Backed out changeset a6d7efb82917 (bug 1772347) for causing devtools failres. CLOSED TREE

devtools/server/actors/targets/content-process.js

@@ -214,17 +214,14 @@ const ContentProcessTargetActor = TargetActorMixin(
     },
 
     destroy: function() {
-      // Avoid reentrancy. We will destroy the Transport when emitting "destroyed",
-      // which will force destroying all actors.
-      if (this.destroying) {
+      if (this.isDestroyed()) {
         return;
       }
-      this.destroying = true;
+      Resources.unwatchAllResources(this);
 
       this.emit("destroyed");
 
       Actor.prototype.destroy.call(this);
-      Resources.unwatchAllResources(this);
 
       if (this.threadActor) {
         this.threadActor = null;

devtools/server/actors/watcher/target-helpers/process-helper.js

@@ -77,26 +77,6 @@ function onContentProcessActorCreated(msg) {
   watcher.notifyTargetAvailable(actor);
 }
 
-function onContentProcessActorDestroyed(msg) {
-  const { watcherActorID } = msg.data;
-  const watcher = WatcherRegistry.getWatcher(watcherActorID);
-  if (!watcher) {
-    throw new Error(
-      `Receiving a content process actor destruction without a watcher actor ${watcherActorID}`
-    );
-  }
-  // Ignore watchers of other connections.
-  // We may have two browser toolbox connected to the same process.
-  // This will spawn two distinct Watcher actor and two distinct process target helper module.
-  // Avoid processing the event many times, otherwise we will notify about the same target
-  // multiple times.
-  if (!watchers.has(watcher)) {
-    return;
-  }
-  const messageManager = msg.target;
-  unregisterWatcherForMessageManager(watcher, messageManager);
-}
-
 function onMessageManagerClose(messageManager, topic, data) {
   const list = actors.get(messageManager);
   if (!list || list.length == 0) {
@@ -113,52 +93,38 @@ function onMessageManagerClose(messageManager, topic, data) {
   actors.delete(messageManager);
 }
 
-/**
- * Unregister everything created for a given watcher against a precise message manager:
- * - clear up things from `actors` WeakMap,
- * - notify all related target actors as being destroyed,
- * - close all DevTools Transports being created for each Message Manager.
- */
-function unregisterWatcherForMessageManager(watcher, messageManager) {
-  const targetActorDescriptions = actors.get(messageManager);
-  if (!targetActorDescriptions || targetActorDescriptions.length == 0) {
-    return;
-  }
-
-  // Destroy all transports related to this watcher and tells the client to purge all related actors
-  const matchingTargetActorDescriptions = targetActorDescriptions.filter(
-    item => item.watcher === watcher
-  );
-  for (const {
-    prefix,
-    childTransport,
-    actor,
-  } of matchingTargetActorDescriptions) {
-    watcher.notifyTargetDestroyed(actor);
-
-    childTransport.close();
-    watcher.conn.cancelForwarding(prefix);
-  }
-
-  // Then update global `actors` WeakMap by stripping all data about this watcher
-  const remainingTargetActorDescriptions = targetActorDescriptions.filter(
-    item => item.watcher !== watcher
-  );
-  if (remainingTargetActorDescriptions.length == 0) {
-    actors.delete(messageManager);
-  } else {
-    actors.set(messageManager, remainingTargetActorDescriptions);
-  }
-}
-
-/**
- * Destroy everything related to a given watcher that has been created in this module:
- * (See unregisterWatcherForMessageManager)
- */
 function closeWatcherTransports(watcher) {
   for (let i = 0; i < Services.ppmm.childCount; i++) {
     const messageManager = Services.ppmm.getChildAt(i);
-    unregisterWatcherForMessageManager(watcher, messageManager);
+    const targetActorDescriptions = actors.get(messageManager);
+    if (!targetActorDescriptions || targetActorDescriptions.length == 0) {
+      continue;
+    }
+
+    // Destroy all transports related to this watcher and tells the client to purge all related actors
+    const matchingTargetActorDescriptions = targetActorDescriptions.filter(
+      item => item.watcher === watcher
+    );
+    for (const {
+      prefix,
+      childTransport,
+      actor,
+    } of matchingTargetActorDescriptions) {
+      watcher.notifyTargetDestroyed(actor);
+
+      childTransport.close();
+      watcher.conn.cancelForwarding(prefix);
+    }
+
+    // Then update global `actors` WeakMap by stripping all data about this watcher
+    const remainingTargetActorDescriptions = targetActorDescriptions.filter(
+      item => item.watcher !== watcher
+    );
+    if (remainingTargetActorDescriptions.length == 0) {
+      actors.delete(messageManager);
+    } else {
+      actors.set(messageManager, remainingTargetActorDescriptions);
+    }
   }
 }
 
@@ -170,10 +136,6 @@ function maybeRegisterMessageListeners(watcher) {
       "debug:content-process-actor",
       onContentProcessActorCreated
     );
-    Services.ppmm.addMessageListener(
-      "debug:content-process-actor-destroyed",
-      onContentProcessActorDestroyed
-    );
     Services.obs.addObserver(onMessageManagerClose, "message-manager-close");
 
     // Load the content process server startup script only once,
@@ -199,10 +161,6 @@ function maybeUnregisterMessageListeners(watcher) {
       "debug:content-process-actor",
       onContentProcessActorCreated
     );
-    Services.ppmm.removeMessageListener(
-      "debug:content-process-actor-destroyed",
-      onContentProcessActorDestroyed
-    );
     Services.obs.removeObserver(onMessageManagerClose, "message-manager-close");
 
     // We inconditionally remove the process script, while we should only remove it

devtools/server/startup/content-process.jsm

@@ -60,44 +60,29 @@ function initContentProcessTarget(msg) {
   const response = { watcherActorID, prefix, actor: actor.form() };
   mm.sendAsyncMessage("debug:content-process-actor", response);
 
-  function onDestroy() {
-    mm.removeMessageListener(
-      "debug:content-process-disconnect",
-      onContentProcessDisconnect
-    );
-    actor.off("destroyed", onDestroy);
-
-    // Notify the parent process that the actor is being destroyed
-    mm.sendAsyncMessage("debug:content-process-actor-destroyed", {
-      watcherActorID,
-    });
-
-    // Call DevToolsServerConnection.close to destroy all child actors. It should end up
-    // calling DevToolsServerConnection.onTransportClosed that would actually cleanup all actor
-    // pools.
-    conn.close();
-
-    // Destroy the related loader when the target is destroyed
-    // and we were the last user of the special loader
-    releaseDistinctSystemPrincipalLoader(loaderRequester);
-  }
-  function onContentProcessDisconnect(message) {
+  // Clean up things when the client disconnects
+  mm.addMessageListener("debug:content-process-disconnect", function onDestroy(
+    message
+  ) {
     if (message.data.prefix != prefix) {
       // Several copies of this process script can be running for a single process if
       // we are debugging the same process from multiple clients.
       // If this disconnect request doesn't match a connection known here, ignore it.
       return;
     }
-    onDestroy();
-  }
+    mm.removeMessageListener("debug:content-process-disconnect", onDestroy);
 
-  // Clean up things when the client disconnects
-  mm.addMessageListener(
-    "debug:content-process-disconnect",
-    onContentProcessDisconnect
-  );
-  // And also when the target actor is destroyed
-  actor.on("destroyed", onDestroy);
+    // Call DevToolsServerConnection.close to destroy all child actors. It should end up
+    // calling DevToolsServerConnection.onTransportClosed that would actually cleanup all actor
+    // pools.
+    conn.close();
+  });
+
+  // Destroy the related loader when the target is destroyed
+  // and we were the last user of the special loader
+  actor.once("destroyed", () => {
+    releaseDistinctSystemPrincipalLoader(loaderRequester);
+  });
 
   return {
     actor,