1447870 - Stop flushing throttled animations in nsDOMWindowUtils::UpdateLayerTree()

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Hiroyuki Ikezoe (:hiro)]

As far as I can tell the UpdateLayerTree() is only called from reftest harness, and to sync decode images (only for plugin process?) after 'reftest-wait' has been already removed.  So, we don't need to flush throttled animations there throttled animations in reftest should have already being restyled on the main-thread or don't need to be restyled because they run on the compositor.
  
I am not sure 'DOM' is the right component for this, by the way.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Don't flush throttled animations in nsIDOMWindowUtils.updateLayerTree

Here is a patch that I have locally, I am confident the fix is the right thing to do, but IIRC I haven't finished writing the commit message.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

I did recall what I had trouble with.  That's writing a reftest for this which fails without this fix.  I am going to land the fix without any test cases.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1eb0f29834bd346f1475feb79c3e9595205612cf

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1447870 - Don't flush throttled animations in nsIDOMWindowUtils.updateLayerTree.

updateLayerTree() is called from the callback of MozAfterPaint in
reftest-content.js all the time except when we specify reftest-no-sync-layers
or reftest-no-flush.  If there is a throttled animation,
FlushPendingNotifications flushes the animation, thus it will fire another
MozAfterPaint event.  It means we will end up waiting for the throttled
animation to finish.  This reiteration MozAfterPaint events can be easily
observed in WebRender since WebRender fires MozAfterPaint event without
checking invalidation change.  Whereas current Gecko fires MozAfterPaint only
if invalidation change happens and reftests for such throttled animations
specify the same value for 'from' and 'to' value, so it hasn't been a problem
there.

The FlushPendingNotifications in updateLayerTree() was introduced in bug
1083635 <https://hg.mozilla.org/mozilla-central/rev/5bebfbefb3c2> to
sync-decode images so it shouldn't matter if there are still throttled
animations.

Review commit: https://reviewboard.mozilla.org/r/230022/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/230022/

Comment 5

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8961249 [details]
Bug 1447870 - Don't flush throttled animations in nsIDOMWindowUtils.updateLayerTree.

https://reviewboard.mozilla.org/r/230022/#review236444

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68d08d45977a
Don't flush throttled animations in nsIDOMWindowUtils.updateLayerTree. r=mattwoodrow

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/68d08d45977a