fix: crash on navigator.serial.getPorts() (electron#34280)

* fix: crash on navigator.serial.getPorts() * test: fixup BrowserWindow.setTitlebarOverlay test
solarwindscloud · Feb 22, 2023 · 646f616 · 646f616
1 parent 32b23f9
commit 646f616
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 20 deletions.
diff --git a/shell/browser/electron_permission_manager.cc b/shell/browser/electron_permission_manager.cc
@@ -335,22 +335,32 @@ bool ElectronPermissionManager::CheckDevicePermission(
                    static_cast<blink::PermissionType>(
                        WebContentsPermissionHelper::PermissionType::SERIAL)) {
 #if BUILDFLAG(IS_WIN)
-          if (device->FindStringKey(kDeviceInstanceIdKey) ==
-              granted_device.FindStringKey(kDeviceInstanceIdKey))
+          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) ||
-              *device->FindStringKey(kSerialNumberKey) !=
-                  *granted_device.FindStringKey(kSerialNumberKey)) {
+              (serial_number && port_serial_number &&
+               *port_serial_number != *serial_number)) {
             continue;
           }
 
 #if BUILDFLAG(IS_MAC)
-          if (*device->FindStringKey(kUsbDriverKey) !=
-              *granted_device.FindStringKey(kUsbDriverKey)) {
+          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)

diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts
@@ -2187,8 +2187,10 @@ describe('BrowserWindow module', () => {
       const [, newOverlayRect] = await geometryChange;
       expect(newOverlayRect.width).to.equal(overlayRect.width + 400);
     };
-    afterEach(closeAllWindows);
-    afterEach(() => { ipcMain.removeAllListeners('geometrychange'); });
+    afterEach(async () => {
+      await closeAllWindows();
+      ipcMain.removeAllListeners('geometrychange');
+    });
     it('creates browser window with hidden title bar', () => {
       const w = new BrowserWindow({
         show: false,
@@ -2266,16 +2268,20 @@ describe('BrowserWindow module', () => {
       // Confirm that maximization only affected the height of the buttons and not the title bar
       expect(overlayRectPostMax.height).to.equal(size);
     };
-    afterEach(closeAllWindows);
-    afterEach(() => { ipcMain.removeAllListeners('geometrychange'); });
+    afterEach(async () => {
+      await closeAllWindows();
+      ipcMain.removeAllListeners('geometrychange');
+    });
     it('sets Window Control Overlay with title bar height of 40', async () => {
       await testWindowsOverlayHeight(40);
     });
   });
 
   ifdescribe(process.platform === 'win32')('BrowserWindow.setTitlebarOverlay', () => {
-    afterEach(closeAllWindows);
-    afterEach(() => { ipcMain.removeAllListeners('geometrychange'); });
+    afterEach(async () => {
+      await closeAllWindows();
+      ipcMain.removeAllListeners('geometrychange');
+    });
 
     it('does not crash when an invalid titleBarStyle was initially set', () => {
       const win = new BrowserWindow({
@@ -2299,10 +2305,13 @@ describe('BrowserWindow module', () => {
     });
 
     it('correctly updates the height of the overlay', async () => {
-      const testOverlay = async (w: BrowserWindow, size: Number) => {
+      const testOverlay = async (w: BrowserWindow, size: Number, firstRun: boolean) => {
         const overlayHTML = path.join(__dirname, 'fixtures', 'pages', 'overlay.html');
-        w.loadFile(overlayHTML);
-        await emittedOnce(ipcMain, 'geometrychange');
+        const overlayReady = emittedOnce(ipcMain, 'geometrychange');
+        await w.loadFile(overlayHTML);
+        if (firstRun) {
+          await overlayReady;
+        }
 
         const overlayEnabled = await w.webContents.executeJavaScript('navigator.windowControlsOverlay.visible');
         expect(overlayEnabled).to.be.true('overlayEnabled');
@@ -2339,13 +2348,13 @@ describe('BrowserWindow module', () => {
         }
       });
 
-      await testOverlay(w, INITIAL_SIZE);
+      await testOverlay(w, INITIAL_SIZE, true);
 
       w.setTitleBarOverlay({
         height: INITIAL_SIZE + 10
       });
 
-      await testOverlay(w, INITIAL_SIZE + 10);
+      await testOverlay(w, INITIAL_SIZE + 10, false);
     });
   });
 

diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts
@@ -1678,11 +1678,39 @@ describe('navigator.serial', () => {
   });
 
   it('returns a port when select-serial-port event is defined', async () => {
+    let havePorts = false;
     w.webContents.session.on('select-serial-port', (event, portList, webContents, callback) => {
-      callback(portList[0].portId);
+      if (portList.length > 0) {
+        havePorts = true;
+        callback(portList[0].portId);
+      } else {
+        callback('');
+      }
     });
     const port = await getPorts();
-    expect(port).to.equal('[object SerialPort]');
+    if (havePorts) {
+      expect(port).to.equal('[object SerialPort]');
+    } else {
+      expect(port).to.equal('NotFoundError: No port selected by the user.');
+    }
+  });
+
+  it('navigator.serial.getPorts() returns values', async () => {
+    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('');
+      }
+    });
+    await getPorts();
+    if (havePorts) {
+      const grantedPorts = await w.webContents.executeJavaScript('navigator.serial.getPorts()');
+      expect(grantedPorts).to.not.be.empty();
+    }
   });
 });
 
@@ -2005,7 +2033,6 @@ describe('navigator.hid', () => {
       if (details.deviceList.length > 0) {
         details.deviceList.find((device) => {
           if (device.name && device.name !== '' && device.serialNumber && device.serialNumber !== '') {
-            console.log('device is: ', device);
             if (checkForExcludedDevice) {
               const compareDevice = {
                 vendorId: device.vendorId,