Bug 1506549 - Either pass a front or a form to Target constructor. r=yulia
authorAlexandre Poirot <poirot.alex@gmail.com>
Thu, 15 Nov 2018 10:22:47 +0000
changeset 503054 80ebeb322e14528b47e152cd306795745b4d6b5b
parent 503053 7155e69bc42c3613901dd356371e5529bf85e855
child 503055 bd6fcbbf257de863ca6334fe6683cebb1b6811f5
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyulia
bugs1506549
milestone65.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 1506549 - Either pass a front or a form to Target constructor. r=yulia MozReview-Commit-ID: BWsvAcWthnF Differential Revision: https://phabricator.services.mozilla.com/D11693
devtools/client/framework/target.js
devtools/shared/fronts/targets/worker.js
devtools/shared/webconsole/test/common.js
--- a/devtools/client/framework/target.js
+++ b/devtools/client/framework/target.js
@@ -123,28 +123,16 @@ const TargetFactory = exports.TargetFact
     return targetPromise;
   },
 
   forWorker: function(workerTargetFront) {
     let target = targets.get(workerTargetFront);
     if (target == null) {
       target = new Target({
         client: workerTargetFront.client,
-        // Fake a form attribute until all Target is merged with the Front itself
-        // and will receive form attribute natively.
-        get form() {
-          return {
-            actor: workerTargetFront.actorID,
-            traits: {},
-            // /!\ This depends on WorkerTargetFront.attach being called before this.
-            // It happens that WorkerTargetFront is instantiated from attachWorker,
-            // which instiate this class *and* calls `attach`.
-            consoleActor: workerTargetFront.consoleActor,
-          };
-        },
         activeTab: workerTargetFront,
         chrome: false,
       });
       targets.set(workerTargetFront, target);
     }
     return target;
   },
 
@@ -195,38 +183,39 @@ const TargetFactory = exports.TargetFact
  * a remote device, like a tab on Firefox for Android. But it can also be an add-on,
  * as well as firefox parent process, or just one of its content process.
  * A Target is related to a given TargetActor, for which we pass the form as
  * argument.
  *
  * For now, only workers are having a distinct Target class called WorkerTarget.
  *
  * @param {Object} form
- *                 The TargetActor's form to be connected to.
+ *                  The TargetActor's form to be connected to. Null if front is passed.
+ * @param {Front} activeTab
+ *                  If we already have a front for this target, pass it here. Null if
+ *                  form is passed.
  * @param {DebuggerClient} client
- *                 The DebuggerClient instance to be used to debug this target.
+ *                  The DebuggerClient instance to be used to debug this target.
  * @param {Boolean} chrome
  *                  True, if we allow to see privileged resources like JSM, xpcom,
  *                  frame scripts...
- * @param {Front}   activeTab (optional)
- *                  If we already have a front for this target, pass it here.
  * @param {xul:tab} tab (optional)
  *                  If the target is a local Firefox tab, a reference to the firefox
  *                  frontend tab object.
  */
 function Target({ form, client, chrome, activeTab = null, tab = null }) {
   EventEmitter.decorate(this);
   this.destroy = this.destroy.bind(this);
   this._onTabNavigated = this._onTabNavigated.bind(this);
   this.activeConsole = null;
   this.activeTab = activeTab;
 
   this._form = form;
-  this._url = form.url;
-  this._title = form.title;
+  this._url = this.form.url;
+  this._title = this.form.title;
 
   this._client = client;
   this._chrome = chrome;
 
   // When debugging local tabs, we also have a reference to the Firefox tab
   // This is used to:
   // * distinguish local tabs from remote (see target.isLocalTab)
   // * being able to hookup into Firefox UI (see Hosts)
@@ -242,18 +231,18 @@ function Target({ form, client, chrome, 
   // * xpcshell debugging (it uses ParentProcessTargetActor, which inherits from
   //                       BrowsingContextActor, but doesn't have any valid browsing
   //                       context to attach to.)
   // Starting with FF64, BrowsingContextTargetActor exposes a traits to help identify
   // the target actors inheriting from it. It also help identify the xpcshell debugging
   // target actor that doesn't have any valid browsing context.
   // (Once FF63 is no longer supported, we can remove the `else` branch and only look
   // for the traits)
-  if (this._form.traits && ("isBrowsingContext" in this._form.traits)) {
-    this._isBrowsingContext = this._form.traits.isBrowsingContext;
+  if (this.form.traits && ("isBrowsingContext" in this.form.traits)) {
+    this._isBrowsingContext = this.form.traits.isBrowsingContext;
   } else {
     this._isBrowsingContext = !this.isLegacyAddon && !this.isContentProcess && !this.isWorkerTarget;
   }
 
   // Cache of already created targed-scoped fronts
   // [typeName:string => Front instance]
   this.fronts = new Map();
   // Temporary fix for bug #1493131 - inspector has a different life cycle
@@ -363,17 +352,19 @@ Target.prototype = {
     return this.client.traits[traitName];
   },
 
   get tab() {
     return this._tab;
   },
 
   get form() {
-    return this._form;
+    // Target constructor either receive a form or a Front.
+    // If a front is passed, fetch the form from it.
+    return this._form || this.activeTab.targetForm;
   },
 
   // Get a promise of the RootActor's form
   get root() {
     return this.client.mainRoot.rootForm;
   },
 
   // Temporary fix for bug #1493131 - inspector has a different life cycle
@@ -428,17 +419,17 @@ Target.prototype = {
   // interface and requires to call `attach` request before being used and
   // `detach` during cleanup.
   get isBrowsingContext() {
     return this._isBrowsingContext;
   },
 
   get name() {
     if (this.isAddon) {
-      return this._form.name;
+      return this.form.name;
     }
     return this._title;
   },
 
   get url() {
     return this._url;
   },
 
@@ -446,34 +437,34 @@ Target.prototype = {
     return this.isLegacyAddon || this.isWebExtension;
   },
 
   get isWorkerTarget() {
     return this.activeTab && this.activeTab.typeName === "workerTarget";
   },
 
   get isLegacyAddon() {
-    return !!(this._form && this._form.actor &&
-      this._form.actor.match(/conn\d+\.addon(Target)?\d+/));
+    return !!(this.form && this.form.actor &&
+      this.form.actor.match(/conn\d+\.addon(Target)?\d+/));
   },
 
   get isWebExtension() {
-    return !!(this._form && this._form.actor && (
-      this._form.actor.match(/conn\d+\.webExtension(Target)?\d+/) ||
-      this._form.actor.match(/child\d+\/webExtension(Target)?\d+/)
+    return !!(this.form && this.form.actor && (
+      this.form.actor.match(/conn\d+\.webExtension(Target)?\d+/) ||
+      this.form.actor.match(/child\d+\/webExtension(Target)?\d+/)
     ));
   },
 
   get isContentProcess() {
     // browser content toolbox's form will be of the form:
     //   server0.conn0.content-process0/contentProcessTarget7
     // while xpcshell debugging will be:
     //   server1.conn0.contentProcessTarget7
-    return !!(this._form && this._form.actor &&
-      this._form.actor.match(/conn\d+\.(content-process\d+\/)?contentProcessTarget\d+/));
+    return !!(this.form && this.form.actor &&
+      this.form.actor.match(/conn\d+\.(content-process\d+\/)?contentProcessTarget\d+/));
   },
 
   get isLocalTab() {
     return !!this._tab;
   },
 
   get isMultiProcess() {
     return !this.window;
@@ -529,62 +520,62 @@ Target.prototype = {
    */
   attach() {
     if (this._attach) {
       return this._attach;
     }
 
     // Attach the target actor
     const attachBrowsingContextTarget = async () => {
-      const [, targetFront] = await this._client.attachTarget(this._form.actor);
+      const [, targetFront] = await this._client.attachTarget(this.form.actor);
       this.activeTab = targetFront;
 
       this.activeTab.on("tabNavigated", this._onTabNavigated);
       this._onFrameUpdate = packet => {
         this.emit("frame-update", packet);
       };
       this.activeTab.on("frameUpdate", this._onFrameUpdate);
     };
 
     // Attach the console actor
     const attachConsole = async () => {
       const [, consoleClient] = await this._client.attachConsole(
-        this._form.consoleActor, []);
+        this.form.consoleActor, []);
       this.activeConsole = consoleClient;
 
       this._onInspectObject = packet => this.emit("inspect-object", packet);
       this.activeConsole.on("inspectObject", this._onInspectObject);
     };
 
     this._attach = (async () => {
-      if (this._form.isWebExtension &&
+      if (this.form.isWebExtension &&
           this.client.mainRoot.traits.webExtensionAddonConnect) {
         // The addonTargetActor form is related to a WebExtensionActor instance,
         // which isn't a target actor on its own, it is an actor living in the parent
         // process with access to the addon metadata, it can control the addon (e.g.
         // reloading it) and listen to the AddonManager events related to the lifecycle of
         // the addon (e.g. when the addon is disabled or uninstalled).
         // To retrieve the target actor instance, we call its "connect" method, (which
         // fetches the target actor form from a WebExtensionTargetActor instance).
         const {form} = await this._client.request({
-          to: this._form.actor, type: "connect",
+          to: this.form.actor, type: "connect",
         });
 
         this._form = form;
-        this._url = form.url;
-        this._title = form.title;
+        this._url = this.form.url;
+        this._title = this.form.title;
       }
 
       // AddonTargetActor and ContentProcessTargetActor don't inherit from
       // BrowsingContextTargetActor (i.e. this.isBrowsingContext=false) and don't need
       // to be attached via DebuggerClient.attachTarget.
       if (this.isBrowsingContext) {
         await attachBrowsingContextTarget();
       } else if (this.isLegacyAddon) {
-        const [, addonTargetFront] = await this._client.attachAddon(this._form);
+        const [, addonTargetFront] = await this._client.attachAddon(this.form);
         this.activeTab = addonTargetFront;
       } else if (this.isWorkerTarget || this.isContentProcess) {
         // Worker and Content process targets are the first target to have their front already
         // instantiated. The plan is to have all targets to have their front passed as
         // constructor argument.
       } else {
         throw new Error(`Unsupported type of target. Expected target of one of the` +
           ` following types: BrowsingContext, ContentProcess, Worker or ` +
@@ -671,17 +662,17 @@ Target.prototype = {
       // 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) {
+        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);
@@ -807,31 +798,31 @@ Target.prototype = {
 
   /**
    * Clean up references to what this target points to.
    */
   _cleanup: function() {
     if (this._tab) {
       targets.delete(this._tab);
     } else {
-      promiseTargets.delete(this._form);
+      promiseTargets.delete(this.form);
     }
 
     this.activeTab = null;
     this.activeConsole = null;
     this._client = null;
     this._tab = null;
     this._form = null;
     this._attach = null;
     this._title = null;
     this._url = null;
   },
 
   toString: function() {
-    const id = this._tab ? this._tab : (this._form && this._form.actor);
+    const id = this._tab ? this._tab : (this.form && this.form.actor);
     return `Target:${id}`;
   },
 
   /**
    * Log an error of some kind to the tab's console.
    *
    * @param {String} text
    *                 The text to log.
--- a/devtools/shared/fronts/targets/worker.js
+++ b/devtools/shared/fronts/targets/worker.js
@@ -11,16 +11,20 @@ loader.lazyRequireGetter(this, "ThreadCl
 
 const WorkerTargetFront = protocol.FrontClassWithSpec(workerTargetSpec, {
   initialize: function(client, form) {
     protocol.Front.prototype.initialize.call(this, client, form);
 
     this.thread = null;
     this.traits = {};
 
+    // Save the full form for Target class usage
+    // Do not use `form` name to avoid colliding with protocol.js's `form` method
+    this.targetForm = form;
+
     // TODO: remove once ThreadClient becomes a front
     this.client = client;
 
     this._isClosed = false;
 
     this.destroy = this.destroy.bind(this);
     this.on("close", this.destroy);
   },
@@ -45,17 +49,18 @@ const WorkerTargetFront = protocol.Front
   attach: custom(async function() {
     const response = await this._attach();
 
     this.url = response.url;
 
     // Immediately call `connect` in other to fetch console and thread actors
     // that will be later used by Target.
     const connectResponse = await this.connect({});
-    this.consoleActor = connectResponse.consoleActor;
+    // Set the console actor ID on the form to expose it to Target.attach's attachConsole
+    this.targetForm.consoleActor = connectResponse.consoleActor;
     this.threadActor = connectResponse.threadActor;
 
     return response;
   }, {
     impl: "_attach",
   }),
 
   detach: custom(async function() {
@@ -80,17 +85,17 @@ const WorkerTargetFront = protocol.Front
     return Promise.resolve();
   },
 
   attachThread: async function(options = {}) {
     if (this.thread) {
       const response = [{
         type: "connected",
         threadActor: this.thread._actor,
-        consoleActor: this.consoleActor,
+        consoleActor: this.targetForm.consoleActor,
       }, this.thread];
       return response;
     }
 
     const attachResponse = await this.client.request({
       to: this.threadActor,
       type: "attach",
       options,
--- a/devtools/shared/webconsole/test/common.js
+++ b/devtools/shared/webconsole/test/common.js
@@ -114,18 +114,18 @@ var _attachConsole = async function(
     const [workerResponse, workerTargetFront] =
       await targetFront.attachWorker(workerTargetActor);
     if (!workerTargetFront || workerResponse.error) {
       console.error("attachWorker failed. No worker target front or " +
                     " error: " + workerResponse.error);
       return;
     }
     await workerTargetFront.attachThread({});
-    state.actor = workerTargetFront.consoleActor;
-    state.dbgClient.attachConsole(workerTargetFront.consoleActor, listeners)
+    state.actor = workerTargetFront.targetForm.consoleActor;
+    state.dbgClient.attachConsole(workerTargetFront.targetForm.consoleActor, listeners)
       .then(_onAttachConsole.bind(null, state), _onAttachError.bind(null, state));
   } else {
     state.actor = tab.consoleActor;
     state.dbgClient.attachConsole(tab.consoleActor, listeners)
       .then(_onAttachConsole.bind(null, state), _onAttachError.bind(null, state));
   }
 };