24.99ms uninterruptible reflow at adjustSiteIconStart@chrome://global/content/bindings/autocomplete.xml:2379:13

RESOLVED FIXED in Firefox 57

Status

()

P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: rowbot, Assigned: Paolo)

Tracking

(Blocks: 1 bug)

unspecified
mozilla57
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [ohnoreflow][qf:p2][reserve-photon-performance][fxsearch])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Here's the stack:

adjustSiteIconStart@chrome://global/content/bindings/autocomplete.xml:2379:13
_onChanged@chrome://global/content/bindings/autocomplete.xml:2069:31
_appendCurrentResult/<@chrome://global/content/bindings/autocomplete.xml:1398:17
setTimeout handler*_appendCurrentResult@chrome://global/content/bindings/autocomplete.xml:1397:15
_appendCurrentResult/this._appendResultTimeout<@chrome://global/content/bindings/autocomplete.xml:1411:58
setTimeout handler*_appendCurrentResult@chrome://global/content/bindings/autocomplete.xml:1411:41
_appendCurrentResult/this._appendResultTimeout<@chrome://global/content/bindings/autocomplete.xml:1411:58
setTimeout handler*_appendCurrentResult@chrome://global/content/bindings/autocomplete.xml:1411:41
_invalidate@chrome://global/content/bindings/autocomplete.xml:1193:11
invalidate@chrome://global/content/bindings/autocomplete.xml:1162:11
notifyResults@jar:file:///C:/Program%20Files/Nightly/omni.ja!/components/UnifiedComplete.js:2111:5
finishSearch@jar:file:///C:/Program%20Files/Nightly/omni.ja!/components/UnifiedComplete.js:2284:5
startSearch/<@jar:file:///C:/Program%20Files/Nightly/omni.ja!/components/UnifiedComplete.js:2241:33
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
TaskImpl_run@resource://gre/modules/Task.jsm:324:15
promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:396:7
TaskImpl_run@resource://gre/modules/Task.jsm:327:15
promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:396:7
TaskImpl_run@resource://gre/modules/Task.jsm:327:15
promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:396:7
TaskImpl_run@resource://gre/modules/Task.jsm:327:15
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
Promise.prototype.then@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:455:5
TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:396:7
TaskImpl_run@resource://gre/modules/Task.jsm:327:15
promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:396:7
TaskImpl_run@resource://gre/modules/Task.jsm:327:15
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
TaskImpl_run@resource://gre/modules/Task.jsm:324:15
promise callback*TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:396:7
TaskImpl_run@resource://gre/modules/Task.jsm:327:15
TaskImpl@resource://gre/modules/Task.jsm:277:3
asyncFunction@resource://gre/modules/Task.jsm:252:14
Task_spawn@resource://gre/modules/Task.jsm:166:12
TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:389:16
TaskImpl_run@resource://gre/modules/Task.jsm:327:15
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

This occurred while I was typing an address in the address bar.
The code is:
            // Allow margin-inline-start not to be specified in CSS initially.
            let style = window.getComputedStyle(this._typeIcon);
http://searchfox.org/mozilla-central/rev/6e1c138a06a80f2bb3167d9dac050cc0fd4fb39e/toolkit/content/widgets/autocomplete.xml#2376

Marco, do you know in which case the margin-inline-start value isn't in the CSS?
Flags: needinfo?(mak77)
Component: Untriaged → Autocomplete
Product: Firefox → Toolkit
Flags: qe-verify?
Priority: -- → P2
Few lines above we remove when a non numeric value is passed in, that happens for example when there are more than 2 buttons in the toolbar before the urlbar.
http://searchfox.org/mozilla-central/rev/f225dbcb15ca2e38f7d434a9278a41d2340e7cf3/browser/base/content/urlbarBindings.xml#1613

Also, looks like we never set it initially, the last time I was confused by margin-inline-start, that is set on every platform. We seem to always calculate this dynamically the first time the awesomebar is opened. We could maybe set an initial value based on the default, and override it if the toolbar is customized?

Drew wrote this code and may have better ideas, plus I'm on PTO for a couple days.
Flags: needinfo?(mak77) → needinfo?(adw)
Whiteboard: [ohnoreflow][qf][photon-performance] → [ohnoreflow][qf][photon-performance][fxsearch]
(In reply to Marco Bonardo [::mak] from comment #2)
> Also, looks like we never set it initially [...] We seem to always
> calculate this dynamically the first time the awesomebar is opened.

That's right, the initial value is calculated dynamically so it's guaranteed to be right.  If there's a way to set it statically in CSS, that should be OK too as long as it's right of course.
Flags: needinfo?(adw)
Whiteboard: [ohnoreflow][qf][photon-performance][fxsearch] → [ohnoreflow][qf:p1][photon-performance][fxsearch]

Updated

2 years ago
Blocks: 1363757
No longer blocks: 1348289
Flags: qe-verify? → qe-verify-
Priority: P2 → P3
Whiteboard: [ohnoreflow][qf:p1][photon-performance][fxsearch] → [ohnoreflow][qf:p1][reserve-photon-performance][fxsearch]
Whiteboard: [ohnoreflow][qf:p1][reserve-photon-performance][fxsearch] → [ohnoreflow][qf:p2][reserve-photon-performance][fxsearch]
Priority: P3 → P2
Whiteboard: [ohnoreflow][qf:p2][reserve-photon-performance][fxsearch] → [ohnoreflow][qf:p2][photon-performance][fxsearch]
Priority: P2 → P3
Whiteboard: [ohnoreflow][qf:p2][photon-performance][fxsearch] → [ohnoreflow][qf:p2][reserve-photon-performance][fxsearch]
Comment hidden (mozreview-request)
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Priority: P3 → P1
(Assignee)

Comment 5

2 years ago
I tested this with and without the site icons alignment. Tryserver build:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3133408469a347f6465b4cd3b1aadfa5164a4275
Comment hidden (mozreview-request)
(Assignee)

Comment 7

2 years ago
I tested this on RTL and found out that the icon would change position slightly if we didn't apply rounding.

Comment 8

2 years ago
mozreview-review
Comment on attachment 8899432 [details]
Bug 1358792 - Fix uninterruptible reflow at adjustSiteIconStart.

https://reviewboard.mozilla.org/r/170714/#review176262

Good idea and nice seeing the test agree with it!
Attachment #8899432 - Flags: review?(mak77) → review+

Comment 9

2 years ago
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8633b8546a94
Fix uninterruptible reflow at adjustSiteIconStart. r=mak

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8633b8546a94
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Iteration: --- → 57.2 - Aug 29
You need to log in before you can comment on or make changes to this bug.