Backed out changeset 1ce48405e58a (bug 1447619) for creating bug 1457257 and bug 1457289 on a CLOSED TREE
authorAndreea Pavel <apavel@mozilla.com>
Sat, 28 Apr 2018 20:46:51 +0300
changeset 472270 cd79c351e29628d110cbd01af5fd54edafbedbec
parent 472267 f35994b437e3b0d9e61f74b34de2cf4cb80ab458
child 472293 44c33a20df275d8b7760493b8427f97f01ea3008
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1447619, 1457257, 1457289
milestone61.0a1
backs out1ce48405e58a609299a84192df04d340cd90de63
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 1ce48405e58a (bug 1447619) for creating bug 1457257 and bug 1457289 on a CLOSED TREE
browser/base/content/test/performance/browser_window_resize.js
browser/components/customizableui/CustomizableUI.jsm
browser/components/customizableui/test/browser_976792_insertNodeInWindow.js
browser/components/places/content/browserPlacesViews.js
--- a/browser/base/content/test/performance/browser_window_resize.js
+++ b/browser/base/content/test/performance/browser_window_resize.js
@@ -8,19 +8,37 @@
  * is a whitelist that should slowly go away as we improve the performance of
  * the front-end. Instead of adding more reflows to the whitelist, you should
  * be modifying your code to avoid the reflow.
  *
  * See https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers
  * for tips on how to do that.
  */
 const EXPECTED_REFLOWS = [
-   /**
-   * Nothing here! Please don't add anything new!
-   */
+  {
+    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,
+  },
 ];
 
 const gToolbar = document.getElementById("PersonalToolbar");
 
 /**
  * Sets the visibility state on the Bookmarks Toolbar, and
  * waits for it to transition to fully visible.
  *
--- a/browser/components/customizableui/CustomizableUI.jsm
+++ b/browser/components/customizableui/CustomizableUI.jsm
@@ -4403,46 +4403,28 @@ OverflowableToolbar.prototype = {
     doc.defaultView.updateEditUIVisibility();
     let contextMenuId = this._panel.getAttribute("context");
     if (contextMenuId) {
       let contextMenu = doc.getElementById(contextMenuId);
       gELS.removeSystemEventListener(contextMenu, "command", this, true);
     }
   },
 
-  /**
-   * 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)
+  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))
       return;
 
     let child = this._target.lastChild;
 
-    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) {
+    while (child && this._target.scrollLeftMin != this._target.scrollLeftMax) {
       let prevChild = child.previousSibling;
 
       if (child.getAttribute("overflows") != "false") {
         this._collapsed.set(child.id, this._target.clientWidth);
         child.setAttribute("overflowedItem", true);
         child.setAttribute("cui-anchorid", this._chevron.id);
         CustomizableUIInternal.ensureButtonContextMenu(child, this._toolbar, true);
         CustomizableUIInternal.notifyListeners("onWidgetOverflow", child, this._target);
@@ -4451,70 +4433,40 @@ OverflowableToolbar.prototype = {
         if (!this._addedListener) {
           CustomizableUI.addListener(this);
         }
         if (!CustomizableUI.isSpecialWidget(child.id)) {
           this._toolbar.setAttribute("overflowing", "true");
         }
       }
       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);
     }
     this._lazyResizeHandler.arm();
   },
 
-  /**
-   * 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) {
+  _moveItemsBackToTheirOrigin(shouldMoveAllItems) {
     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) {
-        if (!targetWidth) {
-          let dwu = win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils);
-          targetWidth = Math.floor(dwu.getBoundsWithoutFlushing(this._target).width);
-        }
-        if (targetWidth <= minSize) {
-          break;
-        }
+      if (!shouldMoveAllItems &&
+          minSize &&
+          this._target.clientWidth <= minSize) {
+        break;
       }
 
       this._collapsed.delete(child.id);
       let beforeNodeIndex = placements.indexOf(child.id) + 1;
       // If this is a skipintoolbarset item, meaning it doesn't occur in the placements list,
       // we're inserting it at the end. This will mean first-in, first-out (more or less)
       // leading to as little change in order as possible.
       if (beforeNodeIndex == 0) {
@@ -4536,43 +4488,37 @@ OverflowableToolbar.prototype = {
         this._target.appendChild(child);
       }
       child.removeAttribute("cui-anchorid");
       child.removeAttribute("overflowedItem");
       CustomizableUIInternal.ensureButtonContextMenu(child, this._target);
       CustomizableUIInternal.notifyListeners("onWidgetUnderflow", child, this._target);
     }
 
+    let win = this._target.ownerGlobal;
     win.UpdateUrlbarSearchSplitterState();
 
     let collapsedWidgetIds = Array.from(this._collapsed.keys());
     if (collapsedWidgetIds.every(w => CustomizableUI.isSpecialWidget(w))) {
       this._toolbar.removeAttribute("overflowing");
     }
     if (this._addedListener && !this._collapsed.size) {
       CustomizableUI.removeListener(this);
       this._addedListener = false;
     }
   },
 
-  async _onLazyResize() {
+  _onLazyResize() {
     if (!this._enabled)
       return;
 
-    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) {
+    if (this._target.scrollLeftMin != this._target.scrollLeftMax) {
       this.onOverflow();
     } else {
-      this._moveItemsBackToTheirOrigin(false, targetWidth);
+      this._moveItemsBackToTheirOrigin();
     }
   },
 
   _disable() {
     this._enabled = false;
     this._moveItemsBackToTheirOrigin(true);
     if (this._lazyResizeHandler) {
       this._lazyResizeHandler.disarm();
@@ -4657,17 +4603,17 @@ OverflowableToolbar.prototype = {
     } else if (aNode.previousSibling) {
       // but if it still is, it must have changed places. Bookkeep:
       let prevId = aNode.previousSibling.id;
       let minSize = this._collapsed.get(prevId);
       this._collapsed.set(aNode.id, minSize);
     } else {
       // If it's now the first item in the overflow list,
       // maybe we can return it:
-      this._moveItemsBackToTheirOrigin(false);
+      this._moveItemsBackToTheirOrigin();
     }
   },
 
   findOverflowedInsertionPoints(aNode) {
     let newNodeCanOverflow = aNode.getAttribute("overflows") != "false";
     let areaId = this._toolbar.id;
     let placements = gPlacements.get(areaId);
     let nodeIndex = placements.indexOf(aNode.id);
--- a/browser/components/customizableui/test/browser_976792_insertNodeInWindow.js
+++ b/browser/components/customizableui/test/browser_976792_insertNodeInWindow.js
@@ -221,24 +221,17 @@ add_task(async function() {
   }
 
   for (let id of widgetIds) {
     document.getElementById(id).style.minWidth = "200px";
   }
 
   let originalWindowWidth = window.outerWidth;
   window.resizeTo(kForceOverflowWidthPx, window.outerHeight);
-  // 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";
-  });
+  await waitForCondition(() => navbar.hasAttribute("overflowing"));
 
   // Find last widget that doesn't allow overflowing
   let nonOverflowing = navbar.customizationTarget.lastChild;
   is(nonOverflowing.getAttribute("overflows"), "false", "Last child is expected to not allow overflowing");
   isnot(nonOverflowing.getAttribute("skipintoolbarset"), "true", "Last child is expected to not be skipintoolbarset");
 
   let testWidgetId = kTestWidgetPrefix + 10;
   CustomizableUI.destroyWidget(testWidgetId);
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -1165,25 +1165,21 @@ PlacesToolbar.prototype = {
         // the overflow status of the toolbar.
         if (aEvent.target == aEvent.currentTarget) {
           this.updateNodesVisibility();
         }
         break;
       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":
       case "TabClose":
         this.updateNodesVisibility();
         break;
       case "dragstart":
         this._onDragStart(aEvent);