author | Andreea Pavel <apavel@mozilla.com> |
Sat, 28 Apr 2018 20:46:51 +0300 | |
changeset 416172 | cd79c351e29628d110cbd01af5fd54edafbedbec |
parent 416171 | f35994b437e3b0d9e61f74b34de2cf4cb80ab458 |
child 416173 | 44c33a20df275d8b7760493b8427f97f01ea3008 |
push id | 33918 |
push user | nerli@mozilla.com |
push date | Sun, 29 Apr 2018 09:47:13 +0000 |
treeherder | mozilla-central@afbec7f03bd8 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
bugs | 1447619, 1457257, 1457289 |
milestone | 61.0a1 |
backs out | 1ce48405e58a609299a84192df04d340cd90de63 |
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
|
--- 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);