Bug 1465635 - Always consider that Target.activeTab is set. r=yulia
☠☠ backed out by 93f0d162e246 ☠ ☠
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 30 Jan 2019 13:28:13 +0000
changeset 516960 d9ae7cd34c1a66de8a6426e0dc1a3b772d3738f1
parent 516959 00d325ab677ae8ac7b7261e5b9600f9bc7f11581
child 516961 734646bf982960999cd89f49e84280677f3a0f07
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [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;