Bug 1315044 - Cache ContentActor forms to prevent creating new one when calling RootActor.getProcess multiple times. r=jryans draft
authorAlexandre Poirot <poirot.alex@gmail.com>
Fri, 04 Nov 2016 08:05:15 -0700
changeset 439041 a55dc87844cb16d86a1cab7a4b2c738fee2d4fbc
parent 438112 47e0584afe0ab0b867412189c610b302b6ba0ea7
child 439042 622dfeeb720cde802a0fff228b87f5ee15478447
push id35889
push userbmo:poirot.alex@gmail.com
push dateTue, 15 Nov 2016 10:25:33 +0000
reviewersjryans
bugs1315044
milestone52.0a1
Bug 1315044 - Cache ContentActor forms to prevent creating new one when calling RootActor.getProcess multiple times. r=jryans MozReview-Commit-ID: 3YyShRXQhel
devtools/server/actors/root.js
devtools/server/main.js
devtools/server/tests/mochitest/test_getProcess.html
--- a/devtools/server/actors/root.js
+++ b/devtools/server/actors/root.js
@@ -98,16 +98,17 @@ function RootActor(aConnection, aParamet
   this._onServiceWorkerRegistrationListChanged = this.onServiceWorkerRegistrationListChanged.bind(this);
   this._onProcessListChanged = this.onProcessListChanged.bind(this);
   this._extraActors = {};
 
   this._globalActorPool = new ActorPool(this.conn);
   this.conn.addActorPool(this._globalActorPool);
 
   this._chromeActor = null;
+  this._processActors = new Map();
 }
 
 RootActor.prototype = {
   constructor: RootActor,
   applicationType: "browser",
 
   traits: {
     sources: true,
@@ -225,16 +226,17 @@ RootActor.prototype = {
       this._parameters.onShutdown();
     }
     this._extraActors = null;
     this.conn = null;
     this._tabActorPool = null;
     this._globalActorPool = null;
     this._parameters = null;
     this._chromeActor = null;
+    this._processActors.clear();
   },
 
   /* The 'listTabs' request and the 'tabListChanged' notification. */
 
   /**
    * Handles the listTabs request. The actors will survive until at least
    * the next listTabs request.
    */
@@ -466,23 +468,34 @@ RootActor.prototype = {
         // Create a ChromeActor for the parent process
         let { ChromeActor } = require("devtools/server/actors/chrome");
         this._chromeActor = new ChromeActor(this.conn);
         this._globalActorPool.addActor(this._chromeActor);
       }
 
       return { form: this._chromeActor.form() };
     } else {
-      let mm = ppmm.getChildAt(aRequest.id);
+      let { id } = aRequest;
+      let mm = ppmm.getChildAt(id);
       if (!mm) {
         return { error: "noProcess",
-                 message: "There is no process with id '" + aRequest.id + "'." };
+                 message: "There is no process with id '" + id + "'." };
+      }
+      let form = this._processActors.get(id);
+      if (form) {
+        return { form };
       }
-      return DebuggerServer.connectToContent(this.conn, mm)
-                           .then(form => ({ form }));
+      let onDestroy = () => {
+        this._processActors.delete(id);
+      };
+      return DebuggerServer.connectToContent(this.conn, mm, onDestroy)
+        .then(form => {
+          this._processActors.set(id, form);
+          return { form };
+        });
     }
   },
 
   /* This is not in the spec, but it's used by tests. */
   onEcho: function (aRequest) {
     /*
      * Request packets are frozen. Copy aRequest, so that
      * DebuggerServerConnection.onPacket can attach a 'from' property.
--- a/devtools/server/main.js
+++ b/devtools/server/main.js
@@ -710,17 +710,17 @@ var DebuggerServer = {
 
     let transport = isWorker ?
                     new WorkerDebuggerTransport(scopeOrManager, prefix) :
                     new ChildDebuggerTransport(scopeOrManager, prefix);
 
     return this._onConnection(transport, prefix, true);
   },
 
-  connectToContent(connection, mm) {
+  connectToContent(connection, mm, onDestroy) {
     let deferred = SyncPromise.defer();
 
     let prefix = connection.allocID("content-process");
     let actor, childTransport;
 
     mm.addMessageListener("debug:content-process-actor", function listener(msg) {
       // Arbitrarily choose the first content process to reply
       // XXX: This code needs to be updated if we use more than one content process
@@ -759,16 +759,20 @@ var DebuggerServer = {
 
         // ... and notify the child process to clean the tab actors.
         try {
           mm.sendAsyncMessage("debug:content-process-destroy");
         } catch (e) {
           // Nothing to do
         }
       }
+
+      if (onDestroy) {
+        onDestroy(mm);
+      }
     }
 
     let onMessageManagerClose = DevToolsUtils.makeInfallible((subject, topic, data) => {
       if (subject == mm) {
         onClose();
         connection.send({ from: actor.actor, type: "tabDetached" });
       }
     });
--- a/devtools/server/tests/mochitest/test_getProcess.html
+++ b/devtools/server/tests/mochitest/test_getProcess.html
@@ -92,22 +92,34 @@ function runTests() {
 
         // Ensure sending at least one request to an actor...
         client.request({
           to: actor.consoleActor,
           type: "evaluateJS",
           text: "var a = 42; a"
         }, function (response) {
           ok(response.result, 42, "console.eval worked");
-          cleanup();
+
+          getProcessAgain(actor, content.id);
         });
       });
     });
   }
 
+  // Assert that calling client.getProcess against the same process id is
+  // returning the same actor.
+  function getProcessAgain(firstActor, id) {
+    client.getProcess(id).then(response => {
+      let actor = response.form;
+      is(actor, firstActor,
+         "Second call to getProcess with the same id returns the same form");
+      cleanup();
+    });
+  }
+
   function cleanup() {
     client.close().then(function () {
       DebuggerServer.destroy();
       iframe.remove();
       SimpleTest.finish()
     });
   }