Bug 1601245 - [remote] With Fission enabled use "DOMWindowCreated" event to detect newly created frames. r=webdriver-reviewers,jdescottes

Because with Fission enabled the "DOMWindowCreated" event
is emitted way early it can be used to detect when a new
frame has been created.

But with Fission disabled the event is received too late,
and as such the appropriate network requests and responses
for the document have already been done. That means that
Puppeteer actually hangs waiting for them.

As such keep using the "webnavigation-create" observer
notification for Fission-disabled builds for now.

Differential Revision: https://phabricator.services.mozilla.com/D127521

remote/cdp/domains/content/Page.jsm

@@ -55,14 +55,8 @@ class Page extends ContentProcessDomain {
 
   async enable() {
     if (!this.enabled) {
-      this.session.contextObserver.on(
-        "docshell-created",
-        this._onFrameAttached
-      );
-      this.session.contextObserver.on(
-        "docshell-destroyed",
-        this._onFrameDetached
-      );
+      this.session.contextObserver.on("frame-attached", this._onFrameAttached);
+      this.session.contextObserver.on("frame-detached", this._onFrameDetached);
       this.session.contextObserver.on(
         "frame-navigated",
         this._onFrameNavigated
@@ -96,14 +90,8 @@ class Page extends ContentProcessDomain {
 
   disable() {
     if (this.enabled) {
-      this.session.contextObserver.off(
-        "docshell-created",
-        this._onFrameAttached
-      );
-      this.session.contextObserver.off(
-        "docshell-destroyed",
-        this._onFrameDetached
-      );
+      this.session.contextObserver.off("frame-attached", this._onFrameAttached);
+      this.session.contextObserver.off("frame-detached", this._onFrameDetached);
       this.session.contextObserver.off(
         "frame-navigated",
         this._onFrameNavigated
@@ -260,8 +248,8 @@ class Page extends ContentProcessDomain {
     return this.content.location.href;
   }
 
-  _onFrameAttached(name, { id }) {
-    const bc = BrowsingContext.get(id);
+  _onFrameAttached(name, { frameId, window }) {
+    const bc = BrowsingContext.get(frameId);
 
     // Don't emit for top-level browsing contexts
     if (!bc.parent) {
@@ -270,19 +258,19 @@ class Page extends ContentProcessDomain {
 
     // TODO: Use a unique identifier for frames (bug 1605359)
     this.emit("Page.frameAttached", {
-      frameId: bc.id.toString(),
+      frameId: frameId.toString(),
       parentFrameId: bc.parent.id.toString(),
       stack: null,
     });
 
-    const loaderId = this.frameIdToLoaderId.get(bc.id);
+    const loaderId = this.frameIdToLoaderId.get(frameId);
     const timestamp = Date.now() / 1000;
-    this.emit("Page.frameStartedLoading", { frameId: bc.id.toString() });
-    this.emitLifecycleEvent(bc.id, loaderId, "init", timestamp);
+    this.emit("Page.frameStartedLoading", { frameId: frameId.toString() });
+    this.emitLifecycleEvent(frameId, loaderId, "init", timestamp);
   }
 
-  _onFrameDetached(name, { id }) {
-    const bc = BrowsingContext.get(id);
+  _onFrameDetached(name, { frameId }) {
+    const bc = BrowsingContext.get(frameId);
 
     // Don't emit for top-level browsing contexts
     if (!bc.parent) {
@@ -290,7 +278,7 @@ class Page extends ContentProcessDomain {
     }
 
     // TODO: Use a unique identifier for frames (bug 1605359)
-    this.emit("Page.frameDetached", { frameId: bc.id.toString() });
+    this.emit("Page.frameDetached", { frameId: frameId.toString() });
   }
 
   _onFrameNavigated(name, { frameId }) {

remote/cdp/observers/ContextObserver.jsm

@@ -41,6 +41,8 @@ class ContextObserver {
     this.chromeEventHandler = chromeEventHandler;
     EventEmitter.decorate(this);
 
+    this._fissionEnabled = Services.appinfo.fissionAutostart;
+
     this.chromeEventHandler.addEventListener("DOMWindowCreated", this, {
       mozSystemGroup: true,
     });
@@ -55,7 +57,11 @@ class ContextObserver {
 
     Services.obs.addObserver(this, "inner-window-destroyed");
 
-    Services.obs.addObserver(this, "webnavigation-create");
+    // With Fission disabled the `DOMWindowCreated` event is fired too late.
+    // Use the `webnavigation-create` notification instead.
+    if (!this._fissionEnabled) {
+      Services.obs.addObserver(this, "webnavigation-create");
+    }
     Services.obs.addObserver(this, "webnavigation-destroy");
   }
 
@@ -72,7 +78,9 @@ class ContextObserver {
 
     Services.obs.removeObserver(this, "inner-window-destroyed");
 
-    Services.obs.removeObserver(this, "webnavigation-create");
+    if (!this._fissionEnabled) {
+      Services.obs.removeObserver(this, "webnavigation-create");
+    }
     Services.obs.removeObserver(this, "webnavigation-destroy");
   }
 
@@ -87,6 +95,14 @@ class ContextObserver {
         // that is destroyed. Instead, pass the frameId and let the listener figure out
         // what ExecutionContext(s) to destroy.
         this.emit("context-destroyed", { frameId });
+
+        // With Fission enabled the frame is attached early enough so that
+        // expected network requests and responses are handles afterward.
+        // Otherwise send the event when `webnavigation-create` is received.
+        if (this._fissionEnabled) {
+          this.emit("frame-attached", { frameId, window });
+        }
+
         this.emit("frame-navigated", { frameId, window });
         this.emit("context-created", { windowId: id, window });
         // Delay script-loaded to allow context cleanup to happen first
@@ -134,14 +150,14 @@ class ContextObserver {
   }
 
   onDocShellCreated(docShell) {
-    this.emit("docshell-created", {
-      id: docShell.browsingContext.id,
+    this.emit("frame-attached", {
+      frameId: docShell.browsingContext.id,
     });
   }
 
   onDocShellDestroyed(docShell) {
-    this.emit("docshell-destroyed", {
-      id: docShell.browsingContext.id,
+    this.emit("frame-detached", {
+      frameId: docShell.browsingContext.id,
     });
   }
 }