From 45e2ff9308c095bfb6950a536dc55a512bc44abe Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Thu, 5 Mar 2020 14:01:22 -0800 Subject: [PATCH] browser(firefox): fix flaky permissions in Firefox Review URL: https://github.com/aslushnikov/juggler/commit/9bd6e720056491b9edb9a6c5c638d3578dc76ea4 Wait for permissions to propagate to all context pages. Fixes #720 --- browser_patches/firefox/BUILD_NUMBER | 2 +- .../firefox/patches/bootstrap.diff | 61 ++++++++++++++++--- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/browser_patches/firefox/BUILD_NUMBER b/browser_patches/firefox/BUILD_NUMBER index b633d3d9f0b51..4fba33e90d1b6 100644 --- a/browser_patches/firefox/BUILD_NUMBER +++ b/browser_patches/firefox/BUILD_NUMBER @@ -1 +1 @@ -1036 +1037 diff --git a/browser_patches/firefox/patches/bootstrap.diff b/browser_patches/firefox/patches/bootstrap.diff index 345fdb10f8458..6b12c3fa566b2 100644 --- a/browser_patches/firefox/patches/bootstrap.diff +++ b/browser_patches/firefox/patches/bootstrap.diff @@ -489,7 +489,7 @@ index 6dca2b78830edc1ddbd66264bd332853729dac71..fbe89c9682834e11b9d9219d9eb056ed diff --git a/testing/juggler/BrowserContextManager.js b/testing/juggler/BrowserContextManager.js new file mode 100644 -index 0000000000000000000000000000000000000000..483667dbec8e4c76533e4cf5e69ca9e322f2e708 +index 0000000000000000000000000000000000000000..dfb1f50b3a6ad915b99481c987975cb99c268ed7 --- /dev/null +++ b/testing/juggler/BrowserContextManager.js @@ -0,0 +1,194 @@ @@ -593,7 +593,7 @@ index 0000000000000000000000000000000000000000..483667dbec8e4c76533e4cf5e69ca9e3 + this._principals.push(principal); + for (const permission of ALL_PERMISSIONS) { + const action = permissions.includes(permission) ? Ci.nsIPermissionManager.ALLOW_ACTION : Ci.nsIPermissionManager.DENY_ACTION; -+ Services.perms.addFromPrincipal(principal, permission, action); ++ Services.perms.addFromPrincipal(principal, permission, action, Ci.nsIPermissionManager.EXPIRE_NEVER, 0 /* expireTime */); + } + } + @@ -1628,10 +1628,10 @@ index 0000000000000000000000000000000000000000..ba34976ad05e7f5f1a99777f76ac08b1 +this.SimpleChannel = SimpleChannel; diff --git a/testing/juggler/TargetRegistry.js b/testing/juggler/TargetRegistry.js new file mode 100644 -index 0000000000000000000000000000000000000000..2cb5f24b079289f00d84d0d7b266443635edd2b2 +index 0000000000000000000000000000000000000000..1bcbafed549090fe533fb1340b2598e5962b855d --- /dev/null +++ b/testing/juggler/TargetRegistry.js -@@ -0,0 +1,257 @@ +@@ -0,0 +1,264 @@ +const {EventEmitter} = ChromeUtils.import('resource://gre/modules/EventEmitter.jsm'); +const {Helper} = ChromeUtils.import('chrome://juggler/content/Helper.js'); +const {SimpleChannel} = ChromeUtils.import('chrome://juggler/content/SimpleChannel.js'); @@ -1689,6 +1689,13 @@ index 0000000000000000000000000000000000000000..2cb5f24b079289f00d84d0d7b2664436 + Services.obs.addObserver(this, 'oop-frameloader-crashed'); + } + ++ async ensurePermissionsInContextPages(browserContextId, permissions) { ++ const browserContext = this._contextManager.browserContextForId(browserContextId); ++ const pageTargets = [...this._targets.values()].filter(target => target instanceof PageTarget); ++ const contextPages = pageTargets.filter(target => target._browserContext === browserContext); ++ await Promise.all(contextPages.map(page => page._channel.connect('').send('ensurePermissions', permissions).catch(e => void e))); ++ } ++ + async newPage({browserContextId}) { + const browserContext = this._contextManager.browserContextForId(browserContextId); + const tab = this._mainWindow.gBrowser.addTab('about:blank', { @@ -4166,10 +4173,11 @@ index 0000000000000000000000000000000000000000..3a386425d3796d0a6786dea193b3402d + diff --git a/testing/juggler/content/main.js b/testing/juggler/content/main.js new file mode 100644 -index 0000000000000000000000000000000000000000..887180f71ef78604d2756ffa6a026ac968bda276 +index 0000000000000000000000000000000000000000..212f1c1a218efebe8685b019e79fb553db720453 --- /dev/null +++ b/testing/juggler/content/main.js -@@ -0,0 +1,96 @@ +@@ -0,0 +1,129 @@ ++const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm"); +const {Helper} = ChromeUtils.import('chrome://juggler/content/Helper.js'); +const {FrameTree} = ChromeUtils.import('chrome://juggler/content/content/FrameTree.js'); +const {NetworkMonitor} = ChromeUtils.import('chrome://juggler/content/content/NetworkMonitor.js'); @@ -4178,6 +4186,13 @@ index 0000000000000000000000000000000000000000..887180f71ef78604d2756ffa6a026ac9 +const {RuntimeAgent} = ChromeUtils.import('chrome://juggler/content/content/RuntimeAgent.js'); +const {PageAgent} = ChromeUtils.import('chrome://juggler/content/content/PageAgent.js'); + ++const ALL_PERMISSIONS = [ ++ 'geo', ++ 'microphone', ++ 'camera', ++ 'desktop-notifications', ++]; ++ +const scrollbarManager = new ScrollbarManager(docShell); +let frameTree; +let networkMonitor; @@ -4246,6 +4261,31 @@ index 0000000000000000000000000000000000000000..887180f71ef78604d2756ffa6a026ac9 + frameTree.addScriptToEvaluateOnNewDocument(script); + }, + ++ async ensurePermissions(permissions) { ++ const checkPermissions = () => { ++ for (const permission of ALL_PERMISSIONS) { ++ const actual = Services.perms.testExactPermissionFromPrincipal(this._docShell.domWindow.document.nodePrincipal, permission); ++ const expected = permissions.include(permission) ? Ci.nsIPermissionManager.ALLOW_ACTION : Ci.nsIPermissionManager.DENY_ACTION; ++ if (actual !== expected) ++ return false; ++ } ++ return true; ++ } ++ ++ if (checkPermissions()) ++ return; ++ ++ // Track all 'perm-changed' events and wait until permissions are expected. ++ await new Promise(resolve => { ++ const listeners = [helper.addObserver(() => { ++ if (!checkPermission()) ++ return; ++ helper.removeListeners(listeners); ++ resolve(); ++ }, 'perm-changed')]; ++ }); ++ }, ++ + dispose() { + }, + }); @@ -4348,10 +4388,10 @@ index 0000000000000000000000000000000000000000..2f2b7ca247f6b6dff396fb4b644654de +this.AccessibilityHandler = AccessibilityHandler; diff --git a/testing/juggler/protocol/BrowserHandler.js b/testing/juggler/protocol/BrowserHandler.js new file mode 100644 -index 0000000000000000000000000000000000000000..061bcb4ff8a34610a5ec433393bf7901c4cafe86 +index 0000000000000000000000000000000000000000..c34d0852b2e5b550d063f93e29429c651ec2501e --- /dev/null +++ b/testing/juggler/protocol/BrowserHandler.js -@@ -0,0 +1,81 @@ +@@ -0,0 +1,84 @@ +"use strict"; + +const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm"); @@ -4359,6 +4399,7 @@ index 0000000000000000000000000000000000000000..061bcb4ff8a34610a5ec433393bf7901 + "chrome://marionette/content/cert.js" +); +const {BrowserContextManager} = ChromeUtils.import("chrome://juggler/content/BrowserContextManager.js"); ++const {TargetRegistry} = ChromeUtils.import("chrome://juggler/content/TargetRegistry.js"); + +class BrowserHandler { + /** @@ -4367,6 +4408,7 @@ index 0000000000000000000000000000000000000000..061bcb4ff8a34610a5ec433393bf7901 + constructor() { + this._sweepingOverride = null; + this._contextManager = BrowserContextManager.instance(); ++ this._targetRegistry = TargetRegistry.instance(); + } + + async close() { @@ -4389,8 +4431,9 @@ index 0000000000000000000000000000000000000000..061bcb4ff8a34610a5ec433393bf7901 + } + } + -+ grantPermissions({browserContextId, origin, permissions}) { ++ async grantPermissions({browserContextId, origin, permissions}) { + this._contextManager.browserContextForId(browserContextId).grantPermissions(origin, permissions); ++ await this._targetRegistry.ensurePermissionsInContextPages(browserContextId, permissions); + } + + resetPermissions({browserContextId}) {