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
This commit is contained in:
Andrey Lushnikov 2020-09-30 23:50:02 -07:00 committed by GitHub
parent f885d07cb9
commit b74a6b78ef
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 15 additions and 11 deletions

View file

@ -1,2 +1,2 @@
1177 1178
Changed: lushnikov@chromium.org Wed Sep 30 03:11:29 MDT 2020 Changed: lushnikov@chromium.org Wed Sep 30 23:36:27 PDT 2020

View file

@ -147,8 +147,8 @@ class TargetRegistry {
const sessions = []; const sessions = [];
const readyData = { sessions, target }; const readyData = { sessions, target };
this.emit(TargetRegistry.Events.TargetCreated, readyData);
target.markAsReported(); target.markAsReported();
this.emit(TargetRegistry.Events.TargetCreated, readyData);
return { return {
scriptsToEvaluateOnNewDocument: target.browserContext().scriptsToEvaluateOnNewDocument, scriptsToEvaluateOnNewDocument: target.browserContext().scriptsToEvaluateOnNewDocument,
bindings: target.browserContext().bindings, bindings: target.browserContext().bindings,
@ -321,8 +321,8 @@ class TargetRegistry {
return target.id(); return target.id();
} }
targets() { reportedTargets() {
return Array.from(this._browserToTarget.values()); return Array.from(this._browserToTarget.values()).filter(pageTarget => pageTarget._isReported);
} }
targetForBrowser(browser) { targetForBrowser(browser) {
@ -364,6 +364,7 @@ class PageTarget {
helper.addProgressListener(tab.linkedBrowser, navigationListener, Ci.nsIWebProgress.NOTIFY_LOCATION), helper.addProgressListener(tab.linkedBrowser, navigationListener, Ci.nsIWebProgress.NOTIFY_LOCATION),
]; ];
this._isReported = false;
this._reportedPromise = new Promise(resolve => { this._reportedPromise = new Promise(resolve => {
this._reportedCallback = resolve; this._reportedCallback = resolve;
}); });
@ -379,6 +380,7 @@ class PageTarget {
} }
markAsReported() { markAsReported() {
this._isReported = true;
this._reportedCallback(); this._reportedCallback();
} }
@ -491,7 +493,8 @@ class PageTarget {
this._registry._browserToTarget.delete(this._linkedBrowser); this._registry._browserToTarget.delete(this._linkedBrowser);
this._registry._browserBrowsingContextToTarget.delete(this._linkedBrowser.browsingContext); this._registry._browserBrowsingContextToTarget.delete(this._linkedBrowser.browsingContext);
helper.removeListeners(this._eventListeners); helper.removeListeners(this._eventListeners);
this._registry.emit(TargetRegistry.Events.TargetDestroyed, this); if (this._isReported)
this._registry.emit(TargetRegistry.Events.TargetDestroyed, this);
} }
} }

View file

@ -90,10 +90,11 @@ const applySetting = {
}; };
function initialize() { function initialize() {
const loadContext = docShell.QueryInterface(Ci.nsILoadContext); const response = sendSyncMessage('juggler:content-ready')[0];
const userContextId = loadContext.originAttributes.userContextId; // If we didn't get a response, then we don't want to do anything
// as a part of this frame script.
const response = sendSyncMessage('juggler:content-ready', { userContextId })[0]; if (!response)
return;
const { const {
sessionIds = [], sessionIds = [],
scriptsToEvaluateOnNewDocument = [], scriptsToEvaluateOnNewDocument = [],

View file

@ -29,7 +29,7 @@ class BrowserHandler {
this._enabled = true; this._enabled = true;
this._attachToDefaultContext = attachToDefaultContext; this._attachToDefaultContext = attachToDefaultContext;
for (const target of this._targetRegistry.targets()) { for (const target of this._targetRegistry.reportedTargets()) {
if (!this._shouldAttachToTarget(target)) if (!this._shouldAttachToTarget(target))
continue; continue;
const session = this._dispatcher.createSession(); const session = this._dispatcher.createSession();