chore: refactor persisting permission granted to serial ports (#31181)

shell/browser/electron_browser_context.cc

@@ -8,8 +8,6 @@
 
 #include <utility>
 
-#include <vector>
-
 #include "base/barrier_closure.h"
 #include "base/base_paths.h"
 #include "base/command_line.h"
@@ -95,9 +93,6 @@ std::string MakePartitionName(const std::string& input) {
 
 }  // namespace
 
-const char kSerialGrantedDevicesPref[] =
-    "profile.content_settings.exceptions.serial-chooser-data";
-
 // static
 ElectronBrowserContext::BrowserContextMap&
 ElectronBrowserContext::browser_context_map() {
@@ -194,7 +189,6 @@ void ElectronBrowserContext::InitPrefs() {
   registry->RegisterFilePathPref(prefs::kDownloadDefaultDirectory,
                                  download_dir);
   registry->RegisterDictionaryPref(prefs::kDevToolsFileSystemPaths);
-  registry->RegisterDictionaryPref(kSerialGrantedDevicesPref);
   InspectableWebContents::RegisterPrefs(registry.get());
   MediaDeviceIDSalt::RegisterPrefs(registry.get());
   ZoomLevelDelegate::RegisterPrefs(registry.get());
@@ -420,54 +414,6 @@ void ElectronBrowserContext::SetSSLConfigClient(
   ssl_config_client_ = std::move(client);
 }
 
-void ElectronBrowserContext::GrantObjectPermission(
-    const url::Origin& origin,
-    base::Value object,
-    const std::string& pref_key) {
-  std::string origin_string = origin.Serialize();
-  DictionaryPrefUpdate update(prefs(), pref_key);
-  base::Value* const current_objects = update.Get();
-  if (!current_objects || !current_objects->is_dict()) {
-    base::ListValue objects_for_origin;
-    objects_for_origin.Append(std::move(object));
-    base::DictionaryValue objects_by_origin;
-    objects_by_origin.SetPath(origin_string, std::move(objects_for_origin));
-    prefs()->Set(pref_key, std::move(objects_by_origin));
-  } else {
-    base::Value* const objects_mutable =
-        current_objects->FindListKey(origin_string);
-    if (objects_mutable) {
-      base::Value::ListStorage objects = std::move(*objects_mutable).TakeList();
-      objects.push_back(std::move(object));
-      *objects_mutable = base::Value(std::move(objects));
-    } else {
-      base::Value new_objects(base::Value::Type::LIST);
-      new_objects.Append(std::move(object));
-      current_objects->SetKey(origin_string, std::move(new_objects));
-    }
-  }
-}
-
-std::vector<std::unique_ptr<base::Value>>
-ElectronBrowserContext::GetGrantedObjects(const url::Origin& origin,
-                                          const std::string& pref_key) {
-  auto* current_objects = prefs()->Get(pref_key);
-  if (!current_objects || !current_objects->is_dict()) {
-    return {};
-  }
-
-  const base::Value* objects_for_origin =
-      current_objects->FindPath(origin.Serialize());
-  if (!objects_for_origin)
-    return {};
-
-  std::vector<std::unique_ptr<base::Value>> results;
-  for (const auto& object : objects_for_origin->GetList())
-    results.push_back(std::make_unique<base::Value>(object.Clone()));
-
-  return results;
-}
-
 // static
 ElectronBrowserContext* ElectronBrowserContext::From(
     const std::string& partition,

shell/browser/electron_browser_context.h

@@ -8,7 +8,6 @@
 #include <map>
 #include <memory>
 #include <string>
-#include <vector>
 
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/predictors/preconnect_manager.h"
@@ -47,9 +46,6 @@ class ResolveProxyHelper;
 class WebViewManager;
 class ProtocolRegistry;
 
-// Preference keys for device apis
-extern const char kSerialGrantedDevicesPref[];
-
 class ElectronBrowserContext : public content::BrowserContext {
  public:
   // partition_id => browser_context
@@ -146,19 +142,6 @@ class ElectronBrowserContext : public content::BrowserContext {
   network::mojom::SSLConfigPtr GetSSLConfig();
   void SetSSLConfigClient(mojo::Remote<network::mojom::SSLConfigClient> client);
 
-  // Grants |origin| access to |object| by writing it into the browser context.
-  // To be used in place of ObjectPermissionContextBase::GrantObjectPermission.
-  void GrantObjectPermission(const url::Origin& origin,
-                             base::Value object,
-                             const std::string& pref_key);
-
-  // Returns the list of objects that |origin| has been granted permission to
-  // access. To be used in place of
-  // ObjectPermissionContextBase::GetGrantedObjects.
-  std::vector<std::unique_ptr<base::Value>> GetGrantedObjects(
-      const url::Origin& origin,
-      const std::string& pref_key);
-
   ~ElectronBrowserContext() override;
 
  private:

shell/browser/electron_permission_manager.cc

@@ -22,6 +22,7 @@
 #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"
@@ -328,6 +329,31 @@ bool ElectronPermissionManager::CheckDevicePermission(
           if (serial_number && device_serial_number &&
               *device_serial_number == *serial_number)
             return true;
+        } else if (permission ==
+                   static_cast<content::PermissionType>(
+                       WebContentsPermissionHelper::PermissionType::SERIAL)) {
+#if defined(OS_WIN)
+          if (device->FindStringKey(kDeviceInstanceIdKey) ==
+              granted_device.FindStringKey(kDeviceInstanceIdKey))
+            return true;
+#else
+          if (device->FindIntKey(kVendorIdKey) !=
+                  granted_device.FindIntKey(kVendorIdKey) ||
+              device->FindIntKey(kProductIdKey) !=
+                  granted_device.FindIntKey(kProductIdKey) ||
+              *device->FindStringKey(kSerialNumberKey) !=
+                  *granted_device.FindStringKey(kSerialNumberKey)) {
+            continue;
+          }
+
+#if defined(OS_MAC)
+          if (*device->FindStringKey(kUsbDriverKey) !=
+              *granted_device.FindStringKey(kUsbDriverKey)) {
+            continue;
+          }
+#endif  // defined(OS_MAC)
+          return true;
+#endif  // defined(OS_WIN)
         }
       }
     }

shell/browser/serial/electron_serial_delegate.cc

@@ -60,7 +60,7 @@ bool ElectronSerialDelegate::HasPortPermission(
   auto* chooser_context =
       SerialChooserContextFactory::GetForBrowserContext(browser_context);
   return chooser_context->HasPortPermission(
-      web_contents->GetMainFrame()->GetLastCommittedOrigin(), port);
+      web_contents->GetMainFrame()->GetLastCommittedOrigin(), port, frame);
 }
 
 device::mojom::SerialPortManager* ElectronSerialDelegate::GetPortManager(

shell/browser/serial/serial_chooser_context.cc

@@ -13,22 +13,25 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "content/public/browser/device_service.h"
+#include "content/public/browser/web_contents.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
+#include "shell/browser/web_contents_permission_helper.h"
 
 namespace electron {
 
 constexpr char kPortNameKey[] = "name";
 constexpr char kTokenKey[] = "token";
+
 #if defined(OS_WIN)
-constexpr char kDeviceInstanceIdKey[] = "device_instance_id";
+const char kDeviceInstanceIdKey[] = "device_instance_id";
 #else
-constexpr char kVendorIdKey[] = "vendor_id";
-constexpr char kProductIdKey[] = "product_id";
-constexpr char kSerialNumberKey[] = "serial_number";
+const char kVendorIdKey[] = "vendor_id";
+const char kProductIdKey[] = "product_id";
+const char kSerialNumberKey[] = "serial_number";
 #if defined(OS_MAC)
-constexpr char kUsbDriverKey[] = "usb_driver";
+const char kUsbDriverKey[] = "usb_driver";
 #endif  // defined(OS_MAC)
-#endif  // defined(OS_WIN)
+#endif  // defined(OS_WIN
 
 std::string EncodeToken(const base::UnguessableToken& token) {
   const uint64_t data[2] = {token.GetHighForSerialization(),
@@ -83,21 +86,24 @@ base::Value PortInfoToValue(const device::mojom::SerialPortInfo& port) {
   return value;
 }
 
-SerialChooserContext::SerialChooserContext(
-    ElectronBrowserContext* browser_context)
-    : browser_context_(browser_context) {}
+SerialChooserContext::SerialChooserContext() = default;
 
 SerialChooserContext::~SerialChooserContext() = default;
 
 void SerialChooserContext::GrantPortPermission(
     const url::Origin& origin,
-    const device::mojom::SerialPortInfo& port) {
+    const device::mojom::SerialPortInfo& port,
+    content::RenderFrameHost* render_frame_host) {
   base::Value value = PortInfoToValue(port);
   port_info_.insert({port.token, value.Clone()});
 
   if (CanStorePersistentEntry(port)) {
-    browser_context_->GrantObjectPermission(origin, std::move(value),
-                                            kSerialGrantedDevicesPref);
+    auto* web_contents =
+        content::WebContents::FromRenderFrameHost(render_frame_host);
+    auto* permission_helper =
+        WebContentsPermissionHelper::FromWebContents(web_contents);
+    permission_helper->GrantSerialPortPermission(origin, std::move(value),
+                                                 render_frame_host);
     return;
   }
 
@@ -106,7 +112,8 @@ void SerialChooserContext::GrantPortPermission(
 
 bool SerialChooserContext::HasPortPermission(
     const url::Origin& origin,
-    const device::mojom::SerialPortInfo& port) {
+    const device::mojom::SerialPortInfo& port,
+    content::RenderFrameHost* render_frame_host) {
   auto it = ephemeral_ports_.find(origin);
   if (it != ephemeral_ports_.end()) {
     const std::set<base::UnguessableToken> ports = it->second;
@@ -118,39 +125,13 @@ bool SerialChooserContext::HasPortPermission(
     return false;
   }
 
-  std::vector<std::unique_ptr<base::Value>> object_list =
-      browser_context_->GetGrantedObjects(origin, kSerialGrantedDevicesPref);
-  for (const auto& device : object_list) {
-#if defined(OS_WIN)
-    const std::string& device_instance_id =
-        *device->FindStringKey(kDeviceInstanceIdKey);
-    if (port.device_instance_id == device_instance_id)
-      return true;
-#else
-    const int vendor_id = *device->FindIntKey(kVendorIdKey);
-    const int product_id = *device->FindIntKey(kProductIdKey);
-    const std::string& serial_number = *device->FindStringKey(kSerialNumberKey);
-
-    // Guaranteed by the CanStorePersistentEntry) check above.
-    DCHECK(port.has_vendor_id);
-    DCHECK(port.has_product_id);
-    DCHECK(port.serial_number && !port.serial_number->empty());
-    if (port.vendor_id != vendor_id || port.product_id != product_id ||
-        port.serial_number != serial_number) {
-      continue;
-    }
-
-#if defined(OS_MAC)
-    const std::string& usb_driver_name = *device->FindStringKey(kUsbDriverKey);
-    if (port.usb_driver_name != usb_driver_name) {
-      continue;
-    }
-#endif  // defined(OS_MAC)
-
-    return true;
-#endif  // defined(OS_WIN)
-  }
-  return false;
+  auto* web_contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host);
+  auto* permission_helper =
+      WebContentsPermissionHelper::FromWebContents(web_contents);
+  base::Value value = PortInfoToValue(port);
+  return permission_helper->CheckSerialPortPermission(origin, std::move(value),
+                                                      render_frame_host);
 }
 
 // static

shell/browser/serial/serial_chooser_context.h

@@ -29,19 +29,35 @@ class Value;
 
 namespace electron {
 
+extern const char kHidVendorIdKey[];
+extern const char kHidProductIdKey[];
+
+#if defined(OS_WIN)
+extern const char kDeviceInstanceIdKey[];
+#else
+extern const char kVendorIdKey[];
+extern const char kProductIdKey[];
+extern const char kSerialNumberKey[];
+#if defined(OS_MAC)
+extern const char kUsbDriverKey[];
+#endif  // defined(OS_MAC)
+#endif  // defined(OS_WIN)
+
 class SerialChooserContext : public KeyedService,
                              public device::mojom::SerialPortManagerClient {
  public:
   using PortObserver = content::SerialDelegate::Observer;
 
-  explicit SerialChooserContext(ElectronBrowserContext* browser_context);
+  SerialChooserContext();
   ~SerialChooserContext() override;
 
   // Serial-specific interface for granting and checking permissions.
   void GrantPortPermission(const url::Origin& origin,
-                           const device::mojom::SerialPortInfo& port);
+                           const device::mojom::SerialPortInfo& port,
+                           content::RenderFrameHost* render_frame_host);
   bool HasPortPermission(const url::Origin& origin,
-                         const device::mojom::SerialPortInfo& port);
+                         const device::mojom::SerialPortInfo& port,
+                         content::RenderFrameHost* render_frame_host);
   static bool CanStorePersistentEntry(
       const device::mojom::SerialPortInfo& port);
 
@@ -62,8 +78,6 @@ class SerialChooserContext : public KeyedService,
       mojo::PendingRemote<device::mojom::SerialPortManager> manager);
   void OnPortManagerConnectionError();
 
-  ElectronBrowserContext* browser_context_;
-
   // Tracks the set of ports to which an origin has access to.
   std::map<url::Origin, std::set<base::UnguessableToken>> ephemeral_ports_;
 

shell/browser/serial/serial_chooser_context_factory.cc

@@ -19,9 +19,7 @@ SerialChooserContextFactory::~SerialChooserContextFactory() = default;
 
 KeyedService* SerialChooserContextFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
-  auto* browser_context =
-      static_cast<electron::ElectronBrowserContext*>(context);
-  return new SerialChooserContext(browser_context);
+  return new SerialChooserContext();
 }
 
 // static

shell/browser/serial/serial_chooser_controller.cc

@@ -67,7 +67,8 @@ SerialChooserController::SerialChooserController(
     : WebContentsObserver(web_contents),
       filters_(std::move(filters)),
       callback_(std::move(callback)),
-      serial_delegate_(serial_delegate) {
+      serial_delegate_(serial_delegate),
+      render_frame_host_id_(render_frame_host->GetGlobalId()) {
   origin_ = web_contents->GetMainFrame()->GetLastCommittedOrigin();
 
   chooser_context_ = SerialChooserContextFactory::GetForBrowserContext(
@@ -124,7 +125,8 @@ void SerialChooserController::OnDeviceChosen(const std::string& port_id) {
           return ptr->token.ToString() == port_id;
         });
     if (it != ports_.end()) {
-      chooser_context_->GrantPortPermission(origin_, *it->get());
+      auto* rfh = content::RenderFrameHost::FromID(render_frame_host_id_);
+      chooser_context_->GrantPortPermission(origin_, *it->get(), rfh);
       RunCallback(it->Clone());
     } else {
       RunCallback(/*port=*/nullptr);

shell/browser/serial/serial_chooser_controller.h

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/serial_chooser.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -61,6 +62,8 @@ class SerialChooserController final : public SerialChooserContext::PortObserver,
 
   base::WeakPtr<ElectronSerialDelegate> serial_delegate_;
 
+  content::GlobalRenderFrameHostId render_frame_host_id_;
+
   base::WeakPtrFactory<SerialChooserController> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SerialChooserController);

shell/browser/web_contents_permission_helper.cc

@@ -190,6 +190,24 @@ bool WebContentsPermissionHelper::CheckSerialAccessPermission(
       static_cast<content::PermissionType>(PermissionType::SERIAL), &details);
 }
 
+bool WebContentsPermissionHelper::CheckSerialPortPermission(
+    const url::Origin& origin,
+    base::Value device,
+    content::RenderFrameHost* render_frame_host) const {
+  return CheckDevicePermission(
+      static_cast<content::PermissionType>(PermissionType::SERIAL), origin,
+      &device, render_frame_host);
+}
+
+void WebContentsPermissionHelper::GrantSerialPortPermission(
+    const url::Origin& origin,
+    base::Value device,
+    content::RenderFrameHost* render_frame_host) const {
+  return GrantDevicePermission(
+      static_cast<content::PermissionType>(PermissionType::SERIAL), origin,
+      &device, render_frame_host);
+}
+
 bool WebContentsPermissionHelper::CheckHIDAccessPermission(
     const url::Origin& embedding_origin) const {
   base::DictionaryValue details;

shell/browser/web_contents_permission_helper.h

@@ -42,6 +42,14 @@ class WebContentsPermissionHelper
   bool CheckMediaAccessPermission(const GURL& security_origin,
                                   blink::mojom::MediaStreamType type) const;
   bool CheckSerialAccessPermission(const url::Origin& embedding_origin) const;
+  bool CheckSerialPortPermission(
+      const url::Origin& origin,
+      base::Value device,
+      content::RenderFrameHost* render_frame_host) const;
+  void GrantSerialPortPermission(
+      const url::Origin& origin,
+      base::Value device,
+      content::RenderFrameHost* render_frame_host) const;
   bool CheckHIDAccessPermission(const url::Origin& embedding_origin) const;
   bool CheckHIDDevicePermission(
       const url::Origin& origin,