Bug 1743083 - [remote] Instantiate MessageHandler modules after constructor has returned r=webdriver-reviewers,whimboo

Depends on D132064

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

remote/shared/messagehandler/MessageHandler.jsm

@@ -79,10 +79,8 @@ class MessageHandler extends EventEmitter {
    *     ID of the session the handler is used for.
    * @param {Object} context
    *     The context linked to this MessageHandler instance.
-   * @param {Array<SessionDataItem>} sessionDataItems
-   *     Initial session data items for this MessageHandler.
    */
-  constructor(sessionId, context, sessionDataItems) {
+  constructor(sessionId, context) {
     super();
 
     this._moduleCache = new ModuleCache(this);
@@ -90,12 +88,6 @@ class MessageHandler extends EventEmitter {
     this._sessionId = sessionId;
     this._context = context;
     this._contextId = this.constructor.getIdFromContext(context);
-
-    // If any session data item was provided to the constructor, apply it for
-    // this MessageHandler.
-    if (Array.isArray(sessionDataItems)) {
-      this._applyInitialSessionDataItems(sessionDataItems);
-    }
   }
 
   get contextId() {
@@ -226,8 +218,11 @@ class MessageHandler extends EventEmitter {
    * startup. Implementation is specific to each MessageHandler class.
    *
    * By default the implementation is a no-op.
+   *
+   * @param {Array<SessionDataItem>} sessionDataItems
+   *     Initial session data items for this MessageHandler.
    */
-  async _applyInitialSessionDataItems(sessionDataItems) {}
+  async applyInitialSessionDataItems(sessionDataItems) {}
 
   /**
    * Returns the module path corresponding to this MessageHandler class.

remote/shared/messagehandler/MessageHandlerRegistry.jsm

@@ -193,9 +193,11 @@ class MessageHandlerRegistry extends EventEmitter {
   _createMessageHandler(sessionId, sessionDataItems) {
     const messageHandler = new this._messageHandlerClass(
       sessionId,
-      this._context,
-      sessionDataItems
+      this._context
     );
+
+    messageHandler.applyInitialSessionDataItems(sessionDataItems);
+
     this._messageHandlersMap.set(sessionId, messageHandler);
 
     logger.trace(

remote/shared/messagehandler/WindowGlobalMessageHandler.jsm

@@ -28,9 +28,7 @@ class WindowGlobalMessageHandler extends MessageHandler {
   constructor() {
     super(...arguments);
 
-    // Bug 1743083: Temporarily delay caching of the inner window id until
-    // the session data is no longer applied before super() returns.
-    this._innerWindowId;
+    this._innerWindowId = this._context.window.windowGlobalChild.innerWindowId;
   }
 
   /**
@@ -66,20 +64,14 @@ class WindowGlobalMessageHandler extends MessageHandler {
   }
 
   get innerWindowId() {
-    if (this._innerWindowId === undefined) {
-      this._innerWindowId = this._context.window.windowGlobalChild.innerWindowId;
-    }
-
     return this._innerWindowId;
   }
 
-  forwardCommand(command) {
-    throw new Error(
-      `Cannot forward commands from a "WINDOW_GLOBAL" MessageHandler`
-    );
-  }
+  async applyInitialSessionDataItems(sessionDataItems) {
+    if (!Array.isArray(sessionDataItems)) {
+      return;
+    }
 
-  async _applyInitialSessionDataItems(sessionDataItems) {
     for (const sessionDataItem of sessionDataItems) {
       const {
         moduleName,
@@ -107,6 +99,12 @@ class WindowGlobalMessageHandler extends MessageHandler {
     }
   }
 
+  forwardCommand(command) {
+    throw new Error(
+      `Cannot forward commands from a "WINDOW_GLOBAL" MessageHandler`
+    );
+  }
+
   _isRelevantContext(contextDescriptor) {
     // Once we allow to filter on browsing contexts, the contextDescriptor would
     // for instance contain a browserId on top of a type. For instance:

remote/shared/messagehandler/test/browser/broadcast/head.js

@@ -10,14 +10,6 @@ Services.scriptloader.loadSubScript(
   this
 );
 
-var { CONTEXT_DESCRIPTOR_TYPES } = ChromeUtils.import(
-  "chrome://remote/content/shared/messagehandler/MessageHandler.jsm"
-);
-
-var contextDescriptorAll = {
-  type: CONTEXT_DESCRIPTOR_TYPES.ALL,
-};
-
 /**
  * Broadcast the provided method to WindowGlobal contexts on a MessageHandler
  * network.

remote/shared/messagehandler/test/browser/browser.ini

@@ -13,3 +13,4 @@ prefs =
 [browser_registry.js]
 [browser_session_data.js]
 [browser_session_data_broadcast.js]
+[browser_session_data_constructor_race.js]

remote/shared/messagehandler/test/browser/browser_session_data.js

@@ -6,9 +6,6 @@
 const { MessageHandlerRegistry } = ChromeUtils.import(
   "chrome://remote/content/shared/messagehandler/MessageHandlerRegistry.jsm"
 );
-const { CONTEXT_DESCRIPTOR_TYPES } = ChromeUtils.import(
-  "chrome://remote/content/shared/messagehandler/MessageHandler.jsm"
-);
 const { RootMessageHandler } = ChromeUtils.import(
   "chrome://remote/content/shared/messagehandler/RootMessageHandler.jsm"
 );
@@ -24,7 +21,6 @@ add_task(async function test_sessionData() {
   await loadURL(tab1.linkedBrowser, TEST_PAGE);
 
   const sessionId = "sessionData-test";
-  const contextDescriptorAll = { type: CONTEXT_DESCRIPTOR_TYPES.ALL };
 
   const rootMessageHandlerRegistry = new MessageHandlerRegistry(
     RootMessageHandler.type

remote/shared/messagehandler/test/browser/browser_session_data_constructor_race.js

@@ -0,0 +1,56 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const { WindowGlobalMessageHandler } = ChromeUtils.import(
+  "chrome://remote/content/shared/messagehandler/WindowGlobalMessageHandler.jsm"
+);
+
+const TEST_PAGE = "https://example.com/document-builder.sjs?html=tab";
+
+/**
+ * Check that modules created early for session data are still created with a
+ * fully initialized MessageHandler. See Bug 1743083.
+ */
+add_task(async function() {
+  const tab = BrowserTestUtils.addTab(gBrowser, TEST_PAGE);
+  await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+  const browsingContext = tab.linkedBrowser.browsingContext;
+
+  const root = createRootMessageHandler("session-id-event");
+
+  info("Add some session data for the command module");
+  await root.addSessionData({
+    moduleName: "command",
+    category: "testCategory",
+    contextDescriptor: contextDescriptorAll,
+    values: ["some-value"],
+  });
+
+  info("Reload the current tab to create new message handlers and modules");
+  const finished = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+  gBrowser.reloadTab(tab);
+  await finished;
+
+  info(
+    "Check if the command module was created by the MessageHandler constructor"
+  );
+  const isCreatedByMessageHandlerConstructor = await root.handleCommand({
+    moduleName: "command",
+    commandName: "testIsCreatedByMessageHandlerConstructor",
+    destination: {
+      type: WindowGlobalMessageHandler.type,
+      id: browsingContext.id,
+    },
+  });
+
+  is(
+    isCreatedByMessageHandlerConstructor,
+    false,
+    "The command module from session data should not be created by the MessageHandler constructor"
+  );
+  root.destroy();
+
+  gBrowser.removeTab(tab);
+});

remote/shared/messagehandler/test/browser/head.js

@@ -4,6 +4,14 @@
 
 "use strict";
 
+var { CONTEXT_DESCRIPTOR_TYPES } = ChromeUtils.import(
+  "chrome://remote/content/shared/messagehandler/MessageHandler.jsm"
+);
+
+var contextDescriptorAll = {
+  type: CONTEXT_DESCRIPTOR_TYPES.ALL,
+};
+
 function createRootMessageHandler(sessionId) {
   const { RootMessageHandlerRegistry } = ChromeUtils.import(
     "chrome://remote/content/shared/messagehandler/RootMessageHandlerRegistry.jsm"

remote/shared/messagehandler/test/browser/resources/modules/windowglobal/command.jsm

@@ -14,6 +14,8 @@ class Command extends Module {
   constructor(messageHandler) {
     super(messageHandler);
     this._testCategorySessionData = [];
+
+    this._createdByMessageHandlerConstructor = this._isCreatedByMessageHandlerConstructor();
   }
   destroy() {}
 
@@ -53,6 +55,21 @@ class Command extends Module {
   testForwardToWindowGlobal() {
     return "forward-to-windowglobal-value";
   }
+
+  testIsCreatedByMessageHandlerConstructor() {
+    return this._createdByMessageHandlerConstructor;
+  }
+
+  _isCreatedByMessageHandlerConstructor() {
+    let caller = Components.stack.caller;
+    while (caller) {
+      if (caller.name === this.messageHandler.constructor.name) {
+        return true;
+      }
+      caller = caller.caller;
+    }
+    return false;
+  }
 }
 
 const command = Command;