TooltipTextProvider can cause synchronous reflow when computing element direction

RESOLVED FIXED in Firefox 55

Status

()

Toolkit
General
RESOLVED FIXED
8 months ago
7 months ago

People

(Reporter: mconley, Assigned: Neil Deakin)

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

8 months 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

8 months 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

8 months 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

7 months ago
Created attachment 8863873 [details] [diff] [review]
Only compute tooltip direction when a tooltip would show

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

7 months 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

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8366dcea0753
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Blocks: 1363771
No longer blocks: 1348289
Blocks: 1348289
You need to log in before you can comment on or make changes to this bug.