From b74a6b78ef1336c3722aca169e6026182e822bf5 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 30 Sep 2020 23:50:02 -0700 Subject: [PATCH] browser(firefox): do not double-attach session to the same target (#4027) We currently might double-attach to the target in `BrowserHandler` since we iterate over all targets, and then subscribe to the additional event when target is getting initialized. This patch fixes this race condition and should unblock the roll to r1177. References #3995 --- browser_patches/firefox/BUILD_NUMBER | 4 ++-- browser_patches/firefox/juggler/TargetRegistry.js | 11 +++++++---- browser_patches/firefox/juggler/content/main.js | 9 +++++---- .../firefox/juggler/protocol/BrowserHandler.js | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/browser_patches/firefox/BUILD_NUMBER b/browser_patches/firefox/BUILD_NUMBER index 1630fd5c04..12fc4b2a28 100644 --- a/browser_patches/firefox/BUILD_NUMBER +++ b/browser_patches/firefox/BUILD_NUMBER @@ -1,2 +1,2 @@ -1177 -Changed: lushnikov@chromium.org Wed Sep 30 03:11:29 MDT 2020 +1178 +Changed: lushnikov@chromium.org Wed Sep 30 23:36:27 PDT 2020 diff --git a/browser_patches/firefox/juggler/TargetRegistry.js b/browser_patches/firefox/juggler/TargetRegistry.js index 2f6a80aca0..d3ebab6f89 100644 --- a/browser_patches/firefox/juggler/TargetRegistry.js +++ b/browser_patches/firefox/juggler/TargetRegistry.js @@ -147,8 +147,8 @@ class TargetRegistry { const sessions = []; const readyData = { sessions, target }; - this.emit(TargetRegistry.Events.TargetCreated, readyData); target.markAsReported(); + this.emit(TargetRegistry.Events.TargetCreated, readyData); return { scriptsToEvaluateOnNewDocument: target.browserContext().scriptsToEvaluateOnNewDocument, bindings: target.browserContext().bindings, @@ -321,8 +321,8 @@ class TargetRegistry { return target.id(); } - targets() { - return Array.from(this._browserToTarget.values()); + reportedTargets() { + return Array.from(this._browserToTarget.values()).filter(pageTarget => pageTarget._isReported); } targetForBrowser(browser) { @@ -364,6 +364,7 @@ class PageTarget { helper.addProgressListener(tab.linkedBrowser, navigationListener, Ci.nsIWebProgress.NOTIFY_LOCATION), ]; + this._isReported = false; this._reportedPromise = new Promise(resolve => { this._reportedCallback = resolve; }); @@ -379,6 +380,7 @@ class PageTarget { } markAsReported() { + this._isReported = true; this._reportedCallback(); } @@ -491,7 +493,8 @@ class PageTarget { this._registry._browserToTarget.delete(this._linkedBrowser); this._registry._browserBrowsingContextToTarget.delete(this._linkedBrowser.browsingContext); helper.removeListeners(this._eventListeners); - this._registry.emit(TargetRegistry.Events.TargetDestroyed, this); + if (this._isReported) + this._registry.emit(TargetRegistry.Events.TargetDestroyed, this); } } diff --git a/browser_patches/firefox/juggler/content/main.js b/browser_patches/firefox/juggler/content/main.js index b1f1231403..184096981b 100644 --- a/browser_patches/firefox/juggler/content/main.js +++ b/browser_patches/firefox/juggler/content/main.js @@ -90,10 +90,11 @@ const applySetting = { }; function initialize() { - const loadContext = docShell.QueryInterface(Ci.nsILoadContext); - const userContextId = loadContext.originAttributes.userContextId; - - const response = sendSyncMessage('juggler:content-ready', { userContextId })[0]; + const response = sendSyncMessage('juggler:content-ready')[0]; + // If we didn't get a response, then we don't want to do anything + // as a part of this frame script. + if (!response) + return; const { sessionIds = [], scriptsToEvaluateOnNewDocument = [], diff --git a/browser_patches/firefox/juggler/protocol/BrowserHandler.js b/browser_patches/firefox/juggler/protocol/BrowserHandler.js index 3b8f96514d..07ba2567ab 100644 --- a/browser_patches/firefox/juggler/protocol/BrowserHandler.js +++ b/browser_patches/firefox/juggler/protocol/BrowserHandler.js @@ -29,7 +29,7 @@ class BrowserHandler { this._enabled = true; this._attachToDefaultContext = attachToDefaultContext; - for (const target of this._targetRegistry.targets()) { + for (const target of this._targetRegistry.reportedTargets()) { if (!this._shouldAttachToTarget(target)) continue; const session = this._dispatcher.createSession();