Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: partition HidDelegate observers by browser context #40215

Merged
merged 1 commit into from Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
118 changes: 81 additions & 37 deletions shell/browser/hid/electron_hid_delegate.cc
Expand Up @@ -9,6 +9,7 @@

#include "base/command_line.h"
#include "base/containers/contains.h"
#include "base/scoped_observation.h"
#include "chrome/common/chrome_features.h"
#include "content/public/browser/web_contents.h"
#include "electron/buildflags/buildflags.h"
Expand Down Expand Up @@ -36,6 +37,70 @@ electron::HidChooserContext* GetChooserContext(

namespace electron {

// Manages the HidDelegate observers for a single browser context.
class ElectronHidDelegate::ContextObservation
: public HidChooserContext::DeviceObserver {
public:
ContextObservation(ElectronHidDelegate* parent,
content::BrowserContext* browser_context)
: parent_(parent), browser_context_(browser_context) {
auto* chooser_context = GetChooserContext(browser_context_);
device_observation_.Observe(chooser_context);
}

ContextObservation(ContextObservation&) = delete;
ContextObservation& operator=(ContextObservation&) = delete;
~ContextObservation() override = default;

// HidChooserContext::DeviceObserver:
void OnDeviceAdded(const device::mojom::HidDeviceInfo& device_info) override {
for (auto& observer : observer_list_)
observer.OnDeviceAdded(device_info);
}

void OnDeviceRemoved(
const device::mojom::HidDeviceInfo& device_info) override {
for (auto& observer : observer_list_)
observer.OnDeviceRemoved(device_info);
}

void OnDeviceChanged(
const device::mojom::HidDeviceInfo& device_info) override {
for (auto& observer : observer_list_)
observer.OnDeviceChanged(device_info);
}

void OnHidManagerConnectionError() override {
for (auto& observer : observer_list_)
observer.OnHidManagerConnectionError();
}

void OnHidChooserContextShutdown() override {
parent_->observations_.erase(browser_context_);
// Return since `this` is now deleted.
}

void AddObserver(content::HidDelegate::Observer* observer) {
observer_list_.AddObserver(observer);
}

void RemoveObserver(content::HidDelegate::Observer* observer) {
observer_list_.RemoveObserver(observer);
}

private:
// Safe because `parent_` owns `this`.
const raw_ptr<ElectronHidDelegate> parent_;

// Safe because `this` is destroyed when the context is lost.
const raw_ptr<content::BrowserContext> browser_context_;

base::ScopedObservation<HidChooserContext, HidChooserContext::DeviceObserver>
device_observation_{this};

base::ObserverList<content::HidDelegate::Observer> observer_list_;
};

ElectronHidDelegate::ElectronHidDelegate() = default;

ElectronHidDelegate::~ElectronHidDelegate() = default;
Expand All @@ -45,11 +110,11 @@ std::unique_ptr<content::HidChooser> ElectronHidDelegate::RunChooser(
std::vector<blink::mojom::HidDeviceFilterPtr> filters,
std::vector<blink::mojom::HidDeviceFilterPtr> exclusion_filters,
content::HidChooser::Callback callback) {
DCHECK(render_frame_host);
auto* chooser_context =
GetChooserContext(render_frame_host->GetBrowserContext());
if (!device_observation_.IsObserving())
device_observation_.Observe(chooser_context);
auto* browser_context = render_frame_host->GetBrowserContext();

// Start observing HidChooserContext for permission and device events.
GetContextObserver(browser_context);
DCHECK(base::Contains(observations_, browser_context));

HidChooserController* controller = ControllerForFrame(render_frame_host);
if (controller) {
Expand Down Expand Up @@ -101,16 +166,14 @@ device::mojom::HidManager* ElectronHidDelegate::GetHidManager(

void ElectronHidDelegate::AddObserver(content::BrowserContext* browser_context,
Observer* observer) {
observer_list_.AddObserver(observer);
auto* chooser_context = GetChooserContext(browser_context);
if (!device_observation_.IsObserving())
device_observation_.Observe(chooser_context);
GetContextObserver(browser_context)->AddObserver(observer);
}

void ElectronHidDelegate::RemoveObserver(
content::BrowserContext* browser_context,
content::HidDelegate::Observer* observer) {
observer_list_.RemoveObserver(observer);
DCHECK(base::Contains(observations_, browser_context));
GetContextObserver(browser_context)->RemoveObserver(observer);
}

const device::mojom::HidDeviceInfo* ElectronHidDelegate::GetDeviceInfo(
Expand Down Expand Up @@ -140,33 +203,14 @@ bool ElectronHidDelegate::IsServiceWorkerAllowedForOrigin(
return false;
}

void ElectronHidDelegate::OnDeviceAdded(
const device::mojom::HidDeviceInfo& device_info) {
for (auto& observer : observer_list_)
observer.OnDeviceAdded(device_info);
}

void ElectronHidDelegate::OnDeviceRemoved(
const device::mojom::HidDeviceInfo& device_info) {
for (auto& observer : observer_list_)
observer.OnDeviceRemoved(device_info);
}

void ElectronHidDelegate::OnDeviceChanged(
const device::mojom::HidDeviceInfo& device_info) {
for (auto& observer : observer_list_)
observer.OnDeviceChanged(device_info);
}

void ElectronHidDelegate::OnHidManagerConnectionError() {
device_observation_.Reset();

for (auto& observer : observer_list_)
observer.OnHidManagerConnectionError();
}

void ElectronHidDelegate::OnHidChooserContextShutdown() {
device_observation_.Reset();
ElectronHidDelegate::ContextObservation*
ElectronHidDelegate::GetContextObserver(
content::BrowserContext* browser_context) {
if (!base::Contains(observations_, browser_context)) {
observations_.emplace(browser_context, std::make_unique<ContextObservation>(
this, browser_context));
}
return observations_[browser_context].get();
}

HidChooserController* ElectronHidDelegate::ControllerForFrame(
Expand Down
53 changes: 19 additions & 34 deletions shell/browser/hid/electron_hid_delegate.h
Expand Up @@ -10,17 +10,24 @@
#include <unordered_map>
#include <vector>

#include "base/observer_list.h"
#include "base/scoped_observation.h"
#include "base/containers/flat_map.h"
#include "content/public/browser/hid_chooser.h"
#include "content/public/browser/hid_delegate.h"
#include "services/device/public/mojom/hid.mojom-forward.h"
#include "shell/browser/hid/hid_chooser_context.h"
#include "third_party/blink/public/mojom/hid/hid.mojom-forward.h"
#include "url/origin.h"

namespace content {
class BrowserContext;
class RenderFrameHost;
} // namespace content

namespace electron {

class HidChooserController;

class ElectronHidDelegate : public content::HidDelegate,
public HidChooserContext::DeviceObserver {
class ElectronHidDelegate : public content::HidDelegate {
public:
ElectronHidDelegate();
ElectronHidDelegate(ElectronHidDelegate&) = delete;
Expand Down Expand Up @@ -54,13 +61,6 @@ class ElectronHidDelegate : public content::HidDelegate,
bool IsFidoAllowedForOrigin(content::BrowserContext* browser_context,
const url::Origin& origin) override;
bool IsServiceWorkerAllowedForOrigin(const url::Origin& origin) override;

// HidChooserContext::DeviceObserver:
void OnDeviceAdded(const device::mojom::HidDeviceInfo&) override;
void OnDeviceRemoved(const device::mojom::HidDeviceInfo&) override;
void OnDeviceChanged(const device::mojom::HidDeviceInfo&) override;
void OnHidManagerConnectionError() override;
void OnHidChooserContextShutdown() override;
void IncrementConnectionCount(content::BrowserContext* browser_context,
const url::Origin& origin) override {}
void DecrementConnectionCount(content::BrowserContext* browser_context,
Expand All @@ -69,6 +69,14 @@ class ElectronHidDelegate : public content::HidDelegate,
void DeleteControllerForFrame(content::RenderFrameHost* render_frame_host);

private:
class ContextObservation;

ContextObservation* GetContextObserver(
content::BrowserContext* browser_context);

base::flat_map<content::BrowserContext*, std::unique_ptr<ContextObservation>>
observations_;

HidChooserController* ControllerForFrame(
content::RenderFrameHost* render_frame_host);

Expand All @@ -78,10 +86,6 @@ class ElectronHidDelegate : public content::HidDelegate,
std::vector<blink::mojom::HidDeviceFilterPtr> exclusion_filters,
content::HidChooser::Callback callback);

base::ScopedObservation<HidChooserContext, HidChooserContext::DeviceObserver>
device_observation_{this};
base::ObserverList<content::HidDelegate::Observer> observer_list_;

std::unordered_map<content::RenderFrameHost*,
std::unique_ptr<HidChooserController>>
controller_map_;
Expand All @@ -91,23 +95,4 @@ class ElectronHidDelegate : public content::HidDelegate,

} // namespace electron

namespace base {

template <>
struct ScopedObservationTraits<electron::HidChooserContext,
electron::HidChooserContext::DeviceObserver> {
static void AddObserver(
electron::HidChooserContext* source,
electron::HidChooserContext::DeviceObserver* observer) {
source->AddDeviceObserver(observer);
}
static void RemoveObserver(
electron::HidChooserContext* source,
electron::HidChooserContext::DeviceObserver* observer) {
source->RemoveDeviceObserver(observer);
}
};

} // namespace base

#endif // ELECTRON_SHELL_BROWSER_HID_ELECTRON_HID_DELEGATE_H_
20 changes: 20 additions & 0 deletions shell/browser/hid/hid_chooser_context.h
Expand Up @@ -14,6 +14,7 @@
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
#include "base/scoped_observation_traits.h"
#include "base/unguessable_token.h"
#include "components/keyed_service/core/keyed_service.h"
#include "content/public/browser/web_contents.h"
Expand Down Expand Up @@ -138,4 +139,23 @@ class HidChooserContext : public KeyedService,

} // namespace electron

namespace base {

template <>
struct ScopedObservationTraits<electron::HidChooserContext,
electron::HidChooserContext::DeviceObserver> {
static void AddObserver(
electron::HidChooserContext* source,
electron::HidChooserContext::DeviceObserver* observer) {
source->AddDeviceObserver(observer);
}
static void RemoveObserver(
electron::HidChooserContext* source,
electron::HidChooserContext::DeviceObserver* observer) {
source->RemoveDeviceObserver(observer);
}
};

} // namespace base

#endif // ELECTRON_SHELL_BROWSER_HID_HID_CHOOSER_CONTEXT_H_
1 change: 1 addition & 0 deletions shell/browser/hid/hid_chooser_controller.h
Expand Up @@ -10,6 +10,7 @@
#include <vector>

#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
#include "content/public/browser/global_routing_id.h"
#include "content/public/browser/hid_chooser.h"
#include "content/public/browser/web_contents.h"
Expand Down