Bug 1497905 - Avoid destroying the markup iframe on navigation. r=pbro
authorGabriel Luong <gabriel.luong@gmail.com>
Sun, 14 Oct 2018 22:26:05 -0400
changeset 489519 bbae2c874b4460a5ef7c8a3b3e35ee99cd5c4e78
parent 489518 b56ff378a56d47a954ad6a892b2d258d2eaccbda
child 489529 6e0139775220f26e08be6494978e64617f4220d2
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerspbro
bugs1497905
milestone64.0a1
Bug 1497905 - Avoid destroying the markup iframe on navigation. r=pbro
devtools/client/inspector/inspector.js
devtools/client/inspector/markup/markup.js
devtools/client/inspector/markup/test/browser_markup_update-on-navigtion.js
devtools/client/inspector/test/browser_inspector_select-docshell.js
devtools/client/responsive.html/test/browser/browser_toolbox_rule_view_reload.js
--- a/devtools/client/inspector/inspector.js
+++ b/devtools/client/inspector/inspector.js
@@ -108,18 +108,19 @@ function Inspector(toolbox) {
   EventEmitter.decorate(this);
 
   this._toolbox = toolbox;
   this._target = toolbox.target;
   this.panelDoc = window.document;
   this.panelWin = window;
   this.panelWin.inspector = this;
   this.telemetry = toolbox.telemetry;
+  this.store = Store();
 
-  this.store = Store();
+  this._markupBox = this.panelDoc.getElementById("markup-box");
 
   // Map [panel id => panel instance]
   // Stores all the instances of sidebar panels like rule view, computed view, ...
   this._panels = new Map();
 
   this.reflowTracker = new ReflowTracker(this._target);
   this.styleChangeTracker = new InspectorStyleChangeTracker(this);
   if (Services.prefs.getBoolPref(TRACK_CHANGES_ENABLED)) {
@@ -1160,18 +1161,18 @@ Inspector.prototype = {
       // Cancel this promise resolution as a new one had
       // been queued up.
       if (this._pendingSelection != onNodeSelected) {
         return;
       }
       this._pendingSelection = null;
       this.selection.setNodeFront(defaultNode, { reason: "navigateaway" });
 
+      this.once("markuploaded", this.onMarkupLoaded);
       this._initMarkup();
-      this.once("markuploaded", this.onMarkupLoaded);
 
       // Setup the toolbar again, since its content may depend on the current document.
       this.setupToolbar();
     };
     this._pendingSelection = onNodeSelected;
     this._getDefaultNodeForSelection()
         .then(onNodeSelected, this._handleRejectionIfNotDestroyed);
   },
@@ -1444,16 +1445,21 @@ Inspector.prototype = {
       this.animationinspector.destroy();
     }
 
     if (this._highlighters) {
       this._highlighters.destroy();
       this._highlighters = null;
     }
 
+    if (this._markupFrame) {
+      this._markupFrame.removeEventListener("load", this._onMarkupFrameLoad, true);
+      this._markupFrame.removeEventListener("contextmenu", this._onContextMenu);
+    }
+
     if (this._search) {
       this._search.destroy();
       this._search = null;
     }
 
     const sidebarDestroyer = this.sidebar.destroy();
     const ruleViewSideBarDestroyer = this.ruleViewSideBar ?
       this.ruleViewSideBar.destroy() : null;
@@ -1466,16 +1472,18 @@ Inspector.prototype = {
     this.styleChangeTracker.destroy();
 
     if (this.changesManager) {
       this.changesManager.destroy();
     }
 
     this._is3PaneModeChromeEnabled = null;
     this._is3PaneModeEnabled = null;
+    this._markupBox = null;
+    this._markupFrame = null;
     this._notificationBox = null;
     this._target = null;
     this._toolbox = null;
     this.breadcrumbs = null;
     this.panelDoc = null;
     this.panelWin.inspector = null;
     this.panelWin = null;
     this.resultsLength = null;
@@ -1934,65 +1942,54 @@ Inspector.prototype = {
       linkFollow.label = INSPECTOR_L10N.getFormatStr(
         "inspector.menu.selectElement.label", popupNode.dataset.link);
     }
 
     return [linkFollow, linkCopy];
   },
 
   _initMarkup: function() {
-    const doc = this.panelDoc;
-
-    this._markupBox = doc.getElementById("markup-box");
+    if (!this._markupFrame) {
+      this._markupFrame = this.panelDoc.createElement("iframe");
+      this._markupFrame.setAttribute("aria-label",
+        INSPECTOR_L10N.getStr("inspector.panelLabel.markupView"));
+      this._markupFrame.setAttribute("flex", "1");
+      // This is needed to enable tooltips inside the iframe document.
+      this._markupFrame.setAttribute("tooltip", "aHTMLTooltip");
+      this._markupFrame.addEventListener("contextmenu", this._onContextMenu);
 
-    // create tool iframe
-    this._markupFrame = doc.createElement("iframe");
-    this._markupFrame.setAttribute("flex", "1");
-    // This is needed to enable tooltips inside the iframe document.
-    this._markupFrame.setAttribute("tooltip", "aHTMLTooltip");
-    this._markupFrame.addEventListener("contextmenu", this._onContextMenu);
+      this._markupBox.style.visibility = "hidden";
+      this._markupBox.appendChild(this._markupFrame);
 
-    this._markupBox.style.visibility = "hidden";
-    this._markupBox.appendChild(this._markupFrame);
-
-    this._markupFrame.addEventListener("load", this._onMarkupFrameLoad, true);
-    this._markupFrame.setAttribute("src", "markup/markup.xhtml");
-    this._markupFrame.setAttribute("aria-label",
-      INSPECTOR_L10N.getStr("inspector.panelLabel.markupView"));
+      this._markupFrame.addEventListener("load", this._onMarkupFrameLoad, true);
+      this._markupFrame.setAttribute("src", "markup/markup.xhtml");
+    } else {
+      this._onMarkupFrameLoad();
+    }
   },
 
   _onMarkupFrameLoad: function() {
     this._markupFrame.removeEventListener("load", this._onMarkupFrameLoad, true);
     this._markupFrame.contentWindow.focus();
+    this._markupBox.style.visibility = "visible";
     this.markup = new MarkupView(this, this._markupFrame, this._toolbox.win);
-    this._markupBox.style.visibility = "visible";
     this.emit("markuploaded");
   },
 
   _destroyMarkup: function() {
     let destroyPromise;
 
-    if (this._markupFrame) {
-      this._markupFrame.removeEventListener("load", this._onMarkupFrameLoad, true);
-      this._markupFrame.removeEventListener("contextmenu", this._onContextMenu);
-    }
-
     if (this.markup) {
       destroyPromise = this.markup.destroy();
       this.markup = null;
     } else {
       destroyPromise = promise.resolve();
     }
 
-    if (this._markupFrame) {
-      this._markupFrame.remove();
-      this._markupFrame = null;
-    }
-
-    this._markupBox = null;
+    this._markupBox.style.visibility = "hidden";
 
     return destroyPromise;
   },
 
   onEyeDropperButtonClicked: function() {
     this.eyeDropperButton.classList.contains("checked")
       ? this.hideEyeDropper()
       : this.showEyeDropper();
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -68,17 +68,17 @@ function MarkupView(inspector, frame, co
 
   this.controllerWindow = controllerWindow;
   this.inspector = inspector;
   this.highlighters = inspector.highlighters;
   this.walker = this.inspector.walker;
   this._frame = frame;
   this.win = this._frame.contentWindow;
   this.doc = this._frame.contentDocument;
-  this._elt = this.doc.querySelector("#root");
+  this._elt = this.doc.getElementById("root");
   this.telemetry = this.inspector.telemetry;
 
   this.maxChildren = Services.prefs.getIntPref("devtools.markup.pagesize",
                                                DEFAULT_MAX_CHILDREN);
 
   this.collapseAttributes = Services.prefs.getBoolPref(ATTR_COLLAPSE_ENABLED_PREF);
   this.collapseAttributeLength = Services.prefs.getIntPref(ATTR_COLLAPSE_LENGTH_PREF);
 
@@ -1956,23 +1956,24 @@ MarkupView.prototype = {
     this.win.removeEventListener("mouseup", this._onMouseUp);
 
     this._prefObserver.off(ATTR_COLLAPSE_ENABLED_PREF,
                            this._onCollapseAttributesPrefChange);
     this._prefObserver.off(ATTR_COLLAPSE_LENGTH_PREF,
                            this._onCollapseAttributesPrefChange);
     this._prefObserver.destroy();
 
-    this._elt = null;
-
     for (const [, container] of this._containers) {
       container.destroy();
     }
     this._containers = null;
 
+    this._elt.innerHTML = "";
+    this._elt = null;
+
     this.controllerWindow = null;
     this.doc = null;
     this.highlighters = null;
     this.win = null;
 
     this._lastDropTarget = null;
     this._lastDragTarget = null;
 
--- a/devtools/client/inspector/markup/test/browser_markup_update-on-navigtion.js
+++ b/devtools/client/inspector/markup/test/browser_markup_update-on-navigtion.js
@@ -32,12 +32,12 @@ add_task(async function() {
   await selectNode("#two", inspector);
 
   function assertMarkupViewIsLoaded() {
     const markupViewBox = inspector.panelDoc.getElementById("markup-box");
     is(markupViewBox.childNodes.length, 1, "The markup-view is loaded");
   }
 
   function assertMarkupViewIsEmpty() {
-    const markupViewBox = inspector.panelDoc.getElementById("markup-box");
-    is(markupViewBox.childNodes.length, 0, "The markup-view is unloaded");
+    const markupViewFrame = inspector._markupFrame.contentDocument.getElementById("root");
+    is(markupViewFrame.childNodes.length, 0, "The markup-view is unloaded");
   }
 });
--- a/devtools/client/inspector/test/browser_inspector_select-docshell.js
+++ b/devtools/client/inspector/test/browser_inspector_select-docshell.js
@@ -81,11 +81,12 @@ add_task(async function() {
 });
 
 function assertMarkupViewIsLoaded(inspector) {
   const markupViewBox = inspector.panelDoc.getElementById("markup-box");
   is(markupViewBox.childNodes.length, 1, "The markup-view is loaded");
 }
 
 function assertMarkupViewIsEmpty(inspector) {
-  const markupViewBox = inspector.panelDoc.getElementById("markup-box");
-  is(markupViewBox.childNodes.length, 0, "The markup-view is unloaded");
+  const markupFrame = inspector._markupFrame;
+  is(markupFrame.contentDocument.getElementById("root").childNodes.length, 0,
+    "The markup-view is unloaded");
 }
--- a/devtools/client/responsive.html/test/browser/browser_toolbox_rule_view_reload.js
+++ b/devtools/client/responsive.html/test/browser/browser_toolbox_rule_view_reload.js
@@ -21,19 +21,21 @@ add_task(async function() {
 
   is(numberOfRules(view), 2, "Rule view has two rules.");
 
   info("Open RDM");
   await openRDM(tab);
 
   info("Reload the current page");
   const onNewRoot = inspector.once("new-root");
+  const onRuleViewRefreshed = inspector.once("rule-view-refreshed");
   await testActor.reload();
   await onNewRoot;
   await inspector.markup._waitForChildren();
+  await onRuleViewRefreshed;
 
   is(numberOfRules(view), 2, "Rule view still has two rules and is not empty.");
 
   await closeRDM(tab);
   await removeTab(tab);
 });
 
 function numberOfRules(ruleView) {