Bug 1618695 - Wait for target to be fully destroyed before switching targets r=ochameau

Depends on D73779
Reusing the same tab descriptors across process switches means that calling
getTarget on the tab descriptor must create a new target and not return the old one.

The caching mechanism on descriptor fronts relies on the fact that the TargetFront is still alive.
When TargetFront.actorID is null, the cache is invalidated and we fetch a new target.

However the current target switching mechanism relies on an event fired very early in the target
destruction sequence. This means that when we will call getTarget, the old target might still be
alive.

We also start waiting for an event in RDMs target switching, which was not waiting for anything so far

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

devtools/client/fronts/descriptors/tab.js

@@ -31,6 +31,8 @@ class TabDescriptorFront extends FrontClassWithSpec(tabDescriptorSpec) {
     // (eg, regular tab toolbox) or browsing context targets (eg tab remote
     // debugging).
     this._localTab = null;
+
+    this._onTargetDestroyed = this._onTargetDestroyed.bind(this);
   }
 
   form(json) {
@@ -79,9 +81,17 @@ class TabDescriptorFront extends FrontClassWithSpec(tabDescriptorSpec) {
     front.actorID = form.actor;
     front.form(form);
     this.manage(front);
+    front.on("target-destroyed", this._onTargetDestroyed);
     return front;
   }
 
+  _onTargetDestroyed() {
+    // Clear the cached targetFront when the target is destroyed.
+    // Note that we are also checking that _targetFront has a valid actorID
+    // in getTarget, this acts as an additional security to avoid races.
+    this._targetFront = null;
+  }
+
   /**
    * This method is mostly intended for backward compatibility (FF76 and older).
    *

devtools/client/fronts/targets/local-tab.js

@@ -140,7 +140,7 @@ class LocalTabTargetFront extends BrowsingContextTargetFront {
 
       // If we support target switching, only wait for the target to be
       // destroyed so that TargetFactory clears its memoized target for this tab
-      await this.once("close");
+      await this.once("target-destroyed");
     } else {
       // Otherwise, if we don't support target switching, ensure the toolbox is destroyed.
       // We need to wait for the toolbox destruction because the TargetFactory memoized the targets,

devtools/client/fronts/targets/target-mixin.js

@@ -560,6 +560,10 @@ function TargetMixin(parentClass) {
         }
       }
 
+      // This event should be emitted before calling super.destroy(), because
+      // super.destroy() will remove all event listeners attached to this front.
+      this.emit("target-destroyed");
+
       // Do that very last in order to let a chance to dispatch `detach` requests.
       super.destroy();
 

devtools/client/responsive/ui.js

@@ -1181,6 +1181,11 @@ class ResponsiveUI {
     // We should ignore the remoteness events in case of old RDM
     // as it is firing fake remoteness events.
     if (this.isBrowserUIEnabled) {
+      // The current tab target will be destroyed by the process change.
+      // Wait for the target to be fully destroyed so that the cache of the
+      // corresponding TabDescriptorFront has been cleared. Otherwise, getTab()
+      // might return the soon to be destroyed target again.
+      await this.targetList.targetFront.once("target-destroyed");
       const newTarget = await this.client.mainRoot.getTab();
       await this.targetList.switchToTarget(newTarget);
     } else {