[webhid] Support hotplugging in the device chooser

Device connection events are routed to the HID chooser to allow the
chooser list to be updated with newly connected and disconnected
devices.

BUG=1082303

Change-Id: I1d7ef2efcccba6ecddfde4bc6f1047febf9aefdf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1895751Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Matt Reynolds <mattreynolds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779598}

chrome/browser/BUILD.gn

@@ -6018,6 +6018,8 @@ static_library("test_support") {
       "chooser_controller/fake_usb_chooser_controller.h",
       "download/test_download_shelf.cc",
       "download/test_download_shelf.h",
+      "hid/mock_hid_device_observer.cc",
+      "hid/mock_hid_device_observer.h",
       "profile_resetter/profile_resetter_test_base.cc",
       "profile_resetter/profile_resetter_test_base.h",
       "sessions/session_restore_test_helper.cc",

chrome/browser/hid/chrome_hid_delegate.cc

@@ -24,6 +24,16 @@ std::unique_ptr<content::HidChooser> ChromeHidDelegate::RunChooser(
     content::RenderFrameHost* frame,
     std::vector<blink::mojom::HidDeviceFilterPtr> filters,
     content::HidChooser::Callback callback) {
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderFrameHost(frame);
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  auto* chooser_context = HidChooserContextFactory::GetForProfile(profile);
+  if (!device_observer_.IsObservingSources())
+    device_observer_.Add(chooser_context);
+  if (!permission_observer_.IsObservingSources())
+    permission_observer_.Add(chooser_context);
+
   return std::make_unique<HidChooser>(chrome::ShowDeviceChooserDialog(
       frame, std::make_unique<HidChooserController>(frame, std::move(filters),
                                                     std::move(callback))));
@@ -61,3 +71,39 @@ device::mojom::HidManager* ChromeHidDelegate::GetHidManager(
   auto* chooser_context = HidChooserContextFactory::GetForProfile(profile);
   return chooser_context->GetHidManager();
 }
+
+void ChromeHidDelegate::AddObserver(content::HidDelegate::Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void ChromeHidDelegate::RemoveObserver(
+    content::HidDelegate::Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void ChromeHidDelegate::OnPermissionRevoked(
+    const url::Origin& requesting_origin,
+    const url::Origin& embedding_origin) {
+  for (auto& observer : observer_list_)
+    observer.OnPermissionRevoked(requesting_origin, embedding_origin);
+}
+
+void ChromeHidDelegate::OnDeviceAdded(
+    const device::mojom::HidDeviceInfo& device_info) {
+  for (auto& observer : observer_list_)
+    observer.OnDeviceAdded(device_info);
+}
+
+void ChromeHidDelegate::OnDeviceRemoved(
+    const device::mojom::HidDeviceInfo& device_info) {
+  for (auto& observer : observer_list_)
+    observer.OnDeviceRemoved(device_info);
+}
+
+void ChromeHidDelegate::OnHidManagerConnectionError() {
+  device_observer_.RemoveAll();
+  permission_observer_.RemoveAll();
+
+  for (auto& observer : observer_list_)
+    observer.OnHidManagerConnectionError();
+}

chrome/browser/hid/chrome_hid_delegate.h

@@ -8,11 +8,20 @@
 #include <memory>
 #include <vector>
 
+#include "base/observer_list.h"
+#include "base/scoped_observer.h"
+#include "chrome/browser/hid/hid_chooser_context.h"
+#include "components/permissions/chooser_context_base.h"
 #include "content/public/browser/hid_delegate.h"
 
-class ChromeHidDelegate : public content::HidDelegate {
+class ChromeHidDelegate
+    : public content::HidDelegate,
+      public permissions::ChooserContextBase::PermissionObserver,
+      public HidChooserContext::DeviceObserver {
  public:
   ChromeHidDelegate();
+  ChromeHidDelegate(ChromeHidDelegate&) = delete;
+  ChromeHidDelegate& operator=(ChromeHidDelegate&) = delete;
   ~ChromeHidDelegate() override;
 
   std::unique_ptr<content::HidChooser> RunChooser(
@@ -27,9 +36,28 @@ class ChromeHidDelegate : public content::HidDelegate {
                            const device::mojom::HidDeviceInfo& device) override;
   device::mojom::HidManager* GetHidManager(
       content::WebContents* web_contents) override;
+  void AddObserver(content::HidDelegate::Observer* observer) override;
+  void RemoveObserver(content::HidDelegate::Observer* observer) override;
+
+  // permissions::ChooserContextBase::PermissionObserver:
+  void OnPermissionRevoked(const url::Origin& requesting_origin,
+                           const url::Origin& embedding_origin) override;
+
+  // HidChooserContext::DeviceObserver:
+  void OnDeviceAdded(const device::mojom::HidDeviceInfo&) override;
+  void OnDeviceRemoved(const device::mojom::HidDeviceInfo&) override;
+  void OnHidManagerConnectionError() override;
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(ChromeHidDelegate);
+  ScopedObserver<HidChooserContext,
+                 HidChooserContext::DeviceObserver,
+                 &HidChooserContext::AddDeviceObserver,
+                 &HidChooserContext::RemoveDeviceObserver>
+      device_observer_{this};
+  ScopedObserver<permissions::ChooserContextBase,
+                 permissions::ChooserContextBase::PermissionObserver>
+      permission_observer_{this};
+  base::ObserverList<content::HidDelegate::Observer> observer_list_;
 };
 
 #endif  // CHROME_BROWSER_HID_CHROME_HID_DELEGATE_H_

chrome/browser/hid/hid_chooser_context.cc

@@ -46,6 +46,14 @@ base::Value DeviceInfoToValue(const device::mojom::HidDeviceInfo& device) {
 
 }  // namespace
 
+void HidChooserContext::DeviceObserver::OnDeviceAdded(
+    const device::mojom::HidDeviceInfo& device) {}
+
+void HidChooserContext::DeviceObserver::OnDeviceRemoved(
+    const device::mojom::HidDeviceInfo& device) {}
+
+void HidChooserContext::DeviceObserver::OnHidManagerConnectionError() {}
+
 HidChooserContext::HidChooserContext(Profile* profile)
     : ChooserContextBase(ContentSettingsType::HID_GUARD,
                          ContentSettingsType::HID_CHOOSER_DATA,
@@ -82,8 +90,7 @@ HidChooserContext::GetGrantedObjects(const url::Origin& requesting_origin,
                                             embedding_origin);
 
   if (CanRequestObjectPermission(requesting_origin, embedding_origin)) {
-    auto it = ephemeral_devices_.find(
-        std::make_pair(requesting_origin, embedding_origin));
+    auto it = ephemeral_devices_.find({requesting_origin, embedding_origin});
     if (it != ephemeral_devices_.end()) {
       for (const std::string& guid : it->second) {
         // |devices_| should be initialized when |ephemeral_devices_| is filled.
@@ -164,13 +171,13 @@ void HidChooserContext::GrantDevicePermission(
     const url::Origin& requesting_origin,
     const url::Origin& embedding_origin,
     const device::mojom::HidDeviceInfo& device) {
-  devices_[device.guid] = device.Clone();
+  DCHECK(base::Contains(devices_, device.guid));
   if (CanStorePersistentEntry(device)) {
     GrantObjectPermission(requesting_origin, embedding_origin,
                           DeviceInfoToValue(device));
   } else {
-    ephemeral_devices_[std::make_pair(requesting_origin, embedding_origin)]
-        .insert(device.guid);
+    ephemeral_devices_[{requesting_origin, embedding_origin}].insert(
+        device.guid);
     NotifyPermissionChanged();
   }
 }
@@ -187,8 +194,7 @@ bool HidChooserContext::HasDevicePermission(
   if (!CanRequestObjectPermission(requesting_origin, embedding_origin))
     return false;
 
-  auto it = ephemeral_devices_.find(
-      std::make_pair(requesting_origin, embedding_origin));
+  auto it = ephemeral_devices_.find({requesting_origin, embedding_origin});
   if (it != ephemeral_devices_.end() &&
       base::Contains(it->second, device.guid)) {
     return true;
@@ -212,20 +218,97 @@ bool HidChooserContext::HasDevicePermission(
   return false;
 }
 
+void HidChooserContext::AddDeviceObserver(DeviceObserver* observer) {
+  EnsureHidManagerConnection();
+  device_observer_list_.AddObserver(observer);
+}
+
+void HidChooserContext::RemoveDeviceObserver(DeviceObserver* observer) {
+  device_observer_list_.RemoveObserver(observer);
+}
+
+void HidChooserContext::GetDevices(
+    device::mojom::HidManager::GetDevicesCallback callback) {
+  if (!is_initialized_) {
+    EnsureHidManagerConnection();
+    pending_get_devices_requests_.push(std::move(callback));
+    return;
+  }
+
+  std::vector<device::mojom::HidDeviceInfoPtr> device_list;
+  device_list.reserve(devices_.size());
+  for (const auto& pair : devices_)
+    device_list.push_back(pair.second->Clone());
+  base::SequencedTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(std::move(callback), std::move(device_list)));
+}
+
 device::mojom::HidManager* HidChooserContext::GetHidManager() {
   EnsureHidManagerConnection();
   return hid_manager_.get();
 }
 
 void HidChooserContext::SetHidManagerForTesting(
-    mojo::PendingRemote<device::mojom::HidManager> manager) {
-  SetUpHidManagerConnection(std::move(manager));
+    mojo::PendingRemote<device::mojom::HidManager> manager,
+    device::mojom::HidManager::GetDevicesCallback callback) {
+  hid_manager_.Bind(std::move(manager));
+  hid_manager_.set_disconnect_handler(base::BindOnce(
+      &HidChooserContext::OnHidManagerConnectionError, base::Unretained(this)));
+
+  hid_manager_->GetDevicesAndSetClient(
+      client_receiver_.BindNewEndpointAndPassRemote(), std::move(callback));
 }
 
 base::WeakPtr<HidChooserContext> HidChooserContext::AsWeakPtr() {
   return weak_factory_.GetWeakPtr();
 }
 
+void HidChooserContext::DeviceAdded(device::mojom::HidDeviceInfoPtr device) {
+  DCHECK(device);
+
+  // Update the device list.
+  if (!base::Contains(devices_, device->guid))
+    devices_.insert({device->guid, device->Clone()});
+
+  // Notify all observers.
+  for (auto& observer : device_observer_list_)
+    observer.OnDeviceAdded(*device);
+}
+
+void HidChooserContext::DeviceRemoved(device::mojom::HidDeviceInfoPtr device) {
+  DCHECK(device);
+  DCHECK(base::Contains(devices_, device->guid));
+
+  // Update the device list.
+  devices_.erase(device->guid);
+
+  // Notify all device observers.
+  for (auto& observer : device_observer_list_)
+    observer.OnDeviceRemoved(*device);
+
+  // Next we'll notify observers for revoked permissions. If the device does not
+  // support persistent permissions then device permissions are revoked on
+  // disconnect.
+  if (CanStorePersistentEntry(*device))
+    return;
+
+  std::vector<std::pair<url::Origin, url::Origin>> revoked_url_pairs;
+  for (auto& map_entry : ephemeral_devices_) {
+    if (map_entry.second.erase(device->guid) > 0)
+      revoked_url_pairs.push_back(map_entry.first);
+  }
+  if (revoked_url_pairs.empty())
+    return;
+
+  for (auto& observer : permission_observer_list_) {
+    observer.OnChooserObjectPermissionChanged(guard_content_settings_type_,
+                                              data_content_settings_type_);
+    for (auto& url_pair : revoked_url_pairs) {
+      observer.OnPermissionRevoked(url_pair.first, url_pair.second);
+    }
+  }
+}
+
 void HidChooserContext::EnsureHidManagerConnection() {
   if (hid_manager_)
     return;
@@ -241,11 +324,32 @@ void HidChooserContext::SetUpHidManagerConnection(
   hid_manager_.Bind(std::move(manager));
   hid_manager_.set_disconnect_handler(base::BindOnce(
       &HidChooserContext::OnHidManagerConnectionError, base::Unretained(this)));
-  // TODO(crbug.com/1082303): Register a HidManagerClient to be notified when
-  // devices are disconnected so that ephemeral permissions can be revoked.
+
+  hid_manager_->GetDevicesAndSetClient(
+      client_receiver_.BindNewEndpointAndPassRemote(),
+      base::BindOnce(&HidChooserContext::InitDeviceList,
+                     weak_factory_.GetWeakPtr()));
+}
+
+void HidChooserContext::InitDeviceList(
+    std::vector<device::mojom::HidDeviceInfoPtr> devices) {
+  for (auto& device : devices)
+    devices_.insert({device->guid, std::move(device)});
+
+  while (!pending_get_devices_requests_.empty()) {
+    std::vector<device::mojom::HidDeviceInfoPtr> device_list;
+    device_list.reserve(devices.size());
+    for (const auto& entry : devices_)
+      device_list.push_back(entry.second->Clone());
+    std::move(pending_get_devices_requests_.front())
+        .Run(std::move(device_list));
+    pending_get_devices_requests_.pop();
+  }
 }
 
 void HidChooserContext::OnHidManagerConnectionError() {
+  hid_manager_.reset();
+  client_receiver_.reset();
   devices_.clear();
 
   std::vector<std::pair<url::Origin, url::Origin>> revoked_origins;
@@ -254,6 +358,10 @@ void HidChooserContext::OnHidManagerConnectionError() {
     revoked_origins.push_back(map_entry.first);
   ephemeral_devices_.clear();
 
+  // Notify all device observers.
+  for (auto& observer : device_observer_list_)
+    observer.OnHidManagerConnectionError();
+
   // Notify permission observers that all ephemeral permissions have been
   // revoked.
   for (auto& observer : permission_observer_list_) {

chrome/browser/hid/hid_chooser_context.h

@@ -13,8 +13,10 @@
 #include <vector>
 
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "base/unguessable_token.h"
 #include "components/permissions/chooser_context_base.h"
+#include "mojo/public/cpp/bindings/associated_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "services/device/public/mojom/hid.mojom.h"
@@ -28,13 +30,23 @@ class Value;
 
 // Manages the internal state and connection to the device service for the
 // Human Interface Device (HID) chooser UI.
-class HidChooserContext : public permissions::ChooserContextBase {
+class HidChooserContext : public permissions::ChooserContextBase,
+                          public device::mojom::HidManagerClient {
  public:
   explicit HidChooserContext(Profile* profile);
   HidChooserContext(const HidChooserContext&) = delete;
   HidChooserContext& operator=(const HidChooserContext&) = delete;
   ~HidChooserContext() override;
 
+  // This observer can be used to be notified when HID devices are connected or
+  // disconnected.
+  class DeviceObserver : public base::CheckedObserver {
+   public:
+    virtual void OnDeviceAdded(const device::mojom::HidDeviceInfo&);
+    virtual void OnDeviceRemoved(const device::mojom::HidDeviceInfo&);
+    virtual void OnHidManagerConnectionError();
+  };
+
   // permissions::ChooserContextBase implementation:
   bool IsValidObject(const base::Value& object) override;
   // In addition these methods from ChooserContextBase are overridden in order
@@ -56,19 +68,39 @@ class HidChooserContext : public permissions::ChooserContextBase {
                            const url::Origin& embedding_origin,
                            const device::mojom::HidDeviceInfo& device);
 
+  // For ScopedObserver.
+  void AddDeviceObserver(DeviceObserver* observer);
+  void RemoveDeviceObserver(DeviceObserver* observer);
+
+  // Forward HidManager::GetDevices.
+  void GetDevices(device::mojom::HidManager::GetDevicesCallback callback);
+
   device::mojom::HidManager* GetHidManager();
 
+  // Sets |manager| as the HidManager and registers this context as a
+  // HidManagerClient. Calls |callback| with the set of enumerated devices once
+  // the client is registered and the initial enumeration is complete.
   void SetHidManagerForTesting(
-      mojo::PendingRemote<device::mojom::HidManager> manager);
+      mojo::PendingRemote<device::mojom::HidManager> manager,
+      device::mojom::HidManager::GetDevicesCallback callback);
+
   base::WeakPtr<HidChooserContext> AsWeakPtr();
 
  private:
+  // device::mojom::HidManagerClient implementation:
+  void DeviceAdded(device::mojom::HidDeviceInfoPtr device_info) override;
+  void DeviceRemoved(device::mojom::HidDeviceInfoPtr device_info) override;
+
   void EnsureHidManagerConnection();
   void SetUpHidManagerConnection(
       mojo::PendingRemote<device::mojom::HidManager> manager);
+  void InitDeviceList(std::vector<device::mojom::HidDeviceInfoPtr> devices);
   void OnHidManagerConnectionError();
 
   const bool is_incognito_;
+  bool is_initialized_ = false;
+  base::queue<device::mojom::HidManager::GetDevicesCallback>
+      pending_get_devices_requests_;
 
   // Tracks the set of devices to which an origin (potentially embedded in
   // another origin) has access to. Key is (requesting_origin,
@@ -80,6 +112,9 @@ class HidChooserContext : public permissions::ChooserContextBase {
   std::map<std::string, device::mojom::HidDeviceInfoPtr> devices_;
 
   mojo::Remote<device::mojom::HidManager> hid_manager_;
+  mojo::AssociatedReceiver<device::mojom::HidManagerClient> client_receiver_{
+      this};
+  base::ObserverList<DeviceObserver> device_observer_list_;
 
   base::WeakPtrFactory<HidChooserContext> weak_factory_{this};
 };

chrome/browser/hid/hid_chooser_context_unittest.cc

@@ -5,160 +5,388 @@
 #include "chrome/browser/hid/hid_chooser_context.h"
 
 #include "base/run_loop.h"
+#include "base/test/bind_test_util.h"
+#include "base/test/gmock_callback_support.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/hid/hid_chooser_context_factory.h"
+#include "chrome/browser/hid/mock_hid_device_observer.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/permissions/test/chooser_context_base_mock_permission_observer.h"
 #include "content/public/test/browser_task_environment.h"
 #include "services/device/public/cpp/hid/fake_hid_manager.h"
 #include "services/device/public/mojom/hid.mojom.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::base::test::RunClosure;
+using ::testing::_;
+
 namespace {
 
+// Main text fixture.
 class HidChooserContextTest : public testing::Test {
  public:
-  HidChooserContextTest() = default;
+  HidChooserContextTest()
+      : origin_(url::Origin::Create(GURL("https://google.com"))) {}
+  HidChooserContextTest(const HidChooserContextTest&) = delete;
+  HidChooserContextTest& operator=(const HidChooserContextTest&) = delete;
   ~HidChooserContextTest() override = default;
 
+  const url::Origin& origin() { return origin_; }
   Profile* profile() { return &profile_; }
-  permissions::MockPermissionObserver& observer() { return mock_observer_; }
-  device::FakeHidManager* hid_manager() { return &hid_manager_; }
+  permissions::MockPermissionObserver& permission_observer() {
+    return mock_permission_observer_;
+  }
+  MockHidDeviceObserver& device_observer() { return mock_device_observer_; }
 
-  HidChooserContext* GetContext(Profile* profile) {
-    auto* context = HidChooserContextFactory::GetForProfile(profile);
-    context->AddObserver(&mock_observer_);
+  HidChooserContext* GetContext() {
+    auto* context = HidChooserContextFactory::GetForProfile(&profile_);
+    context->AddObserver(&mock_permission_observer_);
+    context->AddDeviceObserver(&mock_device_observer_);
     return context;
   }
 
   void SetUp() override {
     mojo::PendingRemote<device::mojom::HidManager> hid_manager;
     hid_manager_.Bind(hid_manager.InitWithNewPipeAndPassReceiver());
-    HidChooserContextFactory::GetForProfile(profile())->SetHidManagerForTesting(
-        std::move(hid_manager));
+    auto* chooser_context = HidChooserContextFactory::GetForProfile(&profile_);
+
+    // Connect the HidManager and ensure we've received the initial enumeration
+    // before continuing.
+    base::RunLoop run_loop;
+    chooser_context->SetHidManagerForTesting(
+        std::move(hid_manager),
+        base::BindLambdaForTesting(
+            [&run_loop](std::vector<device::mojom::HidDeviceInfoPtr> devices) {
+              run_loop.Quit();
+            }));
+    run_loop.Run();
+  }
+
+  device::mojom::HidDeviceInfoPtr ConnectEphemeralDevice() {
+    return hid_manager_.CreateAndAddDevice(
+        "physical-device-id", 0x1234, 0xabcd, "product-name",
+        /*serial_number=*/"", device::mojom::HidBusType::kHIDBusTypeUSB);
+  }
+
+  device::mojom::HidDeviceInfoPtr ConnectPersistentUsbDevice() {
+    return hid_manager_.CreateAndAddDevice(
+        "physical-device-id", 0x1234, 0xabcd, "product-name", "serial-number",
+        device::mojom::HidBusType::kHIDBusTypeUSB);
+  }
+
+  void DisconnectDevice(const device::mojom::HidDeviceInfo& device) {
+    hid_manager_.RemoveDevice(device.guid);
+  }
+
+  void SimulateHidManagerConnectionError() {
+    hid_manager_.SimulateConnectionError();
   }
 
  private:
+  url::Origin origin_;
   device::FakeHidManager hid_manager_;
   content::BrowserTaskEnvironment task_environment_;
   TestingProfile profile_;
-  permissions::MockPermissionObserver mock_observer_;
+  permissions::MockPermissionObserver mock_permission_observer_;
+  MockHidDeviceObserver mock_device_observer_;
 };
 
 }  // namespace
 
-TEST_F(HidChooserContextTest, GrantAndRevokeEphemeralPermission) {
-  const auto origin = url::Origin::Create(GURL("https://google.com"));
+TEST_F(HidChooserContextTest, GrantAndRevokeEphemeralDevice) {
+  base::RunLoop device_added_loop;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop.QuitClosure()));
+
+  base::RunLoop permission_granted_loop;
+  EXPECT_CALL(permission_observer(), OnChooserObjectPermissionChanged(
+                                         ContentSettingsType::HID_GUARD,
+                                         ContentSettingsType::HID_CHOOSER_DATA))
+      .WillOnce(RunClosure(permission_granted_loop.QuitClosure()))
+      .WillOnce([]() {
+        // Expect a 2nd permission change event when the permission is revoked.
+      });
+
+  base::RunLoop permission_revoked_loop;
+  EXPECT_CALL(permission_observer(), OnPermissionRevoked(origin(), origin()))
+      .WillOnce(RunClosure(permission_revoked_loop.QuitClosure()));
+
+  HidChooserContext* context = GetContext();
+
+  // 1. Connect a device that is only eligible for ephemeral permissions.
+  auto device = ConnectEphemeralDevice();
+  device_added_loop.Run();
+
+  EXPECT_FALSE(context->HasDevicePermission(origin(), origin(), *device));
+
+  // 2. Grant an ephemeral permission.
+  context->GrantDevicePermission(origin(), origin(), *device);
+  permission_granted_loop.Run();
+
+  EXPECT_TRUE(context->HasDevicePermission(origin(), origin(), *device));
+
+  std::vector<std::unique_ptr<permissions::ChooserContextBase::Object>>
+      origin_objects = context->GetGrantedObjects(origin(), origin());
+  ASSERT_EQ(1u, origin_objects.size());
+
+  std::vector<std::unique_ptr<permissions::ChooserContextBase::Object>>
+      objects = context->GetAllGrantedObjects();
+  ASSERT_EQ(1u, objects.size());
+  EXPECT_EQ(origin().GetURL(), objects[0]->requesting_origin);
+  EXPECT_EQ(origin().GetURL(), objects[0]->embedding_origin);
+  EXPECT_EQ(origin_objects[0]->value, objects[0]->value);
+  EXPECT_EQ(content_settings::SettingSource::SETTING_SOURCE_USER,
+            objects[0]->source);
+  EXPECT_FALSE(objects[0]->incognito);
+
+  // 3. Revoke the permission.
+  context->RevokeObjectPermission(origin(), origin(), objects[0]->value);
+  permission_revoked_loop.Run();
+
+  EXPECT_FALSE(context->HasDevicePermission(origin(), origin(), *device));
+  origin_objects = context->GetGrantedObjects(origin(), origin());
+  EXPECT_EQ(0u, origin_objects.size());
+  objects = context->GetAllGrantedObjects();
+  EXPECT_EQ(0u, objects.size());
+}
+
+TEST_F(HidChooserContextTest, GrantAndDisconnectEphemeralDevice) {
+  base::RunLoop device_added_loop;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop.QuitClosure()));
+
+  EXPECT_CALL(device_observer(), OnDeviceRemoved(_));
+
+  base::RunLoop permission_granted_loop;
+  EXPECT_CALL(permission_observer(), OnChooserObjectPermissionChanged(
+                                         ContentSettingsType::HID_GUARD,
+                                         ContentSettingsType::HID_CHOOSER_DATA))
+      .WillOnce(RunClosure(permission_granted_loop.QuitClosure()))
+      .WillOnce([]() {
+        // Expect a 2nd permission change event when the permission is revoked.
+      });
+
+  base::RunLoop permission_revoked_loop;
+  EXPECT_CALL(permission_observer(), OnPermissionRevoked(origin(), origin()))
+      .WillOnce(RunClosure(permission_revoked_loop.QuitClosure()));
+
+  HidChooserContext* context = GetContext();
 
-  // Leave |serial_number| empty so the device cannot be granted a persistent
-  // permission.
-  auto device = hid_manager()->CreateAndAddDevice(
-      "physical-device-id", 0x1234, 0xabcd, "product-name",
-      /*serial_number=*/"", device::mojom::HidBusType::kHIDBusTypeUSB);
+  // 1. Connect a device that is only eligible for ephemeral permissions.
+  auto device = ConnectEphemeralDevice();
+  device_added_loop.Run();
 
-  HidChooserContext* context = GetContext(profile());
-  EXPECT_FALSE(context->HasDevicePermission(origin, origin, *device));
+  EXPECT_FALSE(context->HasDevicePermission(origin(), origin(), *device));
 
-  EXPECT_CALL(observer(), OnChooserObjectPermissionChanged(
-                              ContentSettingsType::HID_GUARD,
-                              ContentSettingsType::HID_CHOOSER_DATA));
+  // 2. Grant an ephemeral permission.
+  context->GrantDevicePermission(origin(), origin(), *device);
+  permission_granted_loop.Run();
 
-  context->GrantDevicePermission(origin, origin, *device);
-  EXPECT_TRUE(context->HasDevicePermission(origin, origin, *device));
+  EXPECT_TRUE(context->HasDevicePermission(origin(), origin(), *device));
 
   std::vector<std::unique_ptr<permissions::ChooserContextBase::Object>>
-      origin_objects = context->GetGrantedObjects(origin, origin);
+      origin_objects = context->GetGrantedObjects(origin(), origin());
   ASSERT_EQ(1u, origin_objects.size());
 
   std::vector<std::unique_ptr<permissions::ChooserContextBase::Object>>
       objects = context->GetAllGrantedObjects();
   ASSERT_EQ(1u, objects.size());
-  EXPECT_EQ(origin.GetURL(), objects[0]->requesting_origin);
-  EXPECT_EQ(origin.GetURL(), objects[0]->embedding_origin);
+  EXPECT_EQ(origin().GetURL(), objects[0]->requesting_origin);
+  EXPECT_EQ(origin().GetURL(), objects[0]->embedding_origin);
   EXPECT_EQ(origin_objects[0]->value, objects[0]->value);
   EXPECT_EQ(content_settings::SettingSource::SETTING_SOURCE_USER,
             objects[0]->source);
   EXPECT_FALSE(objects[0]->incognito);
 
-  EXPECT_CALL(observer(), OnChooserObjectPermissionChanged(
-                              ContentSettingsType::HID_GUARD,
-                              ContentSettingsType::HID_CHOOSER_DATA));
-  EXPECT_CALL(observer(), OnPermissionRevoked(origin, origin));
+  // 3. Disconnect the device. Because an ephemeral permission was granted, the
+  // permission should be revoked on disconnect.
+  DisconnectDevice(*device);
+  permission_revoked_loop.Run();
 
-  context->RevokeObjectPermission(origin, origin, objects[0]->value);
-  EXPECT_FALSE(context->HasDevicePermission(origin, origin, *device));
-  origin_objects = context->GetGrantedObjects(origin, origin);
+  EXPECT_FALSE(context->HasDevicePermission(origin(), origin(), *device));
+  origin_objects = context->GetGrantedObjects(origin(), origin());
   EXPECT_EQ(0u, origin_objects.size());
   objects = context->GetAllGrantedObjects();
   EXPECT_EQ(0u, objects.size());
 }
 
-TEST_F(HidChooserContextTest, GrantAndRevokeUsbPersistentPermission) {
-  const auto origin = url::Origin::Create(GURL("https://google.com"));
+TEST_F(HidChooserContextTest, GrantDisconnectRevokeUsbPersistentDevice) {
+  base::RunLoop device_added_loop;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop.QuitClosure()));
+
+  base::RunLoop device_removed_loop;
+  EXPECT_CALL(device_observer(), OnDeviceRemoved(_))
+      .WillOnce(RunClosure(device_removed_loop.QuitClosure()));
+
+  base::RunLoop permission_granted_loop;
+  EXPECT_CALL(permission_observer(), OnChooserObjectPermissionChanged(
+                                         ContentSettingsType::HID_GUARD,
+                                         ContentSettingsType::HID_CHOOSER_DATA))
+      .WillOnce(RunClosure(permission_granted_loop.QuitClosure()))
+      .WillOnce([]() {
+        // Expect a 2nd permission change event when the permission is revoked.
+      });
+
+  base::RunLoop permission_revoked_loop;
+  EXPECT_CALL(permission_observer(), OnPermissionRevoked(origin(), origin()))
+      .WillOnce(RunClosure(permission_revoked_loop.QuitClosure()));
+
+  HidChooserContext* context = GetContext();
 
-  // USB devices are eligible for persistent permissions if the device reports
-  // a serial number string.
-  auto device = hid_manager()->CreateAndAddDevice(
-      "physical-device-id", 0x1234, 0xabcd, "product-name", "serial-number",
-      device::mojom::HidBusType::kHIDBusTypeUSB);
+  // 1. Connect a USB device eligible for persistent permissions.
+  auto device = ConnectPersistentUsbDevice();
+  device_added_loop.Run();
 
-  HidChooserContext* context = GetContext(profile());
-  EXPECT_FALSE(context->HasDevicePermission(origin, origin, *device));
+  EXPECT_FALSE(context->HasDevicePermission(origin(), origin(), *device));
 
-  EXPECT_CALL(observer(), OnChooserObjectPermissionChanged(
-                              ContentSettingsType::HID_GUARD,
-                              ContentSettingsType::HID_CHOOSER_DATA));
+  // 2. Grant a persistent permission.
+  context->GrantDevicePermission(origin(), origin(), *device);
+  permission_granted_loop.Run();
 
-  context->GrantDevicePermission(origin, origin, *device);
-  EXPECT_TRUE(context->HasDevicePermission(origin, origin, *device));
+  EXPECT_TRUE(context->HasDevicePermission(origin(), origin(), *device));
 
   std::vector<std::unique_ptr<permissions::ChooserContextBase::Object>>
-      origin_objects = context->GetGrantedObjects(origin, origin);
+      origin_objects = context->GetGrantedObjects(origin(), origin());
   ASSERT_EQ(1u, origin_objects.size());
 
   std::vector<std::unique_ptr<permissions::ChooserContextBase::Object>>
       objects = context->GetAllGrantedObjects();
   ASSERT_EQ(1u, objects.size());
-  EXPECT_EQ(origin.GetURL(), objects[0]->requesting_origin);
-  EXPECT_EQ(origin.GetURL(), objects[0]->embedding_origin);
+  EXPECT_EQ(origin().GetURL(), objects[0]->requesting_origin);
+  EXPECT_EQ(origin().GetURL(), objects[0]->embedding_origin);
   EXPECT_EQ(origin_objects[0]->value, objects[0]->value);
   EXPECT_EQ(content_settings::SettingSource::SETTING_SOURCE_USER,
             objects[0]->source);
   EXPECT_FALSE(objects[0]->incognito);
 
-  EXPECT_CALL(observer(), OnChooserObjectPermissionChanged(
-                              ContentSettingsType::HID_GUARD,
-                              ContentSettingsType::HID_CHOOSER_DATA));
-  EXPECT_CALL(observer(), OnPermissionRevoked(origin, origin));
+  // 3. Disconnect the device. The permission should not be revoked.
+  DisconnectDevice(*device);
+  device_removed_loop.Run();
 
-  context->RevokeObjectPermission(origin, origin, objects[0]->value);
-  EXPECT_FALSE(context->HasDevicePermission(origin, origin, *device));
-  origin_objects = context->GetGrantedObjects(origin, origin);
+  EXPECT_TRUE(context->HasDevicePermission(origin(), origin(), *device));
+
+  // 4. Revoke the persistent permission.
+  context->RevokeObjectPermission(origin(), origin(), objects[0]->value);
+  permission_revoked_loop.Run();
+
+  EXPECT_FALSE(context->HasDevicePermission(origin(), origin(), *device));
+  origin_objects = context->GetGrantedObjects(origin(), origin());
   EXPECT_EQ(0u, origin_objects.size());
   objects = context->GetAllGrantedObjects();
   EXPECT_EQ(0u, objects.size());
 }
 
 TEST_F(HidChooserContextTest, GuardPermission) {
-  const auto origin = url::Origin::Create(GURL("https://google.com"));
+  base::RunLoop device_added_loop;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop.QuitClosure()));
+
+  base::RunLoop permission_granted_loop;
+  EXPECT_CALL(permission_observer(), OnChooserObjectPermissionChanged(
+                                         ContentSettingsType::HID_GUARD,
+                                         ContentSettingsType::HID_CHOOSER_DATA))
+      .WillOnce(RunClosure(permission_granted_loop.QuitClosure()));
+
+  HidChooserContext* context = GetContext();
 
-  auto device = device::mojom::HidDeviceInfo::New();
-  device->guid = "test-guid";
+  // 1. Connect a device that is only eligible for ephemeral permissions.
+  auto device = ConnectEphemeralDevice();
+  device_added_loop.Run();
 
-  HidChooserContext* context = GetContext(profile());
-  context->GrantDevicePermission(origin, origin, *device);
-  EXPECT_TRUE(context->HasDevicePermission(origin, origin, *device));
+  // 2. Grant an ephemeral device permission.
+  context->GrantDevicePermission(origin(), origin(), *device);
+  permission_granted_loop.Run();
 
+  EXPECT_TRUE(context->HasDevicePermission(origin(), origin(), *device));
+
+  // 3. Set the guard permission to CONTENT_SETTING_BLOCK.
   auto* map = HostContentSettingsMapFactory::GetForProfile(profile());
-  map->SetContentSettingDefaultScope(origin.GetURL(), origin.GetURL(),
+  map->SetContentSettingDefaultScope(origin().GetURL(), origin().GetURL(),
                                      ContentSettingsType::HID_GUARD,
                                      std::string(), CONTENT_SETTING_BLOCK);
-  EXPECT_FALSE(context->HasDevicePermission(origin, origin, *device));
 
-  auto objects = context->GetGrantedObjects(origin, origin);
+  // 4. Check that the device permission is no longer granted.
+  EXPECT_FALSE(context->HasDevicePermission(origin(), origin(), *device));
+
+  auto objects = context->GetGrantedObjects(origin(), origin());
   EXPECT_EQ(0u, objects.size());
 
   auto all_origin_objects = context->GetAllGrantedObjects();
   EXPECT_EQ(0u, all_origin_objects.size());
 }
+
+TEST_F(HidChooserContextTest, ConnectionErrorWithEphemeralPermission) {
+  base::RunLoop device_added_loop;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop.QuitClosure()));
+
+  EXPECT_CALL(device_observer(), OnHidManagerConnectionError());
+
+  base::RunLoop permission_granted_loop;
+  EXPECT_CALL(permission_observer(), OnChooserObjectPermissionChanged(
+                                         ContentSettingsType::HID_GUARD,
+                                         ContentSettingsType::HID_CHOOSER_DATA))
+      .WillOnce(RunClosure(permission_granted_loop.QuitClosure()))
+      .WillOnce([]() {
+        // Expect a 2nd permission change event when the permission is revoked.
+      });
+
+  base::RunLoop permission_revoked_loop;
+  EXPECT_CALL(permission_observer(), OnPermissionRevoked(origin(), origin()))
+      .WillOnce(RunClosure(permission_revoked_loop.QuitClosure()));
+
+  HidChooserContext* context = GetContext();
+
+  // 1. Connect a device that is only eligible for persistent permissions.
+  auto device = ConnectEphemeralDevice();
+  device_added_loop.Run();
+
+  // 2. Grant an ephemeral device permission.
+  context->GrantDevicePermission(origin(), origin(), *device);
+  permission_granted_loop.Run();
+
+  // 3. Simulate a connection error. The ephemeral permission should be revoked.
+  SimulateHidManagerConnectionError();
+  permission_revoked_loop.Run();
+
+  EXPECT_FALSE(context->HasDevicePermission(origin(), origin(), *device));
+}
+
+TEST_F(HidChooserContextTest, ConnectionErrorWithPersistentPermission) {
+  base::RunLoop device_added_loop;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop.QuitClosure()));
+
+  base::RunLoop connection_error_loop;
+  EXPECT_CALL(device_observer(), OnHidManagerConnectionError())
+      .WillOnce(RunClosure(connection_error_loop.QuitClosure()));
+
+  base::RunLoop permission_granted_loop;
+  EXPECT_CALL(permission_observer(), OnChooserObjectPermissionChanged(
+                                         ContentSettingsType::HID_GUARD,
+                                         ContentSettingsType::HID_CHOOSER_DATA))
+      .WillOnce(RunClosure(permission_granted_loop.QuitClosure()))
+      .WillOnce([]() {
+        // Expect a 2nd permission change event when the permission is revoked.
+      });
+
+  HidChooserContext* context = GetContext();
+
+  // 1. Connect a device that is only eligible for persistent permissions.
+  auto device = ConnectPersistentUsbDevice();
+  device_added_loop.Run();
+
+  // 2. Grant a persistent device permission.
+  context->GrantDevicePermission(origin(), origin(), *device);
+  permission_granted_loop.Run();
+
+  // 3. Simulate a connection error. The persistent permission should not be
+  // affected.
+  SimulateHidManagerConnectionError();
+  connection_error_loop.Run();
+
+  EXPECT_TRUE(context->HasDevicePermission(origin(), origin(), *device));
+}

chrome/browser/hid/mock_hid_device_observer.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/hid/mock_hid_device_observer.h"
+
+MockHidDeviceObserver::MockHidDeviceObserver() = default;
+
+MockHidDeviceObserver::~MockHidDeviceObserver() = default;

chrome/browser/hid/mock_hid_device_observer.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_HID_MOCK_HID_DEVICE_OBSERVER_H_
+#define CHROME_BROWSER_HID_MOCK_HID_DEVICE_OBSERVER_H_
+
+#include "chrome/browser/hid/hid_chooser_context.h"
+
+#include "testing/gmock/include/gmock/gmock.h"
+
+class MockHidDeviceObserver : public HidChooserContext::DeviceObserver {
+ public:
+  MockHidDeviceObserver();
+  MockHidDeviceObserver(MockHidDeviceObserver&) = delete;
+  MockHidDeviceObserver& operator=(MockHidDeviceObserver&) = delete;
+  ~MockHidDeviceObserver() override;
+
+  MOCK_METHOD1(OnDeviceAdded, void(const device::mojom::HidDeviceInfo&));
+  MOCK_METHOD1(OnDeviceRemoved, void(const device::mojom::HidDeviceInfo&));
+  MOCK_METHOD0(OnHidManagerConnectionError, void());
+};
+
+#endif  // CHROME_BROWSER_HID_MOCK_HID_DEVICE_OBSERVER_H_

chrome/browser/ui/hid/hid_chooser_controller.cc

@@ -156,6 +156,32 @@ void HidChooserController::OpenHelpCenterUrl() const {
   NOTIMPLEMENTED();
 }
 
+void HidChooserController::OnDeviceAdded(
+    const device::mojom::HidDeviceInfo& device) {
+  if (!DisplayDevice(device))
+    return;
+
+  if (AddDeviceInfo(device) && view())
+    view()->OnOptionAdded(items_.size() - 1);
+  return;
+}
+
+void HidChooserController::OnDeviceRemoved(
+    const device::mojom::HidDeviceInfo& device) {
+  auto id = PhysicalDeviceIdFromDeviceInfo(device);
+  auto items_it = std::find(items_.begin(), items_.end(), id);
+  if (items_it == items_.end())
+    return;
+  size_t index = std::distance(items_.begin(), items_it);
+
+  if (RemoveDeviceInfo(device) && view())
+    view()->OnOptionRemoved(index);
+}
+
+void HidChooserController::OnHidManagerConnectionError() {
+  observer_.RemoveAll();
+}
+
 void HidChooserController::OnGotDevices(
     std::vector<device::mojom::HidDeviceInfoPtr> devices) {
   for (auto& device : devices) {
@@ -163,6 +189,11 @@ void HidChooserController::OnGotDevices(
       AddDeviceInfo(*device);
   }
 
+  // Listen to HidChooserContext for OnDeviceAdded/Removed events after the
+  // enumeration.
+  if (chooser_context_)
+    observer_.Add(chooser_context_.get());
+
   if (view())
     view()->OnOptionsInitialized();
 }
@@ -248,3 +279,21 @@ bool HidChooserController::AddDeviceInfo(
   items_.push_back(id);
   return true;
 }
+
+bool HidChooserController::RemoveDeviceInfo(
+    const device::mojom::HidDeviceInfo& device) {
+  auto id = PhysicalDeviceIdFromDeviceInfo(device);
+  auto find_it = device_map_.find(id);
+  DCHECK(find_it != device_map_.end());
+  auto& device_infos = find_it->second;
+  base::EraseIf(device_infos,
+                [&device](const device::mojom::HidDeviceInfoPtr& d) {
+                  return d->guid == device.guid;
+                });
+  if (!device_infos.empty())
+    return false;
+  // A device was disconnected. Remove it from the chooser list.
+  device_map_.erase(find_it);
+  base::Erase(items_, id);
+  return true;
+}

chrome/browser/ui/hid/hid_chooser_controller.h

@@ -9,10 +9,11 @@
 #include <string>
 #include <vector>
 
-#include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
 #include "base/strings/string16.h"
 #include "chrome/browser/chooser_controller/chooser_controller.h"
+#include "chrome/browser/hid/hid_chooser_context.h"
 #include "content/public/browser/hid_chooser.h"
 #include "services/device/public/mojom/hid.mojom-forward.h"
 #include "third_party/blink/public/mojom/hid/hid.mojom.h"
@@ -25,7 +26,8 @@ class RenderFrameHost;
 class HidChooserContext;
 
 // HidChooserController provides data for the WebHID API permission prompt.
-class HidChooserController : public ChooserController {
+class HidChooserController : public ChooserController,
+                             public HidChooserContext::DeviceObserver {
  public:
   // Construct a chooser controller for Human Interface Devices (HID).
   // |render_frame_host| is used to initialize the chooser strings and to access
@@ -36,6 +38,8 @@ class HidChooserController : public ChooserController {
   HidChooserController(content::RenderFrameHost* render_frame_host,
                        std::vector<blink::mojom::HidDeviceFilterPtr> filters,
                        content::HidChooser::Callback callback);
+  HidChooserController(HidChooserController&) = delete;
+  HidChooserController& operator=(HidChooserController&) = delete;
   ~HidChooserController() override;
 
   // ChooserController:
@@ -50,6 +54,12 @@ class HidChooserController : public ChooserController {
   void Close() override;
   void OpenHelpCenterUrl() const override;
 
+  // HidChooserContext::DeviceObserver:
+  void OnDeviceAdded(const device::mojom::HidDeviceInfo& device_info) override;
+  void OnDeviceRemoved(
+      const device::mojom::HidDeviceInfo& device_info) override;
+  void OnHidManagerConnectionError() override;
+
  private:
   void OnGotDevices(std::vector<device::mojom::HidDeviceInfoPtr> devices);
   bool DisplayDevice(const device::mojom::HidDeviceInfo& device) const;
@@ -60,6 +70,12 @@ class HidChooserController : public ChooserController {
   // new item is appended. Returns true if an item was appended.
   bool AddDeviceInfo(const device::mojom::HidDeviceInfo& device_info);
 
+  // Remove |device_info| from |device_map_|. The device info is removed from
+  // the chooser item representing the physical device. If this would cause the
+  // item to be empty, the chooser item is removed. Does nothing if the device
+  // is not in the chooser item. Returns true if an item was removed.
+  bool RemoveDeviceInfo(const device::mojom::HidDeviceInfo& device_info);
+
   std::vector<blink::mojom::HidDeviceFilterPtr> filters_;
   content::HidChooser::Callback callback_;
   const url::Origin requesting_origin_;
@@ -80,9 +96,13 @@ class HidChooserController : public ChooserController {
   // in the chooser.
   std::vector<std::string> items_;
 
-  base::WeakPtrFactory<HidChooserController> weak_factory_{this};
+  ScopedObserver<HidChooserContext,
+                 HidChooserContext::DeviceObserver,
+                 &HidChooserContext::AddDeviceObserver,
+                 &HidChooserContext::RemoveDeviceObserver>
+      observer_{this};
 
-  DISALLOW_COPY_AND_ASSIGN(HidChooserController);
+  base::WeakPtrFactory<HidChooserController> weak_factory_{this};
 };
 
 #endif  // CHROME_BROWSER_UI_HID_HID_CHOOSER_CONTROLLER_H_

chrome/browser/ui/hid/hid_chooser_controller_unittest.cc

@@ -7,12 +7,15 @@
 #include <string>
 #include <utility>
 
-#include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
+#include "base/test/gmock_callback_support.h"
 #include "base/test/mock_callback.h"
 #include "base/test/task_environment.h"
+#include "chrome/browser/chooser_controller/mock_chooser_controller_view.h"
 #include "chrome/browser/hid/hid_chooser_context.h"
 #include "chrome/browser/hid/hid_chooser_context_factory.h"
+#include "chrome/browser/hid/mock_hid_device_observer.h"
 #include "chrome/browser/ui/hid/hid_chooser_controller.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
@@ -25,45 +28,27 @@
 #include "ui/base/resource/resource_bundle.h"
 #include "url/gurl.h"
 
+using ::base::test::RunClosure;
+using ::testing::_;
+
 namespace {
 
 const char kDefaultTestUrl[] = "https://www.google.com/";
 
 const char* const kTestPhysicalDeviceIds[] = {"1", "2", "3"};
 
-const uint16_t kYubicoVendorId = 0x1050;
-const uint16_t kYubicoGnubbyProductId = 0x0200;
-
-class FakeHidChooserView : public ChooserController::View {
- public:
-  FakeHidChooserView() {}
-
-  void set_options_initialized_quit_closure(base::OnceClosure quit_closure) {
-    options_initialized_quit_closure_ = std::move(quit_closure);
-  }
-
-  // ChooserController::View:
-  void OnOptionAdded(size_t index) override {}
-  void OnOptionRemoved(size_t index) override {}
-  void OnOptionsInitialized() override {
-    if (options_initialized_quit_closure_)
-      std::move(options_initialized_quit_closure_).Run();
-  }
-  void OnOptionUpdated(size_t index) override {}
-  void OnAdapterEnabledChanged(bool enabled) override {}
-  void OnRefreshStateChanged(bool enabled) override {}
-
- private:
-  base::OnceClosure options_initialized_quit_closure_;
-
-  DISALLOW_COPY_AND_ASSIGN(FakeHidChooserView);
-};
-
-}  // namespace
+const uint16_t kVendorYubico = 0x1050;
+const uint16_t kProductYubicoGnubby = 0x0200;
 
 class HidChooserControllerTest : public ChromeRenderViewHostTestHarness {
  public:
-  HidChooserControllerTest() {}
+  HidChooserControllerTest() = default;
+  HidChooserControllerTest(HidChooserControllerTest&) = delete;
+  HidChooserControllerTest& operator=(HidChooserControllerTest&) = delete;
+  ~HidChooserControllerTest() override = default;
+
+  MockChooserControllerView& view() { return mock_chooser_controller_view_; }
+  MockHidDeviceObserver& device_observer() { return mock_device_observer_; }
 
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
@@ -75,8 +60,17 @@ class HidChooserControllerTest : public ChromeRenderViewHostTestHarness {
     // Set fake HID manager for HidChooserContext.
     mojo::PendingRemote<device::mojom::HidManager> hid_manager;
     hid_manager_.Bind(hid_manager.InitWithNewPipeAndPassReceiver());
-    HidChooserContextFactory::GetForProfile(profile())->SetHidManagerForTesting(
-        std::move(hid_manager));
+    base::RunLoop run_loop;
+    auto* chooser_context = HidChooserContextFactory::GetForProfile(profile());
+    chooser_context->SetHidManagerForTesting(
+        std::move(hid_manager),
+        base::BindLambdaForTesting(
+            [&run_loop](std::vector<device::mojom::HidDeviceInfoPtr> devices) {
+              run_loop.Quit();
+            }));
+    run_loop.Run();
+
+    chooser_context->AddDeviceObserver(&mock_device_observer_);
   }
 
   std::unique_ptr<HidChooserController> CreateHidChooserController(
@@ -84,7 +78,7 @@ class HidChooserControllerTest : public ChromeRenderViewHostTestHarness {
       content::HidChooser::Callback callback = base::DoNothing()) {
     auto hid_chooser_controller = std::make_unique<HidChooserController>(
         main_rfh(), std::move(filters), std::move(callback));
-    hid_chooser_controller->set_view(&fake_hid_chooser_view_);
+    hid_chooser_controller->set_view(&mock_chooser_controller_view_);
     return hid_chooser_controller;
   }
 
@@ -102,6 +96,10 @@ class HidChooserControllerTest : public ChromeRenderViewHostTestHarness {
         usage);
   }
 
+  void DisconnectDevice(const device::mojom::HidDeviceInfo& device) {
+    hid_manager_.RemoveDevice(device.guid);
+  }
+
   blink::mojom::DeviceIdFilterPtr CreateVendorFilter(uint16_t vendor_id) {
     return blink::mojom::DeviceIdFilter::NewVendor(vendor_id);
   }
@@ -123,33 +121,44 @@ class HidChooserControllerTest : public ChromeRenderViewHostTestHarness {
         device::mojom::HidUsageAndPage::New(usage, usage_page));
   }
 
- protected:
-  device::FakeHidManager hid_manager_;
-  FakeHidChooserView fake_hid_chooser_view_;
-
  private:
-  DISALLOW_COPY_AND_ASSIGN(HidChooserControllerTest);
+  device::FakeHidManager hid_manager_;
+  MockChooserControllerView mock_chooser_controller_view_;
+  MockHidDeviceObserver mock_device_observer_;
 };
 
+}  // namespace
+
 TEST_F(HidChooserControllerTest, EmptyChooser) {
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  // Create the HidChooserController. There should be no options.
   auto hid_chooser_controller = CreateHidChooserController({});
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
-  run_loop.Run();
+  options_initialized_loop.Run();
+
   EXPECT_EQ(0u, hid_chooser_controller->NumOptions());
 }
 
 TEST_F(HidChooserControllerTest, AddBlockedFidoDevice) {
-  // FIDO U2F devices (and other devices on the USB blocklist) should be
-  // excluded from the device chooser.
+  base::RunLoop device_added_loop;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop.QuitClosure()));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  // 1. Connect a device blocked by vendor/product ID.
+  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], kVendorYubico,
+                            kProductYubicoGnubby, "gnubby", "001");
+  device_added_loop.Run();
+
+  // 2. Create the HidChooserController. The blocked device should be excluded.
   auto hid_chooser_controller = CreateHidChooserController({});
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
-  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], kYubicoVendorId,
-                            kYubicoGnubbyProductId, "gnubby", "001");
-  run_loop.Run();
+  options_initialized_loop.Run();
+
   EXPECT_EQ(0u, hid_chooser_controller->NumOptions());
 }
 
@@ -157,37 +166,75 @@ TEST_F(HidChooserControllerTest, AddUnknownFidoDevice) {
   // Devices that expose a top-level collection with the FIDO usage page should
   // be blocked even if they aren't on the USB blocklist.
   const uint16_t kFidoU2fHidUsage = 1;
-  auto hid_chooser_controller = CreateHidChooserController({});
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
+
+  base::RunLoop device_added_loop1;
+  base::RunLoop device_added_loop2;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop1.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop2.QuitClosure()));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  // 1. Connect a device blocked by HID usage.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "fido", "001",
                             device::mojom::kPageFido, kFidoU2fHidUsage);
+  device_added_loop1.Run();
+
+  // 2. Connect a second device blocked by HID usage.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 2, 2, "fido", "002",
                             device::mojom::kPageFido, 0);
-  run_loop.Run();
+  device_added_loop2.Run();
+
+  // 3. Create the HidChooserController. The blocked devices should be excluded.
+  auto hid_chooser_controller = CreateHidChooserController({});
+  options_initialized_loop.Run();
+
   EXPECT_EQ(0u, hid_chooser_controller->NumOptions());
 }
 
 TEST_F(HidChooserControllerTest, AddNamedDevice) {
-  auto hid_chooser_controller = CreateHidChooserController({});
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
+  base::RunLoop device_added_loop;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop.QuitClosure()));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  // 1. Connect a device with a non-empty product name string.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001");
-  run_loop.Run();
+  device_added_loop.Run();
+
+  // 2. Create the HidChooserController. The option text should include the
+  // product name.
+  auto hid_chooser_controller = CreateHidChooserController({});
+  options_initialized_loop.Run();
+
   EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)"),
             hid_chooser_controller->GetOption(0));
 }
 
 TEST_F(HidChooserControllerTest, AddUnnamedDevice) {
-  auto hid_chooser_controller = CreateHidChooserController({});
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
+  base::RunLoop device_added_loop;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop.QuitClosure()));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  // 1. Connect a device with an empty product name string.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "", "001");
-  run_loop.Run();
+  device_added_loop.Run();
+
+  // 2. Create the HidChooserController. The option text should use an alternate
+  // string.
+  auto hid_chooser_controller = CreateHidChooserController({});
+  options_initialized_loop.Run();
+
   EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
   EXPECT_EQ(
       base::ASCIIToUTF16("Unknown Device (Vendor: 0x0001, Product: 0x0001)"),
@@ -195,43 +242,80 @@ TEST_F(HidChooserControllerTest, AddUnnamedDevice) {
 }
 
 TEST_F(HidChooserControllerTest, DeviceIdFilterVendorOnly) {
+  base::RunLoop device_added_loop1;
+  base::RunLoop device_added_loop2;
+  base::RunLoop device_added_loop3;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop1.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop2.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop3.QuitClosure()));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  // 1. Connect a device.
+  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001");
+  device_added_loop1.Run();
+
+  // 2. Connect a second device with the same vendor ID.
+  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 1, 2, "b", "002");
+  device_added_loop2.Run();
+
+  // 3. Connect a device with a different vendor ID.
+  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[2], 2, 2, "c", "003");
+  device_added_loop3.Run();
+
+  // 4. Create the HidChooserController with a vendor ID filter. The third
+  // device should be excluded.
   std::vector<blink::mojom::HidDeviceFilterPtr> filters;
   filters.push_back(
       blink::mojom::HidDeviceFilter::New(CreateVendorFilter(1), nullptr));
   auto hid_chooser_controller = CreateHidChooserController(std::move(filters));
-
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
-  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001");
-  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 1, 2, "b", "002");
-  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[2], 2, 2, "c", "003");
-  run_loop.Run();
+  options_initialized_loop.Run();
 
   EXPECT_EQ(2u, hid_chooser_controller->NumOptions());
 
-  std::set<base::string16> options;
-  options.insert(hid_chooser_controller->GetOption(0));
-  options.insert(hid_chooser_controller->GetOption(1));
-  EXPECT_EQ(1u, options.count(
-                    base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)")));
-  EXPECT_EQ(1u, options.count(
-                    base::ASCIIToUTF16("b (Vendor: 0x0001, Product: 0x0002)")));
+  std::set<base::string16> options{hid_chooser_controller->GetOption(0),
+                                   hid_chooser_controller->GetOption(1)};
+  EXPECT_THAT(options,
+              testing::UnorderedElementsAre(
+                  base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)"),
+                  base::ASCIIToUTF16("b (Vendor: 0x0001, Product: 0x0002)")));
 }
 
 TEST_F(HidChooserControllerTest, DeviceIdFilterVendorAndProduct) {
+  base::RunLoop device_added_loop1;
+  base::RunLoop device_added_loop2;
+  base::RunLoop device_added_loop3;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop1.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop2.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop3.QuitClosure()));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  // 1. Connect a device.
+  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001");
+  device_added_loop1.Run();
+
+  // 2. Connect a device with matching vendor ID but non-matching product ID.
+  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 1, 2, "b", "002");
+  device_added_loop2.Run();
+
+  // 3. Connect a device with non-matching vendor and product IDs.
+  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[2], 2, 2, "c", "003");
+  device_added_loop3.Run();
+
+  // 4. Create the HidChooserController with a vendor and product ID filter. The
+  // second and third devices should be excluded.
   std::vector<blink::mojom::HidDeviceFilterPtr> filters;
   filters.push_back(blink::mojom::HidDeviceFilter::New(
       CreateVendorAndProductFilter(1, 1), nullptr));
   auto hid_chooser_controller = CreateHidChooserController(std::move(filters));
-
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
-  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001");
-  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 1, 2, "b", "002");
-  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[2], 2, 2, "c", "003");
-  run_loop.Run();
+  options_initialized_loop.Run();
 
   EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)"),
@@ -239,20 +323,34 @@ TEST_F(HidChooserControllerTest, DeviceIdFilterVendorAndProduct) {
 }
 
 TEST_F(HidChooserControllerTest, UsageFilterUsagePageOnly) {
-  std::vector<blink::mojom::HidDeviceFilterPtr> filters;
-  filters.push_back(blink::mojom::HidDeviceFilter::New(
-      nullptr, CreatePageFilter(device::mojom::kPageGenericDesktop)));
-  auto hid_chooser_controller = CreateHidChooserController(std::move(filters));
+  base::RunLoop device_added_loop1;
+  base::RunLoop device_added_loop2;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop1.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop2.QuitClosure()));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
 
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
+  // 1. Connect a device.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001",
                             device::mojom::kPageGenericDesktop,
                             device::mojom::kGenericDesktopGamePad);
+  device_added_loop1.Run();
+
+  // 2. Connect a device with a different top-level usage page.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 2, 2, "b", "002",
                             device::mojom::kPageSimulation, 5);
-  run_loop.Run();
+  device_added_loop2.Run();
+
+  // 3. Create the HidChooserController with a usage page filter. The second
+  // device should be excluded.
+  std::vector<blink::mojom::HidDeviceFilterPtr> filters;
+  filters.push_back(blink::mojom::HidDeviceFilter::New(
+      nullptr, CreatePageFilter(device::mojom::kPageGenericDesktop)));
+  auto hid_chooser_controller = CreateHidChooserController(std::move(filters));
+  options_initialized_loop.Run();
 
   EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)"),
@@ -260,25 +358,44 @@ TEST_F(HidChooserControllerTest, UsageFilterUsagePageOnly) {
 }
 
 TEST_F(HidChooserControllerTest, UsageFilterUsageAndPage) {
-  std::vector<blink::mojom::HidDeviceFilterPtr> filters;
-  filters.push_back(blink::mojom::HidDeviceFilter::New(
-      nullptr,
-      CreateUsageAndPageFilter(device::mojom::kPageGenericDesktop,
-                               device::mojom::kGenericDesktopGamePad)));
-  auto hid_chooser_controller = CreateHidChooserController(std::move(filters));
-
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
+  base::RunLoop device_added_loop1;
+  base::RunLoop device_added_loop2;
+  base::RunLoop device_added_loop3;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop1.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop2.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop3.QuitClosure()));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  // 1. Connect a device.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001",
                             device::mojom::kPageGenericDesktop,
                             device::mojom::kGenericDesktopGamePad);
+  device_added_loop1.Run();
+
+  // 2. Connect a device with matching usage page but non-matching usage.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 2, 2, "b", "002",
                             device::mojom::kPageGenericDesktop,
                             device::mojom::kGenericDesktopKeyboard);
+  device_added_loop2.Run();
+
+  // 3. Connect a device with non-matching usage page and usage.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[2], 3, 3, "c", "003",
                             device::mojom::kPageSimulation, 5);
-  run_loop.Run();
+  device_added_loop3.Run();
+
+  // 4. Create the HidChooserController with a usage page and usage filter. The
+  // second and third devices should be excluded.
+  std::vector<blink::mojom::HidDeviceFilterPtr> filters;
+  filters.push_back(blink::mojom::HidDeviceFilter::New(
+      nullptr,
+      CreateUsageAndPageFilter(device::mojom::kPageGenericDesktop,
+                               device::mojom::kGenericDesktopGamePad)));
+  auto hid_chooser_controller = CreateHidChooserController(std::move(filters));
+  options_initialized_loop.Run();
 
   EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)"),
@@ -286,32 +403,87 @@ TEST_F(HidChooserControllerTest, UsageFilterUsageAndPage) {
 }
 
 TEST_F(HidChooserControllerTest, DeviceIdAndUsageFilterIntersection) {
+  base::RunLoop device_added_loop1;
+  base::RunLoop device_added_loop2;
+  base::RunLoop device_added_loop3;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop1.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop2.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop3.QuitClosure()));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  // 1. Connect a device.
+  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001",
+                            device::mojom::kPageGenericDesktop,
+                            device::mojom::kGenericDesktopGamePad);
+  device_added_loop1.Run();
+
+  // 2. Connect a device with matching usage page and usage but non-matching
+  // vendor and product IDs.
+  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 2, 2, "b", "002",
+                            device::mojom::kPageGenericDesktop,
+                            device::mojom::kGenericDesktopGamePad);
+  device_added_loop2.Run();
+
+  // 3. Connect a device with matching vendor and product IDs but non-matching
+  // usage page and usage.
+  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[2], 1, 1, "c", "003",
+                            device::mojom::kPageSimulation, 5);
+  device_added_loop3.Run();
+
+  // 4. Create the HidChooserController with a filter that tests vendor ID,
+  // product ID, usage page, and usage. The second and third devices should be
+  // excluded.
   std::vector<blink::mojom::HidDeviceFilterPtr> filters;
   filters.push_back(blink::mojom::HidDeviceFilter::New(
       CreateVendorAndProductFilter(1, 1),
       CreateUsageAndPageFilter(device::mojom::kPageGenericDesktop,
                                device::mojom::kGenericDesktopGamePad)));
   auto hid_chooser_controller = CreateHidChooserController(std::move(filters));
+  options_initialized_loop.Run();
 
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
+  EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
+  EXPECT_EQ(base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)"),
+            hid_chooser_controller->GetOption(0));
+}
+
+TEST_F(HidChooserControllerTest, DeviceIdAndUsageFilterUnion) {
+  base::RunLoop device_added_loop1;
+  base::RunLoop device_added_loop2;
+  base::RunLoop device_added_loop3;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop1.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop2.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop3.QuitClosure()));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  // 1. Connect a device.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001",
                             device::mojom::kPageGenericDesktop,
                             device::mojom::kGenericDesktopGamePad);
+  device_added_loop1.Run();
+
+  // 2. Connect a device with matching usage page and usage but non-matching
+  // vendor and product IDs.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 2, 2, "b", "002",
                             device::mojom::kPageGenericDesktop,
                             device::mojom::kGenericDesktopGamePad);
+  device_added_loop2.Run();
+
+  // 3. Connect a device with matching vendor and product IDs but non-matching
+  // usage page and usage.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[2], 1, 1, "c", "003",
                             device::mojom::kPageSimulation, 5);
-  run_loop.Run();
+  device_added_loop3.Run();
 
-  EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
-  EXPECT_EQ(base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)"),
-            hid_chooser_controller->GetOption(0));
-}
-
-TEST_F(HidChooserControllerTest, DeviceIdAndUsageFilterUnion) {
+  // 4. Create the HidChooserController with a vendor/product ID filter and a
+  // usage page/usage filter. No devices should be excluded.
   std::vector<blink::mojom::HidDeviceFilterPtr> filters;
   filters.push_back(blink::mojom::HidDeviceFilter::New(
       CreateVendorAndProductFilter(1, 1), nullptr));
@@ -320,102 +492,190 @@ TEST_F(HidChooserControllerTest, DeviceIdAndUsageFilterUnion) {
       CreateUsageAndPageFilter(device::mojom::kPageGenericDesktop,
                                device::mojom::kGenericDesktopGamePad)));
   auto hid_chooser_controller = CreateHidChooserController(std::move(filters));
-
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
-  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001",
-                            device::mojom::kPageGenericDesktop,
-                            device::mojom::kGenericDesktopGamePad);
-  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 2, 2, "b", "002",
-                            device::mojom::kPageGenericDesktop,
-                            device::mojom::kGenericDesktopGamePad);
-  CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[2], 1, 1, "c", "003",
-                            device::mojom::kPageSimulation, 5);
-  run_loop.Run();
+  options_initialized_loop.Run();
 
   EXPECT_EQ(3u, hid_chooser_controller->NumOptions());
 }
 
-TEST_F(HidChooserControllerTest, OneItemForSamePhysicalDevice) {
-  base::MockCallback<content::HidChooser::Callback> callback;
-  auto hid_chooser_controller = CreateHidChooserController({}, callback.Get());
+TEST_F(HidChooserControllerTest, OneOptionForSamePhysicalDevice) {
+  base::RunLoop device_added_loop1;
+  base::RunLoop device_added_loop2;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop1.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop2.QuitClosure()));
 
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
 
-  // These two devices have the same physical device ID and should be coalesced
-  // into a single chooser item.
+  base::MockCallback<content::HidChooser::Callback> callback;
+  base::RunLoop callback_loop;
+  std::vector<device::mojom::HidDeviceInfoPtr> devices;
+  EXPECT_CALL(callback, Run(_))
+      .WillOnce([&devices, &callback_loop](
+                    std::vector<device::mojom::HidDeviceInfoPtr> d) {
+        devices = std::move(d);
+        callback_loop.Quit();
+      });
+
+  // 1. Connect a device.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001",
                             device::mojom::kPageGenericDesktop,
                             device::mojom::kGenericDesktopGamePad);
+  device_added_loop1.Run();
+
+  // 2. Connect a second device with the same physical device ID.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001",
                             device::mojom::kPageSimulation, 5);
+  device_added_loop2.Run();
 
-  run_loop.Run();
+  // 3. Create the HidChooserController and register a callback to get the
+  // returned device list. There should be a single chooser option representing
+  // both devices.
+  auto hid_chooser_controller = CreateHidChooserController({}, callback.Get());
+  options_initialized_loop.Run();
 
   EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
   EXPECT_EQ(base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)"),
             hid_chooser_controller->GetOption(0));
 
-  EXPECT_CALL(callback, Run(testing::_))
-      .WillOnce(testing::Invoke(
-          [&](std::vector<device::mojom::HidDeviceInfoPtr> devices) {
-            EXPECT_EQ(2u, devices.size());
-            EXPECT_EQ(kTestPhysicalDeviceIds[0],
-                      devices[0]->physical_device_id);
-            EXPECT_EQ(kTestPhysicalDeviceIds[0],
-                      devices[1]->physical_device_id);
-            EXPECT_NE(devices[0]->guid, devices[1]->guid);
-
-            // Regression test for https://crbug.com/1069057. Ensure that the
-            // set of options is still valid after the callback is run.
-            EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
-            EXPECT_EQ(base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)"),
-                      hid_chooser_controller->GetOption(0));
-          }));
+  // 4. Select the chooser option. The returned device list should include both
+  // devices.
   hid_chooser_controller->Select({0});
+  callback_loop.Run();
+
+  EXPECT_EQ(2u, devices.size());
+  EXPECT_EQ(kTestPhysicalDeviceIds[0], devices[0]->physical_device_id);
+  EXPECT_EQ(kTestPhysicalDeviceIds[0], devices[1]->physical_device_id);
+  EXPECT_NE(devices[0]->guid, devices[1]->guid);
+
+  // Regression test for https://crbug.com/1069057. Ensure that the
+  // set of options is still valid after the callback is run.
+  EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
+  EXPECT_EQ(base::ASCIIToUTF16("a (Vendor: 0x0001, Product: 0x0001)"),
+            hid_chooser_controller->GetOption(0));
 }
 
 TEST_F(HidChooserControllerTest, NoMergeWithDifferentPhysicalDeviceIds) {
-  auto hid_chooser_controller = CreateHidChooserController({});
+  base::RunLoop device_added_loop1;
+  base::RunLoop device_added_loop2;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop1.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop2.QuitClosure()));
 
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
 
+  // 1. Connect a device.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001",
                             device::mojom::kPageGenericDesktop,
                             device::mojom::kGenericDesktopGamePad);
+  device_added_loop1.Run();
 
-  // This device has the same info as the first device except for the physical
-  // device ID. It should have a separate chooser item.
+  // 2. Connect a second device with the same info as the first device except
+  // for the physical device ID.
   CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[1], 1, 1, "a", "001",
                             device::mojom::kPageGenericDesktop,
                             device::mojom::kGenericDesktopGamePad);
+  device_added_loop2.Run();
 
-  run_loop.Run();
+  // 3. Create the HidChooserController. The devices should have separate
+  // chooser options.
+  auto hid_chooser_controller = CreateHidChooserController({});
+  options_initialized_loop.Run();
 
   EXPECT_EQ(2u, hid_chooser_controller->NumOptions());
 }
 
 TEST_F(HidChooserControllerTest, NoMergeWithEmptyPhysicalDeviceId) {
-  auto hid_chooser_controller = CreateHidChooserController({});
+  base::RunLoop device_added_loop1;
+  base::RunLoop device_added_loop2;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop1.QuitClosure()))
+      .WillOnce(RunClosure(device_added_loop2.QuitClosure()));
 
-  base::RunLoop run_loop;
-  fake_hid_chooser_view_.set_options_initialized_quit_closure(
-      run_loop.QuitClosure());
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
 
-  // These two devices have an empty string for the physical device ID and
-  // should not be coalesced.
+  // 1. Connect a device with an empty physical device ID.
   CreateAndAddFakeHidDevice("", 1, 1, "a", "001",
                             device::mojom::kPageGenericDesktop,
                             device::mojom::kGenericDesktopGamePad);
+  device_added_loop1.Run();
+
+  // 2. Connect a second device with an empty physical device ID.
   CreateAndAddFakeHidDevice("", 1, 1, "a", "001",
                             device::mojom::kPageSimulation, 5);
+  device_added_loop2.Run();
 
-  run_loop.Run();
+  // 3. Create the HidChooserController. The devices should have separate
+  // chooser options.
+  auto hid_chooser_controller = CreateHidChooserController({});
+  options_initialized_loop.Run();
 
   EXPECT_EQ(2u, hid_chooser_controller->NumOptions());
 }
+
+TEST_F(HidChooserControllerTest, DeviceConnectAddsOption) {
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  base::RunLoop option_added_loop;
+  EXPECT_CALL(view(), OnOptionAdded(0))
+      .WillOnce(RunClosure(option_added_loop.QuitClosure()));
+
+  // 1. Create the HidChooserController.
+  auto hid_chooser_controller = CreateHidChooserController({});
+  options_initialized_loop.Run();
+
+  EXPECT_EQ(0u, hid_chooser_controller->NumOptions());
+
+  // 2. Connect a device and verify a chooser option is added.
+  auto device =
+      CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001",
+                                device::mojom::kPageGenericDesktop,
+                                device::mojom::kGenericDesktopGamePad);
+  option_added_loop.Run();
+
+  EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
+}
+
+TEST_F(HidChooserControllerTest, DeviceDisconnectRemovesOption) {
+  base::RunLoop device_added_loop;
+  EXPECT_CALL(device_observer(), OnDeviceAdded(_))
+      .WillOnce(RunClosure(device_added_loop.QuitClosure()));
+
+  EXPECT_CALL(device_observer(), OnDeviceRemoved(_));
+
+  base::RunLoop options_initialized_loop;
+  EXPECT_CALL(view(), OnOptionsInitialized())
+      .WillOnce(RunClosure(options_initialized_loop.QuitClosure()));
+
+  base::RunLoop option_removed_loop;
+  EXPECT_CALL(view(), OnOptionRemoved(0))
+      .WillOnce(RunClosure(option_removed_loop.QuitClosure()));
+
+  // 1. Connect a device.
+  auto device =
+      CreateAndAddFakeHidDevice(kTestPhysicalDeviceIds[0], 1, 1, "a", "001",
+                                device::mojom::kPageGenericDesktop,
+                                device::mojom::kGenericDesktopGamePad);
+  device_added_loop.Run();
+
+  // 2. Create the HidChooserController and verify that the device is included.
+  auto hid_chooser_controller = CreateHidChooserController({});
+  options_initialized_loop.Run();
+
+  EXPECT_EQ(1u, hid_chooser_controller->NumOptions());
+
+  // 3. Disconnect the device and verify the chooser option is removed.
+  DisconnectDevice(*device);
+  option_removed_loop.Run();
+
+  EXPECT_EQ(0u, hid_chooser_controller->NumOptions());
+}

content/browser/hid/hid_service.cc

@@ -7,7 +7,9 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/debug/stack_trace.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/hid_chooser.h"
@@ -24,6 +26,7 @@ HidService::HidService(RenderFrameHost* render_frame_host,
     : FrameServiceBase(render_frame_host, std::move(receiver)) {
   watchers_.set_disconnect_handler(base::BindRepeating(
       &HidService::OnWatcherConnectionError, base::Unretained(this)));
+  delegate_observer_.Add(GetContentClient()->browser()->GetHidDelegate());
 }
 
 HidService::~HidService() {
@@ -114,6 +117,49 @@ void HidService::DecrementActiveFrameCount() {
   web_contents_impl->DecrementHidActiveFrameCount();
 }
 
+void HidService::OnDeviceAdded(
+    const device::mojom::HidDeviceInfo& device_info) {
+  if (!GetContentClient()->browser()->GetHidDelegate()->HasDevicePermission(
+          WebContents::FromRenderFrameHost(render_frame_host()), origin(),
+          device_info)) {
+    return;
+  }
+
+  for (auto& client : clients_)
+    client->DeviceAdded(device_info.Clone());
+}
+
+void HidService::OnDeviceRemoved(
+    const device::mojom::HidDeviceInfo& device_info) {
+  if (!GetContentClient()->browser()->GetHidDelegate()->HasDevicePermission(
+          WebContents::FromRenderFrameHost(render_frame_host()), origin(),
+          device_info)) {
+    return;
+  }
+
+  for (auto& client : clients_)
+    client->DeviceRemoved(device_info.Clone());
+}
+
+void HidService::OnHidManagerConnectionError() {
+  // Close the connection with Blink.
+  clients_.Clear();
+
+  // Remove self from HidDelegate's observer list.
+  delegate_observer_.RemoveAll();
+}
+
+void HidService::OnPermissionRevoked(const url::Origin& requesting_origin,
+                                     const url::Origin& embedding_origin) {
+  if (requesting_origin_ != requesting_origin ||
+      embedding_origin_ != embedding_origin) {
+    return;
+  }
+
+  // TODO(mattreynolds): Close connection between Blink and the device if the
+  // device lost permission.
+}
+
 void HidService::FinishGetDevices(
     GetDevicesCallback callback,
     std::vector<device::mojom::HidDeviceInfoPtr> devices) {

content/browser/hid/hid_service.h

@@ -11,10 +11,13 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
 #include "content/public/browser/frame_service_base.h"
+#include "content/public/browser/hid_delegate.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
+#include "mojo/public/cpp/bindings/remote_set.h"
 #include "services/device/public/mojom/hid.mojom.h"
 #include "third_party/blink/public/mojom/hid/hid.mojom.h"
 
@@ -26,8 +29,12 @@ class RenderFrameHost;
 // HidService provides an implementation of the HidService mojom interface. This
 // interface is used by Blink to implement the WebHID API.
 class HidService : public content::FrameServiceBase<blink::mojom::HidService>,
-                   public device::mojom::HidConnectionWatcher {
+                   public device::mojom::HidConnectionWatcher,
+                   public HidDelegate::Observer {
  public:
+  HidService(HidService&) = delete;
+  HidService& operator=(HidService&) = delete;
+
   static void Create(RenderFrameHost*,
                      mojo::PendingReceiver<blink::mojom::HidService>);
 
@@ -39,6 +46,14 @@ class HidService : public content::FrameServiceBase<blink::mojom::HidService>,
                mojo::PendingRemote<device::mojom::HidConnectionClient> client,
                ConnectCallback callback) override;
 
+  // HidDelegate::Observer:
+  void OnDeviceAdded(const device::mojom::HidDeviceInfo& device_info) override;
+  void OnDeviceRemoved(
+      const device::mojom::HidDeviceInfo& device_info) override;
+  void OnHidManagerConnectionError() override;
+  void OnPermissionRevoked(const url::Origin& requesting_origin,
+                           const url::Origin& embedding_origin) override;
+
  private:
   HidService(RenderFrameHost*, mojo::PendingReceiver<blink::mojom::HidService>);
   ~HidService() override;
@@ -57,14 +72,19 @@ class HidService : public content::FrameServiceBase<blink::mojom::HidService>,
 
   // The last shown HID chooser UI.
   std::unique_ptr<HidChooser> chooser_;
+  url::Origin requesting_origin_;
+  url::Origin embedding_origin_;
+
+  // Used to bind with Blink.
+  mojo::AssociatedRemoteSet<device::mojom::HidManagerClient> clients_;
+
+  ScopedObserver<HidDelegate, HidDelegate::Observer> delegate_observer_{this};
 
   // Each pipe here watches a connection created by Connect() in order to notify
   // the WebContentsImpl when an active connection indicator should be shown.
   mojo::ReceiverSet<device::mojom::HidConnectionWatcher> watchers_;
 
   base::WeakPtrFactory<HidService> weak_factory_{this};
-
-  DISALLOW_COPY_AND_ASSIGN(HidService);
 };
 
 }  // namespace content

content/browser/hid/hid_service_unittest.cc

@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
+#include <vector>
+
 #include "base/command_line.h"
 #include "base/test/bind_test_util.h"
 #include "content/browser/hid/hid_test_utils.h"
@@ -30,6 +33,8 @@ const char kCrossOriginTestUrl[] = "https://www.chromium.org";
 class FakeHidConnectionClient : public device::mojom::HidConnectionClient {
  public:
   FakeHidConnectionClient() = default;
+  FakeHidConnectionClient(FakeHidConnectionClient&) = delete;
+  FakeHidConnectionClient& operator=(FakeHidConnectionClient&) = delete;
   ~FakeHidConnectionClient() override = default;
 
   void Bind(
@@ -43,17 +48,37 @@ class FakeHidConnectionClient : public device::mojom::HidConnectionClient {
 
  private:
   mojo::Receiver<device::mojom::HidConnectionClient> receiver_{this};
+};
+
+class MockHidManagerClient : public device::mojom::HidManagerClient {
+ public:
+  MockHidManagerClient() = default;
+  MockHidManagerClient(MockHidManagerClient&) = delete;
+  MockHidManagerClient& operator=(MockHidManagerClient&) = delete;
+  ~MockHidManagerClient() override = default;
+
+  void Bind(mojo::PendingAssociatedReceiver<device::mojom::HidManagerClient>
+                receiver) {
+    receiver_.Bind(std::move(receiver));
+  }
+
+  MOCK_METHOD1(DeviceAdded, void(device::mojom::HidDeviceInfoPtr device_info));
+  MOCK_METHOD1(DeviceRemoved,
+               void(device::mojom::HidDeviceInfoPtr device_info));
 
-  DISALLOW_COPY_AND_ASSIGN(FakeHidConnectionClient);
+ private:
+  mojo::AssociatedReceiver<device::mojom::HidManagerClient> receiver_{this};
 };
 
+// Main test fixture.
 class HidServiceTest : public RenderViewHostImplTestHarness {
  public:
   HidServiceTest() {
     ON_CALL(hid_delegate(), GetHidManager)
         .WillByDefault(testing::Return(&hid_manager_));
   }
-
+  HidServiceTest(HidServiceTest&) = delete;
+  HidServiceTest& operator=(HidServiceTest&) = delete;
   ~HidServiceTest() override = default;
 
   void SetUp() override {
@@ -69,8 +94,17 @@ class HidServiceTest : public RenderViewHostImplTestHarness {
       SetBrowserClientForTesting(original_client_);
   }
 
+  void ConnectDevice(const device::mojom::HidDeviceInfo& device) {
+    hid_manager_.AddDevice(device.Clone());
+    hid_delegate().OnDeviceAdded(device);
+  }
+
+  void DisconnectDevice(const device::mojom::HidDeviceInfo& device) {
+    hid_manager_.RemoveDevice(device.guid);
+    hid_delegate().OnDeviceRemoved(device);
+  }
+
   MockHidDelegate& hid_delegate() { return test_client_.delegate(); }
-  device::FakeHidManager* hid_manager() { return &hid_manager_; }
   FakeHidConnectionClient* connection_client() { return &connection_client_; }
 
  private:
@@ -78,8 +112,6 @@ class HidServiceTest : public RenderViewHostImplTestHarness {
   ContentBrowserClient* original_client_ = nullptr;
   device::FakeHidManager hid_manager_;
   FakeHidConnectionClient connection_client_;
-
-  DISALLOW_COPY_AND_ASSIGN(HidServiceTest);
 };
 
 }  // namespace
@@ -93,7 +125,7 @@ TEST_F(HidServiceTest, GetDevicesWithPermission) {
 
   auto device_info = device::mojom::HidDeviceInfo::New();
   device_info->guid = kTestGuid;
-  hid_manager()->AddDevice(std::move(device_info));
+  ConnectDevice(*device_info);
 
   EXPECT_CALL(hid_delegate(), HasDevicePermission)
       .WillOnce(testing::Return(true));
@@ -118,7 +150,7 @@ TEST_F(HidServiceTest, GetDevicesWithoutPermission) {
 
   auto device_info = device::mojom::HidDeviceInfo::New();
   device_info->guid = kTestGuid;
-  hid_manager()->AddDevice(std::move(device_info));
+  ConnectDevice(*device_info);
 
   EXPECT_CALL(hid_delegate(), HasDevicePermission)
       .WillOnce(testing::Return(false));
@@ -145,7 +177,7 @@ TEST_F(HidServiceTest, RequestDevice) {
   device_info->guid = kTestGuid;
   std::vector<device::mojom::HidDeviceInfoPtr> device_infos;
   device_infos.push_back(device_info.Clone());
-  hid_manager()->AddDevice(std::move(device_info));
+  ConnectDevice(*device_info);
 
   EXPECT_CALL(hid_delegate(), CanRequestDevicePermission)
       .WillOnce(testing::Return(true));
@@ -175,7 +207,7 @@ TEST_F(HidServiceTest, OpenAndCloseHidConnection) {
 
   auto device_info = device::mojom::HidDeviceInfo::New();
   device_info->guid = kTestGuid;
-  hid_manager()->AddDevice(std::move(device_info));
+  ConnectDevice(*device_info);
 
   mojo::PendingRemote<device::mojom::HidConnectionClient> hid_connection_client;
   connection_client()->Bind(
@@ -217,7 +249,7 @@ TEST_F(HidServiceTest, OpenAndNavigateCrossOrigin) {
 
   auto device_info = device::mojom::HidDeviceInfo::New();
   device_info->guid = kTestGuid;
-  hid_manager()->AddDevice(std::move(device_info));
+  ConnectDevice(*device_info);
 
   mojo::PendingRemote<device::mojom::HidConnectionClient> hid_connection_client;
   connection_client()->Bind(

content/browser/hid/hid_test_utils.cc

@@ -23,6 +23,26 @@ std::unique_ptr<HidChooser> MockHidDelegate::RunChooser(
   return nullptr;
 }
 
+void MockHidDelegate::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void MockHidDelegate::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void MockHidDelegate::OnDeviceAdded(
+    const device::mojom::HidDeviceInfo& device) {
+  for (auto& observer : observer_list_)
+    observer.OnDeviceAdded(device);
+}
+
+void MockHidDelegate::OnDeviceRemoved(
+    const device::mojom::HidDeviceInfo& device) {
+  for (auto& observer : observer_list_)
+    observer.OnDeviceRemoved(device);
+}
+
 HidTestContentBrowserClient::HidTestContentBrowserClient() = default;
 
 HidTestContentBrowserClient::~HidTestContentBrowserClient() = default;

content/browser/hid/hid_test_utils.h

@@ -5,6 +5,9 @@
 #ifndef CONTENT_BROWSER_HID_HID_TEST_UTILS_H_
 #define CONTENT_BROWSER_HID_HID_TEST_UTILS_H_
 
+#include <memory>
+#include <vector>
+
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/hid_delegate.h"
 #include "content/public/browser/web_contents.h"
@@ -19,6 +22,8 @@ namespace content {
 class MockHidDelegate : public HidDelegate {
  public:
   MockHidDelegate();
+  MockHidDelegate(MockHidDelegate&) = delete;
+  MockHidDelegate& operator=(MockHidDelegate&) = delete;
   ~MockHidDelegate() override;
 
   // Simulates opening the HID device chooser dialog and selecting an item. The
@@ -29,6 +34,14 @@ class MockHidDelegate : public HidDelegate {
       std::vector<blink::mojom::HidDeviceFilterPtr> filters,
       HidChooser::Callback callback) override;
 
+  void AddObserver(Observer* observer) override;
+  void RemoveObserver(Observer* observer) override;
+
+  // MockHidDelegate does not register to receive device connection events. Use
+  // these methods to broadcast device connections to all delegate observers.
+  void OnDeviceAdded(const device::mojom::HidDeviceInfo& device);
+  void OnDeviceRemoved(const device::mojom::HidDeviceInfo& device);
+
   MOCK_METHOD0(RunChooserInternal,
                std::vector<device::mojom::HidDeviceInfoPtr>());
   MOCK_METHOD2(CanRequestDevicePermission,
@@ -42,7 +55,7 @@ class MockHidDelegate : public HidDelegate {
                device::mojom::HidManager*(content::WebContents* web_contents));
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(MockHidDelegate);
+  base::ObserverList<Observer> observer_list_;
 };
 
 // Test implementation of ContentBrowserClient for HID tests. The test client
@@ -50,6 +63,8 @@ class MockHidDelegate : public HidDelegate {
 class HidTestContentBrowserClient : public ContentBrowserClient {
  public:
   HidTestContentBrowserClient();
+  HidTestContentBrowserClient(HidTestContentBrowserClient&) = delete;
+  HidTestContentBrowserClient& operator=(HidTestContentBrowserClient&) = delete;
   ~HidTestContentBrowserClient() override;
 
   MockHidDelegate& delegate() { return delegate_; }
@@ -59,8 +74,6 @@ class HidTestContentBrowserClient : public ContentBrowserClient {
 
  private:
   MockHidDelegate delegate_;
-
-  DISALLOW_COPY_AND_ASSIGN(HidTestContentBrowserClient);
 };
 
 }  // namespace content

content/public/browser/hid_delegate.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <vector>
 
+#include "base/observer_list_types.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/hid_chooser.h"
 #include "services/device/public/mojom/hid.mojom-forward.h"
@@ -26,6 +27,18 @@ class CONTENT_EXPORT HidDelegate {
  public:
   virtual ~HidDelegate() = default;
 
+  class Observer : public base::CheckedObserver {
+   public:
+    // Events forwarded from HidChooserContext::DeviceObserver:
+    virtual void OnDeviceAdded(const device::mojom::HidDeviceInfo&) = 0;
+    virtual void OnDeviceRemoved(const device::mojom::HidDeviceInfo&) = 0;
+    virtual void OnHidManagerConnectionError() = 0;
+
+    // Event forwarded from permissions::ChooserContextBase::PermissionObserver:
+    virtual void OnPermissionRevoked(const url::Origin& requesting_origin,
+                                     const url::Origin& embedding_origin) = 0;
+  };
+
   // Shows a chooser for the user to select a HID device. |callback| will be
   // run when the prompt is closed. Deleting the returned object will cancel the
   // prompt.
@@ -58,6 +71,9 @@ class CONTENT_EXPORT HidDelegate {
   // possible.
   virtual device::mojom::HidManager* GetHidManager(
       WebContents* web_contents) = 0;
+
+  virtual void AddObserver(Observer* observer) = 0;
+  virtual void RemoveObserver(Observer* observer) = 0;
 };
 
 }  // namespace content

services/device/public/cpp/hid/fake_hid_manager.cc

@@ -92,7 +92,7 @@ void FakeHidConnection::SendFeatureReport(uint8_t report_id,
 }
 
 // Implementation of FakeHidManager.
-FakeHidManager::FakeHidManager() {}
+FakeHidManager::FakeHidManager() = default;
 FakeHidManager::~FakeHidManager() = default;
 
 void FakeHidManager::Bind(mojo::PendingReceiver<mojom::HidManager> receiver) {
@@ -105,6 +105,9 @@ void FakeHidManager::GetDevicesAndSetClient(
     GetDevicesCallback callback) {
   GetDevices(std::move(callback));
 
+  if (!client.is_valid())
+    return;
+
   clients_.Add(std::move(client));
 }
 
@@ -201,4 +204,9 @@ void FakeHidManager::RemoveDevice(const std::string& guid) {
   }
 }
 
+void FakeHidManager::SimulateConnectionError() {
+  clients_.Clear();
+  receivers_.Clear();
+}
+
 }  // namespace device

services/device/public/cpp/hid/fake_hid_manager.h

@@ -20,6 +20,8 @@ namespace device {
 class FakeHidConnection : public mojom::HidConnection {
  public:
   explicit FakeHidConnection(mojom::HidDeviceInfoPtr device);
+  FakeHidConnection(FakeHidConnection&) = delete;
+  FakeHidConnection& operator=(FakeHidConnection&) = delete;
   ~FakeHidConnection() override;
 
   // mojom::HidConnection implementation:
@@ -41,6 +43,8 @@ class FakeHidConnection : public mojom::HidConnection {
 class FakeHidManager : public mojom::HidManager {
  public:
   FakeHidManager();
+  FakeHidManager(FakeHidManager&) = delete;
+  FakeHidManager& operator=(FakeHidManager&) = delete;
   ~FakeHidManager() override;
 
   void Bind(mojo::PendingReceiver<mojom::HidManager> receiver);
@@ -74,6 +78,7 @@ class FakeHidManager : public mojom::HidManager {
       uint16_t usage);
   void AddDevice(mojom::HidDeviceInfoPtr device);
   void RemoveDevice(const std::string& guid);
+  void SimulateConnectionError();
 
  private:
   std::map<std::string, mojom::HidDeviceInfoPtr> devices_;