Bug 1572460 - Refactor selection out of the InspectorFront. r=yulia

Differential Revision: https://phabricator.services.mozilla.com/D41314

--HG--
extra : moz-landing-system : lando

devtools/client/framework/selection.js

@@ -6,16 +6,25 @@
 
 "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:
  *
@@ -48,14 +57,11 @@ var EventEmitter = require("devtools/shared/event-emitter");
  *   "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.
@@ -63,14 +69,9 @@ function Selection(walker) {
 
   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;
@@ -107,10 +108,10 @@ Selection.prototype = {
   },
 
   destroy: function() {
-    this.setWalker(null);
+    this.setWalker();
   },
 
-  setWalker: function(walker) {
+  setWalker: function(walker = null) {
     if (this._walker) {
       this._walker.off("mutations", this._onMutations);
     }
@@ -153,11 +154,13 @@ Selection.prototype = {
     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() {
@@ -310,3 +313,5 @@ Selection.prototype = {
     return this.isNode() && this.nodeFront.isShadowRoot;
   },
 };
+
+module.exports = Selection;

devtools/client/framework/toolbox.js

@@ -24,21 +24,8 @@ 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,
@@ -57,6 +44,18 @@ 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",
@@ -196,6 +195,7 @@ function Toolbox(
   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
@@ -255,6 +255,8 @@ function Toolbox(
   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);
@@ -299,6 +301,8 @@ function Toolbox(
   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);
 
@@ -527,14 +531,6 @@ Toolbox.prototype = {
     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
    */
@@ -1811,6 +1807,24 @@ Toolbox.prototype = {
     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.
@@ -1832,16 +1846,6 @@ Toolbox.prototype = {
     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.
@@ -3319,7 +3323,6 @@ Toolbox.prototype = {
         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",
@@ -3337,7 +3340,14 @@ Toolbox.prototype = {
           "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)();
     }
@@ -3456,7 +3466,6 @@ Toolbox.prototype = {
 
     this._inspector = null;
     this._highlighter = null;
-    this._selection = null;
     this._walker = null;
   },
 
@@ -3624,7 +3633,10 @@ Toolbox.prototype = {
           .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();

devtools/client/inspector/inspector.js

@@ -229,7 +229,6 @@ Inspector.prototype = {
   async initInspectorFront() {
     this.inspectorFront = await this.target.getFront("inspector");
     this.highlighter = this.inspectorFront.highlighter;
-    this.selection = this.inspectorFront.selection;
     this.walker = this.inspectorFront.walker;
   },
 
@@ -297,6 +296,10 @@ Inspector.prototype = {
     return this._search;
   },
 
+  get selection() {
+    return this.toolbox.selection;
+  },
+
   get cssProperties() {
     return this._cssProperties.cssProperties;
   },

devtools/client/shared/widgets/ShapesInContextEditor.js

@@ -154,6 +154,11 @@ class ShapesInContextEditor {
       // 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");
     }
@@ -334,6 +339,8 @@ class ShapesInContextEditor {
     this.highlighter.off("highlighter-event", this.onHighlighterEvent);
     this.ruleView.off("ruleview-changed", this.onRuleViewChanged);
     this.highligherEventHandlers = {};
+
+    this.destroyed = true;
   }
 }
 

devtools/shared/fronts/inspector.js

@@ -23,12 +23,6 @@ loader.lazyRequireGetter(
   "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";
@@ -496,13 +490,7 @@ class InspectorFront extends FrontClassWithSpec(inspectorSpec) {
   // 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() {
@@ -528,9 +516,6 @@ class InspectorFront extends FrontClassWithSpec(inspectorSpec) {
   }
 
   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.

devtools/shared/fronts/inspector/node-picker.js

@@ -19,15 +19,13 @@ loader.lazyRequireGetter(this, "EventEmitter", "devtools/shared/event-emitter");
  *
  * @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);
@@ -118,7 +116,7 @@ class NodePicker extends EventEmitter {
    * @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();
   }
 
@@ -128,7 +126,7 @@ class NodePicker extends EventEmitter {
    * @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);
   }
 
   /**

devtools/shared/fronts/node.js

@@ -399,6 +399,10 @@ class NodeFront extends FrontClassWithSpec(nodeSpec) {
     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.