From 887b5a7dc7ec7020310db210685f920a12c6c033 Mon Sep 17 00:00:00 2001 From: John Kleinschmidt Date: Mon, 23 May 2022 10:33:39 -0400 Subject: [PATCH] fix: crash on navigator.serial.getPorts() (#34280) * fix: crash on navigator.serial.getPorts() * test: fixup BrowserWindow.setTitlebarOverlay test --- shell/browser/electron_permission_manager.cc | 22 +++++++++---- spec-main/api-browser-window-spec.ts | 31 +++++++++++------- spec-main/chromium-spec.ts | 33 ++++++++++++++++++-- 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/shell/browser/electron_permission_manager.cc b/shell/browser/electron_permission_manager.cc index 6970369b80432..a6235561a1aae 100644 --- a/shell/browser/electron_permission_manager.cc +++ b/shell/browser/electron_permission_manager.cc @@ -335,22 +335,32 @@ bool ElectronPermissionManager::CheckDevicePermission( static_cast( 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 index 4d348bb924c47..0de2068f9590c 100644 --- 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 index ca93d5e0954bd..a69e1b4cc433f 100644 --- 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,