1354956 - TooltipTextProvider can cause synchronous reflow when computing element direction

Status: RESOLVED FIXED

(Toolkit :: General, enhancement)

Description

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 2

[Neil Deakin]

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

[:Gijs (he/him)]

(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.

Comment 4

[Florian Quèze [:florian]]

Here is a profile of this causing 60ms of jank (on a very low-powered netbook) https://perfht.ml/2ptajOq

Comment 8

[Hazel Ward]

	

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

Comment 10

[Marco Castelluccio [:marco]]

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.

Comment 11

[Neil Deakin]

Attachment: 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.

Comment 12

[Mike Conley (: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.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/8366dcea0753