Bug 1437512 - Part 2 - Remove the "panelmultiview" binding. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Sun, 25 Feb 2018 18:43:55 +0000
changeset 759577 d6403d026b4539acc71325c600ee9ff3be667591
parent 759576 11ec16d78e61379d6efd359bd221b1829f89efa6
child 759578 b196a817d47d78579a249ea57f3e9ff5734e4f1f
child 759586 4ae1b051e8cb940b163a81ff851973f985d39941
push id100388
push userpaolo.mozmail@amadzone.org
push dateSun, 25 Feb 2018 18:45:46 +0000
reviewersGijs
bugs1437512
milestone60.0a1
Bug 1437512 - Part 2 - Remove the "panelmultiview" binding. r=Gijs MozReview-Commit-ID: H9R7ahkCr2U
browser/base/content/browser-pageActions.js
browser/base/content/browser.css
browser/components/customizableui/PanelMultiView.jsm
browser/components/customizableui/content/jar.mn
browser/components/customizableui/content/panelUI.js
browser/components/customizableui/content/panelUI.xml
--- a/browser/base/content/browser-pageActions.js
+++ b/browser/base/content/browser-pageActions.js
@@ -246,17 +246,17 @@ var BrowserPageActions = {
       iframeNode = document.createElement("iframe");
       iframeNode.setAttribute("type", "content");
       panelNode.appendChild(iframeNode);
     }
 
     let popupSet = document.getElementById("mainPopupSet");
     popupSet.appendChild(panelNode);
     panelNode.addEventListener("popuphidden", () => {
-      panelNode.remove();
+      PanelMultiView.removePopup(panelNode);
     }, { once: true });
 
     if (iframeNode) {
       panelNode.addEventListener("popupshowing", () => {
         action.onIframeShowing(iframeNode, panelNode);
       }, { once: true });
       panelNode.addEventListener("popuphiding", () => {
         action.onIframeHiding(iframeNode, panelNode);
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -72,31 +72,26 @@ toolbar[customizable="true"] {
 %endif
 
 #toolbar-menubar[autohide="true"] {
   -moz-binding: url("chrome://browser/content/customizableui/toolbar.xml#toolbar-menubar-autohide");
 }
 
 panelmultiview {
   -moz-box-align: start;
-  -moz-binding: url("chrome://browser/content/customizableui/panelUI.xml#panelmultiview");
 }
 
 panelmultiview[transitioning] {
   pointer-events: none;
 }
 
 panelview {
   -moz-box-orient: vertical;
 }
 
-panel[hidden] panelmultiview {
-  -moz-binding: none;
-}
-
 panelview:not([current]):not([in-transition]) {
   visibility: collapse;
 }
 
 /* Hide the header when a subview is reused as a main view. */
 panelview[mainview] > .panel-header {
   display: none;
 }
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -115,16 +115,17 @@ const BLOCKERS_TIMEOUT_MS = 10000;
 const TRANSITION_PHASES = Object.freeze({
   START: 1,
   PREPARE: 2,
   TRANSITION: 3,
   END: 4
 });
 
 let gNodeToObjectMap = new WeakMap();
+let gWindowsWithUnloadHandler = new WeakSet();
 let gMultiLineElementsMap = new WeakMap();
 
 /**
  * Allows associating an object to a node lazily using a weak map.
  *
  * Classes deriving from this one may be easily converted to Custom Elements,
  * although they would lose the ability of being associated lazily.
  */
@@ -247,17 +248,17 @@ var AssociatedToNode = class {
         }
       }
       return cancel;
     });
   }
 };
 
 /**
- * This is associated to <panelmultiview> elements by the panelUI.xml binding.
+ * This is associated to <panelmultiview> elements.
  */
 var PanelMultiView = class extends this.AssociatedToNode {
   /**
    * Tries to open the specified <panel> and displays the main view specified
    * with the "mainViewId" attribute on the <panelmultiview> node it contains.
    *
    * If the panel does not contain a <panelmultiview>, it is opened directly.
    * This allows consumers like page actions to accept different panel types.
@@ -285,16 +286,54 @@ var PanelMultiView = class extends this.
     let panelMultiViewNode = panelNode.querySelector("panelmultiview");
     if (panelMultiViewNode) {
       this.forNode(panelMultiViewNode).hidePopup();
     } else {
       panelNode.hidePopup();
     }
   }
 
+  /**
+   * Removes the specified <panel> from the document, ensuring that any
+   * <panelmultiview> node it contains is destroyed properly.
+   *
+   * If the panel does not contain a <panelmultiview>, it is removed directly.
+   * This allows consumers like page actions to accept different panel types.
+   */
+  static removePopup(panelNode) {
+    try {
+      let panelMultiViewNode = panelNode.querySelector("panelmultiview");
+      if (panelMultiViewNode) {
+        this.forNode(panelMultiViewNode).disconnect();
+      }
+    } finally {
+      // Make sure to remove the panel element even if disconnecting fails.
+      panelNode.remove();
+    }
+  }
+
+  /**
+   * Ensures that when the specified window is closed all the <panelmultiview>
+   * node it contains are destroyed properly.
+   */
+  static ensureUnloadHandlerRegistered(window) {
+    if (gWindowsWithUnloadHandler.has(window)) {
+      return;
+    }
+
+    window.addEventListener("unload", () => {
+      for (let panelMultiViewNode of
+           window.document.querySelectorAll("panelmultiview")) {
+        this.forNode(panelMultiViewNode).disconnect();
+      }
+    }, { once: true });
+
+    gWindowsWithUnloadHandler.add(window);
+  }
+
   get _panel() {
     return this.node.parentNode;
   }
 
   get _mainViewId() {
     return this.node.getAttribute("mainViewId");
   }
   get _mainView() {
@@ -350,16 +389,18 @@ var PanelMultiView = class extends this.
     super(node);
     this._openPopupPromise = Promise.resolve(false);
     this._openPopupCancelCallback = () => {};
   }
 
   connect() {
     this.connected = true;
 
+    PanelMultiView.ensureUnloadHandlerRegistered(this.window);
+
     let viewContainer = this._viewContainer =
       this.document.createElement("box");
     viewContainer.classList.add("panel-viewcontainer");
 
     let viewStack = this._viewStack = this.document.createElement("box");
     viewStack.classList.add("panel-viewstack");
     viewContainer.append(viewStack);
 
@@ -402,19 +443,19 @@ var PanelMultiView = class extends this.
     ["current", "showingSubView"].forEach(property => {
       Object.defineProperty(this.node, property, {
         enumerable: true,
         get: () => this[property]
       });
     });
   }
 
-  destructor() {
+  disconnect() {
     // Guard against re-entrancy.
-    if (!this.node)
+    if (!this.node || !this.connected)
       return;
 
     this._cleanupTransitionPhase();
     let mainView = this._mainView;
     if (mainView) {
       if (this._panelViewCache)
         this._panelViewCache.appendChild(mainView);
       mainView.removeAttribute("mainview");
--- a/browser/components/customizableui/content/jar.mn
+++ b/browser/components/customizableui/content/jar.mn
@@ -1,9 +1,8 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 browser.jar:
   content/browser/customizableui/panelUI.js
-  content/browser/customizableui/panelUI.xml
   content/browser/customizableui/toolbar.xml
 
--- a/browser/components/customizableui/content/panelUI.js
+++ b/browser/components/customizableui/content/panelUI.js
@@ -418,20 +418,17 @@ const PanelUI = {
       let panelRemover = () => {
         viewNode.classList.remove("cui-widget-panelview");
         if (viewShown) {
           CustomizableUI.removePanelCloseListeners(tempPanel);
           tempPanel.removeEventListener("popuphidden", panelRemover);
         }
         aAnchor.open = false;
 
-        // Ensure we run the destructor:
-        PanelMultiView.forNode(multiView).destructor();
-
-        tempPanel.remove();
+        PanelMultiView.removePopup(tempPanel);
       };
 
       if (aAnchor.parentNode.id == "PersonalToolbar") {
         tempPanel.classList.add("bookmarks-toolbar");
       }
 
       let anchor = this._getPanelAnchor(aAnchor);
 
deleted file mode 100644
--- a/browser/components/customizableui/content/panelUI.xml
+++ /dev/null
@@ -1,24 +0,0 @@
-<?xml version="1.0"?>
-<!-- This Source Code Form is subject to the terms of the Mozilla Public
-   - License, v. 2.0. If a copy of the MPL was not distributed with this
-   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
-
-<!DOCTYPE bindings [
-  <!ENTITY % browserDTD SYSTEM "chrome://browser/locale/browser.dtd">
-  %browserDTD;
-]>
-
-<bindings id="browserPanelUIBindings"
-          xmlns="http://www.mozilla.org/xbl"
-          xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
-          xmlns:xbl="http://www.mozilla.org/xbl">
-
-  <binding id="panelmultiview">
-    <implementation>
-      <destructor><![CDATA[
-        const {PanelMultiView} = ChromeUtils.import("resource:///modules/PanelMultiView.jsm", {});
-        PanelMultiView.forNode(this).destructor();
-      ]]></destructor>
-    </implementation>
-  </binding>
-</bindings>