Bug 1465635 - Always consider that Target.activeTab is set. r=yulia
authorAlexandre Poirot <poirot.alex@gmail.com>
Sat, 02 Feb 2019 11:24:41 +0000
changeset 456548 e6efb1191ec609df0fa28c0594efbafb3e84eb98
parent 456547 9c7fe2ba8434594aa5d49149e74ffba6d21303e2
child 456549 fead89ec2d1bb342ed1feb0616ec241411baed8a
push id35489
push userrmaries@mozilla.com
push dateSat, 02 Feb 2019 21:36:03 +0000
treeherdermozilla-central@63348118ef1d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyulia
bugs1465635
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1465635 - Always consider that Target.activeTab is set. r=yulia Depends on D15826 Differential Revision: https://phabricator.services.mozilla.com/D15828
devtools/client/framework/target.js
--- a/devtools/client/framework/target.js
+++ b/devtools/client/framework/target.js
@@ -443,17 +443,17 @@ class Target extends EventEmitter {
     return this._url;
   }
 
   get isAddon() {
     return this.isLegacyAddon || this.isWebExtension;
   }
 
   get isWorkerTarget() {
-    return this.activeTab && this.activeTab.typeName === "workerTarget";
+    return this.activeTab.typeName === "workerTarget";
   }
 
   get isLegacyAddon() {
     return !!(this.form && this.form.actor &&
       this.form.actor.match(/conn\d+\.addon(Target)?\d+/));
   }
 
   get isWebExtension() {
@@ -476,17 +476,17 @@ class Target extends EventEmitter {
     return !!this._tab;
   }
 
   get isMultiProcess() {
     return !this.window;
   }
 
   get canRewind() {
-    return this.activeTab && this.activeTab.traits.canRewind;
+    return this.activeTab.traits.canRewind;
   }
 
   isReplayEnabled() {
     return Services.prefs.getBoolPref("devtools.recordreplay.mvp.enabled")
       && this.canRewind
       && this.isLocalTab;
   }
 
@@ -533,27 +533,17 @@ class Target extends EventEmitter {
    */
   attach() {
     if (this._attach) {
       return this._attach;
     }
 
     // Attach the target actor
     const attachBrowsingContextTarget = async () => {
-      // Some BrowsingContextTargetFront are already instantiated and passed as
-      // contructor's argument, like for ParentProcessTargetActor.
-      // For them, we only need to attach them.
-      // The call to attachTarget is to be removed once all Target are having a front
-      // passed as contructor's argument.
-      if (!this.activeTab) {
-        const [, targetFront] = await this._client.attachTarget(this.form);
-        this.activeTab = targetFront;
-      } else {
-        await this.activeTab.attach();
-      }
+      await this.activeTab.attach();
 
       this.activeTab.on("tabNavigated", this._onTabNavigated);
       this._onFrameUpdate = packet => {
         this.emit("frame-update", packet);
       };
       this.activeTab.on("frameUpdate", this._onFrameUpdate);
     };
 
@@ -649,62 +639,37 @@ class Target extends EventEmitter {
   }
 
   /**
    * Setup listeners for remote debugging, updating existing ones as necessary.
    */
   _setupRemoteListeners() {
     this.client.addListener("closed", this.destroy);
 
-    // For now, only browsing-context inherited actors are using a front,
-    // for which events have to be listened on the front itself.
-    // For other actors (ContentProcessTargetActor and AddonTargetActor), events should
-    // still be listened directly on the client. This should be ultimately cleaned up to
-    // only listen from a front by bug 1465635.
-    if (this.activeTab) {
-      this.activeTab.on("tabDetached", this.destroy);
+    this.activeTab.on("tabDetached", this.destroy);
 
-      // These events should be ultimately listened from the thread client as
-      // they are coming from it and no longer go through the Target Actor/Front.
-      this._onSourceUpdated = packet => this.emit("source-updated", packet);
-      this.activeTab.on("newSource", this._onSourceUpdated);
-      this.activeTab.on("updatedSource", this._onSourceUpdated);
-    } else {
-      this._onTabDetached = (type, packet) => {
-        // We have to filter message to ensure that this detach is for this tab
-        if (packet.from == this.form.actor) {
-          this.destroy();
-        }
-      };
-      this.client.addListener("tabDetached", this._onTabDetached);
-
-      this._onSourceUpdated = (type, packet) => this.emit("source-updated", packet);
-      this.client.addListener("newSource", this._onSourceUpdated);
-      this.client.addListener("updatedSource", this._onSourceUpdated);
-    }
+    // These events should be ultimately listened from the thread client as
+    // they are coming from it and no longer go through the Target Actor/Front.
+    this._onSourceUpdated = packet => this.emit("source-updated", packet);
+    this.activeTab.on("newSource", this._onSourceUpdated);
+    this.activeTab.on("updatedSource", this._onSourceUpdated);
   }
 
   /**
    * Teardown listeners for remote debugging.
    */
   _teardownRemoteListeners() {
     // Remove listeners set in _setupRemoteListeners
     this.client.removeListener("closed", this.destroy);
-    if (this.activeTab) {
-      this.activeTab.off("tabDetached", this.destroy);
-      this.activeTab.off("newSource", this._onSourceUpdated);
-      this.activeTab.off("updatedSource", this._onSourceUpdated);
-    } else {
-      this.client.removeListener("tabDetached", this._onTabDetached);
-      this.client.removeListener("newSource", this._onSourceUpdated);
-      this.client.removeListener("updatedSource", this._onSourceUpdated);
-    }
+    this.activeTab.off("tabDetached", this.destroy);
+    this.activeTab.off("newSource", this._onSourceUpdated);
+    this.activeTab.off("updatedSource", this._onSourceUpdated);
 
     // Remove listeners set in attachTarget
-    if (this.activeTab) {
+    if (this.isBrowsingContext) {
       this.activeTab.off("tabNavigated", this._onTabNavigated);
       this.activeTab.off("frameUpdate", this._onFrameUpdate);
     }
 
     // Remove listeners set in attachConsole
     if (this.activeConsole && this._onInspectObject) {
       this.activeConsole.off("inspectObject", this._onInspectObject);
     }
@@ -777,17 +742,17 @@ class Target extends EventEmitter {
       }
 
       this._teardownRemoteListeners();
 
       if (this.isLocalTab) {
         // We started with a local tab and created the client ourselves, so we
         // should close it.
         await this._client.close();
-      } else if (this.activeTab) {
+      } else {
         // The client was handed to us, so we are not responsible for closing
         // it. We just need to detach from the tab, if already attached.
         // |detach| may fail if the connection is already dead, so proceed with
         // cleanup directly after this.
         try {
           await this.activeTab.detach();
         } catch (e) {
           console.warn(`Error while detaching target: ${e.message}`);
@@ -829,33 +794,33 @@ class Target extends EventEmitter {
    *
    * @param {String} text
    *                 The text to log.
    * @param {String} category
    *                 The category of the message.  @see nsIScriptError.
    * @returns {Promise}
    */
   logErrorInPage(text, category) {
-    if (this.activeTab && this.activeTab.traits.logInPage) {
+    if (this.activeTab.traits.logInPage) {
       const errorFlag = 0;
       return this.activeTab.logInPage({ text, category, flags: errorFlag });
     }
     return Promise.resolve();
   }
 
   /**
    * Log a warning of some kind to the tab's console.
    *
    * @param {String} text
    *                 The text to log.
    * @param {String} category
    *                 The category of the message.  @see nsIScriptError.
    * @returns {Promise}
    */
   logWarningInPage(text, category) {
-    if (this.activeTab && this.activeTab.traits.logInPage) {
+    if (this.activeTab.traits.logInPage) {
       const warningFlag = 1;
       return this.activeTab.logInPage({ text, category, flags: warningFlag });
     }
     return Promise.resolve();
   }
 }
 exports.Target = Target;