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 496932 bbae2c874b4460a5ef7c8a3b3e35ee99cd5c4e78
parent 496931 b56ff378a56d47a954ad6a892b2d258d2eaccbda
child 496933 6e0139775220f26e08be6494978e64617f4220d2
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1497905
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 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) {