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.

browser_patches/firefox/BUILD_NUMBER

@@ -1,2 +1,2 @@
-1198
-Changed: lushnikov@chromium.org Thu 29 Oct 2020 04:23:02 PM PDT
+1199
+Changed: lushnikov@chromium.org Mon 02 Nov 2020 04:10:47 PM PST

browser_patches/firefox/juggler/SimpleChannel.js

@@ -17,10 +17,11 @@ class SimpleChannel {
     };
     mm.addMessageListener(SIMPLE_CHANNEL_MESSAGE_NAME, messageListener);
 
-    channel.transport.sendMessage = obj => mm.sendAsyncMessage(SIMPLE_CHANNEL_MESSAGE_NAME, obj);
-    channel.transport.dispose = () => {
-      mm.removeMessageListener(SIMPLE_CHANNEL_MESSAGE_NAME, messageListener);
-    };
+    channel.setTransport({
+      sendMessage: obj => mm.sendAsyncMessage(SIMPLE_CHANNEL_MESSAGE_NAME, obj),
+      dispose: () => mm.removeMessageListener(SIMPLE_CHANNEL_MESSAGE_NAME, messageListener),
+    });
+
     return channel;
   }
 
@@ -30,14 +31,39 @@ class SimpleChannel {
     this._connectorId = 0;
     this._pendingMessages = new Map();
     this._handlers = new Map();
-    this._bufferedRequests = [];
+    this._bufferedIncomingMessages = [];
+    this._bufferedOutgoingMessages = [];
     this.transport = {
       sendMessage: null,
       dispose: null,
     };
+    this._ready = false;
     this._disposed = false;
   }
 
+  setTransport(transport) {
+    this.transport = transport;
+    // connection handshake:
+    // 1. There are two channel ends in different processes.
+    // 2. Both ends start in the `ready = false` state, meaning that they will
+    //    not send any messages over transport.
+    // 3. Once channel end is created, it sends `READY` message to the other end.
+    // 4. Eventually, at least one of the ends receives `READY` message and responds with
+    //    `READY_ACK`. We assume at least one of the ends will receive "READY" event from the other, since
+    //    channel ends have a "parent-child" relation, i.e. one end is always created before the other one.
+    // 5. Once channel end receives either `READY` or `READY_ACK`, it transitions to `ready` state.
+    this.transport.sendMessage('READY');
+  }
+
+  _markAsReady() {
+    if (this._ready)
+      return;
+    this._ready = true;
+    for (const msg of this._bufferedOutgoingMessages)
+      this.transport.sendMessage(msg);
+    this._bufferedOutgoingMessages = [];
+  }
+
   dispose() {
     if (this._disposed)
       return;
@@ -72,8 +98,8 @@ class SimpleChannel {
       throw new Error('ERROR: double-register for namespace ' + namespace);
     this._handlers.set(namespace, handler);
     // Try to re-deliver all pending messages.
-    const bufferedRequests = this._bufferedRequests;
-    this._bufferedRequests = [];
+    const bufferedRequests = this._bufferedIncomingMessages;
+    this._bufferedIncomingMessages = [];
     for (const data of bufferedRequests) {
       this._onMessage(data);
     }
@@ -98,11 +124,24 @@ class SimpleChannel {
     const promise = new Promise((resolve, reject) => {
       this._pendingMessages.set(id, {connectorId, resolve, reject, methodName, namespace});
     });
-    this.transport.sendMessage({requestId: id, methodName, params, namespace});
+    const message = {requestId: id, methodName, params, namespace};
+    if (this._ready)
+      this.transport.sendMessage(message);
+    else
+      this._bufferedOutgoingMessages.push(message);
     return promise;
   }
 
   async _onMessage(data) {
+    if (data === 'READY') {
+      this.transport.sendMessage('READY_ACK');
+      this._markAsReady();
+      return;
+    }
+    if (data === 'READY_ACK') {
+      this._markAsReady();
+      return;
+    }
     if (data.responseId) {
       const {resolve, reject} = this._pendingMessages.get(data.responseId);
       this._pendingMessages.delete(data.responseId);
@@ -114,7 +153,7 @@ class SimpleChannel {
       const namespace = data.namespace;
       const handler = this._handlers.get(namespace);
       if (!handler) {
-        this._bufferedRequests.push(data);
+        this._bufferedIncomingMessages.push(data);
         return;
       }
       const method = handler[data.methodName];

browser_patches/firefox/juggler/content/FrameTree.js

@@ -527,10 +527,10 @@ class Worker {
     workerDebugger.initialize('chrome://juggler/content/content/WorkerMain.js');
 
     this._channel = new SimpleChannel(`content::worker[${this._workerId}]`);
-    this._channel.transport = {
+    this._channel.setTransport({
       sendMessage: obj => workerDebugger.postMessage(JSON.stringify(obj)),
       dispose: () => {},
-    };
+    });
     this._workerDebuggerListener = {
       QueryInterface: ChromeUtils.generateQI([Ci.nsIWorkerDebuggerListener]),
       onMessage: msg => void this._channel._onMessage(JSON.parse(msg)),

browser_patches/firefox/juggler/content/WorkerMain.js

@@ -9,10 +9,10 @@ loadSubScript('chrome://juggler/content/SimpleChannel.js');
 const channel = new SimpleChannel('worker::worker');
 const eventListener = event => channel._onMessage(JSON.parse(event.data));
 this.addEventListener('message', eventListener);
-channel.transport = {
+channel.setTransport({
   sendMessage: msg => postMessage(JSON.stringify(msg)),
   dispose: () => this.removeEventListener('message', eventListener),
-};
+});
 
 const runtime = new Runtime(true /* isWorker */);
 

browser_patches/firefox/juggler/protocol/PageHandler.js

@@ -73,8 +73,12 @@ class PageHandler {
     this._reportedFrameIds = new Set();
     this._networkEventsForUnreportedFrameIds = new Map();
 
-    for (const dialog of this._pageTarget.dialogs())
-      this._onDialogOpened(dialog);
+    // `Page.ready` protocol event is emitted whenever page has completed initialization, e.g.
+    // finished all the transient navigations to the `about:blank`.
+    //
+    // We'd like to avoid reporting meaningful events before the `Page.ready` since they are likely
+    // to be ignored by the protocol clients.
+    this._isPageReady = false;
 
     if (this._pageTarget.screencastInfo())
       this._onScreencastStarted();
@@ -102,7 +106,7 @@ class PageHandler {
         pageNavigationAborted: emitProtocolEvent('Page.navigationAborted'),
         pageNavigationCommitted: emitProtocolEvent('Page.navigationCommitted'),
         pageNavigationStarted: emitProtocolEvent('Page.navigationStarted'),
-        pageReady: emitProtocolEvent('Page.ready'),
+        pageReady: this._onPageReady.bind(this), 
         pageSameDocumentNavigation: emitProtocolEvent('Page.sameDocumentNavigation'),
         pageUncaughtError: emitProtocolEvent('Page.uncaughtError'),
         pageWorkerCreated: this._onWorkerCreated.bind(this),
@@ -130,7 +134,16 @@ class PageHandler {
     this._session.emitEvent('Page.screencastStarted', { screencastId: info.videoSessionId, file: info.file });
   }
 
+  _onPageReady(event) {
+    this._isPageReady = true;
+    this._session.emitEvent('Page.ready');
+    for (const dialog of this._pageTarget.dialogs())
+      this._onDialogOpened(dialog);
+  }
+
   _onDialogOpened(dialog) {
+    if (!this._isPageReady)
+      return;
     this._session.emitEvent('Page.dialogOpened', {
       dialogId: dialog.id(),
       type: dialog.type(),
@@ -140,6 +153,8 @@ class PageHandler {
   }
 
   _onDialogClosed(dialog) {
+    if (!this._isPageReady)
+      return;
     this._session.emitEvent('Page.dialogClosed', { dialogId: dialog.id(), });
   }