Closed Bug 1437825 Opened 4 years ago Closed 4 years ago

browser_urlbar_keyed_search_reflows.js and browser_urlbar_search_reflows.js failing locally - unexpected uninterruptible reflow @_onLazyResize

Categories

(Firefox :: Address Bar, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: florian, Assigned: florian)

Details

Attachments

(1 file, 1 obsolete file)

browser_urlbar_keyed_search_reflows.js and browser_urlbar_search_reflows.js are failing locally for me on Mac, and mconley confirmed he's experiencing the same behavior.

It turns out CustomizableUI.jsm runs some code doing sync reflows off of a 200ms timer after receiving a resize event. We should force this to happen immediately being sending events to the urlbar.
Attached patch Patch (obsolete) — Splinter Review
Attachment #8950545 - Flags: review?(mconley)
Comment on attachment 8950545 [details] [diff] [review]
Patch

Review of attachment 8950545 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, florian! I have one suggestion, see below.

Also, I suspect the reflows you mentioned are tracked in bug 1358388.

::: browser/base/content/test/performance/browser_urlbar_keyed_search_reflows.js
@@ +122,5 @@
>   */
>  add_task(async function() {
>    let win = await BrowserTestUtils.openNewBrowserWindow();
>    await ensureNoPreloadedBrowser(win);
> +  ensureNoPendingLazyResize(win);

Perhaps we should combine this with ensureNoPreloadedBrowser into a single function, like prepareSettledWindow or something.
Attachment #8950545 - Flags: review?(mconley) → review-
(In reply to Mike Conley (:mconley) (:⚙️) (Totally backlogged on reviews and needinfos) from comment #2)

> Also, I suspect the reflows you mentioned are tracked in bug 1358388.

The reflows annoying me here are:
	"_onLazyResize@resource:///modules/CustomizableUI.jsm:4445:1",
	"_runTask@resource://gre/modules/DeferredTask.jsm:308:15",
	"async*_timerCallback/<@resource://gre/modules/DeferredTask.jsm:278:13",
	"async*_timerCallback@resource://gre/modules/DeferredTask.jsm:276:30",

That's https://searchfox.org/mozilla-central/rev/9d47b5fb14152865ad1375eb3ee2e571d81ecdb9/browser/components/customizableui/CustomizableUI.jsm#4445
    if (this._target.scrollLeftMin != this._target.scrollLeftMax) {
so not the _moveItemsBackToTheirOrigin function covered by bug 1358388.
Attached patch Patch v2Splinter Review
Attachment #8950795 - Flags: review?(mconley)
Attachment #8950545 - Attachment is obsolete: true
Comment on attachment 8950795 [details] [diff] [review]
Patch v2

Review of attachment 8950795 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8950795 - Flags: review?(mconley) → review+
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5654cb32980e
browser_urlbar_*search_reflows.js tests should force CustomizableUI.jsm's deferred resize code to run before the actual tests starts, r=mconley.
https://hg.mozilla.org/mozilla-central/rev/5654cb32980e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in before you can comment on or make changes to this bug.