Skip to content

Commit b74a6b7

Browse files
authored
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
1 parent f885d07 commit b74a6b7

File tree

4 files changed

+15
-11
lines changed

4 files changed

+15
-11
lines changed

browser_patches/firefox/BUILD_NUMBER

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
1177
2-
Changed: [email protected] Wed Sep 30 03:11:29 MDT 2020
1+
1178
2+
Changed: [email protected] Wed Sep 30 23:36:27 PDT 2020

browser_patches/firefox/juggler/TargetRegistry.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,8 @@ class TargetRegistry {
147147

148148
const sessions = [];
149149
const readyData = { sessions, target };
150-
this.emit(TargetRegistry.Events.TargetCreated, readyData);
151150
target.markAsReported();
151+
this.emit(TargetRegistry.Events.TargetCreated, readyData);
152152
return {
153153
scriptsToEvaluateOnNewDocument: target.browserContext().scriptsToEvaluateOnNewDocument,
154154
bindings: target.browserContext().bindings,
@@ -321,8 +321,8 @@ class TargetRegistry {
321321
return target.id();
322322
}
323323

324-
targets() {
325-
return Array.from(this._browserToTarget.values());
324+
reportedTargets() {
325+
return Array.from(this._browserToTarget.values()).filter(pageTarget => pageTarget._isReported);
326326
}
327327

328328
targetForBrowser(browser) {
@@ -364,6 +364,7 @@ class PageTarget {
364364
helper.addProgressListener(tab.linkedBrowser, navigationListener, Ci.nsIWebProgress.NOTIFY_LOCATION),
365365
];
366366

367+
this._isReported = false;
367368
this._reportedPromise = new Promise(resolve => {
368369
this._reportedCallback = resolve;
369370
});
@@ -379,6 +380,7 @@ class PageTarget {
379380
}
380381

381382
markAsReported() {
383+
this._isReported = true;
382384
this._reportedCallback();
383385
}
384386

@@ -491,7 +493,8 @@ class PageTarget {
491493
this._registry._browserToTarget.delete(this._linkedBrowser);
492494
this._registry._browserBrowsingContextToTarget.delete(this._linkedBrowser.browsingContext);
493495
helper.removeListeners(this._eventListeners);
494-
this._registry.emit(TargetRegistry.Events.TargetDestroyed, this);
496+
if (this._isReported)
497+
this._registry.emit(TargetRegistry.Events.TargetDestroyed, this);
495498
}
496499
}
497500

browser_patches/firefox/juggler/content/main.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,11 @@ const applySetting = {
9090
};
9191

9292
function initialize() {
93-
const loadContext = docShell.QueryInterface(Ci.nsILoadContext);
94-
const userContextId = loadContext.originAttributes.userContextId;
95-
96-
const response = sendSyncMessage('juggler:content-ready', { userContextId })[0];
93+
const response = sendSyncMessage('juggler:content-ready')[0];
94+
// If we didn't get a response, then we don't want to do anything
95+
// as a part of this frame script.
96+
if (!response)
97+
return;
9798
const {
9899
sessionIds = [],
99100
scriptsToEvaluateOnNewDocument = [],

browser_patches/firefox/juggler/protocol/BrowserHandler.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class BrowserHandler {
2929
this._enabled = true;
3030
this._attachToDefaultContext = attachToDefaultContext;
3131

32-
for (const target of this._targetRegistry.targets()) {
32+
for (const target of this._targetRegistry.reportedTargets()) {
3333
if (!this._shouldAttachToTarget(target))
3434
continue;
3535
const session = this._dispatcher.createSession();

0 commit comments

Comments
 (0)