1.22ms uninterruptible reflow at onxblpopupshowing@chrome://browser/content/search/search.xml:1087:1

VERIFIED FIXED in Firefox 55

Status

()

Firefox
Search
P1
normal
VERIFIED FIXED
10 months ago
7 months ago

People

(Reporter: Robbie Ward, Assigned: florian)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 verified, firefox56 verified, firefox57 verified)

Details

(Whiteboard: [ohnoreflow][qf][photon-performance][qa-commented])

Attachments

(1 attachment)

(Reporter)

Description

10 months ago
Here's the stack:

onxblpopupshowing@chrome://browser/content/search/search.xml:1087:1
openPopup@chrome://global/content/bindings/popup.xml:53:15
openPopup@chrome://browser/content/search/search.xml:741:13
set_popupOpen@chrome://global/content/bindings/autocomplete.xml:103:10
onResultsReady@jar:file:///C:/Program%20Files/Nightly/omni.ja!/components/nsSearchSuggestions.js:104:7
onResultsReturned@jar:file:///C:/Program%20Files/Nightly/omni.ja!/components/nsSearchSuggestions.js:78:5
_init/this._suggestionController<@jar:file:///C:/Program%20Files/Nightly/omni.ja!/components/nsSearchSuggestions.js:27:72
_dedupeAndReturnResults@resource://gre/modules/SearchSuggestionController.jsm:367:7
process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
onSearchResult@resource://gre/modules/SearchSuggestionController.jsm:205:13
processEntry@jar:file:///C:/Program%20Files/Nightly/omni.ja!/components/nsFormAutoComplete.js:434:21
getAutoCompleteValues/<@jar:file:///C:/Program%20Files/Nightly/omni.ja!/components/nsFormAutoComplete.js:503:13
receiveMessage@jar:file:///C:/Program%20Files/Nightly/omni.ja!/components/nsFormAutoComplete.js:147:9

Updated

10 months ago
Component: Untriaged → Search

Updated

10 months ago
Flags: qe-verify?
Priority: -- → P2
(Assignee)

Comment 1

10 months ago
Created attachment 8860380 [details] [diff] [review]
Patch

The sync reflows here are due to the getComputedStyle calls at http://searchfox.org/mozilla-central/rev/7aa21f3b531ddee90a353215bd86e97d6974e25b/browser/components/search/content/search.xml#1084 and on the next line.

I think we can compute the searchbar panel size and position using getBoundsWithoutFlushing on both the textbox and its input field. This lets us remove the platform-specific hardcoded CSS values that I never liked.

Also, I think no longer requiring a sync layout flush makes this code fast enough that we can probably run it each time the panel is shown without the need for the _computedMinWidth variable, so the attached patch also fixes bug 1358455.
Attachment #8860380 - Flags: review?(adw)
(Assignee)

Updated

10 months ago
Assignee: nobody → florian
Status: NEW → ASSIGNED

Updated

10 months ago
Iteration: --- → 55.4 - May 1
Priority: P2 → P1

Comment 2

10 months ago
Comment on attachment 8860380 [details] [diff] [review]
Patch

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

Nice.
Attachment #8860380 - Flags: review?(adw) → review+

Comment 3

10 months ago
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1db9bb48b28
Compute the searchbar panel size and position without sync reflow, r=adw.
https://hg.mozilla.org/mozilla-central/rev/c1db9bb48b28
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
(Assignee)

Comment 5

10 months ago
This bug could do with some QA to verify that the searchbar panel is still sized and positioned correctly on all platforms.
Flags: qe-verify? → qe-verify+

Updated

10 months ago
QA Contact: adrian.florinescu

Updated

10 months ago
Depends on: 1361195
(Assignee)

Updated

10 months ago
No longer blocks: 1348289
(Assignee)

Updated

10 months ago
Blocks: 1348289
(Assignee)

Comment 6

7 months ago
(In reply to Florian Quèze [:florian] [:flo] from comment #5)
> This bug could do with some QA to verify that the searchbar panel is still
> sized and positioned correctly on all platforms.

Please verify that the panel is sized correctly and that its content is laid out correctly after various operations that could change the searchbar size. I wrote this patch on Mac, and we recently did debugging of related issues (bug 1361195, bug 1104325) on Windows HiDPI. I haven't tested myself on Linux, and haven't seen any bug reports about it.
(Assignee)

Updated

7 months ago
Whiteboard: [ohnoreflow][qf][photon-performance] → [ohnoreflow][qf][photon-performance][qa-commented]
55.0 20170803103124
56.0b1  20170808170225
57.0a1 20170810100255

Focused on regression and exploratory testing on Windows10/Linux 16.04:

-customized position for search panel; 
-multi-display resizing;
-multi-display with different resolutions on display1 vs display2;
-drag and drop + resizing up and down;


I've only found bug 1389080, but as it's an bug that is reproducible on Nightly 53 as well, marking this issue as verified.
Status: RESOLVED → VERIFIED
status-firefox55: fixed → verified
status-firefox56: --- → verified
status-firefox57: --- → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.