Bug 1566450 - replace getList process json with processDescriptor; r=ochameau

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

--HG--
extra : moz-landing-system : lando

devtools/client/shared/workers-listener.js

@@ -28,6 +28,14 @@ class WorkersListener {
 
     this._listener = listener;
     this.rootFront.on("workerListChanged", this._listener);
+    this.rootFront.onFront("processDescriptor", processFront => {
+      processFront.onFront("contentProcessTarget", front => {
+        this._contentProcessFronts.push(front);
+        front.on("workerListChanged", this._listener);
+      });
+    });
+
+    // Support FF69 and older
     this.rootFront.onFront("contentProcessTarget", front => {
       this._contentProcessFronts.push(front);
       front.on("workerListChanged", this._listener);

devtools/server/actors/root.js

@@ -6,7 +6,7 @@
 
 "use strict";
 
-const { Cc, Ci, Cu } = require("chrome");
+const { Cu } = require("chrome");
 const Services = require("Services");
 const { Pool } = require("devtools/shared/protocol");
 const {
@@ -23,14 +23,8 @@ loader.lazyRequireGetter(
 );
 loader.lazyRequireGetter(
   this,
-  "ContentProcessTargetActor",
-  "devtools/server/actors/targets/content-process",
-  true
-);
-loader.lazyRequireGetter(
-  this,
-  "ParentProcessTargetActor",
-  "devtools/server/actors/targets/parent-process",
+  "ProcessDescriptorActor",
+  "devtools/server/actors/descriptors/process",
   true
 );
 
@@ -123,7 +117,6 @@ function RootActor(connection, parameters) {
   this._globalActorPool = new LazyPool(this.conn);
 
   this._parentProcessTargetActor = null;
-  this._processActors = new Map();
 }
 
 RootActor.prototype = {
@@ -241,6 +234,9 @@ RootActor.prototype = {
     if (this._tabTargetActorPool) {
       this._tabTargetActorPool.destroy();
     }
+    if (this._processDescriptorActorPool) {
+      this._processDescriptorActorPool.destroy();
+    }
     if (this._globalActorPool) {
       this._globalActorPool.destroy();
     }
@@ -263,7 +259,6 @@ RootActor.prototype = {
     this._chromeWindowActorPool = null;
     this._parameters = null;
     this._parentProcessTargetActor = null;
-    this._processActors.clear();
   },
 
   /**
@@ -560,8 +555,25 @@ RootActor.prototype = {
       };
     }
     processList.onListChanged = this._onProcessListChanged;
+    const processes = processList.getList();
+    const pool = new Pool(this.conn);
+    for (const metadata of processes) {
+      let processDescriptor = this._getKnownProcessDescriptor(metadata.id);
+      if (!processDescriptor) {
+        processDescriptor = new ProcessDescriptorActor(this.conn, metadata);
+      }
+      pool.manage(processDescriptor);
+    }
+    // Do not destroy the pool before transfering ownership to the newly created
+    // pool, so that we do not accidently destroy actors that are still in use.
+    if (this._processDescriptorActorPool) {
+      this._processDescriptorActorPool.destroy();
+    }
+    this._processDescriptorActorPool = pool;
+    // extract the values in the processActors map
+    const processActors = [...this._processDescriptorActorPool.poolChildren()];
     return {
-      processes: processList.getList(),
+      processes: processActors.map(actor => actor.form()),
     };
   },
 
@@ -585,44 +597,8 @@ RootActor.prototype = {
     }
     // If the request doesn't contains id parameter or id is 0
     // (id == 0, based on onListProcesses implementation)
-    if (!("id" in request) || request.id === 0) {
-      // Check if we are running on xpcshell.
-      // When running on xpcshell, there is no valid browsing context to attach to
-      // and so ParentProcessTargetActor doesn't make sense as it inherits from
-      // BrowsingContextTargetActor. So instead use ContentProcessTargetActor, which
-      // matches xpcshell needs.
-      const env = Cc["@mozilla.org/process/environment;1"].getService(
-        Ci.nsIEnvironment
-      );
-      const isXpcshell = env.exists("XPCSHELL_TEST_PROFILE_DIR");
+    const id = request.id || 0;
 
-      if (
-        !isXpcshell &&
-        this._parentProcessTargetActor &&
-        (!this._parentProcessTargetActor.docShell ||
-          this._parentProcessTargetActor.docShell.isBeingDestroyed)
-      ) {
-        this._parentProcessTargetActor.destroy();
-        this._parentProcessTargetActor = null;
-      }
-      if (!this._parentProcessTargetActor) {
-        // Create the target actor for the parent process
-        if (isXpcshell) {
-          this._parentProcessTargetActor = new ContentProcessTargetActor(
-            this.conn
-          );
-        } else {
-          this._parentProcessTargetActor = new ParentProcessTargetActor(
-            this.conn
-          );
-        }
-        this._globalActorPool.manage(this._parentProcessTargetActor);
-      }
-
-      return { form: this._parentProcessTargetActor.form() };
-    }
-
-    const { id } = request;
     const mm = Services.ppmm.getChildAt(id);
     if (!mm) {
       return {
@@ -630,20 +606,28 @@ RootActor.prototype = {
         message: "There is no process with id '" + id + "'.",
       };
     }
-    let form = this._processActors.get(id);
-    if (form) {
-      return { form };
+    let processDescriptor = this._getKnownProcessDescriptor(id);
+    this._processDescriptorActorPool =
+      this._processDescriptorActorPool || new Pool(this.conn);
+    if (!processDescriptor) {
+      const options = { id, parent: id === 0 };
+      processDescriptor = new ProcessDescriptorActor(this.conn, options);
+      this._processDescriptorActorPool.manage(processDescriptor);
     }
-    const onDestroy = () => {
-      this._processActors.delete(id);
-    };
-    form = await DebuggerServer.connectToContentProcess(
-      this.conn,
-      mm,
-      onDestroy
-    );
-    this._processActors.set(id, form);
-    return { form };
+    return { form: processDescriptor.form() };
+  },
+
+  _getKnownProcessDescriptor(id) {
+    // if there is no pool, then we do not have any descriptors
+    if (!this._processDescriptorActorPool) {
+      return null;
+    }
+    for (const descriptor of this._processDescriptorActorPool.poolChildren()) {
+      if (descriptor.id === id) {
+        return descriptor;
+      }
+    }
+    return null;
   },
 
   /* This is not in the spec, but it's used by tests. */

devtools/server/tests/mochitest/test_getProcess.html

@@ -83,33 +83,28 @@ function runTests() {
          "Got one additional process on the second call to listProcesses");
 
       // Connect to the first content processe available
-      const content = response.processes.filter(p => (!p.parent))[0];
+      const content = response.processes.filter(p => (!p.isParent))[0];
 
-      client.mainRoot.getProcess(content.id).then(front => {
-        const actor = front.targetForm;
-        ok(actor.consoleActor, "Got the console actor");
-        ok(actor.threadActor, "Got the thread actor");
+      client.mainRoot.getProcess(content.id).then(async front => {
+        const targetForm = front.targetForm;
+        ok(targetForm.consoleActor, "Got the console actor");
+        ok(targetForm.threadActor, "Got the thread actor");
 
         // Ensure sending at least one request to an actor...
-        client.request({
-          to: actor.consoleActor,
-          type: "evaluateJS",
-          text: "var a = 42; a",
-        }, function({result}) {
-          is(result, 42, "console.eval worked");
+        const consoleFront = await front.getFront("console");
+        const { result } = await consoleFront.evaluateJS("var a = 42; a");
+        is(result, 42, "console.eval worked");
 
-          getProcessAgain(actor, content.id);
-        });
+        getProcessAgain(front, content.id);
       });
     });
   }
 
   // Assert that calling client.getProcess against the same process id is
   // returning the same actor.
-  function getProcessAgain(firstActor, id) {
+  function getProcessAgain(firstTargetFront, id) {
     client.mainRoot.getProcess(id).then(front => {
-      const actor = front.targetForm;
-      is(actor, firstActor,
+      is(front, firstTargetFront,
          "Second call to getProcess with the same id returns the same form");
       closeClient();
     });

devtools/shared/fronts/root.js

@@ -11,6 +11,12 @@ const {
 } = require("devtools/shared/protocol");
 
 loader.lazyRequireGetter(this, "getFront", "devtools/shared/protocol", true);
+loader.lazyRequireGetter(
+  this,
+  "ProcessDescriptorFront",
+  "devtools/shared/fronts/descriptors/process",
+  true
+);
 loader.lazyRequireGetter(
   this,
   "BrowsingContextTargetFront",
@@ -81,12 +87,12 @@ class RootFront extends FrontClassWithSpec(rootSpec) {
 
       // And then from the Child processes
       const { processes } = await this.listProcesses();
-      for (const process of processes) {
+      for (const processDescriptorFront of processes) {
         // Ignore parent process
-        if (process.parent) {
+        if (processDescriptorFront.isParent) {
           continue;
         }
-        const front = await this.getProcess(process.id);
+        const front = await processDescriptorFront.getTarget();
         const response = await front.listWorkers();
         workers = workers.concat(response.workers);
       }
@@ -159,6 +165,24 @@ class RootFront extends FrontClassWithSpec(rootSpec) {
     return result;
   }
 
+  async listProcesses() {
+    const { processes } = await super.listProcesses();
+    const processDescriptors = processes.map(form => {
+      if (form.actor && form.actor.includes("processDescriptor")) {
+        return this._getProcessDescriptorFront(form);
+      }
+      // Support FF69 and older
+      return {
+        id: form.id,
+        isParent: form.parent,
+        getTarget: () => {
+          return this.getProcess(form.id);
+        },
+      };
+    });
+    return { processes: processDescriptors };
+  }
+
   /**
    * Fetch the ParentProcessTargetActor for the main process.
    *
@@ -170,10 +194,18 @@ class RootFront extends FrontClassWithSpec(rootSpec) {
   }
 
   async getProcess(id) {
+    const { form } = await super.getProcess(id);
+    if (form.actor && form.actor.includes("processDescriptor")) {
+      // The server currently returns a form, when we can drop backwards compatibility,
+      // we can use automatic marshalling here instead, making the next line unnecessary
+      const processDescriptorFront = this._getProcessDescriptorFront(form);
+      return processDescriptorFront.getTarget();
+    }
+
+    // Backwards compatibility for servers up to FF69.
     // Do not use specification automatic marshalling as getProcess may return
     // two different type: ParentProcessTargetActor or ContentProcessTargetActor.
     // Also, we do want to memoize the fronts and return already existing ones.
-    const { form } = await super.getProcess(id);
     let front = this.actor(form.actor);
     if (front) {
       return front;
@@ -198,6 +230,25 @@ class RootFront extends FrontClassWithSpec(rootSpec) {
     return front;
   }
 
+  /**
+   * Get the previous process descriptor front if it exists, create a new one if not.
+   *
+   * If we are using a modern server, we will get a form for a processDescriptorFront.
+   * Until we can switch to auto marshalling, we need to marshal this into a process
+   * descriptor front ourselves.
+   */
+  _getProcessDescriptorFront(form) {
+    let front = this.actor(form.actor);
+    if (front) {
+      return front;
+    }
+    front = new ProcessDescriptorFront(this._client);
+    front.form(form);
+    front.actorID = form.actor;
+    this.manage(front);
+    return front;
+  }
+
   /**
    * Override default listTabs request in order to return a list of
    * BrowsingContextTargetFronts while updating their selected state.