Closed Bug 1354956 Opened 4 years ago Closed 4 years ago
Text Provider can cause synchronous reflow when computing element direction
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
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
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 firstname.lastname@example.org: 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
You need to log in before you can comment on or make changes to this bug.