Browse Source

feat: add support for HIDDevice.forget() (#34210)

* feat: add support for HIDDevice.forget()

* chore: remove whitespace

* chore: use `SetGetter` to serialize the render_frame_host

Co-authored-by: Samuel Maddock <[email protected]>

* fixup chore: use `SetGetter` to serialize the render_frame_host

* fixup after rebase

* fixup for crash on navigator.serial.getPorts()

* fixup for lint

Co-authored-by: Samuel Maddock <[email protected]>
John Kleinschmidt 2 years ago
parent
commit
ba573f5583

+ 13 - 0
docs/api/session.md

@@ -274,6 +274,19 @@ from `select-hid-device` is called.  This event is intended for use when using
 a UI to ask users to pick a device so that the UI can be updated to remove the
 specified device.
 
+#### Event: 'hid-device-revoked'
+
+Returns:
+
+* `event` Event
+* `details` Object
+  * `device` [HIDDevice[]](structures/hid-device.md)
+  * `frame` [WebFrameMain](web-frame-main.md)
+
+Emitted after `HIDDevice.forget()` has been called.  This event can be used
+to help maintain persistent storage of permissions when
+`setDevicePermissionHandler` is used.
+
 #### Event: 'select-serial-port'
 
 Returns:

+ 1 - 0
filenames.gni

@@ -566,6 +566,7 @@ filenames = {
     "shell/common/gin_converters/gfx_converter.h",
     "shell/common/gin_converters/guid_converter.h",
     "shell/common/gin_converters/gurl_converter.h",
+    "shell/common/gin_converters/hid_device_info_converter.h",
     "shell/common/gin_converters/image_converter.cc",
     "shell/common/gin_converters/image_converter.h",
     "shell/common/gin_converters/message_box_converter.cc",

+ 102 - 12
shell/browser/api/electron_api_web_contents.cc

@@ -3445,37 +3445,127 @@ v8::Local<v8::Promise> WebContents::TakeHeapSnapshot(
 void WebContents::GrantDevicePermission(
     const url::Origin& origin,
     const base::Value* device,
-    blink::PermissionType permissionType,
+    blink::PermissionType permission_type,
     content::RenderFrameHost* render_frame_host) {
-  granted_devices_[render_frame_host->GetFrameTreeNodeId()][permissionType]
+  granted_devices_[render_frame_host->GetFrameTreeNodeId()][permission_type]
                   [origin]
                       .push_back(
                           std::make_unique<base::Value>(device->Clone()));
 }
 
-std::vector<base::Value> WebContents::GetGrantedDevices(
+void WebContents::RevokeDevicePermission(
     const url::Origin& origin,
-    blink::PermissionType permissionType,
+    const base::Value* device,
+    blink::PermissionType permission_type,
+    content::RenderFrameHost* render_frame_host) {
+  const auto& devices_for_frame_host_it =
+      granted_devices_.find(render_frame_host->GetFrameTreeNodeId());
+  if (devices_for_frame_host_it == granted_devices_.end())
+    return;
+
+  const auto& current_devices_it =
+      devices_for_frame_host_it->second.find(permission_type);
+  if (current_devices_it == devices_for_frame_host_it->second.end())
+    return;
+
+  const auto& origin_devices_it = current_devices_it->second.find(origin);
+  if (origin_devices_it == current_devices_it->second.end())
+    return;
+
+  for (auto it = origin_devices_it->second.begin();
+       it != origin_devices_it->second.end();) {
+    if (DoesDeviceMatch(device, it->get(), permission_type)) {
+      it = origin_devices_it->second.erase(it);
+    } else {
+      ++it;
+    }
+  }
+}
+
+bool WebContents::DoesDeviceMatch(const base::Value* device,
+                                  const base::Value* device_to_compare,
+                                  blink::PermissionType permission_type) {
+  if (permission_type ==
+      static_cast<blink::PermissionType>(
+          WebContentsPermissionHelper::PermissionType::HID)) {
+    if (device->GetDict().FindInt(kHidVendorIdKey) !=
+            device_to_compare->GetDict().FindInt(kHidVendorIdKey) ||
+        device->GetDict().FindInt(kHidProductIdKey) !=
+            device_to_compare->GetDict().FindInt(kHidProductIdKey)) {
+      return false;
+    }
+
+    const auto* serial_number =
+        device_to_compare->GetDict().FindString(kHidSerialNumberKey);
+    const auto* device_serial_number =
+        device->GetDict().FindString(kHidSerialNumberKey);
+
+    if (serial_number && device_serial_number &&
+        *device_serial_number == *serial_number)
+      return true;
+  } else if (permission_type ==
+             static_cast<blink::PermissionType>(
+                 WebContentsPermissionHelper::PermissionType::SERIAL)) {
+#if BUILDFLAG(IS_WIN)
+    const auto* instance_id =
+        device->GetDict().FindString(kDeviceInstanceIdKey);
+    const auto* port_instance_id =
+        device_to_compare->GetDict().FindString(kDeviceInstanceIdKey);
+    if (instance_id && port_instance_id && *instance_id == *port_instance_id)
+      return true;
+#else
+    const auto* serial_number = device->GetDict().FindString(kSerialNumberKey);
+    const auto* port_serial_number =
+        device_to_compare->GetDict().FindString(kSerialNumberKey);
+    if (device->GetDict().FindInt(kVendorIdKey) !=
+            device_to_compare->GetDict().FindInt(kVendorIdKey) ||
+        device->GetDict().FindInt(kProductIdKey) !=
+            device_to_compare->GetDict().FindInt(kProductIdKey) ||
+        (serial_number && port_serial_number &&
+         *port_serial_number != *serial_number)) {
+      return false;
+    }
+
+#if BUILDFLAG(IS_MAC)
+    const auto* usb_driver_key = device->GetDict().FindString(kUsbDriverKey);
+    const auto* port_usb_driver_key =
+        device_to_compare->GetDict().FindString(kUsbDriverKey);
+    if (usb_driver_key && port_usb_driver_key &&
+        *usb_driver_key != *port_usb_driver_key) {
+      return false;
+    }
+#endif  // BUILDFLAG(IS_MAC)
+    return true;
+#endif  // BUILDFLAG(IS_WIN)
+  }
+  return false;
+}
+
+bool WebContents::CheckDevicePermission(
+    const url::Origin& origin,
+    const base::Value* device,
+    blink::PermissionType permission_type,
     content::RenderFrameHost* render_frame_host) {
   const auto& devices_for_frame_host_it =
       granted_devices_.find(render_frame_host->GetFrameTreeNodeId());
   if (devices_for_frame_host_it == granted_devices_.end())
-    return {};
+    return false;
 
   const auto& current_devices_it =
-      devices_for_frame_host_it->second.find(permissionType);
+      devices_for_frame_host_it->second.find(permission_type);
   if (current_devices_it == devices_for_frame_host_it->second.end())
-    return {};
+    return false;
 
   const auto& origin_devices_it = current_devices_it->second.find(origin);
   if (origin_devices_it == current_devices_it->second.end())
-    return {};
+    return false;
 
-  std::vector<base::Value> results;
-  for (const auto& object : origin_devices_it->second)
-    results.push_back(object->Clone());
+  for (const auto& device_to_compare : origin_devices_it->second) {
+    if (DoesDeviceMatch(device, device_to_compare.get(), permission_type))
+      return true;
+  }
 
-  return results;
+  return false;
 }
 
 void WebContents::UpdatePreferredSize(content::WebContents* web_contents,

+ 15 - 4
shell/browser/api/electron_api_web_contents.h

@@ -443,13 +443,20 @@ class WebContents : public ExclusiveAccessContext,
                              blink::PermissionType permissionType,
                              content::RenderFrameHost* render_frame_host);
 
+  // Revokes |origin| access to |device|.
+  // To be used in place of ObjectPermissionContextBase::RevokeObjectPermission.
+  void RevokeDevicePermission(const url::Origin& origin,
+                              const base::Value* device,
+                              blink::PermissionType permission_type,
+                              content::RenderFrameHost* render_frame_host);
+
   // Returns the list of devices that |origin| has been granted permission to
   // access. To be used in place of
   // ObjectPermissionContextBase::GetGrantedObjects.
-  std::vector<base::Value> GetGrantedDevices(
-      const url::Origin& origin,
-      blink::PermissionType permissionType,
-      content::RenderFrameHost* render_frame_host);
+  bool CheckDevicePermission(const url::Origin& origin,
+                             const base::Value* device,
+                             blink::PermissionType permissionType,
+                             content::RenderFrameHost* render_frame_host);
 
   // disable copy
   WebContents(const WebContents&) = delete;
@@ -746,6 +753,10 @@ class WebContents : public ExclusiveAccessContext,
   // Update the html fullscreen flag in both browser and renderer.
   void UpdateHtmlApiFullscreen(bool fullscreen);
 
+  bool DoesDeviceMatch(const base::Value* device,
+                       const base::Value* device_to_compare,
+                       blink::PermissionType permission_type);
+
   v8::Global<v8::Value> session_;
   v8::Global<v8::Value> devtools_web_contents_;
   v8::Global<v8::Value> debugger_;

+ 15 - 62
shell/browser/electron_permission_manager.cc

@@ -21,8 +21,6 @@
 #include "shell/browser/api/electron_api_web_contents.h"
 #include "shell/browser/electron_browser_client.h"
 #include "shell/browser/electron_browser_main_parts.h"
-#include "shell/browser/hid/hid_chooser_context.h"
-#include "shell/browser/serial/serial_chooser_context.h"
 #include "shell/browser/web_contents_permission_helper.h"
 #include "shell/browser/web_contents_preferences.h"
 #include "shell/common/gin_converters/content_converter.h"
@@ -308,66 +306,8 @@ bool ElectronPermissionManager::CheckDevicePermission(
   api::WebContents* api_web_contents = api::WebContents::From(web_contents);
   if (device_permission_handler_.is_null()) {
     if (api_web_contents) {
-      std::vector<base::Value> granted_devices =
-          api_web_contents->GetGrantedDevices(origin, permission,
-                                              render_frame_host);
-
-      for (const auto& granted_device : granted_devices) {
-        if (permission ==
-            static_cast<blink::PermissionType>(
-                WebContentsPermissionHelper::PermissionType::HID)) {
-          if (device->FindIntKey(kHidVendorIdKey) !=
-                  granted_device.FindIntKey(kHidVendorIdKey) ||
-              device->FindIntKey(kHidProductIdKey) !=
-                  granted_device.FindIntKey(kHidProductIdKey)) {
-            continue;
-          }
-
-          const auto* serial_number =
-              granted_device.FindStringKey(kHidSerialNumberKey);
-          const auto* device_serial_number =
-              device->FindStringKey(kHidSerialNumberKey);
-
-          if (serial_number && device_serial_number &&
-              *device_serial_number == *serial_number)
-            return true;
-        } else if (permission ==
-                   static_cast<blink::PermissionType>(
-                       WebContentsPermissionHelper::PermissionType::SERIAL)) {
-#if BUILDFLAG(IS_WIN)
-          const auto* instance_id = device->FindStringKey(kDeviceInstanceIdKey);
-          const auto* port_instance_id =
-              granted_device.FindStringKey(kDeviceInstanceIdKey);
-          if (instance_id && port_instance_id &&
-              *instance_id == *port_instance_id)
-            return true;
-#else
-          const auto* serial_number =
-              granted_device.FindStringKey(kSerialNumberKey);
-          const auto* port_serial_number =
-              device->FindStringKey(kSerialNumberKey);
-          if (device->FindIntKey(kVendorIdKey) !=
-                  granted_device.FindIntKey(kVendorIdKey) ||
-              device->FindIntKey(kProductIdKey) !=
-                  granted_device.FindIntKey(kProductIdKey) ||
-              (serial_number && port_serial_number &&
-               *port_serial_number != *serial_number)) {
-            continue;
-          }
-
-#if BUILDFLAG(IS_MAC)
-          const auto* usb_driver_key = device->FindStringKey(kUsbDriverKey);
-          const auto* port_usb_driver_key =
-              granted_device.FindStringKey(kUsbDriverKey);
-          if (usb_driver_key && port_usb_driver_key &&
-              *usb_driver_key != *port_usb_driver_key) {
-            continue;
-          }
-#endif  // BUILDFLAG(IS_MAC)
-          return true;
-#endif  // BUILDFLAG(IS_WIN)
-        }
-      }
+      return api_web_contents->CheckDevicePermission(origin, device, permission,
+                                                     render_frame_host);
     }
     return false;
   } else {
@@ -398,6 +338,19 @@ void ElectronPermissionManager::GrantDevicePermission(
   }
 }
 
+void ElectronPermissionManager::RevokeDevicePermission(
+    blink::PermissionType permission,
+    const url::Origin& origin,
+    const base::Value* device,
+    content::RenderFrameHost* render_frame_host) const {
+  auto* web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host);
+  api::WebContents* api_web_contents = api::WebContents::From(web_contents);
+  if (api_web_contents)
+    api_web_contents->RevokeDevicePermission(origin, device, permission,
+                                             render_frame_host);
+}
+
 blink::mojom::PermissionStatus
 ElectronPermissionManager::GetPermissionStatusForFrame(
     blink::PermissionType permission,

+ 6 - 0
shell/browser/electron_permission_manager.h

@@ -103,6 +103,12 @@ class ElectronPermissionManager : public content::PermissionControllerDelegate {
                              const base::Value* object,
                              content::RenderFrameHost* render_frame_host) const;
 
+  void RevokeDevicePermission(
+      blink::PermissionType permission,
+      const url::Origin& origin,
+      const base::Value* object,
+      content::RenderFrameHost* render_frame_host) const;
+
  protected:
   void OnPermissionResponse(int request_id,
                             int permission_id,

+ 5 - 2
shell/browser/hid/electron_hid_delegate.cc

@@ -80,8 +80,11 @@ bool ElectronHidDelegate::HasDevicePermission(
 void ElectronHidDelegate::RevokeDevicePermission(
     content::RenderFrameHost* render_frame_host,
     const device::mojom::HidDeviceInfo& device) {
-  // TODO(jkleinsc) implement this for
-  // https://chromium-review.googlesource.com/c/chromium/src/+/3297868
+  auto* chooser_context = GetChooserContext(render_frame_host);
+  const auto& origin =
+      render_frame_host->GetMainFrame()->GetLastCommittedOrigin();
+  return chooser_context->RevokeDevicePermission(origin, device,
+                                                 render_frame_host);
 }
 
 device::mojom::HidManager* ElectronHidDelegate::GetHidManager(

+ 66 - 0
shell/browser/hid/hid_chooser_context.cc

@@ -19,7 +19,13 @@
 #include "content/public/browser/device_service.h"
 #include "services/device/public/cpp/hid/hid_blocklist.h"
 #include "services/device/public/cpp/hid/hid_switches.h"
+#include "shell/browser/api/electron_api_session.h"
 #include "shell/browser/web_contents_permission_helper.h"
+#include "shell/common/gin_converters/content_converter.h"
+#include "shell/common/gin_converters/frame_converter.h"
+#include "shell/common/gin_converters/hid_device_info_converter.h"
+#include "shell/common/gin_converters/value_converter.h"
+#include "shell/common/gin_helper/dictionary.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace electron {
@@ -99,6 +105,66 @@ void HidChooserContext::GrantDevicePermission(
   }
 }
 
+void HidChooserContext::RevokeDevicePermission(
+    const url::Origin& origin,
+    const device::mojom::HidDeviceInfo& device,
+    content::RenderFrameHost* render_frame_host) {
+  DCHECK(base::Contains(devices_, device.guid));
+  if (CanStorePersistentEntry(device)) {
+    RevokePersistentDevicePermission(origin, device, render_frame_host);
+  } else {
+    RevokeEphemeralDevicePermission(origin, device);
+  }
+  auto* web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host);
+
+  api::Session* session =
+      api::Session::FromBrowserContext(web_contents->GetBrowserContext());
+  if (session) {
+    v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
+    v8::HandleScope scope(isolate);
+    gin_helper::Dictionary details =
+        gin_helper::Dictionary::CreateEmpty(isolate);
+    details.Set("device", device.Clone());
+    details.SetGetter("frame", render_frame_host);
+    session->Emit("hid-device-revoked", details);
+  }
+}
+
+void HidChooserContext::RevokePersistentDevicePermission(
+    const url::Origin& origin,
+    const device::mojom::HidDeviceInfo& device,
+    content::RenderFrameHost* render_frame_host) {
+  auto* web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host);
+  auto* permission_helper =
+      WebContentsPermissionHelper::FromWebContents(web_contents);
+  permission_helper->RevokeHIDDevicePermission(
+      origin, DeviceInfoToValue(device), render_frame_host);
+  RevokeEphemeralDevicePermission(origin, device);
+}
+
+void HidChooserContext::RevokeEphemeralDevicePermission(
+    const url::Origin& origin,
+    const device::mojom::HidDeviceInfo& device) {
+  auto it = ephemeral_devices_.find(origin);
+  if (it != ephemeral_devices_.end()) {
+    std::set<std::string>& devices = it->second;
+    for (auto guid = devices.begin(); guid != devices.end();) {
+      DCHECK(base::Contains(devices_, *guid));
+
+      if (devices_[*guid]->physical_device_id != device.physical_device_id) {
+        ++guid;
+        continue;
+      }
+
+      guid = devices.erase(guid);
+      if (devices.empty())
+        ephemeral_devices_.erase(it);
+    }
+  }
+}
+
 bool HidChooserContext::HasDevicePermission(
     const url::Origin& origin,
     const device::mojom::HidDeviceInfo& device,

+ 12 - 0
shell/browser/hid/hid_chooser_context.h

@@ -76,6 +76,9 @@ class HidChooserContext : public KeyedService,
   void GrantDevicePermission(const url::Origin& origin,
                              const device::mojom::HidDeviceInfo& device,
                              content::RenderFrameHost* render_frame_host);
+  void RevokeDevicePermission(const url::Origin& origin,
+                              const device::mojom::HidDeviceInfo& device,
+                              content::RenderFrameHost* render_frame_host);
   bool HasDevicePermission(const url::Origin& origin,
                            const device::mojom::HidDeviceInfo& device,
                            content::RenderFrameHost* render_frame_host);
@@ -111,6 +114,15 @@ class HidChooserContext : public KeyedService,
       std::vector<device::mojom::HidDeviceInfoPtr> devices);
   void OnHidManagerConnectionError();
 
+  // HID-specific interface for revoking device permissions.
+  void RevokePersistentDevicePermission(
+      const url::Origin& origin,
+      const device::mojom::HidDeviceInfo& device,
+      content::RenderFrameHost* render_frame_host);
+  void RevokeEphemeralDevicePermission(
+      const url::Origin& origin,
+      const device::mojom::HidDeviceInfo& device);
+
   ElectronBrowserContext* browser_context_;
 
   bool is_initialized_ = false;

+ 14 - 27
shell/browser/hid/hid_chooser_controller.cc

@@ -20,6 +20,7 @@
 #include "shell/browser/javascript_environment.h"
 #include "shell/common/gin_converters/callback_converter.h"
 #include "shell/common/gin_converters/content_converter.h"
+#include "shell/common/gin_converters/hid_device_info_converter.h"
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_includes.h"
@@ -28,18 +29,6 @@
 
 namespace {
 
-std::string PhysicalDeviceIdFromDeviceInfo(
-    const device::mojom::HidDeviceInfo& device) {
-  // A single physical device may expose multiple HID interfaces, each
-  // represented by a HidDeviceInfo object. When a device exposes multiple
-  // HID interfaces, the HidDeviceInfo objects will share a common
-  // |physical_device_id|. Group these devices so that a single chooser item
-  // is shown for each physical device. If a device's physical device ID is
-  // empty, use its GUID instead.
-  return device.physical_device_id.empty() ? device.guid
-                                           : device.physical_device_id;
-}
-
 bool FilterMatch(const blink::mojom::HidDeviceFilterPtr& filter,
                  const device::mojom::HidDeviceInfo& device) {
   if (filter->device_ids) {
@@ -83,21 +72,6 @@ bool FilterMatch(const blink::mojom::HidDeviceFilterPtr& filter,
 
 }  // namespace
 
-namespace gin {
-
-template <>
-struct Converter<device::mojom::HidDeviceInfoPtr> {
-  static v8::Local<v8::Value> ToV8(
-      v8::Isolate* isolate,
-      const device::mojom::HidDeviceInfoPtr& device) {
-    base::Value value = electron::HidChooserContext::DeviceInfoToValue(*device);
-    value.SetStringKey("deviceId", PhysicalDeviceIdFromDeviceInfo(*device));
-    return gin::ConvertToV8(isolate, value);
-  }
-};
-
-}  // namespace gin
-
 namespace electron {
 
 HidChooserController::HidChooserController(
@@ -131,6 +105,19 @@ HidChooserController::~HidChooserController() {
     std::move(callback_).Run(std::vector<device::mojom::HidDeviceInfoPtr>());
 }
 
+// static
+std::string HidChooserController::PhysicalDeviceIdFromDeviceInfo(
+    const device::mojom::HidDeviceInfo& device) {
+  // A single physical device may expose multiple HID interfaces, each
+  // represented by a HidDeviceInfo object. When a device exposes multiple
+  // HID interfaces, the HidDeviceInfo objects will share a common
+  // |physical_device_id|. Group these devices so that a single chooser item
+  // is shown for each physical device. If a device's physical device ID is
+  // empty, use its GUID instead.
+  return device.physical_device_id.empty() ? device.guid
+                                           : device.physical_device_id;
+}
+
 api::Session* HidChooserController::GetSession() {
   if (!web_contents()) {
     return nullptr;

+ 4 - 0
shell/browser/hid/hid_chooser_controller.h

@@ -53,6 +53,10 @@ class HidChooserController
   HidChooserController& operator=(HidChooserController&) = delete;
   ~HidChooserController() override;
 
+  // static
+  static std::string PhysicalDeviceIdFromDeviceInfo(
+      const device::mojom::HidDeviceInfo& device);
+
   // HidChooserContext::DeviceObserver:
   void OnDeviceAdded(const device::mojom::HidDeviceInfo& device_info) override;
   void OnDeviceRemoved(

+ 20 - 0
shell/browser/web_contents_permission_helper.cc

@@ -107,6 +107,17 @@ void WebContentsPermissionHelper::GrantDevicePermission(
                                             render_frame_host);
 }
 
+void WebContentsPermissionHelper::RevokeDevicePermission(
+    blink::PermissionType permission,
+    const url::Origin& origin,
+    const base::Value* device,
+    content::RenderFrameHost* render_frame_host) const {
+  auto* permission_manager = static_cast<ElectronPermissionManager*>(
+      web_contents_->GetBrowserContext()->GetPermissionControllerDelegate());
+  permission_manager->RevokeDevicePermission(permission, origin, device,
+                                             render_frame_host);
+}
+
 void WebContentsPermissionHelper::RequestFullscreenPermission(
     base::OnceCallback<void(bool)> callback) {
   RequestPermission(
@@ -230,6 +241,15 @@ void WebContentsPermissionHelper::GrantHIDDevicePermission(
       render_frame_host);
 }
 
+void WebContentsPermissionHelper::RevokeHIDDevicePermission(
+    const url::Origin& origin,
+    base::Value device,
+    content::RenderFrameHost* render_frame_host) const {
+  return RevokeDevicePermission(
+      static_cast<blink::PermissionType>(PermissionType::HID), origin, &device,
+      render_frame_host);
+}
+
 WEB_CONTENTS_USER_DATA_KEY_IMPL(WebContentsPermissionHelper);
 
 }  // namespace electron

+ 10 - 0
shell/browser/web_contents_permission_helper.h

@@ -68,6 +68,10 @@ class WebContentsPermissionHelper
       const url::Origin& origin,
       base::Value device,
       content::RenderFrameHost* render_frame_host) const;
+  void RevokeHIDDevicePermission(
+      const url::Origin& origin,
+      base::Value device,
+      content::RenderFrameHost* render_frame_host) const;
 
  private:
   explicit WebContentsPermissionHelper(content::WebContents* web_contents);
@@ -91,6 +95,12 @@ class WebContentsPermissionHelper
                              const base::Value* device,
                              content::RenderFrameHost* render_frame_host) const;
 
+  void RevokeDevicePermission(
+      blink::PermissionType permission,
+      const url::Origin& origin,
+      const base::Value* device,
+      content::RenderFrameHost* render_frame_host) const;
+
   // TODO(clavin): refactor to use the WebContents provided by the
   // WebContentsUserData base class instead of storing a duplicate ref
   content::WebContents* web_contents_;

+ 31 - 0
shell/common/gin_converters/hid_device_info_converter.h

@@ -0,0 +1,31 @@
+// Copyright (c) 2022 Microsoft, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ELECTRON_SHELL_COMMON_GIN_CONVERTERS_HID_DEVICE_INFO_CONVERTER_H_
+#define ELECTRON_SHELL_COMMON_GIN_CONVERTERS_HID_DEVICE_INFO_CONVERTER_H_
+
+#include "gin/converter.h"
+#include "services/device/public/mojom/hid.mojom.h"
+#include "shell/browser/hid/hid_chooser_context.h"
+#include "shell/browser/hid/hid_chooser_controller.h"
+
+namespace gin {
+
+template <>
+struct Converter<device::mojom::HidDeviceInfoPtr> {
+  static v8::Local<v8::Value> ToV8(
+      v8::Isolate* isolate,
+      const device::mojom::HidDeviceInfoPtr& device) {
+    base::Value value = electron::HidChooserContext::DeviceInfoToValue(*device);
+    value.SetStringKey(
+        "deviceId",
+        electron::HidChooserController::PhysicalDeviceIdFromDeviceInfo(
+            *device));
+    return gin::ConvertToV8(isolate, value);
+  }
+};
+
+}  // namespace gin
+
+#endif  // ELECTRON_SHELL_COMMON_GIN_CONVERTERS_HID_DEVICE_INFO_CONVERTER_H_

+ 45 - 7
spec-main/chromium-spec.ts

@@ -1918,7 +1918,7 @@ describe('navigator.hid', () => {
     serverUrl = `http://localhost:${(server.address() as any).port}`;
   });
 
-  const getDevices: any = () => {
+  const requestDevices: any = () => {
     return w.webContents.executeJavaScript(`
       navigator.hid.requestDevice({filters: []}).then(device => device.toString()).catch(err => err.toString());
     `, true);
@@ -1936,7 +1936,7 @@ describe('navigator.hid', () => {
 
   it('does not return a device if select-hid-device event is not defined', async () => {
     w.loadFile(path.join(fixturesPath, 'pages', 'blank.html'));
-    const device = await getDevices();
+    const device = await requestDevices();
     expect(device).to.equal('');
   });
 
@@ -1947,7 +1947,7 @@ describe('navigator.hid', () => {
       callback();
     });
     session.defaultSession.setPermissionCheckHandler(() => false);
-    const device = await getDevices();
+    const device = await requestDevices();
     expect(selectFired).to.be.false();
     expect(device).to.equal('');
   });
@@ -1965,7 +1965,7 @@ describe('navigator.hid', () => {
         callback();
       }
     });
-    const device = await getDevices();
+    const device = await requestDevices();
     expect(selectFired).to.be.true();
     if (haveDevices) {
       expect(device).to.contain('[object HIDDevice]');
@@ -2014,7 +2014,7 @@ describe('navigator.hid', () => {
       return true;
     });
     await w.webContents.executeJavaScript('navigator.hid.getDevices();', true);
-    const device = await getDevices();
+    const device = await requestDevices();
     expect(selectFired).to.be.true();
     if (haveDevices) {
       expect(device).to.contain('[object HIDDevice]');
@@ -2024,7 +2024,7 @@ describe('navigator.hid', () => {
     }
   });
 
-  it('returns excludes a device when a exclusionFilter is specified', async () => {
+  it('excludes a device when a exclusionFilter is specified', async () => {
     const exclusionFilters = <any>[];
     let haveDevices = false;
     let checkForExcludedDevice = false;
@@ -2053,13 +2053,51 @@ describe('navigator.hid', () => {
       callback();
     });
 
-    await getDevices();
+    await requestDevices();
     if (haveDevices) {
       // We have devices to exclude, so check if exculsionFilters work
       checkForExcludedDevice = true;
       await w.webContents.executeJavaScript(`
         navigator.hid.requestDevice({filters: [], exclusionFilters: ${JSON.stringify(exclusionFilters)}}).then(device => device.toString()).catch(err => err.toString());
+
       `, true);
     }
   });
+
+  it('supports device.forget()', async () => {
+    let deletedDeviceFromEvent;
+    let haveDevices = false;
+    w.webContents.session.on('select-hid-device', (event, details, callback) => {
+      if (details.deviceList.length > 0) {
+        haveDevices = true;
+        callback(details.deviceList[0].deviceId);
+      } else {
+        callback();
+      }
+    });
+    w.webContents.session.on('hid-device-revoked', (event, details) => {
+      deletedDeviceFromEvent = details.device;
+    });
+    await requestDevices();
+    if (haveDevices) {
+      const grantedDevices = await w.webContents.executeJavaScript('navigator.hid.getDevices()');
+      if (grantedDevices.length > 0) {
+        const deletedDevice = await w.webContents.executeJavaScript(`
+          navigator.hid.getDevices().then(devices => {
+            devices[0].forget();
+            return {
+              vendorId: devices[0].vendorId,
+              productId: devices[0].productId,
+              name: devices[0].productName
+            }
+          })
+        `);
+        const grantedDevices2 = await w.webContents.executeJavaScript('navigator.hid.getDevices()');
+        expect(grantedDevices2.length).to.be.lessThan(grantedDevices.length);
+        if (deletedDevice.name !== '' && deletedDevice.productId && deletedDevice.vendorId) {
+          expect(deletedDeviceFromEvent).to.include(deletedDevice);
+        }
+      }
+    }
+  });
 });