Bug 1407347 - Prevent iframes in the inspector sidebar from rerendering when a sidebar tab is removed. r=Honza draft
authorGabriel Luong <gabriel.luong@gmail.com>
Mon, 29 Jan 2018 11:58:22 -0500
changeset 748344 a3ff83dc0d276a79989b7f03bfb7dc527a59397a
parent 748343 15e5dcaa825751d2c7fa9b170e0d3e9306025301
push id97131
push userbmo:gl@mozilla.com
push dateMon, 29 Jan 2018 16:58:41 +0000
reviewersHonza
bugs1407347
milestone60.0a1
Bug 1407347 - Prevent iframes in the inspector sidebar from rerendering when a sidebar tab is removed. r=Honza MozReview-Commit-ID: HlTyODVfXyU
devtools/client/inspector/components/InspectorTabPanel.js
devtools/client/inspector/toolsidebar.js
devtools/client/jsonview/components/MainTabbedArea.js
devtools/client/shared/components/tabs/Tabs.js
devtools/client/webconsole/net/components/net-info-body.js
--- a/devtools/client/inspector/components/InspectorTabPanel.js
+++ b/devtools/client/inspector/components/InspectorTabPanel.js
@@ -20,18 +20,20 @@ const { div } = dom;
  */
 class InspectorTabPanel extends Component {
   static get propTypes() {
     return {
       // ID of the node that should be rendered as the content.
       id: PropTypes.string.isRequired,
       // Optional prefix for panel IDs.
       idPrefix: PropTypes.string,
-      // Optional mount callback
+      // Optional mount callback.
       onMount: PropTypes.func,
+      // Optional unmount callback.
+      onUnmount: PropTypes.func,
     };
   }
 
   static get defaultProps() {
     return {
       idPrefix: "",
     };
   }
@@ -47,16 +49,20 @@ class InspectorTabPanel extends Componen
       this.props.onMount(this.refs.content, this.props);
     }
   }
 
   componentWillUnmount() {
     let doc = this.refs.content.ownerDocument;
     let panels = doc.getElementById("tabpanels");
 
+    if (this.props.onUnmount) {
+      this.props.onUnmount(this.refs.content, this.props);
+    }
+
     // Move panel's content node back into list of tab panels.
     panels.appendChild(this.refs.content.firstChild);
   }
 
   render() {
     return (
       div({
         ref: "content",
--- a/devtools/client/inspector/toolsidebar.js
+++ b/devtools/client/inspector/toolsidebar.js
@@ -132,16 +132,17 @@ ToolSidebar.prototype = {
   addFrameTab: function (id, title, url, selected, index) {
     let panel = this.InspectorTabPanel({
       id: id,
       idPrefix: this.TABPANEL_ID_PREFIX,
       key: id,
       title: title,
       url: url,
       onMount: this.onSidePanelMounted.bind(this),
+      onUnmount: this.onSidePanelUnmounted.bind(this),
     });
 
     this.addTab(id, title, panel, selected, index);
   },
 
   onSidePanelMounted: function (content, props) {
     let iframe = content.querySelector("iframe");
     if (!iframe || iframe.getAttribute("src")) {
@@ -158,16 +159,30 @@ ToolSidebar.prototype = {
       }
       this.emit(props.id + "-ready");
     };
 
     iframe.addEventListener("load", onIFrameLoaded, true);
     iframe.setAttribute("src", props.url);
   },
 
+  onSidePanelUnmounted: function (content, props) {
+    let iframe = content.querySelector("iframe");
+    if (!iframe || !iframe.hasAttribute("src")) {
+      return;
+    }
+
+    let win = iframe.contentWindow;
+    if ("destroy" in win) {
+      win.destroy(this._toolPanel, iframe);
+    }
+
+    iframe.removeAttribute("src");
+  },
+
   /**
    * Remove an existing tab.
    * @param {String} tabId The ID of the tab that was used to register it, or
    * the tab id attribute value if the tab existed before the sidebar
    * got created.
    * @param {String} tabPanelId Optional. If provided, this ID will be used
    * instead of the tabId to retrieve and remove the corresponding <tabpanel>
    */
--- a/devtools/client/jsonview/components/MainTabbedArea.js
+++ b/devtools/client/jsonview/components/MainTabbedArea.js
@@ -56,36 +56,39 @@ define(function (require, exports, modul
     }
 
     render() {
       return (
         Tabs({
           tabActive: this.state.tabActive,
           onAfterChange: this.onTabChanged},
           TabPanel({
+            id: "json",
             className: "json",
             title: JSONView.Locale.$STR("jsonViewer.tab.JSON")},
             JsonPanel({
               data: this.state.json,
               expandedNodes: this.props.expandedNodes,
               actions: this.props.actions,
               searchFilter: this.state.searchFilter
             })
           ),
           TabPanel({
+            id: "rawdata",
             className: "rawdata",
             title: JSONView.Locale.$STR("jsonViewer.tab.RawData")},
             TextPanel({
               isValidJson: !(this.state.json instanceof Error) &&
                            document.readyState != "loading",
               data: this.state.jsonText,
               actions: this.props.actions
             })
           ),
           TabPanel({
+            id: "headers",
             className: "headers",
             title: JSONView.Locale.$STR("jsonViewer.tab.Headers")},
             HeadersPanel({
               data: this.props.headers,
               actions: this.props.actions,
               searchFilter: this.props.searchFilter
             })
           )
--- a/devtools/client/shared/components/tabs/Tabs.js
+++ b/devtools/client/shared/components/tabs/Tabs.js
@@ -67,24 +67,25 @@ define(function (require, exports, modul
     }
 
     constructor(props) {
       super(props);
 
       this.state = {
         tabActive: props.tabActive,
 
-        // This array is used to store an information whether a tab
-        // at specific index has already been created (e.g. selected
-        // at least once).
-        // If yes, it's rendered even if not currently selected.
-        // This is because in some cases we don't want to re-create
-        // tab content when it's being unselected/selected.
-        // E.g. in case of an iframe being used as a tab-content
-        // we want the iframe to stay in the DOM.
+        // This array is used to store an object containing information on whether a tab
+        // at a specified index has already been created (e.g. selected at least once) and
+        // the tab id. An example of the object structure is the following:
+        // [{ isCreated: true, tabId: "ruleview" }, { isCreated: false, tabId: "foo" }].
+        // If the tab at the specified index has already been created, it's rendered even
+        // if not currently selected. This is because in some cases we don't want
+        // to re-create tab content when it's being unselected/selected.
+        // E.g. in case of an iframe being used as a tab-content we want the iframe to
+        // stay in the DOM.
         created: [],
 
         // True if tabs can't fit into available horizontal space.
         overflow: false,
       };
 
       this.onOverflow = this.onOverflow.bind(this);
       this.onUnderflow = this.onUnderflow.bind(this);
@@ -111,34 +112,56 @@ define(function (require, exports, modul
       let index = this.state.tabActive;
       if (this.props.onMount) {
         this.props.onMount(index);
       }
     }
 
     componentWillReceiveProps(nextProps) {
       let { children, tabActive } = nextProps;
+      let panels = children.filter(panel => panel);
+      let created = [...this.state.created];
 
-      // Check type of 'tabActive' props to see if it's valid
-      // (it's 0-based index).
+      // If the children props has changed due to an addition or removal of a tab,
+      // update the state's created array with the latest tab ids and whether or not
+      // the tab is already created.
+      if (this.state.created.length != panels.length) {
+        created = panels.map(panel => {
+          // Get whether or not the tab has already been created from the previous state.
+          let createdEntry = this.state.created.find(entry => {
+            return entry && entry.tabId === panel.props.id;
+          });
+          let isCreated = !!createdEntry && createdEntry.isCreated;
+          let tabId = panel.props.id;
+
+          return {
+            isCreated,
+            tabId,
+          };
+        });
+      }
+
+      // Check type of 'tabActive' props to see if it's valid (it's 0-based index).
       if (typeof tabActive === "number") {
-        let panels = children.filter((panel) => panel);
-
         // Reset to index 0 if index overflows the range of panel array
         tabActive = (tabActive < panels.length && tabActive >= 0) ?
           tabActive : 0;
 
-        let created = [...this.state.created];
-        created[tabActive] = true;
+        created[tabActive] = Object.assign({}, created[tabActive], {
+          isCreated: true,
+        });
 
         this.setState({
-          created,
           tabActive,
         });
       }
+
+      this.setState({
+        created,
+      });
     }
 
     componentWillUnmount() {
       let node = findDOMNode(this);
       node.removeEventListener("keydown", this.onKeyDown);
 
       if (this.props.showAllTabsMenu) {
         node.removeEventListener("overflow", this.onOverflow);
@@ -204,21 +227,23 @@ define(function (require, exports, modul
       if (onBeforeChange) {
         let cancel = onBeforeChange(index);
         if (cancel) {
           return;
         }
       }
 
       let created = [...this.state.created];
-      created[index] = true;
+      created[index] = Object.assign({}, created[index], {
+        isCreated: true,
+      });
 
       let newState = Object.assign({}, this.state, {
+        created,
         tabActive: index,
-        created: created
       });
 
       this.setState(newState, () => {
         // Properly set focus on selected tab.
         let node = findDOMNode(this);
         let selectedTab = node.querySelector(".is-active > a");
         if (selectedTab) {
           selectedTab.focus();
@@ -330,16 +355,18 @@ define(function (require, exports, modul
         .filter((tab) => tab)
         .map((tab, index) => {
           let selected = selectedIndex === index;
           if (renderOnlySelected && !selected) {
             return null;
           }
 
           let id = tab.props.id;
+          let isCreated = this.state.created[index] &&
+            this.state.created[index].isCreated;
 
           // Use 'visibility:hidden' + 'height:0' for hiding content of non-selected
           // tab. It's faster than 'display:none' because it avoids triggering frame
           // destruction and reconstruction. 'width' is not changed to avoid relayout.
           let style = {
             visibility: selected ? "visible" : "hidden",
             height: selected ? "100%" : "0",
           };
@@ -349,23 +376,23 @@ define(function (require, exports, modul
           if (typeof tab.panel == "function" && selected) {
             tab.panel = tab.panel(tab);
           }
           let panel = tab.panel || tab;
 
           return (
             dom.div({
               id: id ? id + "-panel" : "panel-" + index,
-              key: index,
+              key: id,
               style: style,
               className: selected ? "tab-panel-box" : "tab-panel-box hidden",
               role: "tabpanel",
               "aria-labelledby": id ? id + "-tab" : "tab-" + index,
             },
-              (selected || this.state.created[index]) ? panel : null
+              (selected || isCreated) ? panel : null
             )
           );
         });
 
       return (
         dom.div({className: "panels"},
           panels
         )
@@ -383,16 +410,18 @@ define(function (require, exports, modul
   }
 
   /**
    * Renders simple tab 'panel'.
    */
   class Panel extends Component {
     static get propTypes() {
       return {
+        id: PropTypes.string.isRequired,
+        className: PropTypes.string,
         title: PropTypes.string.isRequired,
         children: PropTypes.oneOfType([
           PropTypes.array,
           PropTypes.element
         ]).isRequired
       };
     }
 
--- a/devtools/client/webconsole/net/components/net-info-body.js
+++ b/devtools/client/webconsole/net/components/net-info-body.js
@@ -93,74 +93,82 @@ class NetInfoBody extends Component {
     let hasParams = request.queryString && request.queryString.length;
     let hasPostData = request.bodySize > 0;
 
     let panels = [];
 
     // Headers tab
     panels.push(
       TabPanel({
+        id: "headers",
         className: "headers",
         key: "headers",
         title: Locale.$STR("netRequest.headers")},
         HeadersTab({data: data, actions: actions})
       )
     );
 
     // URL parameters tab
     if (hasParams) {
       panels.push(
         TabPanel({
+          id: "params",
           className: "params",
           key: "params",
           title: Locale.$STR("netRequest.params")},
           ParamsTab({data: data, actions: actions})
         )
       );
     }
 
     // Posted data tab
     if (hasPostData) {
       panels.push(
         TabPanel({
+          id: "post",
           className: "post",
           key: "post",
           title: Locale.$STR("netRequest.post")},
           PostTab({data: data, actions: actions})
         )
       );
     }
 
     // Response tab
     panels.push(
-      TabPanel({className: "response", key: "response",
+      TabPanel({
+        id: "response",
+        className: "response",
+        key: "response",
         title: Locale.$STR("netRequest.response")},
         ResponseTab({data: data, actions: actions})
       )
     );
 
     // Cookies tab
     if (this.hasCookies()) {
       panels.push(
         TabPanel({
+          id: "cookies",
           className: "cookies",
           key: "cookies",
           title: Locale.$STR("netRequest.cookies")},
           CookiesTab({
             data: data,
             actions: actions
           })
         )
       );
     }
 
     // Stacktrace tab
     if (this.hasStackTrace()) {
       panels.push(
         TabPanel({
+          id: "stacktrace-tab",
           className: "stacktrace-tab",
           key: "stacktrace",
           title: Locale.$STR("netRequest.callstack")},
           StackTraceTab({
             data: data,
             actions: actions,
             sourceMapService: sourceMapService,
           })