Repository: electron/electron (language: cpp)

Issue:
[Bug]: serial.getPorts return empty even after setDevicePermissionHandler grants the permissions
### Preflight Checklist

- [X] I have read the [Contributing Guidelines](https://github.com/electron/electron/blob/main/CONTRIBUTING.md) for this project.
- [X] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/main/CODE_OF_CONDUCT.md) that this project adheres to.
- [X] I have searched the [issue tracker](https://www.github.com/electron/electron/issues) for a bug report that matches the one I want to file, without success.

### Electron Version

16.0.0;15.3.5

### What operating system are you using?

macOS

### Operating System Version

12.1

### What arch are you using?

x64

### Last Known Working Electron version

_No response_

### Expected Behavior

According v16.0.0 release notes: 
Changed Web Serial to use ses.setDevicePermissionHandler to allow developers more control over Web Serial permissioning. #31324 (Also in 15)

I expect ```navigator.serial.getPorts().then(ports=>console.log(ports))``` return all serial port without call ```navigator.serial.requestPort()``` after setDevicePermissionHandler grant the permissions.
 
```
    mainWindow.webContents.session.setPermissionCheckHandler((webContents, permission, requestingOrigin, details) => {
        return true;
    });
    mainWindow.webContents.session.setDevicePermissionHandler((details) => {
        return true;
    });
```

I expect ```navigator.serial.getPorts()``` worked like ```navigator.hid.getDevices()``` worked without call ```navigator.hid.requestDevice()```.

### Actual Behavior

```navigator.serial.getPorts().then(ports=>console.log(ports))``` return empty without call ```navigator.serial.requestPort()``` 

### Testcase Gist URL
https://gist.github.com/d9de1c981ef830835c0e05931dc5f8f4

_No response_

### Additional Information

_No response_


Discussion / hints:
cc @jkleinsc 
@belerweb what kind of serial devices do you have attached to your machine when testing?  When I attach a legitimate serial device (eg an Arduino) to my machine and run your test it works. 
It's vehicle exhaust gas detection equipment. I can't identify its manufacturer.
I don't think it has anything to do with the device.

My gist navigator.serial.requestPort() return 4 devices on my computer, but navigator.serial.getPorts() return empty if i do not call navigator.serial.requestPort() and callback with one device.

And I found that "callback" set by setPermissionCheckHandler and  setDevicePermissionHandler not execute when call navigator.serial.getPorts(). But call navigator.hid.requestDevice(), it executed.
I believe what is happening here is due to https://github.com/electron/electron/blob/main/shell/browser/serial/serial_chooser_context.cc#L163, which was copied from this upstream change: https://chromium-review.googlesource.com/c/chromium/src/+/2413176.  Given that we are giving control to the developer as to which ports are being exposed, we can probably skip this check, but there is no guarantee that those ports that fall into this scenario will actually work.

Fix patch:
diff --git a/shell/browser/serial/serial_chooser_context.cc b/shell/browser/serial/serial_chooser_context.cc
index 594c9b41694d5..49a57a4ffdf79 100644
--- a/shell/browser/serial/serial_chooser_context.cc
+++ b/shell/browser/serial/serial_chooser_context.cc
@@ -95,36 +95,18 @@ void SerialChooserContext::GrantPortPermission(
     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)) {
-    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;
-  }
-
-  ephemeral_ports_[origin].insert(port.token);
+  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);
 }
 
 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* web_contents =
       content::WebContents::FromRenderFrameHost(render_frame_host);
   auto* permission_helper =
@@ -194,8 +176,6 @@ void SerialChooserContext::OnPortRemoved(
     device::mojom::SerialPortInfoPtr port) {
   for (auto& observer : port_observer_list_)
     observer.OnPortRemoved(*port);
-
-  port_info_.erase(port->token);
 }
 
 void SerialChooserContext::EnsurePortManagerConnection() {
@@ -221,10 +201,6 @@ void SerialChooserContext::SetUpPortManagerConnection(
 void SerialChooserContext::OnPortManagerConnectionError() {
   port_manager_.reset();
   client_receiver_.reset();
-
-  port_info_.clear();
-
-  ephemeral_ports_.clear();
 }
 
 }  // namespace electron
diff --git a/shell/browser/serial/serial_chooser_context.h b/shell/browser/serial/serial_chooser_context.h
index 313ec7065c0d6..0183b5877a514 100644
--- a/shell/browser/serial/serial_chooser_context.h
+++ b/shell/browser/serial/serial_chooser_context.h
@@ -82,12 +82,6 @@ class SerialChooserContext : public KeyedService,
       mojo::PendingRemote<device::mojom::SerialPortManager> manager);
   void OnPortManagerConnectionError();
 
-  // Tracks the set of ports to which an origin has access to.
-  std::map<url::Origin, std::set<base::UnguessableToken>> ephemeral_ports_;
-
-  // Holds information about ports in |ephemeral_ports_|.
-  std::map<base::UnguessableToken, base::Value> port_info_;
-
   mojo::Remote<device::mojom::SerialPortManager> port_manager_;
   mojo::Receiver<device::mojom::SerialPortManagerClient> client_receiver_{this};
   base::ObserverList<PortObserver> port_observer_list_;


Test patch: