Bug 1439358 - Part 8 - Change how visibility is controlled so knownViews can be removed. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Thu, 22 Feb 2018 15:28:15 +0000
changeset 758545 797624d41f41a2ba0efd943356ade1ed6428b3e6
parent 758544 5b50ac03f80902ccbf64c4dcad6e0505a0cdbfa4
push id100095
push userpaolo.mozmail@amadzone.org
push dateThu, 22 Feb 2018 17:13:44 +0000
reviewersGijs
bugs1439358
milestone60.0a1
Bug 1439358 - Part 8 - Change how visibility is controlled so knownViews can be removed. r=Gijs MozReview-Commit-ID: 9dVZ1cOto8O
browser/base/content/test/performance/browser_appmenu_reflows.js
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/base/content/test/performance/browser_appmenu_reflows.js
+++ b/browser/base/content/test/performance/browser_appmenu_reflows.js
@@ -53,30 +53,20 @@ const EXPECTED_APPMENU_SUBVIEW_REFLOWS =
    *
    * If we add more views where this is necessary, we may need to duplicate
    * these expected reflows further. Bug 1392340 is on file to remove the
    * reflows completely when opening subviews.
    */
   {
     stack: [
       "descriptionHeightWorkaround@resource:///modules/PanelMultiView.jsm",
-      "set current@resource:///modules/PanelMultiView.jsm",
-      "hideAllViewsExcept@resource:///modules/PanelMultiView.jsm",
-    ],
-
-    times: 1, // This number should only ever go down - never up.
-  },
-
-  {
-    stack: [
-      "descriptionHeightWorkaround@resource:///modules/PanelMultiView.jsm",
       "_transitionViews@resource:///modules/PanelMultiView.jsm",
     ],
 
-    times: 3, // This number should only ever go down - never up.
+    times: 4, // This number should only ever go down - never up.
   },
 
   /**
    * Please don't add anything new!
    */
 ];
 
 add_task(async function() {
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -332,31 +332,30 @@ this.PanelMultiView = class extends this
     return this.node && this._currentSubView;
   }
   get _currentSubView() {
     // Peek the top of the stack, but fall back to the main view if the list of
     // opened views is currently empty.
     let panelView = this.openViews[this.openViews.length - 1];
     return (panelView && panelView.node) || this._mainView;
   }
+  get showingSubView() {
+    return this.openViews.length > 1;
+  }
 
   constructor(node) {
     super(node);
     this._openPopupPromise = Promise.resolve(false);
     this._openPopupCancelCallback = () => {};
   }
 
   connect() {
     this.connected = true;
-    this.knownViews = new Set(Array.from(
-      this.node.getElementsByTagName("panelview"),
-      node => PanelView.forNode(node)));
     this.openViews = [];
     this.__transitioning = false;
-    this.showingSubView = false;
 
     const {document, window} = this;
 
     this._viewContainer =
       document.getAnonymousElementByAttribute(this.node, "anonid", "viewContainer");
     this._viewStack =
       document.getAnonymousElementByAttribute(this.node, "anonid", "viewStack");
     this._offscreenViewStack =
@@ -393,18 +392,16 @@ this.PanelMultiView = class extends this
   }
 
   destructor() {
     // Guard against re-entrancy.
     if (!this.node)
       return;
 
     this._cleanupTransitionPhase();
-    if (this._ephemeral)
-      this.hideAllViewsExcept(null);
     let mainView = this._mainView;
     if (mainView) {
       if (this._panelViewCache)
         this._panelViewCache.appendChild(mainView);
       mainView.removeAttribute("mainview");
     }
 
     this._moveOutKids(this._viewStack);
@@ -683,56 +680,34 @@ this.PanelMultiView = class extends this
 
     // The main view of a panel can be a subview in another one. Make sure to
     // reset all the properties that may be set on a subview.
     nextPanelView.mainview = true;
     nextPanelView.headerText = "";
     nextPanelView.minMaxWidth = 0;
 
     await this._cleanupTransitionPhase();
-    this.hideAllViewsExcept(nextPanelView);
+    nextPanelView.visible = true;
+    nextPanelView.descriptionHeightWorkaround();
 
     this._viewShown(nextPanelView);
     return true;
   }
 
   /**
-   * Ensures that all the panelviews, that are currently part of this instance,
-   * are hidden, except one specifically.
-   *
-   * @param {panelview} [nextPanelView]
-   *        The PanelView object to ensure is visible. Optional.
-   */
-  hideAllViewsExcept(nextPanelView = null) {
-    for (let panelView of this.knownViews) {
-      // When the panelview was already reparented, don't interfere any more.
-      if (panelView == nextPanelView || !this.node || panelView.node.panelMultiView != this.node)
-        continue;
-      panelView.current = false;
-    }
-
-    if (!this.node || !nextPanelView)
-      return;
-
-    nextPanelView.current = true;
-    this.showingSubView = nextPanelView.node.id != this._mainViewId;
-  }
-
-  /**
    * Opens the specified PanelView and dispatches the ViewShowing event, which
    * can be used to populate the subview or cancel the operation.
    *
    * @resolves With true if the view was opened, false otherwise.
    */
   async _openView(panelView) {
     if (panelView.node.parentNode != this._viewStack) {
       this._viewStack.appendChild(panelView.node);
     }
 
-    this.knownViews.add(panelView);
     panelView.node.panelMultiView = this.node;
     this.openViews.push(panelView);
 
     let canceled = await panelView.dispatchAsyncEvent("ViewShowing");
 
     // The panel can be hidden while we are processing the ViewShowing event.
     // This results in all the views being closed synchronously, and at this
     // point the ViewHiding event has already been dispatched for all of them.
@@ -767,16 +742,19 @@ this.PanelMultiView = class extends this
    * @note The ViewHiding event is not cancelable and should probably be renamed
    *       to ViewHidden or ViewClosed instead, see bug 1438507.
    */
   _closeLatestView() {
     let panelView = this.openViews.pop();
     panelView.clearNavigation();
     panelView.dispatchCustomEvent("ViewHiding");
     panelView.node.panelMultiView = null;
+    // Views become invisible synchronously when they are closed, and they won't
+    // become visible again until they are opened.
+    panelView.visible = false;
   }
 
   /**
    * Closes all the views that are currently open.
    */
   closeAllViews() {
     // Raise ViewHiding events for open views in reverse order.
     while (this.openViews.length) {
@@ -940,16 +918,23 @@ this.PanelMultiView = class extends this
         this._viewContainer.removeEventListener("transitioncancel", details.cancelListener);
         delete details.cancelListener;
         resolve();
       });
     });
 
     details.phase = TRANSITION_PHASES.END;
 
+    // Apply the final visibility, unless the view was closed in the meantime.
+    if (nextPanelView.node.panelMultiView == this.node) {
+      prevPanelView.visible = false;
+      nextPanelView.visible = true;
+      nextPanelView.descriptionHeightWorkaround();
+    }
+
     // This will complete the operation by removing any transition properties.
     await this._cleanupTransitionPhase(details);
 
     // Focus the correct element, unless the view was closed in the meantime.
     if (nextPanelView.node.panelMultiView == this.node) {
       nextPanelView.focusSelectedElement();
     }
   }
@@ -966,21 +951,18 @@ this.PanelMultiView = class extends this
   async _cleanupTransitionPhase(details = this._transitionDetails) {
     if (!details || !this.node)
       return;
 
     let {phase, previousViewNode, viewNode, reverse, resolve, listener, cancelListener, anchor} = details;
     if (details == this._transitionDetails)
       this._transitionDetails = null;
 
-    let nextPanelView = PanelView.forNode(viewNode);
-
     // Do the things we _always_ need to do whenever the transition ends or is
     // interrupted.
-    this.hideAllViewsExcept(nextPanelView);
     previousViewNode.removeAttribute("in-transition");
     viewNode.removeAttribute("in-transition");
 
     if (anchor)
       anchor.removeAttribute("open");
 
     if (phase >= TRANSITION_PHASES.START) {
       this._panel.removeAttribute("width");
@@ -1108,17 +1090,16 @@ this.PanelMultiView = class extends this
         PanelView.forNode(this._mainView).descriptionHeightWorkaround();
         break;
       case "popuphidden": {
         // WebExtensions consumers can hide the popup from viewshowing, or
         // mid-transition, which disrupts our state:
         this._transitioning = false;
         this.node.removeAttribute("panelopen");
         this._cleanupTransitionPhase();
-        this.hideAllViewsExcept(null);
         this.window.removeEventListener("keydown", this);
         this._panel.removeEventListener("mousemove", this);
         this.closeAllViews();
 
         // Clear the main view size caches. The dimensions could be different
         // when the popup is opened again, e.g. through touch mode sizing.
         this._viewContainer.style.removeProperty("min-height");
         this._viewStack.style.removeProperty("max-height");
@@ -1145,22 +1126,19 @@ this.PanelView = class extends this.Asso
   set mainview(value) {
     if (value) {
       this.node.setAttribute("mainview", true);
     } else {
       this.node.removeAttribute("mainview");
     }
   }
 
-  set current(value) {
+  set visible(value) {
     if (value) {
-      if (!this.node.hasAttribute("current")) {
-        this.node.setAttribute("current", true);
-        this.descriptionHeightWorkaround();
-      }
+      this.node.setAttribute("current", true);
     } else {
       this.node.removeAttribute("current");
     }
   }
 
   /**
    * Constrains the width of this view using the "min-width" and "max-width"
    * styles. Setting this to zero removes the constraints.