Bug 1471754 - Make the Toolbox use the TargetList. r=jdescottes

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

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

devtools/client/debugger/test/mochitest/browser_dbg-worker-scopes.js

@@ -33,7 +33,7 @@ add_task(async function() {
   const workerSource = findSource(dbg, "scopes-worker.js");
 
   await addBreakpoint(dbg, workerSource, 11);
-  await dbg.toolbox._target.waitForRequestsToSettle();
+  await dbg.toolbox.target.waitForRequestsToSettle();
   invokeInTab("startWorker");
   await waitForPaused(dbg, "scopes-worker.js");
   const onRemoved = waitForDispatch(dbg, "REMOVE_BREAKPOINT");

devtools/client/framework/toolbox.js

@@ -33,6 +33,8 @@ var Startup = Cc["@mozilla.org/devtools/startup-clh;1"].getService(
   Ci.nsISupports
 ).wrappedJSObject;
 
+const { TargetList } = require("devtools/shared/resources/target-list");
+
 const { BrowserLoader } = ChromeUtils.import(
   "resource://devtools/client/shared/browser-loader.js"
 );
@@ -208,12 +210,13 @@ function Toolbox(
   frameId,
   msSinceProcessStart
 ) {
-  this._target = target;
   this._win = contentWindow;
   this.frameId = frameId;
   this.selection = new Selection();
   this.telemetry = new Telemetry();
 
+  this.targetList = new TargetList(target.client.mainRoot, target);
+
   // The session ID is used to determine which telemetry events belong to which
   // toolbox session. Because we use Amplitude to analyse the telemetry data we
   // must use the time since the system wide epoch as the session ID.
@@ -288,6 +291,9 @@ function Toolbox(
   this.toggleDragging = this.toggleDragging.bind(this);
   this._onPausedState = this._onPausedState.bind(this);
   this._onResumedState = this._onResumedState.bind(this);
+  this._onTargetAvailable = this._onTargetAvailable.bind(this);
+  this._onTargetDestroyed = this._onTargetDestroyed.bind(this);
+
   this.isPaintFlashing = false;
 
   if (!selectedTool) {
@@ -487,16 +493,18 @@ Toolbox.prototype = {
    * in a distinct process.
    */
   async switchToTarget(newTarget) {
-    // First unregister the current target
+    // Notify gDevTools that the toolbox will be hooked to another target.
-    this.detachTarget();
-
-    this._target = newTarget;
-
-    // Notify gDevTools that the toolbox is now hooked to another tab target.
     this.emit("switch-target", newTarget);
 
+    // TargetList.switchToTarget won't wait for all target listeners, like
+    // Toolbox._onTargetAvailable to be finished before resolving.
+    // But, we do expect the target to be attached before calling listFrames
+    // and initPerformance. So wait for this via an internal event.
+    const onAttached = this.once("top-target-attached");
+    await this.targetList.switchToTarget(newTarget);
+    await onAttached;
+
     // Attach the toolbox to this new target
-    await this._attachTargets(newTarget);
     await this._listFrames();
     await this.initPerformance();
 
@@ -514,12 +522,10 @@ Toolbox.prototype = {
   },
 
   /**
-   * Get/alter the target of a Toolbox so we're debugging something different.
+   * Get the current top level target the toolbox is debugging.
-   * See Target.jsm for more details.
-   * TODO: Do we allow |toolbox.target = null;| ?
    */
   get target() {
-    return this._target;
+    return this.targetList.targetFront;
   },
 
   get threadFront() {
@@ -609,48 +615,34 @@ Toolbox.prototype = {
   },
 
   /**
-   * Attach to a new top-level target.
+   * This method will be called for the top-level target, as well as any potential
-   * This method will attach to the top-level target, as well as any potential
    * additional targets we may care about.
    */
-  async _attachTargets(target) {
+  async _onTargetAvailable(type, targetFront, isTopLevel) {
-    // For now, register these event listeners only on the top level target
+    if (isTopLevel) {
-    this._target.on("will-navigate", this._onWillNavigate);
+      // Attach to a new top-level target.
-    this._target.on("navigate", this._refreshHostTitle);
+      // For now, register these event listeners only on the top level target
-    this._target.on("frame-update", this._updateFrames);
+      targetFront.on("will-navigate", this._onWillNavigate);
-    this._target.on("inspect-object", this._onInspectObject);
+      targetFront.on("navigate", this._refreshHostTitle);
+      targetFront.on("frame-update", this._updateFrames);
+      targetFront.on("inspect-object", this._onInspectObject);
 
-    this._target.onFront("inspector", async inspectorFront => {
+      targetFront.onFront("inspector", async inspectorFront => {
-      registerWalkerListeners(this.store, inspectorFront.walker);
+        registerWalkerListeners(this.store, inspectorFront.walker);
-    });
+      });
 
-    this._threadFront = await this._attachTarget(target);
+      this._threadFront = await this._attachTarget(targetFront);
+      this.emit("top-target-attached");
+    } else {
+      return this._attachTarget(targetFront);
+    }
+  },
 
-    const fissionSupport = Services.prefs.getBoolPref(
+  _onTargetDestroyed(type, targetFront, isTopLevel) {
-      "devtools.browsertoolbox.fission"
+    if (isTopLevel) {
-    );
+      this.detachTarget();
-
+    } else {
-    if (fissionSupport && target.isParentProcess && !target.isAddon) {
+      this._stopThreadFrontListeners(targetFront.threadFront);
-      const { mainRoot } = target.client;
-      const { processes } = await mainRoot.listProcesses();
-
-      for (const processDescriptor of processes) {
-        const targetFront = await processDescriptor.getTarget();
-
-        // Ignore the parent process target, which is the current target
-        if (targetFront === target) {
-          continue;
-        }
-
-        if (!targetFront) {
-          console.warn(
-            "Can't retrieve the target front for process",
-            processDescriptor
-          );
-          continue;
-        }
-        await this._attachTarget(targetFront);
-      }
     }
   },
 
@@ -680,9 +672,9 @@ Toolbox.prototype = {
     threadFront.on("resumed", this._onResumedState);
   },
 
-  _stopThreadFrontListeners: function() {
+  _stopThreadFrontListeners: function(threadFront) {
-    this.threadFront.off("paused", this._onPausedState);
+    threadFront.off("paused", this._onPausedState);
-    this.threadFront.off("resumed", this._onResumedState);
+    threadFront.off("resumed", this._onResumedState);
   },
 
   _attachAndResumeThread: async function(target) {
@@ -737,9 +729,15 @@ Toolbox.prototype = {
         );
       });
 
+      await this.targetList.startListening(TargetList.ALL_TYPES);
+
       // Optimization: fire up a few other things before waiting on
       // the iframe being ready (makes startup faster)
-      await this._attachTargets(this.target);
+      await this.targetList.watchTargets(
+        TargetList.ALL_TYPES,
+        this._onTargetAvailable,
+        this._onTargetDestroyed
+      );
 
       await domReady;
 
@@ -807,7 +805,7 @@ Toolbox.prototype = {
       // remoted, otherwise we could have done it in the toolbox constructor
       // (bug 1072764).
       const toolDef = gDevTools.getToolDefinition(this._defaultToolId);
-      if (!toolDef || !toolDef.isTargetSupported(this._target)) {
+      if (!toolDef || !toolDef.isTargetSupported(this.target)) {
         this._defaultToolId = "webconsole";
       }
 
@@ -892,13 +890,13 @@ Toolbox.prototype = {
   },
 
   detachTarget() {
-    this._target.off("inspect-object", this._onInspectObject);
+    this.target.off("inspect-object", this._onInspectObject);
-    this._target.off("will-navigate", this._onWillNavigate);
+    this.target.off("will-navigate", this._onWillNavigate);
-    this._target.off("navigate", this._refreshHostTitle);
+    this.target.off("navigate", this._refreshHostTitle);
-    this._target.off("frame-update", this._updateFrames);
+    this.target.off("frame-update", this._updateFrames);
 
     // Detach the thread
-    this._stopThreadFrontListeners();
+    this._stopThreadFrontListeners(this._threadFront);
     this._threadFront = null;
   },
 
@@ -1661,7 +1659,7 @@ Toolbox.prototype = {
    * the host changes.
    */
   _buildDockOptions: function() {
-    if (!this._target.isLocalTab) {
+    if (!this.target.isLocalTab) {
       this.component.setDockOptionsEnabled(false);
       this.component.setCanCloseToolbox(false);
       return;
@@ -1720,8 +1718,7 @@ Toolbox.prototype = {
     // Get the definitions that will only affect the main tab area.
     this.panelDefinitions = definitions.filter(
       definition =>
-        definition.isTargetSupported(this._target) &&
+        definition.isTargetSupported(this.target) && definition.id !== "options"
-        definition.id !== "options"
     );
 
     // Do async lookup of disable pop-up auto-hide state.
@@ -2163,7 +2160,7 @@ Toolbox.prototype = {
    *        Tool definition of the tool to build a tab for.
    */
   _buildPanelForTool: function(toolDefinition) {
-    if (!toolDefinition.isTargetSupported(this._target)) {
+    if (!toolDefinition.isTargetSupported(this.target)) {
       return;
     }
 
@@ -3023,7 +3020,7 @@ Toolbox.prototype = {
 
   // Is the disable auto-hide of pop-ups feature available in this context?
   get disableAutohideAvailable() {
-    return this._target.chrome;
+    return this.target.chrome;
   },
 
   async toggleNoAutohide() {
@@ -3214,7 +3211,7 @@ Toolbox.prototype = {
    *        The host type of the new host object
    */
   switchHost: function(hostType) {
-    if (hostType == this.hostType || !this._target.isLocalTab) {
+    if (hostType == this.hostType || !this.target.isLocalTab) {
       return null;
     }
 
@@ -3386,7 +3383,7 @@ Toolbox.prototype = {
       isAdditionalTool = true;
     }
 
-    if (definition.isTargetSupported(this._target)) {
+    if (definition.isTargetSupported(this.target)) {
       if (isAdditionalTool) {
         this.visibleAdditionalTools = [...this.visibleAdditionalTools, toolId];
         this._combineAndSortPanelDefinitions();
@@ -3636,7 +3633,13 @@ Toolbox.prototype = {
     // Reset preferences set by the toolbox
     outstanding.push(this.resetPreference());
 
-    this.detachTarget();
+    this.targetList.unwatchTargets(
+      TargetList.ALL_TYPES,
+      this._onTargetAvailable,
+      this._onTargetDestroyed
+    );
+
+    this.targetList.stopListening(TargetList.ALL_TYPES);
 
     // Unregister buttons listeners
     this.toolbarButtons.forEach(button => {
@@ -3709,12 +3712,7 @@ Toolbox.prototype = {
             // This is done after other destruction tasks since it may tear down
             // fronts and the debugger transport which earlier destroy methods may
             // require to complete.
-            if (!this._target) {
+            return this.target.destroy();
-              return null;
-            }
-            const target = this._target;
-            this._target = null;
-            return target.destroy();
           }, console.error)
           .then(() => {
             this.emit("destroyed");