Closed Bug 1449187 Opened 6 years ago Closed 6 years ago

Perma opt OS X and Windows browser/base/content/test/performance/browser_urlbar_keyed_search.js when Gecko 61 merges to Beta on 2018-04-26

Categories

(Firefox :: Address Bar, defect, P1)

Product:
Firefox
Component:
Address Bar
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 61
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 + verified

People

(Reporter: aryx, Assigned: standard8)

References

Details

(Whiteboard: [fxsearch])

Attachments

(1 file)

Bug 1449187 - Whitelist extra autocomplete reflows that happen due to a default bookmark with a long URL on beta/release.
59 bytes, text/x-review-board-request
mconley
: review+
Details 
central-as-beta simulation: https://treeherder.mozilla.org/#/jobs?repo=try&revision=62fff771b6a6ffadea4f7383c0986f0e5f4f2cda&group_state=expanded&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&selectedJob=170516891

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=170517739&repo=try

03:40:45     INFO - Buffered messages finished
03:40:45     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_urlbar_keyed_search.js | unexpected reflow at _handleOverflow@chrome://global/content/bindings/autocomplete.xml hit 1 times
03:40:45     INFO - Stack:
03:40:45     INFO -   _handleOverflow@chrome://global/content/bindings/autocomplete.xml:2053:26
03:40:45     INFO -   _onOverflow@chrome://global/content/bindings/autocomplete.xml:1444:11
03:40:45     INFO -   onoverflow@chrome://browser/content/browser.xul:1:1
03:40:45     INFO -    - false == true - JS frame :: chrome://mochitests/content/browser/browser/base/content/test/performance/head.js :: reportUnexpectedReflows :: line 190
03:40:45     INFO - Stack trace:
03:40:45     INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:reportUnexpectedReflows:190
03:40:45     INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/head.js:withPerfObserver:630

Push log from last good to first bad: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f0d13136b3580182df241674cf97a3fbf1543c2a%20&tochange=97cdd8febc40ac6025bce5dec9f8dadb8e62f906

Florian, can you spot what causes this (almost opt-only) reflow? Thank you.
Flags: needinfo?(florian)
This seems to be using a different code path than what we expect, eg. I see '_adjustAcItem' frames in the stacks, whereas our whitelisted reflows have '_reuseAcItem'. Mike or Marco may have ideas.
Flags: needinfo?(mconley)
Flags: needinfo?(mak77)
Flags: needinfo?(florian)
I honestly don't have a very specific idea of why this should differ between Beta and Nightly, both code paths are possible, _reuseAcItem is used when we have a match that is absolutely identical to an existing one, while _adjustAcItem is used to "adjust" a match that has some differences.
I can't think of any nightly-only features in the location bar at this time. Thus it just sounds timing-sensitive. Maybe we should just reduce the whitelisted stack, it's clear from quite some time that handleOverUnderflow causes these reflows, there's no big win into restricting the stack further.
Flags: needinfo?(mak77)
Priority: -- → P1
Whiteboard: [fxsearch]
I'm not sure why this would be different. :/ I agree that the most expedient solution is probably to shorten the stack to just _handleOverflow.
Flags: needinfo?(mconley)
Mark, this is an unassigned P1 tracking 61+. Can you please help find an owner?
Flags: needinfo?(standard8)
It's likely the long url changes the timing we take to populate and adds more reflows, because it will send overflow events more often.
I don't think it's worth calling this a regression, those reflows were just not accounted for by the test, but were hiding there.

I suggested shortening the stack, but looking at the logs this doesn't look like a shift, since I don't the see ones we track being reported as "missing"... We may just have to eat the pill and add these new stacks. The scope doesn't change, we should avoid all of these in bug 1356532.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Flags: needinfo?(standard8)
Assignee: mak77 → standard8
Comment on attachment 8965699 [details]
Bug 1449187 - Whitelist extra autocomplete reflows that happen due to a default bookmark with a long URL on beta/release.

https://reviewboard.mozilla.org/r/234554/#review240180

Thanks!
Attachment #8965699 - Flags: review?(mconley) → review+
Comment on attachment 8965699 [details]
Bug 1449187 - Whitelist extra autocomplete reflows that happen due to a default bookmark with a long URL on beta/release.

https://reviewboard.mozilla.org/r/234554/#review240182

::: browser/base/content/test/performance/browser_urlbar_keyed_search.js:18
(Diff revision 1)
>   * See https://developer.mozilla.org/en-US/Firefox/Performance_best_practices_for_Firefox_fe_engineers
>   * for tips on how to do that.
>   */
>  
>  /* These reflows happen only the first time the awesomebar panel opens. */
> -const EXPECTED_REFLOWS_FIRST_OPEN = [
> +let EXPECTED_REFLOWS_FIRST_OPEN = [

Oh, fwiw, this can stay a const.
Something went wrong with the try simulations. Not sure what, but my testing locally, I'm fairly sure this is right. So I'll push it and we'll find out with the next simulation ;-)
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a07ed16f9792
Whitelist extra autocomplete reflows that happen due to a default bookmark with a long URL on beta/release. r=mconley
https://hg.mozilla.org/mozilla-central/rev/a07ed16f9792
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox61: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: