Bug 1673328 - [devtools] Drop ThreadActor.exit in favor of destroy. r=nchevobbe

We no longer need to manuall call threadActor.exit from Target Actors
as the ThreadActor is managed by the Target Actors, so that managed children
will be destroyed when the target actor is destroyed.

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

devtools/server/actors/targets/browsing-context.js

@@ -972,7 +972,7 @@ const browsingContextTargetPrototype = {
    * The content window is no longer being debugged after this call.
    */
   _destroyThreadActor() {
-    this.threadActor.exit();
+    this.threadActor.destroy();
     this.threadActor = null;
 
     if (this._sources) {

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

@@ -206,21 +206,16 @@ const ContentProcessTargetActor = TargetActorMixin(
       }
       Resources.unwatchAllTargetResources(this);
 
-      if (this.threadActor) {
-        // We have to manually call `exit` as calling `destroy` won't completely destroy it.
-        // ThreadActor.destroy do not call protocoljs.Actor.destroy, ThreadActor.exit does it.
-        // Also do this before destroying _dbg and _sources as the thread actor may still
-        // use them while cleaning up things.
-        this.threadActor.exit();
-        this.threadActor = null;
-      }
-
       // Notify the client that this target is being destroyed.
       // So that we can destroy the target front and all its children.
       this.emit("tabDetached");
 
       Actor.prototype.destroy.call(this);
 
+      if (this.threadActor) {
+        this.threadActor = null;
+      }
+
       // Tell the live lists we aren't watching any more.
       if (this._workerList) {
         this._workerList.destroy();

devtools/server/actors/targets/worker.js

@@ -96,15 +96,6 @@ exports.WorkerTargetActor = TargetActorMixin(
     },
 
     destroy() {
-      if (this.threadActor) {
-        // We have to manually call `exit` as calling `destroy` won't completely destroy it.
-        // ThreadActor.destroy do not call protocoljs.Actor.destroy, ThreadActor.exit does it.
-        // We also need do this before destroying _sources as the thread actor may still
-        // use it while cleaning up things.
-        this.threadActor.exit();
-        this.threadActor = null;
-      }
-
       Actor.prototype.destroy.call(this);
 
       if (this._sources) {
@@ -115,6 +106,7 @@ exports.WorkerTargetActor = TargetActorMixin(
       this.workerGlobal = null;
       this._dbg = null;
       this._consoleActor = null;
+      this.threadActor = null;
     },
   }
 );

devtools/server/actors/thread.js

@@ -331,10 +331,10 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
   },
 
   /**
-   * Clean up listeners, debuggees and clear actor pools associated with
+   * Destroy the debugger and put the actor in the exited state.
-   * the lifetime of this actor. This does not destroy the thread actor,
+   *
-   * it resets it. This is used in methods `onDetatch` and
+   * As part of destroy, we: clean up listeners, debuggees and
-   * `exit`. The actor is truely destroyed in the `exit method`.
+   * clear actor pools associated with the lifetime of this actor.
    */
   destroy: function() {
     dumpn("in ThreadActor.prototype.destroy");
@@ -368,24 +368,8 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
     this._threadLifetimePool.destroy();
     this._threadLifetimePool = null;
     this._dbg = null;
-    // Note: here we don't call Actor.prototype.destroy.call(this)
-    // because the real "destroy" method is `exit()`, where
-    // Actor.prototype.destroy.call(this) will be called.
-  },
-
-  /**
-   * destroy the debugger and put the actor in the exited state.
-   */
-  exit: function() {
-    this.destroy();
     this._state = "exited";
-    // This actor has a slightly different destroy behavior than other
+
-    // actors using Protocol.js. The thread actor may detach but still
-    // be in use, however detach calls the destroy method, even though it
-    // expects the actor to still be alive. Therefore, we are calling
-    // `Actor.prototype.destroy` in the `exit` method, after its state has
-    // been set to "exited", where we are certain that the actor is no
-    // longer in use.
     Actor.prototype.destroy.call(this);
   },
 

devtools/server/tests/xpcshell/testactors.js

@@ -217,7 +217,7 @@ const TestTargetActor = protocol.ActorClassWithSpec(browsingContextTargetSpec, {
     if (!this._attached) {
       return { error: "wrongState" };
     }
-    this.threadActor.exit();
+    this.threadActor.destroy();
     return { type: "detached" };
   },