From 4c96ced00aaf861d1f1464fbd66282c9f617ebdb Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Wed, 5 Jun 2024 11:26:58 -0700 Subject: [PATCH] fix: WebUSB should not crash when using in-memory partitions (#42364) Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr --- shell/browser/usb/electron_usb_delegate.cc | 47 +++++++++++++++++----- shell/browser/usb/electron_usb_delegate.h | 9 +++-- spec/chromium-spec.ts | 26 ++++++++++++ 3 files changed, 68 insertions(+), 14 deletions(-) diff --git a/shell/browser/usb/electron_usb_delegate.cc b/shell/browser/usb/electron_usb_delegate.cc index a90896e0fe1f..18d833eaf9e0 100644 --- a/shell/browser/usb/electron_usb_delegate.cc +++ b/shell/browser/usb/electron_usb_delegate.cc @@ -174,6 +174,9 @@ std::unique_ptr ElectronUsbDelegate::RunChooser( bool ElectronUsbDelegate::CanRequestDevicePermission( content::BrowserContext* browser_context, const url::Origin& origin) { + if (!browser_context) + return false; + base::Value::Dict details; details.Set("securityOrigin", origin.GetURL().spec()); auto* permission_manager = static_cast( @@ -188,32 +191,46 @@ void ElectronUsbDelegate::RevokeDevicePermissionWebInitiated( content::BrowserContext* browser_context, const url::Origin& origin, const device::mojom::UsbDeviceInfo& device) { - GetChooserContext(browser_context) - ->RevokeDevicePermissionWebInitiated(origin, device); + auto* chooser_context = GetChooserContext(browser_context); + if (chooser_context) { + chooser_context->RevokeDevicePermissionWebInitiated(origin, device); + } } const device::mojom::UsbDeviceInfo* ElectronUsbDelegate::GetDeviceInfo( content::BrowserContext* browser_context, const std::string& guid) { - return GetChooserContext(browser_context)->GetDeviceInfo(guid); + auto* chooser_context = GetChooserContext(browser_context); + if (!chooser_context) + return nullptr; + return chooser_context->GetDeviceInfo(guid); } bool ElectronUsbDelegate::HasDevicePermission( content::BrowserContext* browser_context, content::RenderFrameHost* frame, const url::Origin& origin, - const device::mojom::UsbDeviceInfo& device) { - if (IsDevicePermissionAutoGranted(origin, device)) + const device::mojom::UsbDeviceInfo& device_info) { + if (IsDevicePermissionAutoGranted(origin, device_info)) return true; + auto* chooser_context = GetChooserContext(browser_context); + if (!chooser_context) + return false; + return GetChooserContext(browser_context) - ->HasDevicePermission(origin, device); + ->HasDevicePermission(origin, device_info); } void ElectronUsbDelegate::GetDevices( content::BrowserContext* browser_context, blink::mojom::WebUsbService::GetDevicesCallback callback) { - GetChooserContext(browser_context)->GetDevices(std::move(callback)); + auto* chooser_context = GetChooserContext(browser_context); + if (!chooser_context) { + std::move(callback).Run(std::vector()); + return; + } + chooser_context->GetDevices(std::move(callback)); } void ElectronUsbDelegate::GetDevice( @@ -222,25 +239,35 @@ void ElectronUsbDelegate::GetDevice( base::span blocked_interface_classes, mojo::PendingReceiver device_receiver, mojo::PendingRemote device_client) { - GetChooserContext(browser_context) - ->GetDevice(guid, blocked_interface_classes, std::move(device_receiver), - std::move(device_client)); + auto* chooser_context = GetChooserContext(browser_context); + if (chooser_context) { + chooser_context->GetDevice(guid, blocked_interface_classes, + std::move(device_receiver), + std::move(device_client)); + } } void ElectronUsbDelegate::AddObserver(content::BrowserContext* browser_context, Observer* observer) { + if (!browser_context) + return; + GetContextObserver(browser_context)->AddObserver(observer); } void ElectronUsbDelegate::RemoveObserver( content::BrowserContext* browser_context, Observer* observer) { + if (!browser_context) + return; + GetContextObserver(browser_context)->RemoveObserver(observer); } ElectronUsbDelegate::ContextObservation* ElectronUsbDelegate::GetContextObserver( content::BrowserContext* browser_context) { + CHECK(browser_context); if (!base::Contains(observations_, browser_context)) { observations_.emplace(browser_context, std::make_unique( this, browser_context)); diff --git a/shell/browser/usb/electron_usb_delegate.h b/shell/browser/usb/electron_usb_delegate.h index 9984e9337867..89afd16da3d4 100644 --- a/shell/browser/usb/electron_usb_delegate.h +++ b/shell/browser/usb/electron_usb_delegate.h @@ -56,10 +56,11 @@ class ElectronUsbDelegate : public content::UsbDelegate { const device::mojom::UsbDeviceInfo* GetDeviceInfo( content::BrowserContext* browser_context, const std::string& guid) override; - bool HasDevicePermission(content::BrowserContext* browser_context, - content::RenderFrameHost* frame, - const url::Origin& origin, - const device::mojom::UsbDeviceInfo& device) override; + bool HasDevicePermission( + content::BrowserContext* browser_context, + content::RenderFrameHost* frame, + const url::Origin& origin, + const device::mojom::UsbDeviceInfo& device_info) override; void GetDevices( content::BrowserContext* browser_context, blink::mojom::WebUsbService::GetDevicesCallback callback) override; diff --git a/spec/chromium-spec.ts b/spec/chromium-spec.ts index 7a549f764a38..5ffe2578c63f 100644 --- a/spec/chromium-spec.ts +++ b/spec/chromium-spec.ts @@ -3678,18 +3678,44 @@ describe('navigator.usb', () => { `, true); }; + const getDevices: any = () => { + return w.webContents.executeJavaScript(` + navigator.usb.getDevices().then(devices => devices.map(device => device.toString())).catch(err => err.toString()); + `, true); + }; + const notFoundError = 'NotFoundError: Failed to execute \'requestDevice\' on \'USB\': No device selected.'; after(() => { server.close(); closeAllWindows(); }); + afterEach(() => { session.defaultSession.setPermissionCheckHandler(null); session.defaultSession.setDevicePermissionHandler(null); session.defaultSession.removeAllListeners('select-usb-device'); }); + it('does not crash when using in-memory partitions', async () => { + const sesWin = new BrowserWindow({ + webPreferences: { + partition: 'test-partition' + } + }); + + await sesWin.loadFile(path.join(fixturesPath, 'pages', 'blank.html')); + server = http.createServer((req, res) => { + res.setHeader('Content-Type', 'text/html'); + res.end(''); + }); + + serverUrl = (await listen(server)).url; + + const devices = await getDevices(); + expect(devices).to.be.an('array').that.is.empty(); + }); + it('does not return a device if select-usb-device event is not defined', async () => { w.loadFile(path.join(fixturesPath, 'pages', 'blank.html')); const device = await requestDevices();