TooltipTextProvider can cause synchronous reflow when computing element direction

RESOLVED FIXED in Firefox 55

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mconley, Assigned: enndeakin)

Tracking

(Blocks 2 bugs)

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [qf:p1])

Attachments

(1 attachment)

Here's a stack from an nsIReflowObserver:

getNodeText@/components/TooltipTextProvider.js:124:21
fillInPageTooltip@chrome://global/content/bindings/popup.xml:573:34
onxblpopupshowing@chrome://global/content/bindings/popup.xml:631:27

and that appears to refer to this line:

http://searchfox.org/mozilla-central/rev/eace920a0372051a11d8d275bd9b8f14f3024ecd/toolkit/components/tooltiptext/TooltipTextProvider.js#124-125

We seem to trip this synchronous reflow all of the time. I just need to float my mouse around the content a bit, and it spams my reflow observer.
Ah, to be clear, I seem to need to float my mouse over non-remote content - for example, the tiles in the still-not-remote about:newtab page, or anything in about:preferences.
Assignee

Comment 2

2 years ago
We should at least only get the direction once we know a tooltip needs to be displayed; that is, move the compuation to just before the 'return true'.

Comment 3

2 years ago
(In reply to Neil Deakin from comment #2)
> We should at least only get the direction once we know a tooltip needs to be
> displayed; that is, move the compuation to just before the 'return true'.

Can we get the computed style without flushing style all the time (similar to nsIDOMWindowUtils' getBoundsWithoutFlushing), like, can we say we're OK with slightly stale data or something? That would be even better, IMO.
Here is a profile of this causing 60ms of jank (on a very low-powered netbook) https://perfht.ml/2ptajOq
Whiteboard: [qf] → [qf:p1]
Duplicate of this bug: 1356767
Duplicate of this bug: 1358440
Depends on: 1358495
Duplicate of this bug: 1358723

Comment 8

2 years ago
	

getNodeText@jar:file:///C:/program%20files/nightly/omni.ja!/components/TooltipTextProvider.js:124:21
fillInPageTooltip@chrome://global/content/bindings/popup.xml:573:34
onxblpopupshowing@chrome://global/content/bindings/popup.xml:631:27
waitForSyncCallback@resource://services-common/async.js:98:7
_sleep@resource://services-sync/engines.js:313:5
walkBookmarksTree@resource://services-sync/engines/bookmarks.js:311:13
walkBookmarksTree@resource://services-sync/engines/bookmarks.js:312:20
walkBookmarksRoots@resource://services-sync/engines/bookmarks.js:321:18
_buildGUIDMap@resource://services-sync/engines/bookmarks.js:326:5
_syncStart/<@resource://services-sync/engines/bookmarks.js:465:19
_mapDupe@resource://services-sync/engines/bookmarks.js:405:9
_createRecord@resource://services-sync/engines/bookmarks.js:544:17
_uploadOutgoing@resource://services-sync/engines.js:1649:17
_sync@resource://services-sync/engines.js:1803:7
WrappedNotify@resource://services-sync/util.js:162:21
sync@resource://services-sync/engines.js:720:5
_syncEngine@resource://services-sync/stages/enginesync.js:218:7
sync@resource://services-sync/stages/enginesync.js:165:15
onNotify@resource://services-sync/service.js:1075:7
WrappedNotify@resource://services-sync/util.js:162:21
WrappedLock@resource://services-sync/util.js:118:16
_lockedSync@resource://services-sync/service.js:1065:12
sync/<@resource://services-sync/service.js:1057:7
WrappedCatch@resource://services-sync/util.js:93:16
sync@resource://services-sync/service.js:1046:5

Here's another stack
Duplicate of this bug: 1359990
I often see this when scrolling/clicking around/keeping the mouse pointer on an element (also when no tooltip is shown) on the reflow addon report page.
Assignee

Comment 11

2 years ago
While we wait for a way to get the direction without flushing, we can significantly reduce the amount of delays by only getting the direction once we know that a tooltip will appear.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #8863873 - Flags: review?(mconley)
Comment on attachment 8863873 [details] [diff] [review]
Only compute tooltip direction when a tooltip would show

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

This optimization makes sense to me. Thanks Enn!

I wonder if it's worth opening up another bug for the remaining flush though.
Attachment #8863873 - Flags: review?(mconley) → review+

Comment 13

2 years ago
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8366dcea0753
delay the tooltip direction computation until it is known that the tooltip will appear, avoiding a flush, r=mconley

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8366dcea0753
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.