feat: support `serialPort.forget()` (#36062)

feat: enable serialPort.revoke()

Co-authored-by: Shelley Vohr <[email protected]>

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>

docs/api/session.md

@@ -385,6 +385,50 @@ callback from `select-serial-port` is called.  This event is intended for use
 when using a UI to ask users to pick a port so that the UI can be updated
 to remove the specified port.
 
+#### Event: 'serial-port-revoked'
+
+Returns:
+
+* `event` Event
+* `details` Object
+  * `port` [SerialPort](structures/serial-port.md)
+  * `frame` [WebFrameMain](web-frame-main.md)
+  * `origin` string - The origin that the device has been revoked from.
+
+Emitted after `SerialPort.forget()` has been called.  This event can be used
+to help maintain persistent storage of permissions when `setDevicePermissionHandler` is used.
+
+```js
+// Browser Process
+const { app, BrowserWindow } = require('electron')
+
+app.whenReady().then(() => {
+  const win = new BrowserWindow({
+    width: 800,
+    height: 600
+  })
+
+  win.webContents.session.on('serial-port-revoked', (event, details) => {
+    console.log(`Access revoked for serial device from origin ${details.origin}`)
+  })
+})
+```
+
+```js
+// Renderer Process
+
+const portConnect = async () => {
+  // Request a port.
+  const port = await navigator.serial.requestPort()
+
+  // Wait for the serial port to open.
+  await port.open({ baudRate: 9600 })
+
+  // ...later, revoke access to the serial port.
+  await port.forget()
+}
+```
+
 ### Instance Methods
 
 The following methods are available on instances of `Session`:

filenames.gni

@@ -582,6 +582,7 @@ filenames = {
     "shell/common/gin_converters/native_window_converter.h",
     "shell/common/gin_converters/net_converter.cc",
     "shell/common/gin_converters/net_converter.h",
+    "shell/common/gin_converters/serial_port_info_converter.h",
     "shell/common/gin_converters/std_converter.h",
     "shell/common/gin_converters/time_converter.cc",
     "shell/common/gin_converters/time_converter.h",

shell/browser/serial/electron_serial_delegate.cc

@@ -61,6 +61,21 @@ bool ElectronSerialDelegate::HasPortPermission(
       frame);
 }
 
+void ElectronSerialDelegate::RevokePortPermissionWebInitiated(
+    content::RenderFrameHost* frame,
+    const base::UnguessableToken& token) {
+  auto* web_contents = content::WebContents::FromRenderFrameHost(frame);
+  return GetChooserContext(frame)->RevokePortPermissionWebInitiated(
+      web_contents->GetPrimaryMainFrame()->GetLastCommittedOrigin(), token,
+      frame);
+}
+
+const device::mojom::SerialPortInfo* ElectronSerialDelegate::GetPortInfo(
+    content::RenderFrameHost* frame,
+    const base::UnguessableToken& token) {
+  return GetChooserContext(frame)->GetPortInfo(token);
+}
+
 device::mojom::SerialPortManager* ElectronSerialDelegate::GetPortManager(
     content::RenderFrameHost* frame) {
   return GetChooserContext(frame)->GetPortManager();
@@ -81,18 +96,6 @@ void ElectronSerialDelegate::RemoveObserver(
   observer_list_.RemoveObserver(observer);
 }
 
-void ElectronSerialDelegate::RevokePortPermissionWebInitiated(
-    content::RenderFrameHost* frame,
-    const base::UnguessableToken& token) {
-  // TODO(nornagon/jkleinsc): pass this on to the chooser context
-}
-
-const device::mojom::SerialPortInfo* ElectronSerialDelegate::GetPortInfo(
-    content::RenderFrameHost* frame,
-    const base::UnguessableToken& token) {
-  return GetChooserContext(frame)->GetPortInfo(token);
-}
-
 SerialChooserController* ElectronSerialDelegate::ControllerForFrame(
     content::RenderFrameHost* render_frame_host) {
   auto mapping = controller_map_.find(render_frame_host);

shell/browser/serial/electron_serial_delegate.h

@@ -36,18 +36,18 @@ class ElectronSerialDelegate : public content::SerialDelegate,
   bool CanRequestPortPermission(content::RenderFrameHost* frame) override;
   bool HasPortPermission(content::RenderFrameHost* frame,
                          const device::mojom::SerialPortInfo& port) override;
-  device::mojom::SerialPortManager* GetPortManager(
-      content::RenderFrameHost* frame) override;
-  void AddObserver(content::RenderFrameHost* frame,
-                   content::SerialDelegate::Observer* observer) override;
-  void RemoveObserver(content::RenderFrameHost* frame,
-                      content::SerialDelegate::Observer* observer) override;
   void RevokePortPermissionWebInitiated(
       content::RenderFrameHost* frame,
       const base::UnguessableToken& token) override;
   const device::mojom::SerialPortInfo* GetPortInfo(
       content::RenderFrameHost* frame,
       const base::UnguessableToken& token) override;
+  device::mojom::SerialPortManager* GetPortManager(
+      content::RenderFrameHost* frame) override;
+  void AddObserver(content::RenderFrameHost* frame,
+                   Observer* observer) override;
+  void RemoveObserver(content::RenderFrameHost* frame,
+                      Observer* observer) override;
 
   void DeleteControllerForFrame(content::RenderFrameHost* render_frame_host);
 

shell/browser/serial/serial_chooser_context.cc

@@ -15,14 +15,16 @@
 #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/api/electron_api_session.h"
 #include "shell/browser/electron_permission_manager.h"
 #include "shell/browser/web_contents_permission_helper.h"
+#include "shell/common/gin_converters/frame_converter.h"
+#include "shell/common/gin_converters/serial_port_info_converter.h"
 
 namespace electron {
 
 constexpr char kPortNameKey[] = "name";
 constexpr char kTokenKey[] = "token";
-
 #if BUILDFLAG(IS_WIN)
 const char kDeviceInstanceIdKey[] = "device_instance_id";
 #else
@@ -56,35 +58,35 @@ base::UnguessableToken DecodeToken(base::StringPiece input) {
 }
 
 base::Value PortInfoToValue(const device::mojom::SerialPortInfo& port) {
-  base::Value value(base::Value::Type::DICTIONARY);
+  base::Value::Dict value;
   if (port.display_name && !port.display_name->empty())
-    value.SetStringKey(kPortNameKey, *port.display_name);
+    value.Set(kPortNameKey, *port.display_name);
   else
-    value.SetStringKey(kPortNameKey, port.path.LossyDisplayName());
+    value.Set(kPortNameKey, port.path.LossyDisplayName());
 
   if (!SerialChooserContext::CanStorePersistentEntry(port)) {
-    value.SetStringKey(kTokenKey, EncodeToken(port.token));
-    return value;
+    value.Set(kTokenKey, EncodeToken(port.token));
+    return base::Value(std::move(value));
   }
 
 #if BUILDFLAG(IS_WIN)
   // Windows provides a handy device identifier which we can rely on to be
   // sufficiently stable for identifying devices across restarts.
-  value.SetStringKey(kDeviceInstanceIdKey, port.device_instance_id);
+  value.Set(kDeviceInstanceIdKey, port.device_instance_id);
 #else
   DCHECK(port.has_vendor_id);
-  value.SetIntKey(kVendorIdKey, port.vendor_id);
+  value.Set(kVendorIdKey, port.vendor_id);
   DCHECK(port.has_product_id);
-  value.SetIntKey(kProductIdKey, port.product_id);
+  value.Set(kProductIdKey, port.product_id);
   DCHECK(port.serial_number);
-  value.SetStringKey(kSerialNumberKey, *port.serial_number);
+  value.Set(kSerialNumberKey, *port.serial_number);
 
 #if BUILDFLAG(IS_MAC)
   DCHECK(port.usb_driver_name && !port.usb_driver_name->empty());
-  value.SetStringKey(kUsbDriverKey, *port.usb_driver_name);
+  value.Set(kUsbDriverKey, *port.usb_driver_name);
 #endif  // BUILDFLAG(IS_MAC)
 #endif  // BUILDFLAG(IS_WIN)
-  return value;
+  return base::Value(std::move(value));
 }
 
 SerialChooserContext::SerialChooserContext(ElectronBrowserContext* context)
@@ -105,18 +107,33 @@ void SerialChooserContext::GrantPortPermission(
     content::RenderFrameHost* render_frame_host) {
   port_info_.insert({port.token, port.Clone()});
 
-  auto* permission_manager = static_cast<ElectronPermissionManager*>(
-      browser_context_->GetPermissionControllerDelegate());
-  return permission_manager->GrantDevicePermission(
-      static_cast<blink::PermissionType>(
-          WebContentsPermissionHelper::PermissionType::SERIAL),
-      origin, PortInfoToValue(port), browser_context_);
+  if (CanStorePersistentEntry(port)) {
+    auto* permission_manager = static_cast<ElectronPermissionManager*>(
+        browser_context_->GetPermissionControllerDelegate());
+    permission_manager->GrantDevicePermission(
+        static_cast<blink::PermissionType>(
+            WebContentsPermissionHelper::PermissionType::SERIAL),
+        origin, PortInfoToValue(port), browser_context_);
+    return;
+  }
+
+  ephemeral_ports_[origin].insert(port.token);
 }
 
 bool SerialChooserContext::HasPortPermission(
     const url::Origin& origin,
     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;
+    if (base::Contains(ports, port.token))
+      return true;
+  }
+
+  if (!CanStorePersistentEntry(port))
+    return false;
+
   auto* permission_manager = static_cast<ElectronPermissionManager*>(
       browser_context_->GetPermissionControllerDelegate());
   return permission_manager->CheckDevicePermission(
@@ -127,10 +144,39 @@ bool SerialChooserContext::HasPortPermission(
 
 void SerialChooserContext::RevokePortPermissionWebInitiated(
     const url::Origin& origin,
-    const base::UnguessableToken& token) {
+    const base::UnguessableToken& token,
+    content::RenderFrameHost* render_frame_host) {
   auto it = port_info_.find(token);
-  if (it == port_info_.end())
-    return;
+  if (it != port_info_.end()) {
+    auto* permission_manager = static_cast<ElectronPermissionManager*>(
+        browser_context_->GetPermissionControllerDelegate());
+    permission_manager->RevokeDevicePermission(
+        static_cast<blink::PermissionType>(
+            WebContentsPermissionHelper::PermissionType::SERIAL),
+        origin, PortInfoToValue(*it->second), browser_context_);
+  }
+
+  auto ephemeral = ephemeral_ports_.find(origin);
+  if (ephemeral != ephemeral_ports_.end()) {
+    std::set<base::UnguessableToken>& ports = ephemeral->second;
+    ports.erase(token);
+  }
+
+  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("port", it->second);
+    details.SetGetter("frame", render_frame_host);
+    details.Set("origin", origin.Serialize());
+    session->Emit("serial-port-revoked", details);
+  }
 }
 
 // static
@@ -195,6 +241,11 @@ void SerialChooserContext::OnPortAdded(device::mojom::SerialPortInfoPtr port) {
   if (!base::Contains(port_info_, port->token))
     port_info_.insert({port->token, port->Clone()});
 
+  for (auto& map_entry : ephemeral_ports_) {
+    std::set<base::UnguessableToken>& ports = map_entry.second;
+    ports.erase(port->token);
+  }
+
   for (auto& observer : port_observer_list_)
     observer.OnPortAdded(*port);
 }
@@ -239,6 +290,8 @@ void SerialChooserContext::OnGetDevices(
 void SerialChooserContext::OnPortManagerConnectionError() {
   port_manager_.reset();
   client_receiver_.reset();
-}
 
+  port_info_.clear();
+  ephemeral_ports_.clear();
+}
 }  // namespace electron

shell/browser/serial/serial_chooser_context.h

@@ -67,9 +67,19 @@ class SerialChooserContext : public KeyedService,
   bool HasPortPermission(const url::Origin& origin,
                          const device::mojom::SerialPortInfo& port,
                          content::RenderFrameHost* render_frame_host);
+  void RevokePortPermissionWebInitiated(
+      const url::Origin& origin,
+      const base::UnguessableToken& token,
+      content::RenderFrameHost* render_frame_host);
   static bool CanStorePersistentEntry(
       const device::mojom::SerialPortInfo& port);
 
+  // Only call this if you're sure |port_info_| has been initialized
+  // before-hand. The returned raw pointer is owned by |port_info_| and will be
+  // destroyed when the port is removed.
+  const device::mojom::SerialPortInfo* GetPortInfo(
+      const base::UnguessableToken& token);
+
   device::mojom::SerialPortManager* GetPortManager();
 
   void AddPortObserver(PortObserver* observer);
@@ -77,21 +87,9 @@ class SerialChooserContext : public KeyedService,
 
   base::WeakPtr<SerialChooserContext> AsWeakPtr();
 
-  bool is_initialized_ = false;
-
-  // Map from port token to port info.
-  std::map<base::UnguessableToken, device::mojom::SerialPortInfoPtr> port_info_;
-
   // SerialPortManagerClient implementation.
   void OnPortAdded(device::mojom::SerialPortInfoPtr port) override;
   void OnPortRemoved(device::mojom::SerialPortInfoPtr port) override;
-  void RevokePortPermissionWebInitiated(const url::Origin& origin,
-                                        const base::UnguessableToken& token);
-  // Only call this if you're sure |port_info_| has been initialized
-  // before-hand. The returned raw pointer is owned by |port_info_| and will be
-  // destroyed when the port is removed.
-  const device::mojom::SerialPortInfo* GetPortInfo(
-      const base::UnguessableToken& token);
 
  private:
   void EnsurePortManagerConnection();
@@ -99,9 +97,14 @@ class SerialChooserContext : public KeyedService,
       mojo::PendingRemote<device::mojom::SerialPortManager> manager);
   void OnGetDevices(std::vector<device::mojom::SerialPortInfoPtr> ports);
   void OnPortManagerConnectionError();
-  void RevokeObjectPermissionInternal(const url::Origin& origin,
-                                      const base::Value& object,
-                                      bool revoked_by_website);
+
+  bool is_initialized_ = false;
+
+  // Tracks the set of ports to which an origin has access to.
+  std::map<url::Origin, std::set<base::UnguessableToken>> ephemeral_ports_;
+
+  // Map from port token to port info.
+  std::map<base::UnguessableToken, device::mojom::SerialPortInfoPtr> port_info_;
 
   mojo::Remote<device::mojom::SerialPortManager> port_manager_;
   mojo::Receiver<device::mojom::SerialPortManagerClient> client_receiver_{this};

shell/common/gin_converters/serial_port_info_converter.h

@@ -0,0 +1,43 @@
+// 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_SERIAL_PORT_INFO_CONVERTER_H_
+#define ELECTRON_SHELL_COMMON_GIN_CONVERTERS_SERIAL_PORT_INFO_CONVERTER_H_
+
+#include "gin/converter.h"
+#include "shell/common/gin_helper/dictionary.h"
+#include "third_party/blink/public/mojom/serial/serial.mojom.h"
+
+namespace gin {
+
+template <>
+struct Converter<device::mojom::SerialPortInfoPtr> {
+  static v8::Local<v8::Value> ToV8(
+      v8::Isolate* isolate,
+      const device::mojom::SerialPortInfoPtr& port) {
+    gin_helper::Dictionary dict = gin::Dictionary::CreateEmpty(isolate);
+    dict.Set("portId", port->token.ToString());
+    dict.Set("portName", port->path.BaseName().LossyDisplayName());
+    if (port->display_name && !port->display_name->empty())
+      dict.Set("displayName", *port->display_name);
+    if (port->has_vendor_id)
+      dict.Set("vendorId", base::StringPrintf("%u", port->vendor_id));
+    if (port->has_product_id)
+      dict.Set("productId", base::StringPrintf("%u", port->product_id));
+    if (port->serial_number && !port->serial_number->empty())
+      dict.Set("serialNumber", *port->serial_number);
+#if BUILDFLAG(IS_MAC)
+    if (port->usb_driver_name && !port->usb_driver_name->empty())
+      dict.Set("usbDriverName", *port->usb_driver_name);
+#elif BUILDFLAG(IS_WIN)
+    if (!port->device_instance_id.empty())
+      dict.Set("deviceInstanceId", port->device_instance_id);
+#endif
+    return gin::ConvertToV8(isolate, dict);
+  }
+};
+
+}  // namespace gin
+
+#endif  // ELECTRON_SHELL_COMMON_GIN_CONVERTERS_SERIAL_PORT_INFO_CONVERTER_H_

spec/chromium-spec.ts

@@ -2423,6 +2423,50 @@ describe('navigator.serial', () => {
       expect(grantedPorts).to.not.be.empty();
     }
   });
+
+  it('supports port.forget()', async () => {
+    let forgottenPortFromEvent = {};
+    let havePorts = false;
+
+    w.webContents.session.on('select-serial-port', (event, portList, webContents, callback) => {
+      if (portList.length > 0) {
+        havePorts = true;
+        callback(portList[0].portId);
+      } else {
+        callback('');
+      }
+    });
+
+    w.webContents.session.on('serial-port-revoked', (event, details) => {
+      forgottenPortFromEvent = details.port;
+    });
+
+    await getPorts();
+    if (havePorts) {
+      const grantedPorts = await w.webContents.executeJavaScript('navigator.serial.getPorts()');
+      if (grantedPorts.length > 0) {
+        const forgottenPort = await w.webContents.executeJavaScript(`
+          navigator.serial.getPorts().then(async(ports) => {
+            const portInfo = await ports[0].getInfo();
+            await ports[0].forget();
+            if (portInfo.usbVendorId && portInfo.usbProductId) {
+              return {
+                vendorId: '' + portInfo.usbVendorId,
+                productId: '' + portInfo.usbProductId
+              }
+            } else {
+              return {};
+            }
+          })
+        `);
+        const grantedPorts2 = await w.webContents.executeJavaScript('navigator.serial.getPorts()');
+        expect(grantedPorts2.length).to.be.lessThan(grantedPorts.length);
+        if (forgottenPort.vendorId && forgottenPort.productId) {
+          expect(forgottenPortFromEvent).to.include(forgottenPort);
+        }
+      }
+    }
+  });
 });
 
 describe('navigator.clipboard', () => {