Bug 1315044 - Cache ContentActor forms to prevent creating new one when calling RootActor.getProcess multiple times. r=jryans
authorAlexandre Poirot <poirot.alex@gmail.com>
Fri, 04 Nov 2016 08:05:15 -0700
changeset 322589 f57c3052af5a813e8b25620ef6f343be8fe09b58
parent 322588 e6f4bb5cf32188efda9eab669f6829ba5f7750db
child 322590 eca8f585871bbe8074fe5f4476f3157fc7848adf
push id34253
push userapoirot@mozilla.com
push dateTue, 15 Nov 2016 13:35:28 +0000
treeherderautoland@eca8f585871b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans
bugs1315044
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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()
     });
   }