Bug 1181100 - Fix actors being registered dynamically when closing the first connected tab. r=jryans,mratcliffe

toolkit/devtools/server/actors/storage.js

@@ -649,9 +649,9 @@ StorageActors.createActor({
     }
 
     const { sendSyncMessage, addMessageListener } =
-      DebuggerServer.parentMessageManager;
+      this.conn.parentMessageManager;
 
-    DebuggerServer.setupInParent({
+    this.conn.setupInParent({
       module: "devtools/server/actors/storage",
       setupParent: "setupParentProcessForCookies"
     });
@@ -749,7 +749,6 @@ let cookieHelpers = {
         break;
       case "removeCookieObservers":
         return cookieHelpers.removeCookieObservers();
-        return null;
       default:
         console.error("ERR_DIRECTOR_PARENT_UNKNOWN_METHOD", msg.json.method);
         throw new Error("ERR_DIRECTOR_PARENT_UNKNOWN_METHOD");
@@ -761,7 +760,7 @@ let cookieHelpers = {
  * E10S parent/child setup helpers
  */
 
-exports.setupParentProcessForCookies = function({mm, childID}) {
+exports.setupParentProcessForCookies = function({mm, prefix}) {
   cookieHelpers.onCookieChanged =
     callChildProcess.bind(null, "onCookieChanged");
 
@@ -769,7 +768,7 @@ exports.setupParentProcessForCookies = function({mm, childID}) {
   mm.addMessageListener("storage:storage-cookie-request-parent",
                         cookieHelpers.handleChildRequest);
 
-  DebuggerServer.once("disconnected-from-child:" + childID,
+  DebuggerServer.once("disconnected-from-child:" + prefix,
                       handleMessageManagerDisconnected);
 
   gTrackedMessageManager.set("cookies", mm);
@@ -1038,6 +1037,9 @@ StorageActors.createActor({
 }, {
   initialize: function(storageActor) {
     protocol.Actor.prototype.initialize.call(this, null);
+
+    this.storageActor = storageActor;
+
     this.maybeSetupChildProcess();
 
     this.objectsSize = {};
@@ -1236,9 +1238,9 @@ StorageActors.createActor({
     }
 
     const { sendSyncMessage, addMessageListener } =
-      DebuggerServer.parentMessageManager;
+      this.conn.parentMessageManager;
 
-    DebuggerServer.setupInParent({
+    this.conn.setupInParent({
       module: "devtools/server/actors/storage",
       setupParent: "setupParentProcessForIndexedDB"
     });
@@ -1602,12 +1604,12 @@ let indexedDBHelpers = {
  * E10S parent/child setup helpers
  */
 
-exports.setupParentProcessForIndexedDB = function({mm, childID}) {
+exports.setupParentProcessForIndexedDB = function({mm, prefix}) {
   // listen for director-script requests from the child process
   mm.addMessageListener("storage:storage-indexedDB-request-parent",
                         indexedDBHelpers.handleChildRequest);
 
-  DebuggerServer.once("disconnected-from-child:" + childID,
+  DebuggerServer.once("disconnected-from-child:" + prefix,
                       handleMessageManagerDisconnected);
 
   gTrackedMessageManager.set("indexedDB", mm);

toolkit/devtools/server/child.js

@@ -17,14 +17,11 @@ let chromeGlobal = this;
   const { dumpn } = DevToolsUtils;
   const { DebuggerServer, ActorPool } = Cu.import("resource://gre/modules/devtools/dbg-server.jsm", {});
 
+  // Note that this frame script may be evaluated in non-e10s build
+  // In such case, DebuggerServer is already going to be initialized.
   if (!DebuggerServer.initialized) {
     DebuggerServer.init();
-
-    // message manager helpers provided for actor module parent/child message exchange
-    DebuggerServer.parentMessageManager = {
-      sendSyncMessage: sendSyncMessage,
-      addMessageListener: addMessageListener
-    };
+    DebuggerServer.isInChildProcess = true;
   }
 
   // In case of apps being loaded in parent process, DebuggerServer is already
@@ -42,6 +39,7 @@ let chromeGlobal = this;
     let prefix = msg.data.prefix;
 
     let conn = DebuggerServer.connectToParent(prefix, mm);
+    conn.parentMessageManager = mm;
     connections.set(prefix, conn);
 
     let actor = new DebuggerServer.ContentActor(conn, chromeGlobal, prefix);

toolkit/devtools/server/docs/actor-e10s-handling.md

@@ -25,7 +25,9 @@ E.g. in the **director-registry**:
   }
 
   function setupChildProcess() {
-    DebuggerServer.setupInParent({
+    // `setupInParent`  is defined on DebuggerServerConnection,
+    // your actor receive a reference to one instance in its constructor.
+    conn.setupInParent({
       module: "devtools/server/actors/director-registry",
       setupParent: "setupParentProcess"
     });
@@ -33,7 +35,7 @@ E.g. in the **director-registry**:
   }
 ```
 
-The `setupChildProcess` helper defined and used in the previous example uses the `DebuggerServer.setupInParent` to run a given setup function in the parent process Debugger Server, e.g. in the **director-registry** module.
+The `setupChildProcess` helper defined and used in the previous example uses the `DebuggerServerConnection.setupInParent` to run a given setup function in the parent process Debugger Server, e.g. in the **director-registry** module.
 
 With this, the `DebuggerServer` running in the parent process will require the requested module (**director-registry**) and call its `setupParentProcess` function (which should be exported on the module).
 
@@ -81,7 +83,7 @@ exports.setupParentProcess = function setupParentProcess({ mm, prefix }) {
   }
 ```
 
-The `DebuggerServer` emits "disconnected-from-child:CHILDID" events to give the actor modules the chance to cleanup their handlers registered on the disconnected message manager.
+The `DebuggerServer` emits "disconnected-from-child:PREFIX" events to give the actor modules the chance to cleanup their handlers registered on the disconnected message manager.
 
 ## Summary of the setup flow
 
@@ -89,13 +91,13 @@ In the child process:
 
 * The `DebuggerServer` loads an actor module,
 * the actor module checks `DebuggerServer.isInChildProcess` to know whether it runs in a child process or not,
-* the actor module then uses the `DebuggerServer.setupInParent` helper to start setting up a parent-process counterpart,
-* the `DebuggerServer.setupInParent` helper asks the parent process to run the required module's setup function,
-* the actor module uses the `DebuggerServer.parentMessageManager.sendSyncMessage` and `DebuggerServer.parentMessageManager.addMessageListener` helpers to send or listen to message.
+* the actor module then uses the `DebuggerServerConnection.setupInParent` helper to start setting up a parent-process counterpart,
+* the `DebuggerServerConnection.setupInParent` helper asks the parent process to run the required module's setup function,
+* the actor module uses the `DebuggerServerConnection.parentMessageManager.sendSyncMessage` and `DebuggerServerConnection.parentMessageManager.addMessageListener` helpers to send or listen to message.
 
 In the parent process:
 
-* The DebuggerServer receives the `DebuggerServer.setupInParent` request,
+* The DebuggerServer receives the `DebuggerServerConnection.setupInParent` request,
 * tries to load the required module,
 * tries to call the `module[setupParent]` function with the frame message manager and the prefix as parameters `{ mm, prefix }`,
 * the `setupParent` function then uses the mm to subscribe the messagemanager events,

toolkit/devtools/server/main.js

@@ -878,13 +878,12 @@ var DebuggerServer = {
 
   /**
    * Check if the caller is running in a content child process.
+   * (Eventually set by child.js)
    *
    * @return boolean
    *         true if the caller is running in a content
    */
-  get isInChildProcess() {
-    return !!this.parentMessageManager;
-  },
+  isInChildProcess: false,
 
   /**
    * In a chrome parent process, ask all content child processes
@@ -901,40 +900,19 @@ var DebuggerServer = {
       return;
     }
 
-    const gMessageManager = Cc["@mozilla.org/globalmessagemanager;1"].
-      getService(Ci.nsIMessageListenerManager);
-
-    gMessageManager.broadcastAsyncMessage("debug:setup-in-child", {
+    this._childMessageManagers.forEach(mm => {
+      mm.sendAsyncMessage("debug:setup-in-child", {
         module: module,
         setupChild: setupChild,
         args: args,
       });
+    });
   },
 
   /**
-   * In a content child process, ask the DebuggerServer in the parent process
-   * to execute a given module setup helper.
-   *
-   * @param module
-   *        The module to be required
-   * @param setupParent
-   *        The name of the setup helper exported by the above module
-   *        (setup helper signature: function ({mm}) { ... })
-   * @return boolean
-   *         true if the setup helper returned successfully
+   * Live list of all currenctly attached child's message managers.
    */
-  setupInParent: function({ module, setupParent }) {
-    if (!this.isInChildProcess) {
-      return false;
-    }
-
-    let { sendSyncMessage } = DebuggerServer.parentMessageManager;
-
-    return sendSyncMessage("debug:setup-in-parent", {
-      module: module,
-      setupParent: setupParent
-    });
-  },
+  _childMessageManagers: new Set(),
 
   /**
    * Connect to a child process.
@@ -957,6 +935,7 @@ var DebuggerServer = {
     let mm = aFrame.QueryInterface(Ci.nsIFrameLoaderOwner).frameLoader
              .messageManager;
     mm.loadFrameScript("resource://gre/modules/devtools/server/child.js", false);
+    this._childMessageManagers.add(mm);
 
     let actor, childTransport;
     let prefix = aConnection.allocID("child");
@@ -1062,6 +1041,8 @@ var DebuggerServer = {
         mm.removeMessageListener("debug:actor", onActorCreated);
       }
       events.off(aConnection, "closed", destroy);
+
+      DebuggerServer._childMessageManagers.delete(mm);
     });
 
     // Listen for app process exit
@@ -1333,6 +1314,13 @@ DebuggerServerConnection.prototype = {
   _transport: null,
   get transport() { return this._transport },
 
+  /**
+   * Message manager used to communicate with the parent process,
+   * set by child.js. Is only defined for connections instantiated
+   * within a child process.
+   */
+  parentMessageManager: null,
+
   close: function() {
     this._transport.close();
   },
@@ -1721,5 +1709,30 @@ DebuggerServerConnection.prototype = {
     dumpn("/-------------------- dumping pool:");
     dumpn("--------------------- actorPool actors: " +
           uneval(Object.keys(aPool._actors)));
+  },
+
+  /**
+   * In a content child process, ask the DebuggerServer in the parent process
+   * to execute a given module setup helper.
+   *
+   * @param module
+   *        The module to be required
+   * @param setupParent
+   *        The name of the setup helper exported by the above module
+   *        (setup helper signature: function ({mm}) { ... })
+   * @return boolean
+   *         true if the setup helper returned successfully
+   */
+  setupInParent: function({ conn, module, setupParent }) {
+    if (!this.parentMessageManager) {
+      return false;
     }
+
+    let { sendSyncMessage } = this.parentMessageManager;
+
+    return sendSyncMessage("debug:setup-in-parent", {
+      module: module,
+      setupParent: setupParent
+    });
+  },
 };

toolkit/devtools/server/tests/mochitest/chrome.ini

@@ -16,6 +16,8 @@ support-files =
   memory-helpers.js
   nonchrome_unsafeDereference.html
   small-image.gif
+  setup-in-child.js
+  setup-in-parent.js
 
 [test_connection-manager.html]
 skip-if = buildapp == 'mulet'
@@ -86,6 +88,7 @@ skip-if = buildapp == 'mulet'
 [test_registerActor.html]
 [test_SaveHeapSnapshot.html]
 [test_settings.html]
+[test_setupInParentChild.html]
 [test_styles-applied.html]
 [test_styles-computed.html]
 [test_styles-layout.html]

toolkit/devtools/server/tests/mochitest/setup-in-child.js

@@ -0,0 +1,20 @@
+const {Cc, Ci} = require("chrome");
+const cpmm = Cc["@mozilla.org/childprocessmessagemanager;1"].
+  getService(Ci.nsIMessageListenerManager);
+const { DebuggerServer } = require("devtools/server/main");
+
+exports.setupChild = function (a, b, c) {
+  cpmm.sendAsyncMessage("test:setupChild", [a, b, c]);
+}
+
+exports.callParent = function () {
+  // Hack! Fetch DebuggerServerConnection objects directly within DebuggerServer guts.
+  for (let id in DebuggerServer._connections) {
+    let conn = DebuggerServer._connections[id];
+    conn.setupInParent({
+      module: "chrome://mochitests/content/chrome/toolkit/devtools/server/tests/mochitest/setup-in-parent.js",
+      setupParent: "setupParent",
+      args: [{one: true}, 2, "three"]
+    });
+  }
+}

toolkit/devtools/server/tests/mochitest/setup-in-parent.js

@@ -0,0 +1,10 @@
+let {Ci} = require("chrome");
+let Services = require("Services");
+
+exports.setupParent = function ({mm, prefix}) {
+  let args = [
+    !!mm.QueryInterface(Ci.nsIMessageSender),
+    prefix
+  ];
+  Services.obs.notifyObservers(null, "test:setupParent", JSON.stringify(args));
+}

toolkit/devtools/server/tests/mochitest/test_setupInParentChild.html

@@ -0,0 +1,109 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+Bug 1181100 - Test DebuggerServerConnection.setupInParent and DebuggerServer.setupInChild
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Mozilla Bug</title>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css">
+</head>
+<body>
+<pre id="test">
+<script type="application/javascript;version=1.8">
+
+let Cu = Components.utils;
+let Cc = Components.classes;
+let Ci = Components.interfaces;
+
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/devtools/dbg-client.jsm");
+Cu.import("resource://gre/modules/devtools/dbg-server.jsm");
+
+window.onload = function() {
+  SimpleTest.waitForExplicitFinish();
+
+  SpecialPowers.pushPrefEnv({
+    "set": [
+      // Always log packets when running tests.
+      ["devtools.debugger.log", true],
+      ["dom.mozBrowserFramesEnabled", true]
+    ]
+  }, runTests);
+}
+
+function runTests() {
+  // Create a minimal iframe with a message manager
+  let iframe = document.createElement("iframe");
+  iframe.mozbrowser = true;
+  document.body.appendChild(iframe);
+
+  let mm = iframe.QueryInterface(Ci.nsIFrameLoaderOwner).frameLoader.messageManager;
+
+  // Instantiate a minimal server
+  if (!DebuggerServer.initialized) {
+    DebuggerServer.init();
+  }
+  if (!DebuggerServer.createRootActor) {
+    DebuggerServer.addBrowserActors();
+  }
+
+  // Fake a connection to an iframe
+  let transport = DebuggerServer.connectPipe();
+  let conn = transport._serverConnection;
+  let client = new DebuggerClient(transport);
+
+  // Wait for a response from setupInChild
+  const ppmm = Cc["@mozilla.org/parentprocessmessagemanager;1"]
+                 .getService(Ci.nsIMessageListenerManager);
+  let onChild = msg => {
+    ppmm.removeMessageListener("test:setupChild", onChild);
+    let args = msg.json;
+
+    is(args[0], 1, "Got first numeric argument");
+    is(args[1], "two", "Got second string argument");
+    is(args[2].three, true, "Got last JSON argument");
+
+    // Ask the child to call setupInParent
+    DebuggerServer.setupInChild({
+      module: "chrome://mochitests/content/chrome/toolkit/devtools/server/tests/mochitest/setup-in-child.js",
+      setupChild: "callParent"
+    });
+  };
+  ppmm.addMessageListener("test:setupChild", onChild);
+
+  // Wait also for a reponse from setupInParent called from setup-in-child.js
+  let onParent = (_, topic, args) => {
+    Services.obs.removeObserver(onParent, "test:setupParent", false);
+    args = JSON.parse(args);
+
+    is(args[0], true, "Got `mm` argument, a message manager");
+    ok(args[1].match(/server\d+.conn\d+.child\d+/), "Got `prefix` argument");
+
+    cleanup();
+  };
+  Services.obs.addObserver(onParent, "test:setupParent", false);
+
+  // Instanciate e10s machinery and call setupInChild
+  DebuggerServer.connectToChild(conn, iframe).then(actor => {
+    DebuggerServer.setupInChild({
+      module: "chrome://mochitests/content/chrome/toolkit/devtools/server/tests/mochitest/setup-in-child.js",
+      setupChild: "setupChild",
+      args: [1, "two", {three: true}]
+    });
+  });
+
+  function cleanup() {
+    client.close(function () {
+      DebuggerServer.destroy();
+      iframe.remove();
+      SimpleTest.finish()
+    });
+  }
+
+}
+</script>
+</pre>
+</body>
+</html>

toolkit/devtools/server/tests/unit/head_dbg.js

@@ -318,6 +318,7 @@ function startTestDebuggerServer(title, server = DebuggerServer) {
 function finishClient(aClient)
 {
   aClient.close(function() {
+    DebuggerServer.destroy();
     do_test_finished();
   });
 }