Bug 1428839 - Part 7 - Reduce calls to _cleanupTransitionPhase. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Tue, 27 Feb 2018 22:56:15 +0000
changeset 761008 28706bcd77ce1002ef633614002a99a791faa2eb
parent 761007 2a0bc71f245bc93095497d5f75ae6fc5f3ae505a
child 761009 c647f89d710631c7a3f6252b68967d606652158f
push id100802
push userpaolo.mozmail@amadzone.org
push dateWed, 28 Feb 2018 13:44:56 +0000
reviewersGijs
bugs1428839
milestone60.0a1
Bug 1428839 - Part 7 - Reduce calls to _cleanupTransitionPhase. r=Gijs The transition code now returns early if the panel was closed during an "await" statement. Given that transitions can only be interrupted when closing the panel, and the _cleanupTransitionPhase method handles exclusively state related to the panel rather than the individual views, it is now possible to call the _cleanupTransitionPhase method only when the panel is hidden or at the end of a transition. MozReview-Commit-ID: GYRKyyhJBPK
browser/components/customizableui/PanelMultiView.jsm
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -414,18 +414,16 @@ var PanelMultiView = class extends this.
     });
   }
 
   disconnect() {
     // Guard against re-entrancy.
     if (!this.node || !this.connected)
       return;
 
-    this._cleanupTransitionPhase();
-
     this._panel.removeEventListener("mousemove", this);
     this._panel.removeEventListener("popupshowing", this);
     this._panel.removeEventListener("popuppositioned", this);
     this._panel.removeEventListener("popupshown", this);
     this._panel.removeEventListener("popuphidden", this);
     this.window.removeEventListener("keydown", this);
     this.node = this._openPopupPromise = this._openPopupCancelCallback =
       this._viewContainer = this._viewStack = this.__dwu =
@@ -731,17 +729,17 @@ var 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;
 
-    this._cleanupTransitionPhase();
+    // Ensure the view will be visible once the panel is opened.
     nextPanelView.visible = true;
     nextPanelView.descriptionHeightWorkaround();
 
     return true;
   }
 
   /**
    * Opens the specified PanelView and dispatches the ViewShowing event, which
@@ -837,19 +835,16 @@ var PanelMultiView = class extends this.
    *                                     is about to be transitioned away. This
    *                                     must be already inactive at this point.
    * @param {panelview} viewNode         Node that will becode the active view,
    *                                     after the transition has finished.
    * @param {Boolean}   reverse          Whether we're navigation back to a
    *                                     previous view or forward to a next view.
    */
   async _transitionViews(previousViewNode, viewNode, reverse) {
-    // Clean up any previous transition that may be active at this point.
-    this._cleanupTransitionPhase();
-
     const { window } = this;
 
     let nextPanelView = PanelView.forNode(viewNode);
     let prevPanelView = PanelView.forNode(previousViewNode);
 
     let details = this._transitionDetails = {
       phase: TRANSITION_PHASES.START,
     };
@@ -892,16 +887,20 @@ var PanelMultiView = class extends this.
 
       // Now that the subview is visible, we can check the height of the
       // description elements it contains.
       nextPanelView.descriptionHeightWorkaround();
 
       viewRect = await window.promiseDocumentFlushed(() => {
         return this._dwu.getBoundsWithoutFlushing(viewNode);
       });
+      // Bail out if the panel was closed in the meantime.
+      if (!nextPanelView.isOpenIn(this)) {
+        return;
+      }
 
       try {
         this._viewStack.insertBefore(viewNode, oldSibling);
       } catch (ex) {
         this._viewStack.appendChild(viewNode);
       }
 
       this._offscreenViewStack.style.removeProperty("min-height");
@@ -929,16 +928,20 @@ var PanelMultiView = class extends this.
       " var(--panelui-subview-transition-duration)";
     this._viewStack.style.willChange = "transform";
     // Use an outline instead of a border so that the size is not affected.
     deepestNode.style.outline = "1px solid var(--panel-separator-color)";
 
     // Now that all the elements are in place for the start of the transition,
     // give the layout code a chance to set the initial values.
     await window.promiseDocumentFlushed(() => {});
+    // Bail out if the panel was closed in the meantime.
+    if (!nextPanelView.isOpenIn(this)) {
+      return;
+    }
 
     // Now set the viewContainer dimensions to that of the new view, which
     // kicks of the height animation.
     this._viewContainer.style.height = viewRect.height + "px";
     this._viewContainer.style.width = viewRect.width + "px";
     this._panel.removeAttribute("width");
     this._panel.removeAttribute("height");
     // We're setting the width property to prevent flickering during the
@@ -974,37 +977,33 @@ var PanelMultiView = class extends this.
     if (!nextPanelView.isOpenIn(this)) {
       return;
     }
     prevPanelView.visible = false;
 
     // This will complete the operation by removing any transition properties.
     nextPanelView.node.style.removeProperty("width");
     deepestNode.style.removeProperty("outline");
-    this._cleanupTransitionPhase(details);
+    this._cleanupTransitionPhase();
 
     nextPanelView.focusSelectedElement();
   }
 
   /**
    * Attempt to clean up the attributes and properties set by `_transitionViews`
    * above. Which attributes and properties depends on the phase the transition
    * was left from.
-   *
-   * @param {Object} details Dictionary object containing details of the transition
-   *                         that should be cleaned up after. Defaults to the most
-   *                         recent details.
    */
-  _cleanupTransitionPhase(details = this._transitionDetails) {
-    if (!details || !this.node)
+  _cleanupTransitionPhase() {
+    if (!this._transitionDetails) {
       return;
+    }
 
-    let {phase, resolve, listener, cancelListener} = details;
-    if (details == this._transitionDetails)
-      this._transitionDetails = null;
+    let {phase, resolve, listener, cancelListener} = this._transitionDetails;
+    this._transitionDetails = null;
 
     if (phase >= TRANSITION_PHASES.START) {
       this._panel.removeAttribute("width");
       this._panel.removeAttribute("height");
       this._viewContainer.style.removeProperty("height");
       this._viewContainer.style.removeProperty("width");
     }
     if (phase >= TRANSITION_PHASES.PREPARE) {