Bug 1499683 - Remove Highlightable Trait; r=ochameau
authoryulia <ystartsev@mozilla.com>
Thu, 18 Oct 2018 14:12:00 +0000
changeset 500404 42a7995a7d3655675925610c92fe13c9b86cd7ec
parent 500403 5e123c35d1e82866b42ee86dbe74bdf4c02956f5
child 500405 f95e8040037226253522aea53a8b24dee6b75098
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1499683
milestone64.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 1499683 - Remove Highlightable Trait; r=ochameau Differential Revision: https://phabricator.services.mozilla.com/D8987
devtools/client/framework/toolbox-highlighter-utils.js
devtools/client/framework/toolbox.js
devtools/server/actors/root.js
devtools/server/actors/targets/addon.js
devtools/server/actors/targets/content-process.js
--- a/devtools/client/framework/toolbox-highlighter-utils.js
+++ b/devtools/client/framework/toolbox-highlighter-utils.js
@@ -23,48 +23,35 @@ const flags = require("devtools/shared/f
  * This should be done once only by the toolbox itself and stored there so that
  * panels can get it from there. That's because the API returned has a stateful
  * scope that would be different for another instance returned by this function.
  *
  * @param {Toolbox} toolbox
  * @return {Object} the highlighterUtils public API
  */
 exports.getHighlighterUtils = function(toolbox) {
-  if (!toolbox || !toolbox.target) {
+  if (!toolbox) {
     throw new Error("Missing or invalid toolbox passed to getHighlighterUtils");
   }
 
   // Exported API properties will go here
   const exported = {};
 
-  // The current toolbox target
-  let target = toolbox.target;
-
   // Is the highlighter currently in pick mode
   let isPicking = false;
 
   // Is the box model already displayed, used to prevent dispatching
   // unnecessary requests, especially during toolbox shutdown
   let isNodeFrontHighlighted = false;
 
   /**
    * Release this utils, nullifying the references to the toolbox
    */
   exported.release = function() {
-    toolbox = target = null;
-  };
-
-  /**
-   * Does the target have the highlighter actor.
-   * The devtools must be backwards compatible with at least B2G 1.3 (28),
-   * which doesn't have the highlighter actor. This can be removed as soon as
-   * the minimal supported version becomes 1.4 (29)
-   */
-  const isRemoteHighlightable = exported.isRemoteHighlightable = function() {
-    return target.client.traits.highlightable;
+    toolbox = null;
   };
 
   /**
    * Make a function that initializes the inspector before it runs.
    * Since the init of the inspector is asynchronous, the return value will be
    * produced by Task.async and the argument should be a generator
    * @param {Function*} generator A generator function
    * @return {Function} A function
@@ -110,59 +97,44 @@ exports.getHighlighterUtils = function(t
         return;
       }
       isPicking = true;
 
       toolbox.pickerButton.isChecked = true;
       await toolbox.selectTool("inspector", "inspect_dom");
       toolbox.on("select", cancelPicker);
 
-      if (isRemoteHighlightable()) {
-        toolbox.walker.on("picker-node-hovered", onPickerNodeHovered);
-        toolbox.walker.on("picker-node-picked", onPickerNodePicked);
-        toolbox.walker.on("picker-node-previewed", onPickerNodePreviewed);
-        toolbox.walker.on("picker-node-canceled", onPickerNodeCanceled);
+      toolbox.walker.on("picker-node-hovered", onPickerNodeHovered);
+      toolbox.walker.on("picker-node-picked", onPickerNodePicked);
+      toolbox.walker.on("picker-node-previewed", onPickerNodePreviewed);
+      toolbox.walker.on("picker-node-canceled", onPickerNodeCanceled);
 
-        await toolbox.highlighter.pick(doFocus);
-        toolbox.emit("picker-started");
-      } else {
-        // If the target doesn't have the highlighter actor, we can use the
-        // walker's pick method instead, knowing that it only responds when a node
-        // is picked (instead of emitting events)
-        toolbox.emit("picker-started");
-        const node = await toolbox.walker.pick();
-        onPickerNodePicked({node: node});
-      }
+      await toolbox.highlighter.pick(doFocus);
+      toolbox.emit("picker-started");
     });
 
   /**
    * Stop the element picker. Note that the picker is automatically stopped when
    * an element is picked
    * @return A promise that resolves when the picker has stopped or immediately
    * if it is already stopped
    */
   const stopPicker = exported.stopPicker = requireInspector(async function() {
     if (!isPicking) {
       return;
     }
     isPicking = false;
 
     toolbox.pickerButton.isChecked = false;
 
-    if (isRemoteHighlightable()) {
-      await toolbox.highlighter.cancelPick();
-      toolbox.walker.off("picker-node-hovered", onPickerNodeHovered);
-      toolbox.walker.off("picker-node-picked", onPickerNodePicked);
-      toolbox.walker.off("picker-node-previewed", onPickerNodePreviewed);
-      toolbox.walker.off("picker-node-canceled", onPickerNodeCanceled);
-    } else {
-      // If the target doesn't have the highlighter actor, use the walker's
-      // cancelPick method instead
-      await toolbox.walker.cancelPick();
-    }
+    await toolbox.highlighter.cancelPick();
+    toolbox.walker.off("picker-node-hovered", onPickerNodeHovered);
+    toolbox.walker.off("picker-node-picked", onPickerNodePicked);
+    toolbox.walker.off("picker-node-previewed", onPickerNodePreviewed);
+    toolbox.walker.off("picker-node-canceled", onPickerNodeCanceled);
 
     toolbox.off("select", cancelPicker);
     toolbox.emit("picker-stopped");
   });
 
   /**
    * Stop the picker, but also emit an event that the picker was canceled.
    */
@@ -216,23 +188,17 @@ exports.getHighlighterUtils = function(t
    */
   const highlightNodeFront = exported.highlightNodeFront = requireInspector(
   async function(nodeFront, options = {}) {
     if (!nodeFront) {
       return;
     }
 
     isNodeFrontHighlighted = true;
-    if (isRemoteHighlightable()) {
-      await toolbox.highlighter.showBoxModel(nodeFront, options);
-    } else {
-      // If the target doesn't have the highlighter actor, revert to the
-      // walker's highlight method, which draws a simple outline
-      await toolbox.walker.highlight(nodeFront);
-    }
+    await toolbox.highlighter.showBoxModel(nodeFront, options);
 
     toolbox.emit("node-highlight", nodeFront);
   });
 
   /**
    * This is a convenience method in case you don't have a nodeFront but a
    * valueGrip. This is often the case with VariablesView properties.
    * This method will simply translate the grip into a nodeFront and call
@@ -266,20 +232,17 @@ exports.getHighlighterUtils = function(t
    * in the markup view doesn't hide/show the highlighter to ease testing. The
    * highlighter stays visible at all times, except when the mouse leaves the
    * markup view, which is when this param is passed to true
    * @return a promise that resolves when the highlighter is hidden
    */
   exported.unhighlight = async function(forceHide = false) {
     forceHide = forceHide || !flags.testing;
 
-    // Note that if isRemoteHighlightable is true, there's no need to hide the
-    // highlighter as the walker uses setTimeout to hide it after some time
-    if (isNodeFrontHighlighted && forceHide && toolbox.highlighter &&
-        isRemoteHighlightable()) {
+    if (isNodeFrontHighlighted && forceHide && toolbox.highlighter) {
       isNodeFrontHighlighted = false;
       await toolbox.highlighter.hideBoxModel();
     }
 
     // unhighlight is called when destroying the toolbox, which means that by
     // now, the toolbox reference might have been nullified already.
     if (toolbox) {
       toolbox.emit("node-unhighlight");
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -2675,23 +2675,21 @@ Toolbox.prototype = {
         // TODO: replace with getFront once inspector is separated from the toolbox
         this._inspector = this.target.getInspector();
         const pref = "devtools.inspector.showAllAnonymousContent";
         const showAllAnonymousContent = Services.prefs.getBoolPref(pref);
         this._walker = await this._inspector.getWalker({ showAllAnonymousContent });
         this._selection = new Selection(this._walker);
         this._selection.on("new-node-front", this._onNewSelectedNodeFront);
 
-        if (this.highlighterUtils.isRemoteHighlightable()) {
-          this.walker.on("highlighter-ready", this._highlighterReady);
-          this.walker.on("highlighter-hide", this._highlighterHidden);
-
-          const autohide = !flags.testing;
-          this._highlighter = await this._inspector.getHighlighter(autohide);
-        }
+        this.walker.on("highlighter-ready", this._highlighterReady);
+        this.walker.on("highlighter-hide", this._highlighterHidden);
+
+        const autohide = !flags.testing;
+        this._highlighter = await this._inspector.getHighlighter(autohide);
         if (!("_supportsFrameHighlight" in this)) {
           // Only works with FF58+ targets
           this._supportsFrameHighlight =
             await this.target.actorHasMethod("domwalker", "getNodeActorFromWindowID");
         }
       }.bind(this))();
     }
     return this._initInspector;
--- a/devtools/server/actors/root.js
+++ b/devtools/server/actors/root.js
@@ -111,19 +111,16 @@ function RootActor(connection, parameter
 }
 
 RootActor.prototype = {
   constructor: RootActor,
   applicationType: "browser",
 
   traits: {
     sources: true,
-    // Whether the server-side highlighter actor exists and can be used to
-    // remotely highlight nodes (see server/actors/highlighters.js)
-    highlightable: true,
     networkMonitor: true,
     // Whether the storage inspector actor to inspect cookies, etc.
     storageInspector: true,
     // Whether storage inspector is read only
     storageInspectorReadOnly: true,
     // Whether conditional breakpoints are supported
     conditionalBreakpoints: true,
     // Whether the server supports full source actors (breakpoints on
--- a/devtools/server/actors/targets/addon.js
+++ b/devtools/server/actors/targets/addon.js
@@ -91,17 +91,16 @@ AddonTargetActor.prototype = {
       debuggable: this._addon.isDebuggable,
       temporarilyInstalled: this._addon.temporarilyInstalled,
       type: this._addon.type,
       isWebExtension: this._addon.isWebExtension,
       isAPIExtension: this._addon.isAPIExtension,
       consoleActor: this._consoleActor.actorID,
 
       traits: {
-        highlightable: false,
         networkMonitor: false,
       },
     };
   },
 
   destroy() {
     this.conn.removeActorPool(this._contextPool);
     this._contextPool = null;
--- a/devtools/server/actors/targets/content-process.js
+++ b/devtools/server/actors/targets/content-process.js
@@ -114,17 +114,16 @@ ContentProcessTargetActor.prototype = {
       name: "Content process",
 
       consoleActor: this._consoleActor.actorID,
       chromeDebugger: this.threadActor.actorID,
       memoryActor: this.memoryActor.actorID,
       promisesActor: this._promisesActor.actorID,
 
       traits: {
-        highlightable: false,
         networkMonitor: false,
       },
     };
   },
 
   onListWorkers: function() {
     if (!this._workerList) {
       this._workerList = new WorkerTargetActorList(this.conn, {});