Bug 1572460 - Refactor `selection` out of the `InspectorFront`. r=yulia
authorGabriel Luong <gabriel.luong@gmail.com>
Wed, 14 Aug 2019 05:07:38 +0000
changeset 487845 f7dd2503e50989bd39121a7a2be4fd48d83ee12c
parent 487844 59052299b371a05552f03523f19de48a4c40c325
child 487846 214fac6eb1c05177c7ef8eb29b87cceb09eb4110
push id36431
push useraciure@mozilla.com
push dateWed, 14 Aug 2019 09:42:16 +0000
treeherdermozilla-central@a6ba020c9f7c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyulia
bugs1572460
milestone70.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 1572460 - Refactor `selection` out of the `InspectorFront`. r=yulia Differential Revision: https://phabricator.services.mozilla.com/D41314
devtools/client/framework/selection.js
devtools/client/framework/toolbox.js
devtools/client/inspector/inspector.js
devtools/client/shared/widgets/ShapesInContextEditor.js
devtools/shared/fronts/inspector.js
devtools/shared/fronts/inspector/node-picker.js
devtools/shared/fronts/node.js
--- a/devtools/client/framework/selection.js
+++ b/devtools/client/framework/selection.js
@@ -1,26 +1,35 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* 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 nodeConstants = require("devtools/shared/dom-node-constants");
-var EventEmitter = require("devtools/shared/event-emitter");
+const EventEmitter = require("devtools/shared/event-emitter");
+
+loader.lazyRequireGetter(
+  this,
+  "nodeConstants",
+  "devtools/shared/dom-node-constants"
+);
 
 /**
+ * Selection is a singleton belonging to the Toolbox that manages the current selected
+ * NodeFront. In addition, it provides some helpers about the context of the selected
+ * node.
+ *
  * API
  *
- *   new Selection(walker=null)
+ *   new Selection()
  *   destroy()
- *   node (readonly)
- *   setNode(node, origin="unknown")
+ *   nodeFront (readonly)
+ *   setNodeFront(node, origin="unknown")
  *
  * Helpers:
  *
  *   window
  *   document
  *   isRoot()
  *   isNode()
  *   isHTMLNode()
@@ -43,39 +52,31 @@ var EventEmitter = require("devtools/sha
  * Events:
  *   "new-node-front" when the inner node changed
  *   "attribute-changed" when an attribute is changed
  *   "detached-front" when the node (or one of its parents) is removed from
  *   the document
  *   "reparented" when the node (or one of its parents) is moved under
  *   a different node
  */
-
-/**
- * A Selection object. Hold a reference to a node.
- * Includes some helpers, fire some helpful events.
- */
-function Selection(walker) {
+function Selection() {
   EventEmitter.decorate(this);
 
+  // The WalkerFront is dynamic and is always set to the selected NodeFront's WalkerFront.
+  this._walker = null;
   // A single node front can be represented twice on the client when the node is a slotted
   // element. It will be displayed once as a direct child of the host element, and once as
   // a child of a slot in the "shadow DOM". The latter is called the slotted version.
   this._isSlotted = false;
 
   this._onMutations = this._onMutations.bind(this);
   this.setNodeFront = this.setNodeFront.bind(this);
-  this.setWalker(walker);
 }
 
-exports.Selection = Selection;
-
 Selection.prototype = {
-  _walker: null,
-
   _onMutations: function(mutations) {
     let attributeChange = false;
     let pseudoChange = false;
     let detached = false;
     let parentNode = null;
 
     for (const m of mutations) {
       if (!attributeChange && m.type == "attributes") {
@@ -102,20 +103,20 @@ Selection.prototype = {
       this.emit("pseudoclass");
     }
     if (detached) {
       this.emit("detached-front", parentNode);
     }
   },
 
   destroy: function() {
-    this.setWalker(null);
+    this.setWalker();
   },
 
-  setWalker: function(walker) {
+  setWalker: function(walker = null) {
     if (this._walker) {
       this._walker.off("mutations", this._onMutations);
     }
     this._walker = walker;
     if (this._walker) {
       this._walker.on("mutations", this._onMutations);
     }
   },
@@ -148,21 +149,23 @@ Selection.prototype = {
       // (e.g. once when the webpage start to navigate away from the current webpage,
       // and then again while the new page is being loaded).
       return;
     }
 
     this._isSlotted = isSlotted;
     this._nodeFront = nodeFront;
 
-    this.emit("new-node-front", nodeFront, this.reason);
-  },
+    if (nodeFront) {
+      this.setWalker(nodeFront.walkerFront);
+    } else {
+      this.setWalker();
+    }
 
-  get documentFront() {
-    return this._walker.document(this._nodeFront);
+    this.emit("new-node-front", nodeFront, this.reason);
   },
 
   get nodeFront() {
     return this._nodeFront;
   },
 
   isRoot: function() {
     return (
@@ -305,8 +308,10 @@ Selection.prototype = {
   isSlotted: function() {
     return this._isSlotted;
   },
 
   isShadowRootNode: function() {
     return this.isNode() && this.nodeFront.isShadowRoot;
   },
 };
+
+module.exports = Selection;
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -19,31 +19,18 @@ const HTML_NS = "http://www.w3.org/1999/
 
 var { Ci, Cc } = require("chrome");
 var promise = require("promise");
 const { debounce } = require("devtools/shared/debounce");
 var Services = require("Services");
 var ChromeUtils = require("ChromeUtils");
 var { gDevTools } = require("devtools/client/framework/devtools");
 var EventEmitter = require("devtools/shared/event-emitter");
+const Selection = require("devtools/client/framework/selection");
 var Telemetry = require("devtools/client/shared/telemetry");
-
-loader.lazyRequireGetter(
-  this,
-  "createToolboxStore",
-  "devtools/client/framework/store",
-  true
-);
-loader.lazyRequireGetter(
-  this,
-  "registerWalkerListeners",
-  "devtools/client/framework/actions/index",
-  true
-);
-
 const { getUnicodeUrl } = require("devtools/client/shared/unicode-url");
 var {
   DOMHelpers,
 } = require("resource://devtools/client/shared/DOMHelpers.jsm");
 const { KeyCodes } = require("devtools/client/shared/keycodes");
 var Startup = Cc["@mozilla.org/devtools/startup-clh;1"].getService(
   Ci.nsISupports
 ).wrappedJSObject;
@@ -54,16 +41,28 @@ const { BrowserLoader } = ChromeUtils.im
 
 const { LocalizationHelper } = require("devtools/shared/l10n");
 const L10N = new LocalizationHelper(
   "devtools/client/locales/toolbox.properties"
 );
 
 loader.lazyRequireGetter(
   this,
+  "createToolboxStore",
+  "devtools/client/framework/store",
+  true
+);
+loader.lazyRequireGetter(
+  this,
+  "registerWalkerListeners",
+  "devtools/client/framework/actions/index",
+  true
+);
+loader.lazyRequireGetter(
+  this,
   "AppConstants",
   "resource://gre/modules/AppConstants.jsm",
   true
 );
 loader.lazyRequireGetter(this, "flags", "devtools/shared/flags");
 loader.lazyRequireGetter(
   this,
   "KeyShortcuts",
@@ -191,16 +190,17 @@ function Toolbox(
   hostType,
   contentWindow,
   frameId,
   msSinceProcessStart
 ) {
   this._target = target;
   this._win = contentWindow;
   this.frameId = frameId;
+  this.selection = new Selection();
   this.telemetry = new Telemetry();
 
   // The session ID is used to determine which telemetry events belong to which
   // toolbox session. Because we use Amplitude to analyse the telemetry data we
   // must use the time since the system wide epoch as the session ID.
   this.sessionId = msSinceProcessStart;
 
   // Map of the available DevTools WebExtensions:
@@ -250,16 +250,18 @@ function Toolbox(
   this._onToolbarFocus = this._onToolbarFocus.bind(this);
   this._onToolbarArrowKeypress = this._onToolbarArrowKeypress.bind(this);
   this._onPickerClick = this._onPickerClick.bind(this);
   this._onPickerKeypress = this._onPickerKeypress.bind(this);
   this._onPickerStarting = this._onPickerStarting.bind(this);
   this._onPickerStarted = this._onPickerStarted.bind(this);
   this._onPickerStopped = this._onPickerStopped.bind(this);
   this._onPickerCanceled = this._onPickerCanceled.bind(this);
+  this._onPickerPicked = this._onPickerPicked.bind(this);
+  this._onPickerPreviewed = this._onPickerPreviewed.bind(this);
   this._onInspectObject = this._onInspectObject.bind(this);
   this._onNewSelectedNodeFront = this._onNewSelectedNodeFront.bind(this);
   this._onToolSelected = this._onToolSelected.bind(this);
   this._onTargetClosed = this._onTargetClosed.bind(this);
   this._onContextMenu = this._onContextMenu.bind(this);
   this.updateToolboxButtonsVisibility = this.updateToolboxButtonsVisibility.bind(
     this
   );
@@ -294,16 +296,18 @@ function Toolbox(
   this._target.on("will-navigate", this._onWillNavigate);
   this._target.on("navigate", this._refreshHostTitle);
   this._target.on("frame-update", this._updateFrames);
   this._target.on("inspect-object", this._onInspectObject);
 
   this.on("host-changed", this._refreshHostTitle);
   this.on("select", this._onToolSelected);
 
+  this.selection.on("new-node-front", this._onNewSelectedNodeFront);
+
   gDevTools.on("tool-registered", this._toolRegistered);
   gDevTools.on("tool-unregistered", this._toolUnregistered);
 
   /**
    * Get text direction for the current locale direction.
    *
    * `getComputedStyle` forces a synchronous reflow, so use a lazy getter in order to
    * call it only once.
@@ -523,24 +527,16 @@ Toolbox.prototype = {
    * Get the toolbox's walker front. Note that it may not always have been
    * initialized first. Use `initInspector()` if needed.
    */
   get walker() {
     return this._walker;
   },
 
   /**
-   * Get the toolbox's node selection. Note that it may not always have been
-   * initialized first. Use `initInspector()` if needed.
-   */
-  get selection() {
-    return this._selection;
-  },
-
-  /**
    * Get the toggled state of the split console
    */
   get splitConsole() {
     return this._splitConsole;
   },
 
   /**
    * Get the focused state of the split console
@@ -1807,16 +1803,34 @@ Toolbox.prototype = {
   _onPickerStopped: function() {
     this.tellRDMAboutPickerState(false);
     this.off("select", this.inspectorFront.nodePicker.stop);
     this.doc.removeEventListener("keypress", this._onPickerKeypress, true);
     this.pickerButton.isChecked = false;
   },
 
   /**
+   * When the picker is canceled, make sure the toolbox
+   * gets the focus.
+   */
+  _onPickerCanceled: function() {
+    if (this.hostType !== Toolbox.HostType.WINDOW) {
+      this.win.focus();
+    }
+  },
+
+  _onPickerPicked: function(nodeFront) {
+    this.selection.setNodeFront(nodeFront, { reason: "picker-node-picked" });
+  },
+
+  _onPickerPreviewed: function(nodeFront) {
+    this.selection.setNodeFront(nodeFront, { reason: "picker-node-previewed" });
+  },
+
+  /**
    * RDM sometimes simulates touch events. For this to work correctly at all times, it
    * needs to know when the picker is active or not.
    * This method communicates with the RDM Manager if it exists.
    *
    * @param {Boolean} state
    */
   tellRDMAboutPickerState: async function(state) {
     const { tab } = this.target;
@@ -1828,26 +1842,16 @@ Toolbox.prototype = {
       return;
     }
 
     const ui = ResponsiveUIManager.getResponsiveUIForTab(tab);
     await ui.emulationFront.setElementPickerState(state);
   },
 
   /**
-   * When the picker is canceled, make sure the toolbox
-   * gets the focus.
-   */
-  _onPickerCanceled: function() {
-    if (this.hostType !== Toolbox.HostType.WINDOW) {
-      this.win.focus();
-    }
-  },
-
-  /**
    * The element picker button enables the ability to select a DOM node by clicking
    * it on the page.
    */
   _buildPickerButton() {
     this.pickerButton = this._createButtonState({
       id: "command-button-pick",
       description: this._getPickerTooltip(),
       onClick: this._onPickerClick,
@@ -3314,17 +3318,16 @@ Toolbox.prototype = {
       this._initInspector = async function() {
         // Temporary fix for bug #1493131 - inspector has a different life cycle
         // than most other fronts because it is closely related to the toolbox.
         // TODO: replace with getFront once inspector is separated from the toolbox
         // TODO: remove these bindings
         this._inspector = await this.target.getFront("inspector");
         this._walker = this.inspectorFront.walker;
         this._highlighter = this.inspectorFront.highlighter;
-        this._selection = this.inspectorFront.selection;
 
         this.inspectorFront.nodePicker.on(
           "picker-starting",
           this._onPickerStarting
         );
         this.inspectorFront.nodePicker.on(
           "picker-started",
           this._onPickerStarted
@@ -3332,17 +3335,24 @@ Toolbox.prototype = {
         this.inspectorFront.nodePicker.on(
           "picker-stopped",
           this._onPickerStopped
         );
         this.inspectorFront.nodePicker.on(
           "picker-node-canceled",
           this._onPickerCanceled
         );
-        this._selection.on("new-node-front", this._onNewSelectedNodeFront);
+        this.inspectorFront.nodePicker.on(
+          "picker-node-picked",
+          this._onPickerPicked
+        );
+        this.inspectorFront.nodePicker.on(
+          "picker-node-previewed",
+          this._onPickerPreviewed
+        );
         registerWalkerListeners(this);
       }.bind(this)();
     }
     return this._initInspector;
   },
 
   /**
    * An helper function that returns an object contain a highlighter and unhighlighter
@@ -3451,17 +3461,16 @@ Toolbox.prototype = {
     }
 
     // Temporary fix for bug #1493131 - inspector has a different life cycle
     // than most other fronts because it is closely related to the toolbox.
     this._inspector.destroy();
 
     this._inspector = null;
     this._highlighter = null;
-    this._selection = null;
     this._walker = null;
   },
 
   /**
    * Get the toolbox's notification component
    *
    * @return The notification box component.
    */
@@ -3619,17 +3628,20 @@ Toolbox.prototype = {
     // then destroying the host, successfully or not) before destroying the
     // target.
     const onceDestroyed = new Promise(resolve => {
       resolve(
         settleAll(outstanding)
           .catch(console.error)
           .then(async () => {
             // Destroying the walker and inspector fronts
-            await this.destroyInspector();
+            this.destroyInspector();
+
+            this.selection.destroy();
+            this.selection = null;
 
             if (this._netMonitorAPI) {
               this._netMonitorAPI.destroy();
               this._netMonitorAPI = null;
             }
 
             this._removeWindowListeners();
             this._removeChromeEventHandlerEvents();
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -224,17 +224,16 @@ Inspector.prototype = {
     this._markupBox = this.panelDoc.getElementById("markup-box");
 
     return this._deferredOpen();
   },
 
   async initInspectorFront() {
     this.inspectorFront = await this.target.getFront("inspector");
     this.highlighter = this.inspectorFront.highlighter;
-    this.selection = this.inspectorFront.selection;
     this.walker = this.inspectorFront.walker;
   },
 
   get toolbox() {
     return this._toolbox;
   },
 
   get highlighters() {
@@ -292,16 +291,20 @@ Inspector.prototype = {
         this.searchBox,
         this.searchClearButton
       );
     }
 
     return this._search;
   },
 
+  get selection() {
+    return this.toolbox.selection;
+  },
+
   get cssProperties() {
     return this._cssProperties.cssProperties;
   },
 
   /**
    * Handle promise rejections for various asynchronous actions, and only log errors if
    * the inspector panel still exists.
    * This is useful to silence useless errors that happen when the inspector is closed
--- a/devtools/client/shared/widgets/ShapesInContextEditor.js
+++ b/devtools/client/shared/widgets/ShapesInContextEditor.js
@@ -149,16 +149,21 @@ class ShapesInContextEditor {
    */
   async hide() {
     try {
       await this.highlighter.hide();
     } catch (err) {
       // silent error
     }
 
+    // Stop if the panel has been destroyed during the call to hide.
+    if (this.destroyed) {
+      return;
+    }
+
     if (this.swatch) {
       this.swatch.classList.remove("active");
     }
     this.swatch = null;
     this.rule = null;
     this.textPropIndex = -1;
     this.textPropName = null;
 
@@ -329,12 +334,14 @@ class ShapesInContextEditor {
 
     this.textProperty.setValue(value);
   }
 
   destroy() {
     this.highlighter.off("highlighter-event", this.onHighlighterEvent);
     this.ruleView.off("ruleview-changed", this.onRuleViewChanged);
     this.highligherEventHandlers = {};
+
+    this.destroyed = true;
   }
 }
 
 module.exports = ShapesInContextEditor;
--- a/devtools/shared/fronts/inspector.js
+++ b/devtools/shared/fronts/inspector.js
@@ -18,22 +18,16 @@ const {
   walkerSpec,
 } = require("devtools/shared/specs/inspector");
 
 loader.lazyRequireGetter(
   this,
   "nodeConstants",
   "devtools/shared/dom-node-constants"
 );
-loader.lazyRequireGetter(
-  this,
-  "Selection",
-  "devtools/client/framework/selection",
-  true
-);
 loader.lazyRequireGetter(this, "flags", "devtools/shared/flags");
 
 const TELEMETRY_EYEDROPPER_OPENED = "DEVTOOLS_EYEDROPPER_OPENED_COUNT";
 const TELEMETRY_EYEDROPPER_OPENED_MENU =
   "DEVTOOLS_MENU_EYEDROPPER_OPENED_COUNT";
 const SHOW_ALL_ANONYMOUS_CONTENT_PREF =
   "devtools.inspector.showAllAnonymousContent";
 const SHOW_UA_SHADOW_ROOTS_PREF = "devtools.inspector.showUserAgentShadowRoots";
@@ -491,23 +485,17 @@ class InspectorFront extends FrontClassW
 
     // Attribute name from which to retrieve the actorID out of the target actor's form
     this.formAttributeName = "inspectorActor";
   }
 
   // async initialization
   async initialize() {
     await Promise.all([this._getWalker(), this._getHighlighter()]);
-
-    this.selection = new Selection(this.walker);
-    this.nodePicker = new NodePicker(
-      this.highlighter,
-      this.walker,
-      this.selection
-    );
+    this.nodePicker = new NodePicker(this.highlighter, this.walker);
   }
 
   async _getWalker() {
     const showAllAnonymousContent = Services.prefs.getBoolPref(
       SHOW_ALL_ANONYMOUS_CONTENT_PREF
     );
     const showUserAgentShadowRoots = Services.prefs.getBoolPref(
       SHOW_UA_SHADOW_ROOTS_PREF
@@ -523,19 +511,16 @@ class InspectorFront extends FrontClassW
     this.highlighter = await this.getHighlighter(autohide);
   }
 
   hasHighlighter(type) {
     return this._highlighters.has(type);
   }
 
   destroy() {
-    // Selection isn't a Front and so isn't managed by InspectorFront
-    // and has to be destroyed manually
-    this.selection.destroy();
     // Highlighter fronts are managed by InspectorFront and so will be
     // automatically destroyed. But we have to clear the `_highlighters`
     // Map as well as explicitly call `finalize` request on all of them.
     this.destroyHighlighters();
     super.destroy();
   }
 
   destroyHighlighters() {
--- a/devtools/shared/fronts/inspector/node-picker.js
+++ b/devtools/shared/fronts/inspector/node-picker.js
@@ -14,25 +14,23 @@ loader.lazyRequireGetter(this, "EventEmi
 /**
  * Get the NodePicker instance for an inspector front.
  * The NodePicker wraps the highlighter so that it can interact with the
  * walkerFront and selection api. The nodeFront is stateless, with the
  * HighlighterFront managing it's own state.
  *
  * @param {highlighter} highlighterFront
  * @param {walker} walkerFront
- * @param {selection} selection api
  * @return {Object} the NodePicker public API
  */
 class NodePicker extends EventEmitter {
-  constructor(highlighter, walker, selection) {
+  constructor(highlighter, walker) {
     super();
     this.highlighter = highlighter;
     this.walker = walker;
-    this.selection = selection;
 
     this.cancel = this.cancel.bind(this);
     this.start = this.start.bind(this);
     this.stop = this.stop.bind(this);
     this.togglePicker = this.togglePicker.bind(this);
 
     this._onHovered = this._onHovered.bind(this);
     this._onPicked = this._onPicked.bind(this);
@@ -113,27 +111,27 @@ class NodePicker extends EventEmitter {
     this.emit("picker-node-hovered", data.node);
   }
 
   /**
    * When a node has been picked while the highlighter is in picker mode
    * @param {Object} data Information about the picked node
    */
   _onPicked(data) {
-    this.selection.setNodeFront(data.node, { reason: "picker-node-picked" });
+    this.emit("picker-node-picked", data.node);
     return this.stop();
   }
 
   /**
    * When a node has been shift-clicked (previewed) while the highlighter is in
    * picker mode
    * @param {Object} data Information about the picked node
    */
   _onPreviewed(data) {
-    this.selection.setNodeFront(data.node, { reason: "picker-node-previewed" });
+    this.emit("picker-node-previewed", data.node);
   }
 
   /**
    * When the picker is canceled, stop the picker, and make sure the toolbox
    * gets the focus.
    */
   _onCanceled() {
     return this.cancel();
--- a/devtools/shared/fronts/node.js
+++ b/devtools/shared/fronts/node.js
@@ -394,16 +394,20 @@ class NodeFront extends FrontClassWithSp
       if (!parent.isDisplayed) {
         return false;
       }
       parent = parent.parentNode();
     }
     return true;
   }
 
+  get walkerFront() {
+    return this.parentFront;
+  }
+
   getNodeValue() {
     // backward-compatibility: if nodevalue is null and shortValue is defined, the actual
     // value of the node needs to be fetched on the server.
     if (this._form.nodeValue === null && this._form.shortValue) {
       return super.getNodeValue();
     }
 
     const str = this._form.nodeValue || "";