Bug 1485660 - Convert TabClient to a front. r=jdescottes
authorAlexandre Poirot <poirot.alex@gmail.com>
Mon, 15 Oct 2018 08:36:07 +0000
changeset 489601 174ed200a5d2d4c4e4c8ab98a9b2300aa4f2a9f7
parent 489600 f0d97e8bb0813840fce9d2dea039b3deed3c5898
child 489602 8cf41a5970be7b559c9df821bd5cba3fd3cfad9c
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersjdescottes
bugs1485660
milestone64.0a1
Bug 1485660 - Convert TabClient to a front. r=jdescottes TabClient appears to be a client for any actor that inherits from browsing context target actor. So let it be a front for that. MozReview-Commit-ID: KmpClxJ53N7 Depends on D7457 Differential Revision: https://phabricator.services.mozilla.com/D7458
devtools/client/framework/target.js
devtools/client/framework/toolbox-options.js
devtools/client/framework/toolbox.js
devtools/server/actors/targets/browsing-context.js
devtools/shared/client/constants.js
devtools/shared/client/debugger-client.js
devtools/shared/client/moz.build
devtools/shared/client/tab-client.js
devtools/shared/client/thread-client.js
devtools/shared/fronts/moz.build
devtools/shared/fronts/targets/browsing-context.js
devtools/shared/fronts/targets/moz.build
devtools/shared/specs/targets/browsing-context.js
--- a/devtools/client/framework/target.js
+++ b/devtools/client/framework/target.js
@@ -736,17 +736,21 @@ TabTarget.prototype = {
           // We started with a local tab and created the client ourselves, so we
           // should close it.
           this._client.close().then(cleanupAndResolve);
         } else if (this.activeTab) {
           // 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.
-          this.activeTab.detach();
+          try {
+            await this.activeTab.detach();
+          } catch (e) {
+            console.warn(`Error while detaching target: ${e.message}`);
+          }
           cleanupAndResolve();
         } else {
           cleanupAndResolve();
         }
       }
     });
 
     return this._destroyer;
--- a/devtools/client/framework/toolbox-options.js
+++ b/devtools/client/framework/toolbox-options.js
@@ -455,17 +455,17 @@ OptionsPanel.prototype = {
       prefSelect.addEventListener("change", function(e) {
         const select = e.target;
         SetPref(select.getAttribute("data-pref"),
           select.options[select.selectedIndex].value);
       });
     }
 
     if (this.target.activeTab && !this.target.chrome) {
-      const [ response ] = await this.target.client.attachTarget(this.target.activeTab._actor);
+      const response = await this.target.activeTab.attach();
       this._origJavascriptEnabled = !response.javascriptEnabled;
       this.disableJSNode.checked = this._origJavascriptEnabled;
       this.disableJSNode.addEventListener("click", this._disableJSClicked);
     } else {
       // Hide the checkbox and label
       this.disableJSNode.parentNode.style.display = "none";
 
       const triggersPageRefreshLabel =
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -2323,17 +2323,17 @@ Toolbox.prototype = {
   },
 
   /**
    * Select a frame by sending 'switchToFrame' packet to the backend.
    */
   onSelectFrame: function(frameId) {
     // Send packet to the backend to select specified frame and
     // wait for 'frameUpdate' event packet to update the UI.
-    this.target.activeTab.switchToFrame(frameId);
+    this.target.activeTab.switchToFrame({ windowId: frameId });
   },
 
   /**
    * Highlight a frame in the page
    */
   onHighlightFrame: async function(frameId) {
     // Need to initInspector to check presence of getNodeActorFromWindowID
     // and use the highlighter later
--- a/devtools/server/actors/targets/browsing-context.js
+++ b/devtools/server/actors/targets/browsing-context.js
@@ -961,17 +961,17 @@ const browsingContextTargetPrototype = {
   /**
    * Navigate this browsing context to a new location
    */
   navigateTo(request) {
     // Wait a tick so that the response packet can be dispatched before the
     // subsequent navigation event packet.
     Services.tm.dispatchToMainThread(DevToolsUtils.makeInfallible(() => {
       this.window.location = request.url;
-    }, "BrowsingContextTargetActor.prototype.navigateTo's delayed body"));
+    }, "BrowsingContextTargetActor.prototype.navigateTo's delayed body:" + request.url));
     return {};
   },
 
   /**
    * Reconfigure options.
    */
   reconfigure(request) {
     const options = request.options || {};
--- a/devtools/shared/client/constants.js
+++ b/devtools/shared/client/constants.js
@@ -28,23 +28,26 @@ const UnsolicitedNotifications = {
   "networkEventUpdate": "networkEventUpdate",
   "documentEvent": "documentEvent",
   "tabDetached": "tabDetached",
   "tabListChanged": "tabListChanged",
   "reflowActivity": "reflowActivity",
   "addonListChanged": "addonListChanged",
   "workerListChanged": "workerListChanged",
   "serviceWorkerRegistrationListChanged": "serviceWorkerRegistrationList",
-  "tabNavigated": "tabNavigated",
-  "frameUpdate": "frameUpdate",
   "pageError": "pageError",
   "evaluationResult": "evaluationResult",
+  "updatedSource": "updatedSource",
+  "inspectObject": "inspectObject",
+
+  // newSource is still emitted on the ThreadActor, in addition to the
+  // BrowsingContextActor we have to keep it here until ThreadClient is converted to
+  // ThreadFront and/or we stop emitting this duplicated events.
+  // See ThreadActor.onNewSourceEvent.
   "newSource": "newSource",
-  "updatedSource": "updatedSource",
-  "inspectObject": "inspectObject"
 };
 
 /**
  * Set of pause types that are sent by the server and not as an immediate
  * response to a client request.
  */
 const UnsolicitedPauses = {
   "resumeLimit": "resumeLimit",
--- a/devtools/shared/client/debugger-client.js
+++ b/devtools/shared/client/debugger-client.js
@@ -19,20 +19,21 @@ const {
 
 loader.lazyRequireGetter(this, "Authentication", "devtools/shared/security/auth");
 loader.lazyRequireGetter(this, "DebuggerSocket", "devtools/shared/security/socket", true);
 loader.lazyRequireGetter(this, "EventEmitter", "devtools/shared/event-emitter");
 
 loader.lazyRequireGetter(this, "WebConsoleClient", "devtools/shared/webconsole/client", true);
 loader.lazyRequireGetter(this, "AddonClient", "devtools/shared/client/addon-client");
 loader.lazyRequireGetter(this, "RootClient", "devtools/shared/client/root-client");
-loader.lazyRequireGetter(this, "TabClient", "devtools/shared/client/tab-client");
+loader.lazyRequireGetter(this, "BrowsingContextFront", "devtools/shared/fronts/targets/browsing-context", true);
 loader.lazyRequireGetter(this, "ThreadClient", "devtools/shared/client/thread-client");
 loader.lazyRequireGetter(this, "WorkerClient", "devtools/shared/client/worker-client");
 loader.lazyRequireGetter(this, "ObjectClient", "devtools/shared/client/object-client");
+loader.lazyRequireGetter(this, "Pool", "devtools/shared/protocol", true);
 
 // Retrieve the major platform version, i.e. if we are on Firefox 64.0a1, it will be 64.
 const PLATFORM_MAJOR_VERSION = AppConstants.MOZ_APP_VERSION.match(/\d+/)[0];
 
 // Define the minimum officially supported version of Firefox when connecting to a remote
 // runtime. (Use ".0a1" to support the very first nightly version)
 // This matches the release channel's version when we are on nightly,
 // or 2 versions before when we are on other channels.
@@ -45,18 +46,29 @@ const MS_PER_DAY = 86400000;
  * provides the means to communicate with the server and exchange the messages
  * required by the protocol in a traditional JavaScript API.
  */
 function DebuggerClient(transport) {
   this._transport = transport;
   this._transport.hooks = this;
 
   // Map actor ID to client instance for each actor type.
+  // To be removed once all clients are refactored to protocol.js
   this._clients = new Map();
 
+  // Pool of fronts instanciated by this class.
+  // This is useful for actors that have already been transitioned to protocol.js
+  // Once RootClient becomes a protocol.js actor, these actors can be attached to it
+  // instead of this pool.
+  // This Pool will automatically be added to this._pools via addActorPool once the first
+  // Front will be added to it (in attachTarget, attachWorker,...).
+  // And it does not need to destroyed explicitly as all Pools are destroyed on client
+  // closing.
+  this._frontPool = new Pool(this);
+
   this._pendingRequests = new Map();
   this._activeRequests = new Map();
   this._eventsEnabled = true;
 
   this.traits = {};
 
   this.request = this.request.bind(this);
   this.localTransport = this._transport.onOutputStreamReady === undefined;
@@ -351,39 +363,25 @@ DebuggerClient.prototype = {
    *  - start watching for inner iframe updates (emits `frameUpdate` messages)
    *  - retrieve the thread actor:
    *    Instantiates a new ThreadActor that can be later attached to in order to
    *    debug JS sources in the document.
    *
    * @param string targetActor
    *        The target actor ID for the tab to attach.
    */
-  attachTarget: function(targetActor) {
-    if (this._clients.has(targetActor)) {
-      const cachedTarget = this._clients.get(targetActor);
-      const cachedResponse = {
-        cacheDisabled: cachedTarget.cacheDisabled,
-        javascriptEnabled: cachedTarget.javascriptEnabled,
-        traits: cachedTarget.traits,
-      };
-      return promise.resolve([cachedResponse, cachedTarget]);
+  attachTarget: async function(targetActor) {
+    let front = this._frontPool.actor(targetActor);
+    if (!front) {
+      front = new BrowsingContextFront(this, { actor: targetActor });
+      this._frontPool.manage(front);
     }
 
-    const packet = {
-      to: targetActor,
-      type: "attach"
-    };
-    return this.request(packet).then(response => {
-      // TabClient can actually represent targets other than a tab.
-      // It is planned to be renamed while being converted to a front
-      // in bug 1485660.
-      const targetClient = new TabClient(this, response);
-      this.registerClient(targetClient);
-      return [response, targetClient];
-    });
+    const response = await front.attach();
+    return [response, front];
   },
 
   attachWorker: function(workerTargetActor) {
     let workerClient = this._clients.get(workerTargetActor);
     if (workerClient !== undefined) {
       const response = {
         from: workerClient.actor,
         type: "attached",
--- a/devtools/shared/client/moz.build
+++ b/devtools/shared/client/moz.build
@@ -14,12 +14,11 @@ DevToolsModules(
     'environment-client.js',
     'event-source.js',
     'long-string-client.js',
     'object-client.js',
     'property-iterator-client.js',
     'root-client.js',
     'source-client.js',
     'symbol-iterator-client.js',
-    'tab-client.js',
     'thread-client.js',
     'worker-client.js',
 )
deleted file mode 100644
--- a/devtools/shared/client/tab-client.js
+++ /dev/null
@@ -1,163 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-"use strict";
-
-const promise = require("devtools/shared/deprecated-sync-thenables");
-
-const eventSource = require("devtools/shared/client/event-source");
-const {arg, DebuggerClient} = require("devtools/shared/client/debugger-client");
-loader.lazyRequireGetter(this, "ThreadClient", "devtools/shared/client/thread-client");
-
-/**
- * Creates a tab client for the remote debugging protocol server. This client is a front
- * to the target actor for a tab created in the server side, hiding the protocol details
- * in a traditional JavaScript API.
- *
- * @param client DebuggerClient
- *        The debugger client parent.
- * @param form object
- *        The protocol form for this tab.
- */
-function TabClient(client, form) {
-  this.client = client;
-  this._actor = form.from;
-  this._threadActor = form.threadActor;
-  this.javascriptEnabled = form.javascriptEnabled;
-  this.cacheDisabled = form.cacheDisabled;
-  this.thread = null;
-  this.request = this.client.request;
-  this.traits = form.traits || {};
-  this.events = ["workerListChanged"];
-}
-
-TabClient.prototype = {
-  get actor() {
-    return this._actor;
-  },
-  get _transport() {
-    return this.client._transport;
-  },
-
-  /**
-   * Attach to a thread actor.
-   *
-   * @param object options
-   *        Configuration options.
-   *        - useSourceMaps: whether to use source maps or not.
-   */
-  attachThread: function(options = {}) {
-    if (this.thread) {
-      return promise.resolve([{}, this.thread]);
-    }
-
-    const packet = {
-      to: this._threadActor,
-      type: "attach",
-      options,
-    };
-    return this.request(packet).then(response => {
-      this.thread = new ThreadClient(this, this._threadActor);
-      this.client.registerClient(this.thread);
-      return [response, this.thread];
-    });
-  },
-
-  /**
-   * Detach the client from the target actor.
-   */
-  detach: DebuggerClient.requester({
-    type: "detach"
-  }, {
-    before: function(packet) {
-      if (this.thread) {
-        this.thread.detach();
-      }
-      return packet;
-    },
-    after: function(response) {
-      this.client.unregisterClient(this);
-      return response;
-    },
-  }),
-
-  /**
-   * Bring the window to the front.
-   */
-  focus: DebuggerClient.requester({
-    type: "focus"
-  }, {}),
-
-  /**
-   * Ensure relevant pages have error reporting enabled.
-   */
-  ensureCSSErrorReportingEnabled: DebuggerClient.requester({
-    type: "ensureCSSErrorReportingEnabled",
-  }, {}),
-
-  /**
-   * Reload the page in this tab.
-   *
-   * @param [optional] object options
-   *        An object with a `force` property indicating whether or not
-   *        this reload should skip the cache
-   */
-  reload: function(options = { force: false }) {
-    return this._reload(options);
-  },
-  _reload: DebuggerClient.requester({
-    type: "reload",
-    options: arg(0)
-  }),
-
-  /**
-   * Navigate to another URL.
-   *
-   * @param string url
-   *        The URL to navigate to.
-   */
-  navigateTo: DebuggerClient.requester({
-    type: "navigateTo",
-    url: arg(0)
-  }),
-
-  /**
-   * Reconfigure the target actor.
-   *
-   * @param object options
-   *        A dictionary object of the new options to use in the target actor.
-   */
-  reconfigure: DebuggerClient.requester({
-    type: "reconfigure",
-    options: arg(0)
-  }),
-
-  listWorkers: DebuggerClient.requester({
-    type: "listWorkers"
-  }),
-
-  attachWorker: function(workerTargetActor) {
-    return this.client.attachWorker(workerTargetActor);
-  },
-
-  logInPage: DebuggerClient.requester({
-    type: "logInPage",
-    text: arg(0),
-    category: arg(1),
-    flags: arg(2),
-  }),
-
-  listFrames: DebuggerClient.requester({
-    type: "listFrames",
-  }),
-
-  switchToFrame: DebuggerClient.requester({
-    type: "switchToFrame",
-    windowId: arg(0),
-  }),
-};
-
-eventSource(TabClient.prototype);
-
-module.exports = TabClient;
--- a/devtools/shared/client/thread-client.js
+++ b/devtools/shared/client/thread-client.js
@@ -20,17 +20,17 @@ loader.lazyRequireGetter(this, "SourceCl
 
 const noop = () => {};
 
 /**
  * Creates a thread client for the remote debugging protocol server. This client
  * is a front to the thread actor created in the server side, hiding the
  * protocol details in a traditional JavaScript API.
  *
- * @param client DebuggerClient|TabClient
+ * @param client DebuggerClient, WorkerClient or BrowsingContextFront
  *        The parent of the thread (tab for target-scoped debuggers,
  *        DebuggerClient for chrome debuggers).
  * @param actor string
  *        The actor ID for this thread.
  */
 function ThreadClient(client, actor) {
   this._parent = client;
   this.client = client instanceof DebuggerClient ? client : client.client;
--- a/devtools/shared/fronts/moz.build
+++ b/devtools/shared/fronts/moz.build
@@ -1,16 +1,17 @@
 # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DIRS += [
     'addon',
+    'targets',
 ]
 
 DevToolsModules(
     'accessibility.js',
     'actor-registry.js',
     'animation.js',
     'canvas.js',
     'css-properties.js',
new file mode 100644
--- /dev/null
+++ b/devtools/shared/fronts/targets/browsing-context.js
@@ -0,0 +1,88 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+"use strict";
+
+const {browsingContextTargetSpec} = require("devtools/shared/specs/targets/browsing-context");
+const protocol = require("devtools/shared/protocol");
+const {custom} = protocol;
+
+loader.lazyRequireGetter(this, "ThreadClient", "devtools/shared/client/thread-client");
+
+const BrowsingContextFront = protocol.FrontClassWithSpec(browsingContextTargetSpec, {
+  initialize: function(client, form) {
+    protocol.Front.prototype.initialize.call(this, client, form);
+
+    this.thread = null;
+
+    // TODO: remove once ThreadClient becomes a front
+    this.client = client;
+  },
+
+  /**
+   * Attach to a thread actor.
+   *
+   * @param object options
+   *        Configuration options.
+   *        - useSourceMaps: whether to use source maps or not.
+   */
+  attachThread: function(options = {}) {
+    if (this.thread) {
+      return Promise.resolve([{}, this.thread]);
+    }
+
+    const packet = {
+      to: this._threadActor,
+      type: "attach",
+      options,
+    };
+    return this.client.request(packet).then(response => {
+      this.thread = new ThreadClient(this, this._threadActor);
+      this.client.registerClient(this.thread);
+      return [response, this.thread];
+    });
+  },
+
+  attach: custom(async function() {
+    const response = await this._attach();
+
+    this._threadActor = response.threadActor;
+    this.javascriptEnabled = response.javascriptEnabled;
+    this.cacheDisabled = response.cacheDisabled;
+    this.traits = response.traits || {};
+
+    return response;
+  }, {
+    impl: "_attach"
+  }),
+
+  detach: custom(async function() {
+    let response;
+    try {
+      response = await this._detach();
+    } catch (e) {
+      console.warn(
+        `Error while detaching the browsing context target front: ${e.message}`);
+    }
+
+    if (this.thread) {
+      try {
+        await this.thread.detach();
+      } catch (e) {
+        console.warn(`Error while detaching the thread front: ${e.message}`);
+      }
+    }
+
+    this.destroy();
+
+    return response;
+  }, {
+    impl: "_detach"
+  }),
+
+  attachWorker: function(workerTargetActor) {
+    return this.client.attachWorker(workerTargetActor);
+  },
+});
+
+exports.BrowsingContextFront = BrowsingContextFront;
new file mode 100644
--- /dev/null
+++ b/devtools/shared/fronts/targets/moz.build
@@ -0,0 +1,9 @@
+# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
+# vim: set filetype=python:
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+DevToolsModules(
+    'browsing-context.js',
+)
--- a/devtools/shared/specs/targets/browsing-context.js
+++ b/devtools/shared/specs/targets/browsing-context.js
@@ -122,20 +122,27 @@ const browsingContextTargetSpecPrototype
     },
     frameUpdate: {
       type: "frameUpdate",
       frames: Option(0, "nullable:array:browsingContextTarget.window"),
       selected: Option(0, "nullable:number"),
       destroyAll: Option(0, "nullable:boolean")
     },
     tabDetached: {
-      type: "tabDetached"
+      type: "tabDetached",
+      // This is to make browser_dbg_navigation.js to work as it expect to
+      // see a packet object when listening for tabDetached
+      from: Option(0, "string"),
     },
     workerListChanged: {
       type: "workerListChanged"
+    },
+    newSource: {
+      type: "newSource",
+      source: Option(0, "json")
     }
   }
 };
 
 const browsingContextTargetSpec = generateActorSpec(browsingContextTargetSpecPrototype);
 
 exports.browsingContextTargetSpecPrototype = browsingContextTargetSpecPrototype;
 exports.browsingContextTargetSpec = browsingContextTargetSpec;