From 3bb4e6bda53b622904e552ac4910e53e11f49196 Mon Sep 17 00:00:00 2001 From: Alexandre Poirot Date: Mon, 3 Aug 2015 07:52:42 -0700 Subject: [PATCH] Bug 1181100 - Fix actors being registered dynamically when closing the first connected tab. r=jryans,mratcliffe --- toolkit/devtools/server/actors/storage.js | 20 ++-- toolkit/devtools/server/child.js | 10 +- .../server/docs/actor-e10s-handling.md | 18 +-- toolkit/devtools/server/main.js | 79 +++++++------ .../server/tests/mochitest/chrome.ini | 3 + .../server/tests/mochitest/setup-in-child.js | 20 ++++ .../server/tests/mochitest/setup-in-parent.js | 10 ++ .../mochitest/test_setupInParentChild.html | 109 ++++++++++++++++++ .../devtools/server/tests/unit/head_dbg.js | 1 + 9 files changed, 214 insertions(+), 56 deletions(-) create mode 100644 toolkit/devtools/server/tests/mochitest/setup-in-child.js create mode 100644 toolkit/devtools/server/tests/mochitest/setup-in-parent.js create mode 100644 toolkit/devtools/server/tests/mochitest/test_setupInParentChild.html diff --git a/toolkit/devtools/server/actors/storage.js b/toolkit/devtools/server/actors/storage.js index b4baf2ade01..339c8dbb81c 100644 --- a/toolkit/devtools/server/actors/storage.js +++ b/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); diff --git a/toolkit/devtools/server/child.js b/toolkit/devtools/server/child.js index 88cce054c06..ee9c6c6b5bf 100644 --- a/toolkit/devtools/server/child.js +++ b/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); diff --git a/toolkit/devtools/server/docs/actor-e10s-handling.md b/toolkit/devtools/server/docs/actor-e10s-handling.md index 5c09556ce6b..ba9ddee7426 100644 --- a/toolkit/devtools/server/docs/actor-e10s-handling.md +++ b/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,14 +91,14 @@ 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, -* the `setupParent` function also uses the DebuggerServer object to subscribe *once* to the `"disconnected-from-child:PREFIX"` event to unsubscribe from messagemanager events. \ No newline at end of file +* the `setupParent` function also uses the DebuggerServer object to subscribe *once* to the `"disconnected-from-child:PREFIX"` event to unsubscribe from messagemanager events. diff --git a/toolkit/devtools/server/main.js b/toolkit/devtools/server/main.js index e6fd02cde2c..0d7fbe4e21c 100644 --- a/toolkit/devtools/server/main.js +++ b/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", { - module: module, - setupChild: setupChild, - args: args, + 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 + }); + }, }; diff --git a/toolkit/devtools/server/tests/mochitest/chrome.ini b/toolkit/devtools/server/tests/mochitest/chrome.ini index 5c08202d905..804bb378596 100644 --- a/toolkit/devtools/server/tests/mochitest/chrome.ini +++ b/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] diff --git a/toolkit/devtools/server/tests/mochitest/setup-in-child.js b/toolkit/devtools/server/tests/mochitest/setup-in-child.js new file mode 100644 index 00000000000..487f2982614 --- /dev/null +++ b/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"] + }); + } +} diff --git a/toolkit/devtools/server/tests/mochitest/setup-in-parent.js b/toolkit/devtools/server/tests/mochitest/setup-in-parent.js new file mode 100644 index 00000000000..1709d7379d0 --- /dev/null +++ b/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)); +} diff --git a/toolkit/devtools/server/tests/mochitest/test_setupInParentChild.html b/toolkit/devtools/server/tests/mochitest/test_setupInParentChild.html new file mode 100644 index 00000000000..29ce55526fe --- /dev/null +++ b/toolkit/devtools/server/tests/mochitest/test_setupInParentChild.html @@ -0,0 +1,109 @@ + + + + + + Mozilla Bug + + + + +
+
+
+ + diff --git a/toolkit/devtools/server/tests/unit/head_dbg.js b/toolkit/devtools/server/tests/unit/head_dbg.js index 9526a32b47e..2788e1c0f7e 100644 --- a/toolkit/devtools/server/tests/unit/head_dbg.js +++ b/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(); }); }