Bug 1356532: Part 4 - Avoid layout flushes during adjustSiteIconStart() and _handleOverflow(). r?florian
MozReview-Commit-ID: LBbElXZOEnf
--- a/browser/base/content/test/performance/browser_urlbar_search_reflows.js
+++ b/browser/base/content/test/performance/browser_urlbar_search_reflows.js
@@ -30,49 +30,20 @@ const EXPECTED_REFLOWS_FIRST_OPEN = [
"openPopup@chrome://global/content/bindings/autocomplete.xml",
"set_popupOpen@chrome://global/content/bindings/autocomplete.xml"
],
times: 1, // This number should only ever go down - never up.
},
{
stack: [
- "adjustHeight@chrome://global/content/bindings/autocomplete.xml",
- "onxblpopupshown@chrome://global/content/bindings/autocomplete.xml"
- ],
- times: 5, // This number should only ever go down - never up.
- },
-
- {
- stack: [
- "_handleOverflow@chrome://global/content/bindings/autocomplete.xml",
- "handleOverUnderflow@chrome://global/content/bindings/autocomplete.xml",
- "set_siteIconStart@chrome://global/content/bindings/autocomplete.xml",
- ],
- times: 6, // This number should only ever go down - never up.
- },
-
- {
- stack: [
- "adjustHeight@chrome://global/content/bindings/autocomplete.xml",
- "_invalidate/this._adjustHeightTimeout<@chrome://global/content/bindings/autocomplete.xml",
+ "forceReflow@resource://gre/modules/BrowserUtils.jsm",
],
times: 3, // This number should only ever go down - never up.
- },
-
- {
- stack: [
- "_handleOverflow@chrome://global/content/bindings/autocomplete.xml",
- "handleOverUnderflow@chrome://global/content/bindings/autocomplete.xml",
- "_reuseAcItem@chrome://global/content/bindings/autocomplete.xml",
- "_appendCurrentResult@chrome://global/content/bindings/autocomplete.xml",
- "_invalidate@chrome://global/content/bindings/autocomplete.xml",
- "invalidate@chrome://global/content/bindings/autocomplete.xml"
- ],
- times: 390, // This number should only ever go down - never up.
+ minTimes: 2,
},
{
stack: [
"_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openPopup@chrome://global/content/bindings/autocomplete.xml",
"set_popupOpen@chrome://global/content/bindings/autocomplete.xml",
@@ -89,44 +60,16 @@ const EXPECTED_REFLOWS_FIRST_OPEN = [
"openPopup@chrome://global/content/bindings/autocomplete.xml",
"set_popupOpen@chrome://global/content/bindings/autocomplete.xml",
],
},
];
/* These reflows happen everytime the awesomebar panel opens. */
const EXPECTED_REFLOWS_SECOND_OPEN = [
- {
- stack: [
- "adjustHeight@chrome://global/content/bindings/autocomplete.xml",
- "onxblpopupshown@chrome://global/content/bindings/autocomplete.xml"
- ],
- times: 3, // This number should only ever go down - never up.
- },
-
- {
- stack: [
- "adjustHeight@chrome://global/content/bindings/autocomplete.xml",
- "_invalidate/this._adjustHeightTimeout<@chrome://global/content/bindings/autocomplete.xml",
- ],
- times: 3, // This number should only ever go down - never up.
- },
-
- {
- stack: [
- "_handleOverflow@chrome://global/content/bindings/autocomplete.xml",
- "handleOverUnderflow@chrome://global/content/bindings/autocomplete.xml",
- "_reuseAcItem@chrome://global/content/bindings/autocomplete.xml",
- "_appendCurrentResult@chrome://global/content/bindings/autocomplete.xml",
- "_invalidate@chrome://global/content/bindings/autocomplete.xml",
- "invalidate@chrome://global/content/bindings/autocomplete.xml"
- ],
- times: 444, // This number should only ever go down - never up.
- },
-
// Bug 1384256
{
stack: [
"_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openPopup@chrome://global/content/bindings/autocomplete.xml",
"set_popupOpen@chrome://global/content/bindings/autocomplete.xml",
],
@@ -138,16 +81,24 @@ const EXPECTED_REFLOWS_SECOND_OPEN = [
stack: [
"openPopup@chrome://global/content/bindings/popup.xml",
"_openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openAutocompletePopup@chrome://browser/content/urlbarBindings.xml",
"openPopup@chrome://global/content/bindings/autocomplete.xml",
"set_popupOpen@chrome://global/content/bindings/autocomplete.xml",
],
},
+
+ {
+ stack: [
+ "forceReflow@resource://gre/modules/BrowserUtils.jsm",
+ ],
+ times: 3, // This number should only ever go down - never up.
+ minTimes: 1,
+ },
];
const SEARCH_TERM = "urlbar-reflows";
add_task(async function setup() {
await addDummyHistoryEntries();
});
--- a/browser/base/content/test/performance/head.js
+++ b/browser/base/content/test/performance/head.js
@@ -1,13 +1,22 @@
"use strict";
XPCOMUtils.defineLazyModuleGetter(this, "PlacesTestUtils",
"resource://testing-common/PlacesTestUtils.jsm");
+const {ReflowScheduler} = Cu.import("resource://gre/modules/BrowserUtils.jsm", {});
+
+// Force a reflow that doesn't immediately dirty the layout state, so
+// that promiseLayoutFlushed callbacks can query layout state with
+// impunity.
+ReflowScheduler.prototype.maybeForceReflow = function maybeForceReflow() {
+ this.forceReflow();
+};
+
/**
* Async utility function for ensuring that no unexpected uninterruptible
* reflows occur during some period of time in a window.
*
* @param testFn (async function)
* The async function that will exercise the browser activity that is
* being tested for reflows.
*
@@ -49,16 +58,17 @@ XPCOMUtils.defineLazyModuleGetter(this,
* Note that line numbers are not included in the stacks.
*
* Order of the reflows doesn't matter. Expected reflows that aren't seen
* will cause an assertion failure. When this argument is not passed,
* it defaults to the empty Array, meaning no reflows are expected.
* @param window (browser window, optional)
* The browser window to monitor. Defaults to the current window.
*/
+let times = {};
async function withReflowObserver(testFn, expectedReflows = [], win = window) {
let dwu = win.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIDOMWindowUtils);
let dirtyFrameFn = () => {
try {
dwu.ensureDirtyRootFrame();
} catch (e) {
// If this fails, we should probably make note of it, but it's not fatal.
@@ -83,17 +93,22 @@ async function withReflowObserver(testFn
// Gather information about the current code path, slicing out the current
// frame.
let path = (new Error().stack).split("\n").slice(1).map(line => {
return line.replace(/:\d+:\d+$/, "");
}).join("|");
let pathWithLineNumbers = (new Error().stack).split("\n").slice(1);
- // Just in case, dirty the frame now that we've reflowed.
+ // Reflows triggered by promiseLayoutFlushed callbacks get a free pass.
+ if (path.startsWith("forceReflow@resource://gre/modules/BrowserUtils.jsm|" +
+ "maybeForceReflow@chrome://mochitests/content/browser/browser/base/content/test/performance/head.js")) {
+ return;
+ }
+
dirtyFrameFn();
// Stack trace is empty. Reflow was triggered by native code, which
// we ignore.
if (path === "") {
return;
}
@@ -103,16 +118,17 @@ async function withReflowObserver(testFn
return;
}
let index = expectedReflows.findIndex(reflow => path.startsWith(reflow.stack.join("|")));
if (index != -1) {
Assert.ok(true, "expected uninterruptible reflow: '" +
JSON.stringify(pathWithLineNumbers, null, "\t") + "'");
+ --expectedReflows[index].minTimes;
if (--expectedReflows[index].times == 0) {
expectedReflows.splice(index, 1);
}
} else {
Assert.ok(false, "unexpected uninterruptible reflow \n" +
JSON.stringify(pathWithLineNumbers, null, "\t") + "\n");
}
},
@@ -134,19 +150,20 @@ async function withReflowObserver(testFn
els.addListenerForAllEvents(win, dirtyFrameFn, true);
try {
dirtyFrameFn();
await testFn(dirtyFrameFn);
} finally {
for (let remainder of expectedReflows) {
- Assert.ok(false,
+ Assert.ok(remainder.minTimes <= 0,
`Unused expected reflow: ${JSON.stringify(remainder.stack, null, "\t")}\n` +
- `This reflow was supposed to be hit ${remainder.times} more time(s).\n` +
+ `This reflow was supposed to be hit ${remainder.minTimes || remainder.times} ` +
+ `more time(s).\n` +
"This is probably a good thing - just remove it from the " +
"expected list.");
}
els.removeListenerForAllEvents(win, dirtyFrameFn, true);
docShell.removeWeakReflowObserver(observer);
}
}
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1431,20 +1431,17 @@ extends="chrome://global/content/binding
items' site icons (favicons). -->
<property name="siteIconStart"
onget="return this._siteIconStart;">
<setter>
<![CDATA[
if (val != this._siteIconStart) {
this._siteIconStart = val;
for (let item of this.richlistbox.childNodes) {
- let changed = item.adjustSiteIconStart(val);
- if (changed) {
- item.handleOverUnderflow();
- }
+ item.adjustSiteIconStart(val);
}
}
return val;
]]>
</setter>
</property>
<property name="overflowPadding"
@@ -2079,21 +2076,17 @@ extends="chrome://global/content/binding
]]>
</body>
</method>
<method name="_onChanged">
<body>
<![CDATA[
let popup = this.parentNode.parentNode;
- let iconChanged = this.adjustSiteIconStart(popup._siteIconStart);
-
- if (iconChanged) {
- this.handleOverUnderflow();
- }
+ this.adjustSiteIconStart(popup._siteIconStart);
]]>
</body>
</method>
<method name="_reuseAcItem">
<body>
<![CDATA[
@@ -2107,19 +2100,16 @@ extends="chrome://global/content/binding
action.type != "searchengine" ||
!popup.overrideSearchEngineName ||
action.params.engineName == popup.overrideSearchEngineName) {
this.collapsed = false;
// Call adjustSiteIconStart only after setting collapsed=
// false. The calculations it does may be wrong otherwise.
this.adjustSiteIconStart(popup._siteIconStart);
- // The popup may have changed size between now and the last
- // time the item was shown, so always handle over/underflow.
- this.handleOverUnderflow();
return true;
}
return false;
]]>
</body>
</method>
@@ -2374,108 +2364,124 @@ extends="chrome://global/content/binding
@param newStart The new x-coordinate, relative to the leading edge of
the window. Pass undefined to reset the icon's position to
whatever is specified in CSS.
@return True if the icon's position changed, false if not. -->
<method name="adjustSiteIconStart">
<parameter name="newStart"/>
<body>
<![CDATA[
- if (typeof(newStart) != "number") {
- this._typeIconSpacer.style.removeProperty("width");
- return true;
- }
-
- let utils = window.QueryInterface(Ci.nsIInterfaceRequestor)
- .getInterface(Ci.nsIDOMWindowUtils);
- let rect = utils.getBoundsWithoutFlushing(this._siteIcon);
-
- let dir = this.getAttribute("dir");
- let delta = dir == "rtl" ? rect.right - Math.round(newStart)
- : Math.round(newStart) - rect.left;
- if (delta) {
- let currentSpacerWidth = this._typeIconSpacer.style.width || "0px";
- this._typeIconSpacer.style.width =
- (parseInt(currentSpacerWidth, 10) + delta) + "px";
- return true;
- }
-
- return false;
+ this._siteIconStart = Math.round(newStart);
+ this.handleOverUnderflow();
]]>
</body>
</method>
<!-- This method truncates the displayed strings as necessary. -->
<method name="_handleOverflow">
<body><![CDATA[
- let itemRect = this.parentNode.getBoundingClientRect();
- let titleRect = this._titleText.getBoundingClientRect();
- let tagsRect = this._tagsText.getBoundingClientRect();
- let separatorRect = this._separator.getBoundingClientRect();
- let urlRect = this._urlText.getBoundingClientRect();
- let actionRect = this._actionText.getBoundingClientRect();
- let separatorURLActionWidth =
- separatorRect.width + Math.max(urlRect.width, actionRect.width);
-
- // Total width for the title and URL/action is the width of the item
- // minus the start of the title text minus a little optional extra padding.
- // This extra padding amount is basically arbitrary but keeps the text
- // from getting too close to the popup's edge.
- let dir = this.getAttribute("dir");
- let titleStart = dir == "rtl" ? itemRect.right - titleRect.right
- : titleRect.left - itemRect.left;
-
- let popup = this.parentNode.parentNode;
- let itemWidth = itemRect.width - titleStart - popup.overflowPadding;
-
- if (this._tags.hasAttribute("empty")) {
- // The tags box is not displayed in this case.
- tagsRect.width = 0;
+ if (this._overflowPromise) {
+ return;
}
- let titleTagsWidth = titleRect.width + tagsRect.width;
- if (titleTagsWidth + separatorURLActionWidth > itemWidth) {
- // Title + tags + URL/action overflows the item width.
+ this._overflowPromise = (async () => {
+ if (typeof this._siteIconStart != "number") {
+ this.typeIconSpacer.style.removeProperty("width");
+ }
- // The percentage of the item width allocated to the title and tags.
- let titleTagsPct = 0.66;
+ let [itemRect, iconSpacerRect, siteIconRect, titleRect, tagsRect, separatorRect, urlRect, actionRect] =
+ await BrowserUtils.promiseSlowLayoutFlush(document, "layout", () => [
+ this.parentNode.getBoundingClientRect(),
+ this._typeIconSpacer.getBoundingClientRect(),
+ this._siteIcon.getBoundingClientRect(),
+ this._titleText.getBoundingClientRect(),
+ this._tagsText.getBoundingClientRect(),
+ this._separator.getBoundingClientRect(),
+ this._urlText.getBoundingClientRect(),
+ this._actionText.getBoundingClientRect(),
+ ]);
+
+ this._overflowPromise = null;
- let titleTagsAvailable = itemWidth - separatorURLActionWidth;
- let titleTagsMaxWidth = Math.max(
- titleTagsAvailable,
- itemWidth * titleTagsPct
- );
- if (titleTagsWidth > titleTagsMaxWidth) {
- // Title + tags overflows the max title + tags width.
+ let dir = this.getAttribute("dir");
+ let typeIconDelta = 0;
+ if (typeof this._siteIconStart == "number") {
+ typeIconDelta = dir == "rtl" ? siteIconRect.right - this._siteIconStart
+ : this._siteIconStart - siteIconRect.left;
- // The percentage of the title + tags width allocated to the
- // title.
- let titlePct = 0.33;
+ if (typeIconDelta != 0) {
+ let newWidth = typeIconDelta + iconSpacerRect.width;
+ BrowserUtils.promiseAnimationFrame(window, () => {
+ this._typeIconSpacer.style.width = `${newWidth}px`;
+ });
+ }
+ }
+
+ let separatorURLActionWidth =
+ separatorRect.width + Math.max(urlRect.width, actionRect.width) + typeIconDelta;
+
+ // Total width for the title and URL/action is the width of the item
+ // minus the start of the title text minus a little optional extra padding.
+ // This extra padding amount is basically arbitrary but keeps the text
+ // from getting too close to the popup's edge.
+ let titleStart = dir == "rtl" ? itemRect.right - titleRect.right
+ : titleRect.left - itemRect.left;
+
+ let popup = this.parentNode.parentNode;
+ let itemWidth = itemRect.width - titleStart - popup.overflowPadding;
- let titleAvailable = titleTagsMaxWidth - tagsRect.width;
- let titleMaxWidth = Math.max(
- titleAvailable,
- titleTagsMaxWidth * titlePct
- );
- let tagsAvailable = titleTagsMaxWidth - titleRect.width;
- let tagsMaxWidth = Math.max(
- tagsAvailable,
- titleTagsMaxWidth * (1 - titlePct)
+ if (this._tags.hasAttribute("empty")) {
+ // The tags box is not displayed in this case.
+ tagsRect.width = 0;
+ }
+
+ let titleTagsWidth = titleRect.width + tagsRect.width;
+ if (titleTagsWidth + separatorURLActionWidth > itemWidth) {
+ // Title + tags + URL/action overflows the item width.
+
+ // The percentage of the item width allocated to the title and tags.
+ let titleTagsPct = 0.66;
+
+ let titleTagsAvailable = itemWidth - separatorURLActionWidth;
+ let titleTagsMaxWidth = Math.max(
+ titleTagsAvailable,
+ itemWidth * titleTagsPct
);
- this._titleText.style.maxWidth = titleMaxWidth + "px";
- this._tagsText.style.maxWidth = tagsMaxWidth + "px";
+
+ await BrowserUtils.promiseAnimationFrame(window, () => {
+ if (titleTagsWidth > titleTagsMaxWidth) {
+ // Title + tags overflows the max title + tags width.
+
+ // The percentage of the title + tags width allocated to the
+ // title.
+ let titlePct = 0.33;
+
+ let titleAvailable = titleTagsMaxWidth - tagsRect.width;
+ let titleMaxWidth = Math.max(
+ titleAvailable,
+ titleTagsMaxWidth * titlePct
+ );
+ let tagsAvailable = titleTagsMaxWidth - titleRect.width;
+ let tagsMaxWidth = Math.max(
+ tagsAvailable,
+ titleTagsMaxWidth * (1 - titlePct)
+ );
+ this._titleText.style.maxWidth = titleMaxWidth + "px";
+ this._tagsText.style.maxWidth = tagsMaxWidth + "px";
+ }
+ let urlActionMaxWidth = Math.max(
+ itemWidth - titleTagsWidth,
+ itemWidth * (1 - titleTagsPct)
+ );
+ urlActionMaxWidth -= separatorRect.width;
+ this._urlText.style.maxWidth = urlActionMaxWidth + "px";
+ this._actionText.style.maxWidth = urlActionMaxWidth + "px";
+ });
}
- let urlActionMaxWidth = Math.max(
- itemWidth - titleTagsWidth,
- itemWidth * (1 - titleTagsPct)
- );
- urlActionMaxWidth -= separatorRect.width;
- this._urlText.style.maxWidth = urlActionMaxWidth + "px";
- this._actionText.style.maxWidth = urlActionMaxWidth + "px";
- }
+ })();
]]></body>
</method>
<method name="handleOverUnderflow">
<body>
<![CDATA[
this._removeMaxWidths();
this._handleOverflow();
--- a/toolkit/modules/BrowserUtils.jsm
+++ b/toolkit/modules/BrowserUtils.jsm
@@ -42,25 +42,31 @@ ReflowScheduler.get = function(doc) {
reflowSchedulers.set(doc, scheduler);
}
return scheduler;
}
ReflowScheduler.prototype = {
QueryInterface: XPCOMUtils.generateQI(["nsIReflowObserver", "nsISupportsWeakReference"]),
+ // This is overridden by mochitests to allow us skip dirtying the
+ // layout state for layoutFlushed callbacks.
+ maybeForceReflow() {},
+
_onReflow() {
this._doc.docShell.removeWeakReflowObserver(this);
this._reflowRequested = false;
if (this._idleRequest) {
this._win.cancelIdleCallback(this._idleRequest);
this._idleRequest = null;
}
+ this.maybeForceReflow();
+
for (let callback of this.reflowCallbacks.slice()) {
try {
callback();
} catch (e) {
Cu.reportError(e);
}
}
},