Bug 1548102 - Coordinate Page.frameNavigated and Runtime.executionContextDestroyed/Created events. r=remote-protocol-reviewers,ato
authorAlexandre Poirot <poirot.alex@gmail.com>
Tue, 14 May 2019 15:18:51 +0000
changeset 473821 1bb020dacad9998406a9bd9a7a6fe1568f718b19
parent 473820 5e5a2115d0da387463725c290d8f9b9a6cbd8e93
child 473822 5df94e4b78cc5fcb8a1cf8f5664b256c346f2f21
push id36017
push userrgurzau@mozilla.com
push dateWed, 15 May 2019 09:25:56 +0000
treeherdermozilla-central@76bbedc1ec1a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersremote-protocol-reviewers, ato
bugs1548102
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1548102 - Coordinate Page.frameNavigated and Runtime.executionContextDestroyed/Created events. r=remote-protocol-reviewers,ato Differential Revision: https://phabricator.services.mozilla.com/D30237
remote/domains/ContentProcessDomain.jsm
remote/domains/ContextObserver.jsm
remote/domains/content/Page.jsm
remote/domains/content/Runtime.jsm
remote/domains/content/runtime/ExecutionContext.jsm
remote/jar.mn
remote/test/browser/browser.ini
remote/test/browser/browser_page_runtime_events.js
--- a/remote/domains/ContentProcessDomain.jsm
+++ b/remote/domains/ContentProcessDomain.jsm
@@ -3,23 +3,41 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 var EXPORTED_SYMBOLS = ["ContentProcessDomain"];
 
 const {Domain} = ChromeUtils.import("chrome://remote/content/domains/Domain.jsm");
 
+ChromeUtils.defineModuleGetter(this, "ContextObserver",
+                               "chrome://remote/content/domains/ContextObserver.jsm");
+
 class ContentProcessDomain extends Domain {
+  destructor() {
+    super.destructor();
+
+    if (this._contextObserver) {
+      this._contextObserver.destructor();
+    }
+  }
+
   // helpers
 
   get content() {
     return this.session.content;
   }
 
   get docShell() {
     return this.session.docShell;
   }
 
   get chromeEventHandler() {
     return this.docShell.chromeEventHandler;
   }
+
+  get contextObserver() {
+    if (!this._contextObserver) {
+      this._contextObserver = new ContextObserver(this.chromeEventHandler);
+    }
+    return this._contextObserver;
+  }
 }
new file mode 100644
--- /dev/null
+++ b/remote/domains/ContextObserver.jsm
@@ -0,0 +1,103 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+/**
+ * Helper class to coordinate Runtime and Page events.
+ * Events have to be sent in the following order:
+ *  - Runtime.executionContextDestroyed
+ *  - Page.frameNavigated
+ *  - Runtime.executionContextCreated
+ *
+ * This class also handles the special case of Pages going from/to the BF cache.
+ * When you navigate to a new URL, the previous document may be stored in the BF Cache.
+ * All its asynchronous operations are frozen (XHR, timeouts, ...) and a `pagehide` event
+ * is fired for this document. We then navigate to the new URL.
+ * If the user navigates back to the previous page, the page is resurected from the
+ * cache. A `pageshow` event is fired and its asynchronous operations are resumed.
+ *
+ * When a page is in the BF Cache, we should consider it as frozen and shouldn't try
+ * to execute any javascript. So that the ExecutionContext should be considered as
+ * being destroyed and the document navigated.
+ */
+
+var EXPORTED_SYMBOLS = ["ContextObserver"];
+
+const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
+const {EventEmitter} = ChromeUtils.import("resource://gre/modules/EventEmitter.jsm");
+
+class ContextObserver {
+  constructor(chromeEventHandler) {
+    this.chromeEventHandler = chromeEventHandler;
+    EventEmitter.decorate(this);
+
+    this.chromeEventHandler.addEventListener("DOMWindowCreated", this,
+      {mozSystemGroup: true});
+
+    // Listen for pageshow and pagehide to track pages going in/out to/from the BF Cache
+    this.chromeEventHandler.addEventListener("pageshow", this,
+      {mozSystemGroup: true});
+    this.chromeEventHandler.addEventListener("pagehide", this,
+      {mozSystemGroup: true});
+
+    Services.obs.addObserver(this, "inner-window-destroyed");
+  }
+
+  destructor() {
+    this.chromeEventHandler.removeEventListener("DOMWindowCreated", this,
+      {mozSystemGroup: true});
+    this.chromeEventHandler.removeEventListener("pageshow", this,
+      {mozSystemGroup: true});
+    this.chromeEventHandler.removeEventListener("pagehide", this,
+      {mozSystemGroup: true});
+    Services.obs.removeObserver(this, "inner-window-destroyed");
+  }
+
+  handleEvent({type, target, persisted}) {
+    const window = target.defaultView;
+    if (window.top != this.chromeEventHandler.ownerGlobal) {
+      // Ignore iframes for now.
+      return;
+    }
+    const { windowUtils } = window;
+    const frameId = windowUtils.outerWindowID;
+    const id = windowUtils.currentInnerWindowID;
+    switch (type) {
+    case "DOMWindowCreated":
+      // Do not pass `id` here as that's the new document ID instead of the old one
+      // that is destroyed. Instead, pass the frameId and let the listener figure out
+      // what ExecutionContext to destroy.
+      this.emit("context-destroyed", { frameId });
+      this.emit("frame-navigated", { frameId, window });
+      this.emit("context-created", { id, window });
+      break;
+    case "pageshow":
+      // `persisted` is true when this is about a page being resurected from BF Cache
+      if (!persisted) {
+        return;
+      }
+      // XXX(ochameau) we might have to emit FrameNavigate here to properly handle BF Cache
+      // scenario in Page domain events
+      this.emit("context-created", { id, window });
+      break;
+
+    case "pagehide":
+      // `persisted` is true when this is about a page being frozen into BF Cache
+      if (!persisted) {
+        return;
+      }
+      this.emit("context-destroyed", { id });
+      break;
+    }
+  }
+
+  // "inner-window-destroyed" observer service listener
+  observe(subject, topic, data) {
+    const innerWindowID = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
+    this.emit("context-destroyed", { id: innerWindowID });
+  }
+}
+
+
--- a/remote/domains/content/Page.jsm
+++ b/remote/domains/content/Page.jsm
@@ -9,49 +9,42 @@ var EXPORTED_SYMBOLS = ["Page"];
 const {ContentProcessDomain} = ChromeUtils.import("chrome://remote/content/domains/ContentProcessDomain.jsm");
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const {UnsupportedError} = ChromeUtils.import("chrome://remote/content/Error.jsm");
 
 class Page extends ContentProcessDomain {
   constructor(session) {
     super(session);
     this.enabled = false;
+
+    this.onFrameNavigated = this.onFrameNavigated.bind(this);
   }
 
   destructor() {
     this.disable();
   }
 
-  QueryInterface(iid) {
-    if (iid.equals(Ci.nsIWebProgressListener) ||
-      iid.equals(Ci.nsISupportsWeakReference) ||
-      iid.equals(Ci.nsIObserver)) {
-      return this;
-    }
-    throw Cr.NS_ERROR_NO_INTERFACE;
-  }
-
   // commands
 
   async enable() {
     if (!this.enabled) {
       this.enabled = true;
-      this.chromeEventHandler.addEventListener("DOMWindowCreated", this,
-        {mozSystemGroup: true});
+      this.contextObserver.on("frame-navigated", this.onFrameNavigated);
+
       this.chromeEventHandler.addEventListener("DOMContentLoaded", this,
         {mozSystemGroup: true});
       this.chromeEventHandler.addEventListener("pageshow", this,
         {mozSystemGroup: true});
     }
   }
 
   disable() {
     if (this.enabled) {
-      this.chromeEventHandler.removeEventListener("DOMWindowCreated", this,
-        {mozSystemGroup: true});
+      this.contextObserver.off("frame-navigated", this.onFrameNavigated);
+
       this.chromeEventHandler.removeEventListener("DOMContentLoaded", this,
         {mozSystemGroup: true});
       this.chromeEventHandler.removeEventListener("pageshow", this,
         {mozSystemGroup: true});
       this.enabled = false;
     }
   }
 
@@ -95,38 +88,40 @@ class Page extends ContentProcessDomain 
   setLifecycleEventsEnabled() {}
   addScriptToEvaluateOnNewDocument() {}
   createIsolatedWorld() {}
 
   url() {
     return this.content.location.href;
   }
 
+  onFrameNavigated(name, { frameId, window }) {
+    const url = window.location.href;
+    this.emit("Page.frameNavigated", {
+      frame: {
+        id: frameId,
+        // frameNavigated is only emitted for the top level document
+        // so that it never has a parent.
+        parentId: null,
+        url,
+      },
+    });
+  }
+
   handleEvent({type, target}) {
     if (target.defaultView != this.content) {
       // Ignore iframes for now
       return;
     }
 
     const timestamp = Date.now();
     const frameId = target.defaultView.windowUtils.outerWindowID;
     const url = target.location.href;
 
     switch (type) {
-    case "DOMWindowCreated":
-      this.emit("Page.frameNavigated", {
-        frame: {
-          id: frameId,
-          // frameNavigated is only emitted for the top level document
-          // so that it never has a parent.
-          parentId: null,
-          url,
-        },
-      });
-      break;
     case "DOMContentLoaded":
       this.emit("Page.domContentEventFired", {timestamp});
       break;
 
     case "pageshow":
       this.emit("Page.loadEventFired", {timestamp, frameId});
       // XXX this should most likely be sent differently
       this.emit("Page.navigatedWithinDocument", {timestamp, frameId, url});
--- a/remote/domains/content/Runtime.jsm
+++ b/remote/domains/content/Runtime.jsm
@@ -17,56 +17,49 @@ addDebuggerToGlobal(Cu.getGlobalForObjec
 class Runtime extends ContentProcessDomain {
   constructor(session) {
     super(session);
     this.enabled = false;
 
     // Map of all the ExecutionContext instances:
     // [Execution context id (Number) => ExecutionContext instance]
     this.contexts = new Map();
+
+    this.onContextCreated = this.onContextCreated.bind(this);
+    this.onContextDestroyed = this.onContextDestroyed.bind(this);
   }
 
   destructor() {
     this.disable();
   }
 
   // commands
 
   async enable() {
     if (!this.enabled) {
       this.enabled = true;
-      this.chromeEventHandler.addEventListener("DOMWindowCreated", this,
-        {mozSystemGroup: true});
-
-      // Listen for pageshow and pagehide to track pages going in/out to/from the BF Cache
-      this.chromeEventHandler.addEventListener("pageshow", this,
-        {mozSystemGroup: true});
-      this.chromeEventHandler.addEventListener("pagehide", this,
-        {mozSystemGroup: true});
-
-      Services.obs.addObserver(this, "inner-window-destroyed");
+      this.contextObserver.on("context-created", this.onContextCreated);
+      this.contextObserver.on("context-destroyed", this.onContextDestroyed);
 
       // Spin the event loop in order to send the `executionContextCreated` event right
       // after we replied to `enable` request.
       Services.tm.dispatchToMainThread(() => {
-        this._createContext(this.content);
+        this.onContextCreated("context-created", {
+          id: this.content.windowUtils.currentInnerWindowID,
+          window: this.content,
+        });
       });
     }
   }
 
   disable() {
     if (this.enabled) {
       this.enabled = false;
-      this.chromeEventHandler.removeEventListener("DOMWindowCreated", this,
-        {mozSystemGroup: true});
-      this.chromeEventHandler.removeEventListener("pageshow", this,
-        {mozSystemGroup: true});
-      this.chromeEventHandler.removeEventListener("pagehide", this,
-        {mozSystemGroup: true});
-      Services.obs.removeObserver(this, "inner-window-destroyed");
+      this.contextObserver.off("context-created", this.onContextCreated);
+      this.contextObserver.off("context-destroyed", this.onContextDestroyed);
     }
   }
 
   evaluate(request) {
     const context = this.contexts.get(request.contextId);
     if (!context) {
       throw new Error(`Unable to find execution context with id: ${request.contextId}`);
     }
@@ -115,91 +108,77 @@ class Runtime extends ContentProcessDoma
   get _debugger() {
     if (this.__debugger) {
       return this.__debugger;
     }
     this.__debugger = new Debugger();
     return this.__debugger;
   }
 
-  handleEvent({type, target, persisted}) {
-    if (target.defaultView != this.content) {
-      // Ignore iframes for now.
-      return;
-    }
-    switch (type) {
-    case "DOMWindowCreated":
-      this._createContext(target.defaultView);
-      break;
-
-    case "pageshow":
-      // `persisted` is true when this is about a page being resurected from BF Cache
-      if (!persisted) {
-        return;
+  getContextByFrameId(frameId) {
+    for (const ctx of this.contexts.values()) {
+      if (ctx.frameId === frameId) {
+        return ctx;
       }
-      this._createContext(target.defaultView);
-      break;
-
-    case "pagehide":
-      // `persisted` is true when this is about a page being frozen into BF Cache
-      if (!persisted) {
-        return;
-      }
-      const id = target.defaultView.windowUtils.currentInnerWindowID;
-      this._destroyContext(id);
-      break;
     }
-  }
-
-  observe(subject, topic, data) {
-    const innerWindowID = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
-    this._destroyContext(innerWindowID);
+    return null;
   }
 
   /**
    * Helper method in order to instantiate the ExecutionContext for a given
    * DOM Window as well as emitting the related `Runtime.executionContextCreated`
    * event.
    *
    * @param {Window} window
    *     The window object of the newly instantiated document.
    */
-  _createContext(window) {
-    const { windowUtils } = window;
-    const id = windowUtils.currentInnerWindowID;
+  onContextCreated(name, { id, window }) {
     if (this.contexts.has(id)) {
       return;
     }
 
     const context = new ExecutionContext(this._debugger, window);
     this.contexts.set(id, context);
 
-    const frameId = windowUtils.outerWindowID;
     this.emit("Runtime.executionContextCreated", {
       context: {
         id,
         auxData: {
           isDefault: window == this.content,
-          frameId,
+          frameId: context.frameId,
         },
       },
     });
   }
 
   /**
    * Helper method to destroy the ExecutionContext of the given id. Also emit
    * the related `Runtime.executionContextDestroyed` event.
+   * ContextObserver will call this method with either `id` or `frameId` argument
+   * being set.
    *
    * @param {Number} id
    *     The execution context id to destroy.
+   * @param {Number} frameId
+   *     The frame id of execution context to destroy.
+   * Eiter `id` or `frameId` is passed.
    */
-  _destroyContext(id) {
-    const context = this.contexts.get(id);
+  onContextDestroyed(name, { id, frameId }) {
+    let context;
+    if (id && frameId) {
+      throw new Error("Expects only id *or* frameId argument to be passed");
+    }
+
+    if (id) {
+      context = this.contexts.get(id);
+    } else {
+      context = this.getContextByFrameId(frameId);
+    }
 
     if (context) {
       context.destructor();
-      this.contexts.delete(id);
+      this.contexts.delete(context.id);
       this.emit("Runtime.executionContextDestroyed", {
-        executionContextId: id,
+        executionContextId: context.id,
       });
     }
   }
 }
--- a/remote/domains/content/runtime/ExecutionContext.jsm
+++ b/remote/domains/content/runtime/ExecutionContext.jsm
@@ -26,16 +26,22 @@ function uuid() {
  *   The debuggable context's global object. This is typically the document window
  *   object. But it can also be any global object, like a worker global scope object.
  */
 class ExecutionContext {
   constructor(dbg, debuggee) {
     this._debugger = dbg;
     this._debuggee = this._debugger.addDebuggee(debuggee);
 
+    // Here, we assume that debuggee is a window object and we will propably have
+    // to adapt that once we cover workers or contexts that aren't a document.
+    const { windowUtils } = debuggee;
+    this.id = windowUtils.currentInnerWindowID;
+    this.frameId = windowUtils.outerWindowID;
+
     this._remoteObjects = new Map();
   }
 
   destructor() {
     this._debugger.removeDebuggee(this._debuggee);
   }
 
   hasRemoteObject(id) {
--- a/remote/jar.mn
+++ b/remote/jar.mn
@@ -26,16 +26,17 @@ remote.jar:
   content/targets/MainProcessTarget.jsm (targets/MainProcessTarget.jsm)
   content/targets/TabTarget.jsm (targets/TabTarget.jsm)
   content/targets/Target.jsm (targets/Target.jsm)
   content/targets/Targets.jsm (targets/Targets.jsm)
 
   # domains
   content/domains/ContentProcessDomain.jsm (domains/ContentProcessDomain.jsm)
   content/domains/ContentProcessDomains.jsm (domains/ContentProcessDomains.jsm)
+  content/domains/ContextObserver.jsm (domains/ContextObserver.jsm)
   content/domains/Domain.jsm (domains/Domain.jsm)
   content/domains/Domains.jsm (domains/Domains.jsm)
   content/domains/ParentProcessDomains.jsm (domains/ParentProcessDomains.jsm)
   content/domains/content/DOM.jsm (domains/content/DOM.jsm)
   content/domains/content/Emulation.jsm (domains/content/Emulation.jsm)
   content/domains/content/Input.jsm (domains/content/Input.jsm)
   content/domains/content/Log.jsm (domains/content/Log.jsm)
   content/domains/content/Network.jsm (domains/content/Network.jsm)
--- a/remote/test/browser/browser.ini
+++ b/remote/test/browser/browser.ini
@@ -4,14 +4,15 @@ prefs = remote.enabled=true
 support-files =
   chrome-remote-interface.js
   head.js
 skip-if = debug || asan # bug 1546945
 
 [browser_cdp.js]
 [browser_main_target.js]
 [browser_page_frameNavigated.js]
+[browser_page_runtime_events.js]
 [browser_runtime_evaluate.js]
 [browser_runtime_callFunctionOn.js]
 [browser_runtime_executionContext.js]
 skip-if = os == "mac" || (verify && os == 'win') # bug 1547961
 [browser_tabs.js]
 [browser_target.js]
new file mode 100644
--- /dev/null
+++ b/remote/test/browser/browser_page_runtime_events.js
@@ -0,0 +1,115 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/* global getCDP */
+
+// Assert the order of Runtime.executionContextDestroyed, Page.frameNavigated and Runtime.executionContextCreated
+
+const TEST_URI = "data:text/html;charset=utf-8,default-test-page";
+
+add_task(async function testCDP() {
+  // Open a test page, to prevent debugging the random default page
+  await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URI);
+
+  // Start the CDP server
+  RemoteAgent.listen(Services.io.newURI("http://localhost:9222"));
+
+  // Retrieve the chrome-remote-interface library object
+  const CDP = await getCDP();
+
+  // Connect to the server
+  const client = await CDP({
+    target(list) {
+      // Ensure debugging the right target, i.e. the one for our test tab.
+      return list.find(target => {
+        return target.url == TEST_URI;
+      });
+    },
+  });
+  ok(true, "CDP client has been instantiated");
+
+  const {Page, Runtime} = client;
+
+  const events = [];
+  function assertReceivedEvents(expected, message) {
+    Assert.deepEqual(events, expected, message);
+    // Empty the list of received events
+    events.splice(0);
+  }
+  Page.frameNavigated(() => {
+    events.push("frameNavigated");
+  });
+  Runtime.executionContextCreated(() => {
+    events.push("executionContextCreated");
+  });
+  Runtime.executionContextDestroyed(() => {
+    events.push("executionContextDestroyed");
+  });
+
+  // turn on navigation related events, such as DOMContentLoaded et al.
+  await Page.enable();
+  ok(true, "Page domain has been enabled");
+
+  const onExecutionContextCreated = Runtime.executionContextCreated();
+  await Runtime.enable();
+  ok(true, "Runtime domain has been enabled");
+
+  // Runtime.enable will dispatch `executionContextCreated` for the existing document
+  let { context } = await onExecutionContextCreated;
+  ok(!!context.id, "The execution context has an id");
+  ok(context.auxData.isDefault, "The execution context is the default one");
+  ok(!!context.auxData.frameId, "The execution context has a frame id set");
+
+  assertReceivedEvents(["executionContextCreated"], "Received only executionContextCreated event after Runtime.enable call");
+
+  const { frameTree } = await Page.getFrameTree();
+  ok(!!frameTree.frame, "getFrameTree exposes one frame");
+  is(frameTree.childFrames.length, 0, "getFrameTree reports no child frame");
+  ok(!!frameTree.frame.id, "getFrameTree's frame has an id");
+  is(frameTree.frame.url, TEST_URI, "getFrameTree's frame has the right url");
+  is(frameTree.frame.id, context.auxData.frameId, "getFrameTree and executionContextCreated refers about the same frame Id");
+
+  const onFrameNavigated = Page.frameNavigated();
+  const onExecutionContextDestroyed = Runtime.executionContextDestroyed();
+  const onExecutionContextCreated2 = Runtime.executionContextCreated();
+  const url = "data:text/html;charset=utf-8,test-page";
+  const { frameId } = await Page.navigate({ url  });
+  ok(true, "A new page has been loaded");
+  ok(frameId, "Page.navigate returned a frameId");
+  is(frameId, frameTree.frame.id, "The Page.navigate's frameId is the same than " +
+    "getFrameTree's one");
+
+  const frameNavigated = await onFrameNavigated;
+  ok(!frameNavigated.frame.parentId, "frameNavigated is for the top level document and" +
+    " has a null parentId");
+  is(frameNavigated.frame.id, frameId, "frameNavigated id is the same than the one " +
+    "returned by Page.navigate");
+  is(frameNavigated.frame.name, undefined, "frameNavigated name isn't implemented yet");
+  is(frameNavigated.frame.url, url, "frameNavigated url is the same being given to " +
+    "Page.navigate");
+
+  const { executionContextId } = await onExecutionContextDestroyed;
+  ok(executionContextId, "The destroyed event reports an id");
+  is(executionContextId, context.id, "The destroyed event is for the first reported execution context");
+
+  ({ context } = await onExecutionContextCreated2);
+  ok(!!context.id, "The execution context has an id");
+  ok(context.auxData.isDefault, "The execution context is the default one");
+  is(context.auxData.frameId, frameId, "The execution context frame id is the same " +
+    "the one returned by Page.navigate");
+
+  isnot(executionContextId, context.id, "The destroyed id is different from the " +
+    "created one");
+
+  assertReceivedEvents(["executionContextDestroyed", "frameNavigated", "executionContextCreated"],
+    "Received frameNavigated between the two execution context events during navigation to another URL");
+
+  await client.close();
+  ok(true, "The client is closed");
+
+  BrowserTestUtils.removeTab(gBrowser.selectedTab);
+
+  await RemoteAgent.close();
+});