Closed Bug 1232563 Opened 7 years ago Closed 7 years ago

Move restyle requests to the animation effect


(Core :: DOM: Animation, defect)

Not set



Tracking Status
firefox46 --- fixed


(Reporter: birtles, Assigned: birtles)


(Blocks 1 open bug)



(4 files)

This covers the second part of bug 1230040:

  * Move request restyle down to the effect
  * Store previous progress and don't request restyle if it hasn't changed
  * Post a layer update whenever an Animation is newly finished
Blocks: 1232577
When requesting restyles we take special care to detect when an animation has
newly finished so we perform the necessary restyle to represent the fill state.
However, we should really explicitly pull the animation off the layer at this
point by requesting a layer update. (That is, when an animation is
newly-finished we should use RestyleType::Layer instead of
RestyleType::Standard. Currently we just use RestyleType::Standard.)

In this bug we plan to move restyle requests down the effect (since it is the
*effect* that is restyled). However, only the Animation has the notion of
"finished" or not so we detect this particular case in the Animation and
request the layer update there. We already request layer updates in the
Animation for other situations such as pausing so doing *layer* updates in the
Animation and regular restyles in the effect is not inconsistent.

This patch also tweaks test_animations_omta.html since it was previously
erroneously testing that a finished animation was still running on the
Attachment #8701712 - Flags: review?(cam)
Blocks: 1235002
Comment on attachment 8701714 [details] [diff] [review]
part 2 - Move RequestRestyle calls to the effect

This patch actually introduces a fairly significant change. In the case of the effect having no properties or not being "in effect", rather than simply requesting a throttled restyle, it actually causes us to not request a restyle at all. This is actually not correct in all cases and so the next two patches in the series correct this.

As a result, this next patch is not strictly atomic but needs to be landed together with parts 3 and 4. I normally try to avoid this but in this case I think it's probably ok since there are only 4 parts to this bug.
This is because rather than simply requesting a throttled restyle when there
were no properties, as of the previous patch, we no longer request a restyle at
all in this case.

We should be able to restore this optimization in bug 1235002 when we properly
encapsulate the properties of a keyframe effect.
Attachment #8701715 - Flags: review?(cam)
Now that restyle requests are handled by the effect, we can more easily detect
cases where we don't need to trigger a style update by looking for when the
output of the effect could actually differ.

Currently, any changes that require updates where the progress does *not* change
(e.g. pausing) are triggered by the Animation. The exception is when we
update timing properties (e.g. animation-iteration-count) from CSS but
current nsAnimationManager takes care to adjust the animation generation in
this case.
Attachment #8701716 - Flags: review?(cam)
Comment on attachment 8701712 [details] [diff] [review]
part 1 - Request a layer update if an animation is newly finished

Review of attachment 8701712 [details] [diff] [review]:

s/down/down to/ in the commit message.
Attachment #8701712 - Flags: review?(cam) → review+
Attachment #8701714 - Flags: review?(cam) → review+
Attachment #8701715 - Flags: review?(cam) → review+
Attachment #8701716 - Flags: review?(cam) → review+
Blocks: 1237453
You need to log in before you can comment on or make changes to this bug.