Bug 1447619 - avoid reflowing when returning items to the toolbar from the overflow menu, r=florian

MozReview-Commit-ID: BFGRssWb9F

--HG--
extra : rebase_source : e8932810584e10a1de0c1b11c1a152d2dc0f48dd

browser/base/content/test/performance/browser_window_resize.js

@@ -13,27 +13,9 @@
  * for tips on how to do that.
  */
 const EXPECTED_REFLOWS = [
-  {
-    stack: [
-      "onOverflow@resource:///modules/CustomizableUI.jsm",
-    ],
-    maxCount: 48,
-  },
-
-  {
-    stack: [
-      "_moveItemsBackToTheirOrigin@resource:///modules/CustomizableUI.jsm",
-      "_onLazyResize@resource:///modules/CustomizableUI.jsm",
-    ],
-    maxCount: 5,
-  },
-
-  {
-    stack: [
-      "_onLazyResize@resource:///modules/CustomizableUI.jsm",
-    ],
-    maxCount: 4,
-  },
+   /**
+   * Nothing here! Please don't add anything new!
+   */
 ];
 
 const gToolbar = document.getElementById("PersonalToolbar");

browser/components/customizableui/CustomizableUI.jsm

@@ -4408,18 +4408,36 @@ OverflowableToolbar.prototype = {
     }
   },
 
-  onOverflow(aEvent) {
-    // The rangeParent check is here because of bug 1111986 and ensuring that
-    // overflow events from the bookmarks toolbar items or similar things that
-    // manage their own overflow don't trigger an overflow on the entire toolbar
-    if (!this._enabled ||
-        (aEvent && aEvent.target != this._toolbar.customizationTarget) ||
-        (aEvent && aEvent.rangeParent))
+  /**
+   * Avoid re-entrancy in the overflow handling by keeping track of invocations:
+   */
+  _lastOverflowCounter: 0,
+
+  /**
+   * Handle overflow in the toolbar by moving items to the overflow menu.
+   * @param {Event} aEvent
+   *        The overflow event that triggered handling overflow. May be omitted
+   *        in some cases (e.g. when we run this method after overflow handling
+   *        is re-enabled from customize mode, to ensure correct handling of
+   *        initial overflow).
+   */
+  async onOverflow(aEvent) {
+    if (!this._enabled)
       return;
 
     let child = this._target.lastChild;
 
-    while (child && this._target.scrollLeftMin != this._target.scrollLeftMax) {
+    let thisOverflowResponse = ++this._lastOverflowCounter;
+
+    let win = this._target.ownerGlobal;
+    let [scrollLeftMin, scrollLeftMax] = await win.promiseDocumentFlushed(() => {
+      return [this._target.scrollLeftMin, this._target.scrollLeftMax];
+    });
+    if (win.closed || this._lastOverflowCounter != thisOverflowResponse) {
+      return;
+    }
+
+    while (child && scrollLeftMin != scrollLeftMax) {
       let prevChild = child.previousSibling;
 
       if (child.getAttribute("overflows") != "false") {
@@ -4438,13 +4456,26 @@ OverflowableToolbar.prototype = {
         }
       }
       child = prevChild;
+      [scrollLeftMin, scrollLeftMax] = await win.promiseDocumentFlushed(() => {
+        return [this._target.scrollLeftMin, this._target.scrollLeftMax];
+      });
+      // If the window has closed or if we re-enter because we were waiting
+      // for layout, stop.
+      if (win.closed || this._lastOverflowCounter != thisOverflowResponse) {
+        return;
+      }
     }
 
-    let win = this._target.ownerGlobal;
     win.UpdateUrlbarSearchSplitterState();
+    // Reset the counter because we finished handling overflow.
+    this._lastOverflowCounter = 0;
   },
 
   _onResize(aEvent) {
+    // Ignore bubbled-up resize events.
+    if (aEvent.target != aEvent.target.ownerGlobal.top) {
+      return;
+    }
     if (!this._lazyResizeHandler) {
       this._lazyResizeHandler = new DeferredTask(this._onLazyResize.bind(this),
                                                  LAZY_RESIZE_INTERVAL_MS, 0);
@@ -4452,17 +4483,34 @@ OverflowableToolbar.prototype = {
     this._lazyResizeHandler.arm();
   },
 
-  _moveItemsBackToTheirOrigin(shouldMoveAllItems) {
+  /**
+   * Try to move toolbar items back to the toolbar from the overflow menu.
+   * @param {boolean} shouldMoveAllItems
+   *        Whether we should move everything (e.g. because we're being disabled)
+   * @param {number} targetWidth
+   *        Optional; the width of the toolbar in which we can put things.
+   *        Some consumers pass this to avoid reflows.
+   *        While there are items in the list, this width won't change, and so
+   *        we can avoid flushing layout by providing it and/or caching it.
+   *        Note that if `shouldMoveAllItems` is true, we never need the width
+   *        anyway.
+   */
+  _moveItemsBackToTheirOrigin(shouldMoveAllItems, targetWidth) {
     let placements = gPlacements.get(this._toolbar.id);
+    let win = this._target.ownerGlobal;
     while (this._list.firstChild) {
       let child = this._list.firstChild;
       let minSize = this._collapsed.get(child.id);
 
-      if (!shouldMoveAllItems &&
-          minSize &&
-          this._target.clientWidth <= minSize) {
+      if (!shouldMoveAllItems && minSize) {
+        if (!targetWidth) {
+          let dwu = win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils);
+          targetWidth = Math.floor(dwu.getBoundsWithoutFlushing(this._target).width);
+        }
+        if (targetWidth <= minSize) {
           break;
         }
+      }
 
       this._collapsed.delete(child.id);
       let beforeNodeIndex = placements.indexOf(child.id) + 1;
@@ -4493,7 +4541,6 @@ OverflowableToolbar.prototype = {
       CustomizableUIInternal.notifyListeners("onWidgetUnderflow", child, this._target);
     }
 
-    let win = this._target.ownerGlobal;
     win.UpdateUrlbarSearchSplitterState();
 
     let collapsedWidgetIds = Array.from(this._collapsed.keys());
@@ -4506,14 +4553,21 @@ OverflowableToolbar.prototype = {
     }
   },
 
-  _onLazyResize() {
+  async _onLazyResize() {
     if (!this._enabled)
       return;
 
-    if (this._target.scrollLeftMin != this._target.scrollLeftMax) {
+    let win = this._target.ownerGlobal;
+    let [min, max, targetWidth] = await win.promiseDocumentFlushed(() => {
+      return [this._target.scrollLeftMin, this._target.scrollLeftMax, this._target.clientWidth];
+    });
+    if (win.closed) {
+      return;
+    }
+    if (min != max) {
       this.onOverflow();
     } else {
-      this._moveItemsBackToTheirOrigin();
+      this._moveItemsBackToTheirOrigin(false, targetWidth);
     }
   },
 
@@ -4608,7 +4662,7 @@ OverflowableToolbar.prototype = {
     } else {
       // If it's now the first item in the overflow list,
       // maybe we can return it:
-      this._moveItemsBackToTheirOrigin();
+      this._moveItemsBackToTheirOrigin(false);
     }
   },
 

browser/components/customizableui/test/browser_976792_insertNodeInWindow.js

@@ -226,7 +226,14 @@ add_task(async function() {
 
   let originalWindowWidth = window.outerWidth;
   window.resizeTo(kForceOverflowWidthPx, window.outerHeight);
-  await waitForCondition(() => navbar.hasAttribute("overflowing"));
+  // Wait for all the widgets to overflow. We can't just wait for the
+  // `overflowing` attribute because we leave time for layout flushes
+  // inbetween, so it's possible for the timeout to run before the
+  // navbar has "settled"
+  await waitForCondition(() => {
+    return navbar.hasAttribute("overflowing") &&
+      navbar.customizationTarget.lastChild.getAttribute("overflows") == "false";
+  });
 
   // Find last widget that doesn't allow overflowing
   let nonOverflowing = navbar.customizationTarget.lastChild;

browser/components/places/content/browserPlacesViews.js

@@ -1172,11 +1172,15 @@ PlacesToolbar.prototype = {
       case "overflow":
         if (!this._isOverflowStateEventRelevant(aEvent))
           return;
+        // Avoid triggering overflow in containers if possible
+        aEvent.stopPropagation();
         this._onOverflow();
         break;
       case "underflow":
         if (!this._isOverflowStateEventRelevant(aEvent))
           return;
+        // Avoid triggering underflow in containers if possible
+        aEvent.stopPropagation();
         this._onUnderflow();
         break;
       case "TabOpen":