Bug 1733523 - [devtools] Handle moving node selection when selected node's target gets destroyed. r=jdescottes.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Tue, 05 Oct 2021 09:58:19 +0000
changeset 594599 a26075e0ca2211c18798363eefd007c6650f9d89
parent 594598 04a3446f3ae37f371aa015ca86720352e57bd442
child 594600 1149ad68222b58e4cf1f0d9b2cf91fa7b0d70533
push id150882
push usernchevobbe@mozilla.com
push dateTue, 05 Oct 2021 10:01:18 +0000
treeherderautoland@a26075e0ca22 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1733523
milestone95.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 1733523 - [devtools] Handle moving node selection when selected node's target gets destroyed. r=jdescottes. Toolbox's Selection instance listens for `mutations` walkerFront event in order to detect if the selected node was removed from the DOM tree, and in such case, emits a `detached-front` event that can be consumed by the markup view. But, when an iframe gets removed and EFT (or Fission for a remote iframe) is enabled we don't get the `mutations` event. To fix this, we add a `onTargetDestroyed` method to `Selection`, that we call from the toolbox. This fixes the browser_inspector_delete-selected-node-* tests when EFT is enabled. Differential Revision: https://phabricator.services.mozilla.com/D127224
devtools/client/framework/selection.js
devtools/client/framework/toolbox.js
devtools/client/inspector/test/browser_inspector_delete-selected-node-02.js
devtools/client/inspector/test/browser_inspector_delete-selected-node-03.js
--- a/devtools/client/framework/selection.js
+++ b/devtools/client/framework/selection.js
@@ -105,23 +105,65 @@ Selection.prototype = {
     }
   },
 
   destroy: function() {
     this.setWalker();
     this._nodeFront = null;
   },
 
+  /**
+   * @param {WalkerFront|null} walker
+   */
   setWalker: function(walker = null) {
     if (this._walker) {
-      this._walker.off("mutations", this._onMutations);
+      this._removeWalkerFrontEventListeners(this._walker);
     }
+
     this._walker = walker;
     if (this._walker) {
-      this._walker.on("mutations", this._onMutations);
+      this._setWalkerFrontEventListeners(this._walker);
+    }
+  },
+
+  /**
+   * Set event listeners on the passed walker front
+   *
+   * @param {WalkerFront} walker
+   */
+  _setWalkerFrontEventListeners(walker) {
+    walker.on("mutations", this._onMutations);
+  },
+
+  /**
+   * Remove event listeners we previously set on walker front
+   *
+   * @param {WalkerFront} walker
+   */
+  _removeWalkerFrontEventListeners(walker) {
+    walker.off("mutations", this._onMutations);
+  },
+
+  /**
+   * Called when a target front is destroyed.
+   *
+   * @param {TargetFront} front
+   * @emits detached-front
+   */
+  onTargetDestroyed: function(targetFront) {
+    // if the current walker belongs to the target that is destroyed, emit a `detached-front`
+    // event so consumers can act accordingly (e.g. in the inspector, another node will be
+    // selected)
+    if (
+      this._walker &&
+      !targetFront.isTopLevel &&
+      this._walker.targetFront == targetFront
+    ) {
+      this._removeWalkerFrontEventListeners(this._walker);
+      this.emit("detached-front");
     }
   },
 
   /**
    * Update the currently selected node-front.
    *
    * @param {NodeFront} nodeFront
    *        The NodeFront being selected.
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -715,16 +715,18 @@ Toolbox.prototype = {
       await this.initPerformance();
     }
   },
 
   _onTargetDestroyed({ targetFront }) {
     if (targetFront.isTopLevel) {
       this.target.off("inspect-object", this._onInspectObject);
       this.target.off("frame-update", this._updateFrames);
+    } else if (this.selection) {
+      this.selection.onTargetDestroyed(targetFront);
     }
 
     if (this.hostType !== Toolbox.HostType.PAGE) {
       this.store.dispatch(unregisterTarget(targetFront));
     }
   },
 
   _onTargetThreadFrontResumeWrongOrder() {
--- a/devtools/client/inspector/test/browser_inspector_delete-selected-node-02.js
+++ b/devtools/client/inspector/test/browser_inspector_delete-selected-node-02.js
@@ -62,21 +62,24 @@ add_task(async function() {
         "and checking the parent node of the iframe is selected and " +
         "breadcrumbs are updated."
     );
 
     info("Selecting an element inside iframe.");
     await selectNodeInFrames(["#deleteIframe", "#deleteInIframe"], inspector);
 
     info("Deleting parent iframe node via javascript.");
-    const iframe = await getNodeFront("#deleteIframe", inspector);
-    await inspector.walker.removeNode(iframe);
+    const onInspectorUpdated = inspector.once("inspector-updated");
+
+    SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
+      content.document.querySelector("iframe#deleteIframe").remove();
+    });
 
     info("Waiting for inspector to update.");
-    await inspector.once("inspector-updated");
+    await onInspectorUpdated;
 
     info("Inspector updated, performing checks.");
     await assertNodeSelectedAndPanelsUpdated("body", "body");
   }
 
   async function testDeleteWithNonElementNode() {
     info(
       "Selecting a node, deleting it via context menu and checking that " +
--- a/devtools/client/inspector/test/browser_inspector_delete-selected-node-03.js
+++ b/devtools/client/inspector/test/browser_inspector_delete-selected-node-03.js
@@ -1,26 +1,24 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
-// Test to ensure inspector can handle destruction of selected node inside an
-// iframe.
+// Test to ensure inspector can handle destruction of selected node inside an iframe.
 
 const TEST_URL = URL_ROOT + "doc_inspector_delete-selected-node-01.html";
 
 add_task(async function() {
   const { inspector } = await openInspectorForURL(TEST_URL);
 
   info("Select a node inside the iframe");
   await selectNodeInFrames(["iframe", "span"], inspector);
 
   info("Removing iframe.");
-  const iframe = await getNodeFront("iframe", inspector);
-  await inspector.walker.removeNode(iframe);
-  await inspector.selection.once("detached-front");
+  const onInspectorUpdated = inspector.once("inspector-updated");
+  SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
+    content.document.querySelector("iframe").remove();
+  });
+  await onInspectorUpdated;
 
   const body = await getNodeFront("body", inspector);
-
   is(inspector.selection.nodeFront, body, "Selection is now the body node");
-
-  await inspector.once("inspector-updated");
 });