Bug 1597877 - Allow many ExecutionContexts per inner window; r=remote-protocol-reviewers,ato,whimboo

Dismantle the assumption that there is one ExecutionContext per
inner window and generate a fresh id for each ExecutionContext
rather than reusing the inner window id.

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

--HG--
extra : moz-landing-system : lando

remote/domains/ContextObserver.jsm

@@ -76,10 +76,10 @@ class ContextObserver {
       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.
+        // what ExecutionContext(s) to destroy.
         this.emit("context-destroyed", { frameId });
         this.emit("frame-navigated", { frameId, window });
-        this.emit("context-created", { id, window });
+        this.emit("context-created", { windowId: id, window });
         break;
       case "pageshow":
         // `persisted` is true when this is about a page being resurected from BF Cache
@@ -88,7 +88,7 @@ class ContextObserver {
         }
         // 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 });
+        this.emit("context-created", { windowId: id, window });
         break;
 
       case "pagehide":
@@ -96,7 +96,7 @@ class ContextObserver {
         if (!persisted) {
           return;
         }
-        this.emit("context-destroyed", { id });
+        this.emit("context-destroyed", { windowId: id });
         break;
     }
   }
@@ -104,6 +104,6 @@ class ContextObserver {
   // "inner-window-destroyed" observer service listener
   observe(subject, topic, data) {
     const innerWindowID = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
-    this.emit("context-destroyed", { id: innerWindowID });
+    this.emit("context-destroyed", { windowId: innerWindowID });
   }
 }

remote/domains/content/Runtime.jsm

@@ -21,14 +21,42 @@ const { addDebuggerToGlobal } = ChromeUtils.import(
 // Import the `Debugger` constructor in the current scope
 addDebuggerToGlobal(Cu.getGlobalForObject(this));
 
+class SetMap extends Map {
+  constructor() {
+    super();
+    this._count = 1;
+  }
+  // Every key in the map is associated with a Set.
+  // The first time `key` is used `obj.set(key, value)` maps `key` to
+  // to `Set(value)`. Subsequent calls add more values to the Set for `key`.
+  // Note that `obj.get(key)` will return undefined if there's no such key,
+  // as in a regular Map.
+  set(key, value) {
+    const innerSet = this.get(key);
+    if (innerSet) {
+      innerSet.add(value);
+    } else {
+      super.set(key, new Set([value]));
+    }
+    this._count++;
+    return this;
+  }
+  // used as ExecutionContext id
+  get count() {
+    return this._count;
+  }
+}
+
 class Runtime extends ContentProcessDomain {
   constructor(session) {
     super(session);
     this.enabled = false;
 
     // Map of all the ExecutionContext instances:
-    // [Execution context id (Number) => ExecutionContext instance]
+    // [id (Number) => ExecutionContext instance]
     this.contexts = new Map();
+    // [innerWindowId (Number) => Set of ExecutionContext instances]
+    this.contextsByWindow = new SetMap();
 
     this._onContextCreated = this._onContextCreated.bind(this);
     this._onContextDestroyed = this._onContextDestroyed.bind(this);
@@ -52,8 +80,9 @@ class Runtime extends ContentProcessDomain {
       // after we replied to `enable` request.
       Services.tm.dispatchToMainThread(() => {
         this._onContextCreated("context-created", {
-          id: this.content.windowUtils.currentInnerWindowID,
+          windowId: this.content.windowUtils.currentInnerWindowID,
           window: this.content,
+          isDefault: true,
         });
       });
     }
@@ -77,7 +106,7 @@ class Runtime extends ContentProcessDomain {
         );
       }
     } else {
-      context = this._getCurrentContext();
+      context = this._getDefaultContextForWindow();
     }
 
     if (typeof expression != "string") {
@@ -186,45 +215,93 @@ class Runtime extends ContentProcessDomain {
     return null;
   }
 
-  _getCurrentContext() {
-    const { windowUtils } = this.content;
-    return this.contexts.get(windowUtils.currentInnerWindowID);
-  }
-
-  _getContextByFrameId(frameId) {
-    for (const ctx of this.contexts.values()) {
-      if (ctx.frameId == frameId) {
-        return ctx;
+  _getDefaultContextForWindow(innerWindowId) {
+    if (!innerWindowId) {
+      const { windowUtils } = this.content;
+      innerWindowId = windowUtils.currentInnerWindowID;
+    }
+    const curContexts = this.contextsByWindow.get(innerWindowId);
+    if (curContexts) {
+      for (const ctx of curContexts) {
+        if (ctx.isDefault) {
+          return ctx;
+        }
       }
     }
     return null;
   }
 
+  _getContextsForFrame(frameId) {
+    const frameContexts = [];
+    for (const ctx of this.contexts.values()) {
+      if (ctx.frameId == frameId) {
+        frameContexts.push(ctx);
+      }
+    }
+    return frameContexts;
+  }
+
   /**
    * Helper method in order to instantiate the ExecutionContext for a given
-   * DOM Window as well as emitting the related `Runtime.executionContextCreated`
-   * event.
+   * DOM Window as well as emitting the related
+   * `Runtime.executionContextCreated` event
    *
-   * @param {Window} window
+   * @param {string} name
+   *     Event name
+   * @param {Object=} options
+   * @param {number} options.windowId
+   *     The inner window id of the newly instantiated document.
+   * @param {Window} options.window
    *     The window object of the newly instantiated document.
+   * @param {string=} options.contextName
+   *     Human-readable name to describe the execution context.
+   * @param {boolean=} options.isDefault
+   *     Whether the execution context is the default one.
+   * @param {string=} options.contextType
+   *     "default" or "isolated"
+   *
    */
-  _onContextCreated(name, { id, window }) {
-    if (this.contexts.has(id)) {
-      return;
+  _onContextCreated(name, options = {}) {
+    const {
+      windowId,
+      window,
+      contextName = "",
+      isDefault = options.window == this.content,
+      contextType = options.contextType ||
+        (options.window == this.content ? "default" : ""),
+    } = options;
+
+    if (windowId === undefined) {
+      throw new Error("windowId is required");
     }
 
-    const context = new ExecutionContext(this._debugger, window);
-    this.contexts.set(id, context);
+    // allow only one default context per inner window
+    if (isDefault && this.contextsByWindow.has(windowId)) {
+      for (const ctx of this.contextsByWindow.get(windowId)) {
+        if (ctx.isDefault) {
+          return;
+        }
+      }
+    }
+
+    const context = new ExecutionContext(
+      this._debugger,
+      window,
+      this.contextsByWindow.count,
+      isDefault
+    );
+    this.contexts.set(context.id, context);
+    this.contextsByWindow.set(windowId, context);
 
     this.emit("Runtime.executionContextCreated", {
       context: {
-        id,
+        id: context.id,
         origin: window.location.href,
-        name: "",
+        name: contextName,
         auxData: {
-          isDefault: window == this.content,
+          isDefault,
           frameId: context.frameId,
-          type: window == this.content ? "default" : "",
+          type: contextType,
         },
       },
     });
@@ -236,30 +313,41 @@ class Runtime extends ContentProcessDomain {
    * ContextObserver will call this method with either `id` or `frameId` argument
    * being set.
    *
+   * @param {string} name
+   *     Event name
+   * @param {Object=} options
    * @param {number} id
    *     The execution context id to destroy.
+   * @param {number} windowId
+   *     The inner-window id of the execution context to destroy.
    * @param {string} frameId
    *     The frame id of execution context to destroy.
-   * Eiter `id` or `frameId` is passed.
+   * Either `id` or `frameId` or `windowId` is passed.
    */
-  _onContextDestroyed(name, { id, frameId }) {
-    let context;
-    if (id && frameId) {
-      throw new Error("Expects only id *or* frameId argument to be passed");
+  _onContextDestroyed(name, { id, frameId, windowId }) {
+    let contexts;
+    if ([id, frameId, windowId].filter(id => !!id).length > 1) {
+      throw new Error("Expects only *one* of id, frameId, windowId");
     }
 
     if (id) {
-      context = this.contexts.get(id);
+      contexts = [this.contexts.get(id)];
+    } else if (frameId) {
+      contexts = this._getContextsForFrame(frameId);
     } else {
-      context = this._getContextByFrameId(frameId);
+      contexts = this.contextsByWindow.get(windowId) || [];
     }
 
-    if (context) {
-      context.destructor();
-      this.contexts.delete(context.id);
+    for (const ctx of contexts) {
+      ctx.destructor();
+      this.contexts.delete(ctx.id);
+      this.contextsByWindow.get(ctx.windowId).delete(ctx);
       this.emit("Runtime.executionContextDestroyed", {
-        executionContextId: context.id,
+        executionContextId: ctx.id,
       });
+      if (this.contextsByWindow.get(ctx.windowId).size == 0) {
+        this.contextsByWindow.delete(ctx.windowId);
+      }
     }
   }
 }

remote/domains/content/runtime/ExecutionContext.jsm

@@ -40,15 +40,17 @@ function uuid() {
  *   object. But it can also be any global object, like a worker global scope object.
  */
 class ExecutionContext {
-  constructor(dbg, debuggee) {
+  constructor(dbg, debuggee, id, isDefault) {
     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.windowId = windowUtils.currentInnerWindowID;
+    this.id = id;
     this.frameId = windowUtils.outerWindowID.toString();
+    this.isDefault = isDefault;
 
     this._remoteObjects = new Map();
   }

remote/test/browser/page/browser_runtimeEvents.js

@@ -41,7 +41,7 @@ add_task(async function testCDP(client) {
 
   // Runtime.enable will dispatch `executionContextCreated` for the existing document
   let { context } = await onExecutionContextCreated;
-  ok(!!context.id, "The execution context has an id");
+  ok(!!context.id, `The execution context has an id ${context.id}`);
   ok(context.auxData.isDefault, "The execution context is the default one");
   ok(!!context.auxData.frameId, "The execution context has a frame id set");
 

remote/test/browser/runtime/browser_executionContext.js

@@ -10,12 +10,12 @@ const TEST_DOC = toDataURL("default-test-page");
 add_task(async function(client) {
   await loadURL(TEST_DOC);
 
-  const firstContext = await testRuntimeEnable(client);
-  await testEvaluate(client, firstContext);
-  const secondContext = await testNavigate(client, firstContext);
-  await testNavigateBack(client, firstContext, secondContext);
-  const thirdContext = await testNavigateViaLocation(client, firstContext);
-  await testReload(client, thirdContext);
+  const context1 = await testRuntimeEnable(client);
+  await testEvaluate(client, context1);
+  const context2 = await testNavigate(client, context1);
+  const context3 = await testNavigateBack(client, context1, context2);
+  const context4 = await testNavigateViaLocation(client, context3);
+  await testReload(client, context4);
 });
 
 async function testRuntimeEnable({ Runtime }) {
@@ -60,7 +60,7 @@ async function testNavigate({ Runtime, Page }, previousContext) {
   is(
     frameId,
     previousContext.auxData.frameId,
-    "Page.navigate returns the same frameId than executionContextCreated"
+    "Page.navigate returns the same frameId as executionContextCreated"
   );
 
   const { executionContextId } = await executionContextDestroyed;
@@ -84,6 +84,9 @@ async function testNavigate({ Runtime, Page }, previousContext) {
     "The execution context frame id is the same " +
       "than the one returned by Page.navigate"
   );
+  is(context.auxData.type, "default", "Execution context has 'default' type");
+  ok(!!context.origin, "The execution context has an origin");
+  is(context.name, "", "The default execution context is named ''");
 
   isnot(
     executionContextId,
@@ -95,7 +98,7 @@ async function testNavigate({ Runtime, Page }, previousContext) {
 }
 
 // Navigates back to the previous page.
-// This should resurect the original document from the BF Cache and recreate the
+// This should resurrect the original document from the BF Cache and recreate the
 // context for it
 async function testNavigateBack({ Runtime }, firstContext, previousContext) {
   info("Navigate back to the previous document");
@@ -106,10 +109,16 @@ async function testNavigateBack({ Runtime }, firstContext, previousContext) {
   gBrowser.selectedBrowser.goBack();
 
   const { context } = await executionContextCreated;
+  ok(!!context.origin, "The execution context has an origin");
   is(
+    context.origin,
+    firstContext.origin,
+    "The new execution context should have the same origin as the first."
+  );
+  isnot(
     context.id,
     firstContext.id,
-    "The new execution context should be the same than the first one"
+    "The new execution context should have a different id"
   );
   ok(context.auxData.isDefault, "The execution context is the default one");
   is(
@@ -117,6 +126,8 @@ async function testNavigateBack({ Runtime }, firstContext, previousContext) {
     firstContext.auxData.frameId,
     "The execution context frame id is always the same"
   );
+  is(context.auxData.type, "default", "Execution context has 'default' type");
+  is(context.name, "", "The default execution context is named ''");
 
   const { executionContextId } = await executionContextDestroyed;
   is(
@@ -134,6 +145,8 @@ async function testNavigateBack({ Runtime }, firstContext, previousContext) {
     TEST_DOC,
     "Runtime.evaluate works and is against the page we just navigated to"
   );
+
+  return context;
 }
 
 async function testNavigateViaLocation({ Runtime }, previousContext) {
@@ -164,6 +177,9 @@ async function testNavigateViaLocation({ Runtime }, previousContext) {
     "The execution context frame id is the same " +
       "the one returned by Page.navigate"
   );
+  is(context.auxData.type, "default", "Execution context has 'default' type");
+  ok(!!context.origin, "The execution context has an origin");
+  is(context.name, "", "The default execution context is named ''");
 
   isnot(
     executionContextId,
@@ -197,6 +213,9 @@ async function testReload({ Runtime, Page }, previousContext) {
     previousContext.auxData.frameId,
     "The execution context frame id is the same one"
   );
+  is(context.auxData.type, "default", "Execution context has 'default' type");
+  ok(!!context.origin, "The execution context has an origin");
+  is(context.name, "", "The default execution context is named ''");
 
   isnot(
     executionContextId,