1358458 - 6.27ms uninterruptible reflow at _showNotification@chrome://global/content/bindings/notification.xml:270:17

Status: RESOLVED INCOMPLETE

(Toolkit :: UI Widgets, enhancement, P4)

Description

[Stefan Fleiter (:sfleiter)]

Here's the stack:

_showNotification@chrome://global/content/bindings/notification.xml:270:17
removeCurrentNotification@chrome://global/content/bindings/notification.xml:222:13
removeNotification@chrome://global/content/bindings/notification.xml:193:15
removeTransientNotifications@chrome://global/content/bindings/notification.xml:256:17
onLocationChange@chrome://browser/content/browser.js:5092:5
callListeners@chrome://browser/content/tabbrowser.xml:492:24
_callProgressListeners@chrome://browser/content/tabbrowser.xml:513:13
_callProgressListeners@chrome://browser/content/tabbrowser.xml:563:22
onLocationChange@chrome://browser/content/tabbrowser.xml:845:17
_callProgressListeners@resource://gre/modules/RemoteWebProgress.jsm:179:11
receiveMessage@resource://gre/modules/RemoteWebProgress.jsm:265:7
waitForSyncCallback@resource://services-common/async.js:98:7
makeSpinningCallback/callback.wait@resource://services-common/async.js:168:27
promiseSpinningly@resource://services-common/async.js:234:12
get changedIDs@resource://services-sync/engines.js:97:5
addChangedID@resource://services-sync/engines.js:158:1
onVisit@resource://services-sync/engines/history.js:491:9

Comment 1

[Florian Quèze [:florian]]

The sync layout flush is "aNotification.boxObject.height;" at http://searchfox.org/mozilla-central/rev/7aa21f3b531ddee90a353215bd86e97d6974e25b/toolkit/content/widgets/notification.xml#269 to decide if we should skip an animation.

I think this could safely be changed to getBoundsWithoutFlushing.

Comment 2

[Dão Gottwald [:dao]]

(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> The sync layout flush is "aNotification.boxObject.height;" at
> http://searchfox.org/mozilla-central/rev/
> 7aa21f3b531ddee90a353215bd86e97d6974e25b/toolkit/content/widgets/
> notification.xml#269 to decide if we should skip an animation.
> 
> I think this could safely be changed to getBoundsWithoutFlushing.

I don't think so. If the height is off, the slide animation will be off. If we fail to detect the height==0 case, we'll erroneously wait for the transitionend event.

Comment 3

[Florian Quèze [:florian]]

Yeah, I read this code too quickly, I had in mind the call stack from comment 0 where we are removing a notification due to a location change, so it's very likely (and probably guaranteed) that the notification we remove is already on screen, so with correct bounds without flushing). But that's the specific (but common) case where we are removing a notification and it's the only one. _showNotification can be called in other situations. I think we should special case the common case where it's fine to not flush.

Comment 4

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

This reflow looks like it still exists, but showing notification bars _should_ be rare enough that I feel comfortable relegating this to a qf:p3.

Comment 5

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

This binding is gone now. I suspect the reflow has moved elsewhere, but I'm going to just close this one out as INCOMPLETE.