Skip to content

Commit 2b495c9

Browse files
authored
browser(firefox): fix SimpleChannel to await initialization (#4311)
As Joel noticed recently, MessageManager in firefox doesn't guarantee message delivery if the opposite end hasn't been initialized yet. In this case, message will be silently dropped on the ground. To fix this, we establish a handshake in SimpleChannel to make sure that both ends are initialized, end buffer outgoing messages until this happens. Drive-by: serialize dialog events to only deliver *after* the `Page.ready` protocol event. Otherwise, we deliver dialog events to the unreported page.
1 parent f80f815 commit 2b495c9

File tree

5 files changed

+72
-18
lines changed

5 files changed

+72
-18
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-
1198
2-
Changed: [email protected] Thu 29 Oct 2020 04:23:02 PM PDT
1+
1199
2+
Changed: [email protected] Mon 02 Nov 2020 04:10:47 PM PST

browser_patches/firefox/juggler/SimpleChannel.js

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@ class SimpleChannel {
1717
};
1818
mm.addMessageListener(SIMPLE_CHANNEL_MESSAGE_NAME, messageListener);
1919

20-
channel.transport.sendMessage = obj => mm.sendAsyncMessage(SIMPLE_CHANNEL_MESSAGE_NAME, obj);
21-
channel.transport.dispose = () => {
22-
mm.removeMessageListener(SIMPLE_CHANNEL_MESSAGE_NAME, messageListener);
23-
};
20+
channel.setTransport({
21+
sendMessage: obj => mm.sendAsyncMessage(SIMPLE_CHANNEL_MESSAGE_NAME, obj),
22+
dispose: () => mm.removeMessageListener(SIMPLE_CHANNEL_MESSAGE_NAME, messageListener),
23+
});
24+
2425
return channel;
2526
}
2627

@@ -30,14 +31,39 @@ class SimpleChannel {
3031
this._connectorId = 0;
3132
this._pendingMessages = new Map();
3233
this._handlers = new Map();
33-
this._bufferedRequests = [];
34+
this._bufferedIncomingMessages = [];
35+
this._bufferedOutgoingMessages = [];
3436
this.transport = {
3537
sendMessage: null,
3638
dispose: null,
3739
};
40+
this._ready = false;
3841
this._disposed = false;
3942
}
4043

44+
setTransport(transport) {
45+
this.transport = transport;
46+
// connection handshake:
47+
// 1. There are two channel ends in different processes.
48+
// 2. Both ends start in the `ready = false` state, meaning that they will
49+
// not send any messages over transport.
50+
// 3. Once channel end is created, it sends `READY` message to the other end.
51+
// 4. Eventually, at least one of the ends receives `READY` message and responds with
52+
// `READY_ACK`. We assume at least one of the ends will receive "READY" event from the other, since
53+
// channel ends have a "parent-child" relation, i.e. one end is always created before the other one.
54+
// 5. Once channel end receives either `READY` or `READY_ACK`, it transitions to `ready` state.
55+
this.transport.sendMessage('READY');
56+
}
57+
58+
_markAsReady() {
59+
if (this._ready)
60+
return;
61+
this._ready = true;
62+
for (const msg of this._bufferedOutgoingMessages)
63+
this.transport.sendMessage(msg);
64+
this._bufferedOutgoingMessages = [];
65+
}
66+
4167
dispose() {
4268
if (this._disposed)
4369
return;
@@ -72,8 +98,8 @@ class SimpleChannel {
7298
throw new Error('ERROR: double-register for namespace ' + namespace);
7399
this._handlers.set(namespace, handler);
74100
// Try to re-deliver all pending messages.
75-
const bufferedRequests = this._bufferedRequests;
76-
this._bufferedRequests = [];
101+
const bufferedRequests = this._bufferedIncomingMessages;
102+
this._bufferedIncomingMessages = [];
77103
for (const data of bufferedRequests) {
78104
this._onMessage(data);
79105
}
@@ -98,11 +124,24 @@ class SimpleChannel {
98124
const promise = new Promise((resolve, reject) => {
99125
this._pendingMessages.set(id, {connectorId, resolve, reject, methodName, namespace});
100126
});
101-
this.transport.sendMessage({requestId: id, methodName, params, namespace});
127+
const message = {requestId: id, methodName, params, namespace};
128+
if (this._ready)
129+
this.transport.sendMessage(message);
130+
else
131+
this._bufferedOutgoingMessages.push(message);
102132
return promise;
103133
}
104134

105135
async _onMessage(data) {
136+
if (data === 'READY') {
137+
this.transport.sendMessage('READY_ACK');
138+
this._markAsReady();
139+
return;
140+
}
141+
if (data === 'READY_ACK') {
142+
this._markAsReady();
143+
return;
144+
}
106145
if (data.responseId) {
107146
const {resolve, reject} = this._pendingMessages.get(data.responseId);
108147
this._pendingMessages.delete(data.responseId);
@@ -114,7 +153,7 @@ class SimpleChannel {
114153
const namespace = data.namespace;
115154
const handler = this._handlers.get(namespace);
116155
if (!handler) {
117-
this._bufferedRequests.push(data);
156+
this._bufferedIncomingMessages.push(data);
118157
return;
119158
}
120159
const method = handler[data.methodName];

browser_patches/firefox/juggler/content/FrameTree.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -527,10 +527,10 @@ class Worker {
527527
workerDebugger.initialize('chrome://juggler/content/content/WorkerMain.js');
528528

529529
this._channel = new SimpleChannel(`content::worker[${this._workerId}]`);
530-
this._channel.transport = {
530+
this._channel.setTransport({
531531
sendMessage: obj => workerDebugger.postMessage(JSON.stringify(obj)),
532532
dispose: () => {},
533-
};
533+
});
534534
this._workerDebuggerListener = {
535535
QueryInterface: ChromeUtils.generateQI([Ci.nsIWorkerDebuggerListener]),
536536
onMessage: msg => void this._channel._onMessage(JSON.parse(msg)),

browser_patches/firefox/juggler/content/WorkerMain.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ loadSubScript('chrome://juggler/content/SimpleChannel.js');
99
const channel = new SimpleChannel('worker::worker');
1010
const eventListener = event => channel._onMessage(JSON.parse(event.data));
1111
this.addEventListener('message', eventListener);
12-
channel.transport = {
12+
channel.setTransport({
1313
sendMessage: msg => postMessage(JSON.stringify(msg)),
1414
dispose: () => this.removeEventListener('message', eventListener),
15-
};
15+
});
1616

1717
const runtime = new Runtime(true /* isWorker */);
1818

browser_patches/firefox/juggler/protocol/PageHandler.js

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,12 @@ class PageHandler {
7373
this._reportedFrameIds = new Set();
7474
this._networkEventsForUnreportedFrameIds = new Map();
7575

76-
for (const dialog of this._pageTarget.dialogs())
77-
this._onDialogOpened(dialog);
76+
// `Page.ready` protocol event is emitted whenever page has completed initialization, e.g.
77+
// finished all the transient navigations to the `about:blank`.
78+
//
79+
// We'd like to avoid reporting meaningful events before the `Page.ready` since they are likely
80+
// to be ignored by the protocol clients.
81+
this._isPageReady = false;
7882

7983
if (this._pageTarget.screencastInfo())
8084
this._onScreencastStarted();
@@ -102,7 +106,7 @@ class PageHandler {
102106
pageNavigationAborted: emitProtocolEvent('Page.navigationAborted'),
103107
pageNavigationCommitted: emitProtocolEvent('Page.navigationCommitted'),
104108
pageNavigationStarted: emitProtocolEvent('Page.navigationStarted'),
105-
pageReady: emitProtocolEvent('Page.ready'),
109+
pageReady: this._onPageReady.bind(this),
106110
pageSameDocumentNavigation: emitProtocolEvent('Page.sameDocumentNavigation'),
107111
pageUncaughtError: emitProtocolEvent('Page.uncaughtError'),
108112
pageWorkerCreated: this._onWorkerCreated.bind(this),
@@ -130,7 +134,16 @@ class PageHandler {
130134
this._session.emitEvent('Page.screencastStarted', { screencastId: info.videoSessionId, file: info.file });
131135
}
132136

137+
_onPageReady(event) {
138+
this._isPageReady = true;
139+
this._session.emitEvent('Page.ready');
140+
for (const dialog of this._pageTarget.dialogs())
141+
this._onDialogOpened(dialog);
142+
}
143+
133144
_onDialogOpened(dialog) {
145+
if (!this._isPageReady)
146+
return;
134147
this._session.emitEvent('Page.dialogOpened', {
135148
dialogId: dialog.id(),
136149
type: dialog.type(),
@@ -140,6 +153,8 @@ class PageHandler {
140153
}
141154

142155
_onDialogClosed(dialog) {
156+
if (!this._isPageReady)
157+
return;
143158
this._session.emitEvent('Page.dialogClosed', { dialogId: dialog.id(), });
144159
}
145160

0 commit comments

Comments
 (0)