Closed Bug 1354956 Opened 4 years ago Closed 4 years ago

TooltipTextProvider can cause synchronous reflow when computing element direction

Categories

(Toolkit :: General, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: mconley, Assigned: enndeakin)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [qf:p1])

Attachments

(1 file)

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

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.
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+
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
https://hg.mozilla.org/mozilla-central/rev/8366dcea0753
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.