Closed Bug 1384582 Opened 4 years ago Closed 4 years ago

Re-enable browser_urlbar_search_reflows.js for macOS opt builds

Categories

(Firefox :: Address Bar, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 58
Tracking Status
firefox57 --- fix-optional
firefox58 --- fixed

People

(Reporter: mconley, Assigned: daleharvey)

Details

Attachments

(1 file)

browser_urlbar_search_reflows.js was added in bug 1356271, but disabled for opt macOS due to frequent failures with this stack:

21:09:21     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_urlbar_search_reflows.js | unexpected uninterruptible reflow 
21:09:21     INFO - [
21:09:21     INFO - 	"_onLazyResize@resource:///modules/CustomizableUI.jsm:4376:1",
21:09:21     INFO - 	"_runTask@resource://gre/modules/DeferredTask.jsm:304:20",
21:09:21     INFO - 	"async*_timerCallback/<@resource://gre/modules/DeferredTask.jsm:277:13",
21:09:21     INFO - 	"async*_timerCallback@resource://gre/modules/DeferredTask.jsm:275:30",
21:09:21     INFO - 	""
21:09:21     INFO - ]
21:09:21     INFO -  - false == true - JS frame :: chrome://mochitests/content/browser/browser/base/content/test/performance/head.js :: reflow :: line 104
21:09:21     INFO - Stack trace:
21:09:21     INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:reflow:104
21:09:21     INFO - resource:///modules/CustomizableUI.jsm:_onLazyResize:4376
21:09:21     INFO - resource://gre/modules/DeferredTask.jsm:_runTask:304
21:09:21     INFO - Not taking screenshot here: see the one that was previously logged
21:09:21     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_urlbar_search_reflows.js | unexpected uninterruptible reflow 
21:09:21     INFO - [
21:09:21     INFO - 	"_onLazyResize@resource:///modules/CustomizableUI.jsm:4376:1",
21:09:21     INFO - 	"_runTask@resource://gre/modules/DeferredTask.jsm:304:20",
21:09:21     INFO - 	"async*_timerCallback/<@resource://gre/modules/DeferredTask.jsm:277:13",
21:09:21     INFO - 	"async*_timerCallback@resource://gre/modules/DeferredTask.jsm:275:30",
21:09:21     INFO - 	""
21:09:21     INFO - ]
21:09:21     INFO -  - false == true - JS frame :: chrome://mochitests/content/browser/browser/base/content/test/performance/head.js :: reflow :: line 104
21:09:21     INFO - Stack trace:
21:09:21     INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:reflow:104
21:09:21     INFO - resource:///modules/CustomizableUI.jsm:_onLazyResize:4376
21:09:21     INFO - resource://gre/modules/DeferredTask.jsm:_runTask:304

We should figure out what's going on there and then re-enable the test.
Summary: Re-enabled browser_urlbar_search_reflows.js for macOS opt builds → Re-enable browser_urlbar_search_reflows.js for macOS opt builds
Been taking a look at this if nobody else is picking it up
Assignee: nobody → dharvey
Just some WIP comments, so this test has never failed on a single run, only as part of --verify, and in --verify it seems to fail fairly early on which makes me think that something isnt completely isolated during the test runs

It is no longer failing with the above stack, its now runs an expected reflow stack (http://searchfox.org/mozilla-central/source/browser/base/content/test/performance/browser_urlbar_search_reflows.js#64) too many times, the current times is 6 however 60(!) is needed to pass, 60 is the exact minimum, if I set it to 59 it will fail pretty reliably after 219 tests

Trying to figure out the reason for the 4th test run (test suite is 83 tests, 219 failing is the 4th run) being different from the other 3
ok so first bug after spending too long looking for it is simple, this is never going to pass with --verify as it assumes a fresh browser instance when first opening the awesomebar, the second time the tests run it has to clear the items from the awesomebar from the previous run @ http://searchfox.org/mozilla-central/source/browser/base/content/urlbarBindings.xml#1970 (1st run childnodes.length === 1, 2nd run childnodes.length === 10), not entirely sure what the bet way to do cleanup here is
So the issue causing --verify to permanently fail is that places will return stale results to a query across new windows even with clearHistory being called, per maks suggestion I give each test its own unique id (just a timestamp) which prevents subsequent test runs from interfering with each others results
Comment on attachment 8926366 [details]
Bug 1384582 - Ensure browser_urlbar_search_reflows.js test runs are isolated.

https://reviewboard.mozilla.org/r/197652/#review202892

Thanks for figuring this out, daleharvey!

::: commit-message-57482:1
(Diff revision 1)
> +Bug 1384582 - Ensure reflow test runs are isolated. r?mconley

Nit: let's maybe make it clear that we're making sure that the browser_urlbar_search_reflows.js tests are isolated.

Probably wouldn't hurt to briefly explain what you mean by "isolated" in a short paragraph below too, but I won't harp on that. :)

::: browser/base/content/test/performance/head.js:4
(Diff revision 1)
>  "use strict";
>  
> +XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
> +                                  "resource://gre/modules/PlacesUtils.jsm");

Nit: can you align this like the `defineLazyModuleGetter` on the next line?
Attachment #8926366 - Flags: review?(mconley) → review+
Pushed by dharvey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/09381b4fe070
Ensure browser_urlbar_search_reflows.js test runs are isolated. r=mconley
https://hg.mozilla.org/mozilla-central/rev/09381b4fe070
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
You need to log in before you can comment on or make changes to this bug.