feat: add support for HIDDevice.forget() (electron#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]>
zljj0818 · May 23, 2022 · ba573f5 · ba573f5
1 parent df9383c
commit ba573f5
Show file tree

Hide file tree

Showing 15 changed files with 359 additions and 114 deletions.
diff --git a/docs/api/session.md b/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:

diff --git a/filenames.gni b/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",

diff --git a/shell/browser/api/electron_api_web_contents.cc b/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,

diff --git a/shell/browser/api/electron_api_web_contents.h b/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_;

diff --git a/shell/browser/electron_permission_manager.cc b/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,

diff --git a/shell/browser/electron_permission_manager.h b/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,

diff --git a/shell/browser/hid/electron_hid_delegate.cc b/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(