Since bug 960465 patch 14, we've retained finished transitions in order
to handle the issues described in
https://lists.w3.org/Archives/Public/www-style/2015Jan/0444.html . The
code that did this made the assumption that the transition manager is
notified of the full sequence of style changes that happen to an
element. However, when an element becomes part of a display:none
subtree and then later becomes displayed again, the transition manager
is not notified of a style change when it becomes displayed again (when
we do not have the old style context).
This patch introduces code to prune the finished transitions when that
happens.
This really fixes only part of the set of problems described in bug
1158431, which also affect running transitions. However, it's the part
of that set that was a regression from bug 960465, which introduced the
retention of finished transitions, and which makes these issues
substantially easier to hit.
I'd like to fix this part quickly because it's a regression and we
should backport the fix.
Without the patch, I confirmed that the following two tests fail:
INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_dynamic_changes.html | bug 1144410 test - opacity after starting second transition - got 0, expected 1
INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_transitions_dynamic_changes.html | bug 1144410 test - opacity during second transition - got 0, expected 0.5
With the patch, all the added tests pass.
--HG--
extra : transplant_source : %B4%A5%5Ck%E1%B7%F5Et%CF%9B%9F%40%97c%C5NM%D3%A8
This patch makes Cancel() call PostUpdate which clobbers certain state in style
so that animated style is correctly flushed when an animation is cancelled.
The main difficulty with this is that we *don't* want to call this when we're
cancelling an animation as a result of a style update or else we'll trigger
needless work. The pattern elsewhere has been to define a *FromStyle() method
for this case (e.g. CSSAnimation::PlayFromStyle, PauseFromStyle). This isn't
ideal because there's always the danger we will forget to call the appropriate
*FromStyle method. It is, however, consistent. Hopefully in bug 1151731 we'll
find a better way of expressing this.
This patch also updates the method names on PendingAnimationTracker but leaves
a number of local variables which will be fixed in a subsequent patch.
--HG--
rename : dom/animation/PendingPlayerTracker.cpp => dom/animation/PendingAnimationTracker.cpp
rename : dom/animation/PendingPlayerTracker.h => dom/animation/PendingAnimationTracker.h
This patch is a fairly minimal rename of the AnimationPlayer interface. It
leaves a bunch of local variables and helper classes still using the word
"player". These will be addressed in subsequent patches that don't require DOM
peer review.
--HG--
rename : dom/animation/AnimationPlayer.cpp => dom/animation/Animation.cpp
rename : dom/animation/AnimationPlayer.h => dom/animation/Animation.h
rename : dom/webidl/AnimationPlayer.webidl => dom/webidl/Animation.webidl
There are still some other references to "source" in AnimationPlayer such as
HasInPlayerSource and UpdateSourceContent. These are renamed in a subsequent
patch (that doesn't require DOM peer review).
We define KeyframeEffectReadonly in KeyframeEffect.cpp since Web Animations also
defines KeyframeEffect and when we come to implement that I expect we'll define
it in the same class, maybe even using the same object.
This patch also adds a few missing includes in places where
KeyframeEffectReadonly is used so that we're not just cargo-culting it in.
--HG--
rename : dom/animation/Animation.cpp => dom/animation/KeyframeEffect.cpp
rename : dom/animation/Animation.h => dom/animation/KeyframeEffect.h
rename : dom/animation/test/css-animations/test_animation-name.html => dom/animation/test/css-animations/test_effect-name.html
rename : dom/animation/test/css-animations/test_animation-target.html => dom/animation/test/css-animations/test_effect-target.html
rename : dom/animation/test/css-transitions/test_animation-name.html => dom/animation/test/css-transitions/test_effect-name.html
rename : dom/animation/test/css-transitions/test_animation-target.html => dom/animation/test/css-transitions/test_effect-target.html
rename : dom/webidl/Animation.webidl => dom/webidl/KeyframeEffect.webidl
This is a bit awkward. We lazily set mName to the transition property and then
return it. The reasons for this approach are:
* We don't really want to eagerly fill in mName for all transitions since in
99% of cases we'll never use it and this will lead to wasted allocations.
* The signature of Name() returns a const nsString reference. This is because
Name() is used when building CSS Animations (to compare different copies of
the same animation when updating). For that case we don't really want to
generate unnecessary copies of nsString objects so we return a reference.
However, that means for transitions as well we need to return a reference so
we can't just generate a temporary string on-demand.
As a result we also have to const-cast ourselves so we can update the mName
member. We could make mName mutable but seeing as it's only set once, the
const_cast seems more appropriate.
I don't have a test case that requires this, but it seems like a good
idea. (It was an incorrect theory for fixing a test failure that I was
debugging, but still seems worth doing.)
This is the main patch for the bug; it makes us use the mechanism added
in bug 1125455 to avoid sending animations that aren't currently
applying to the compositor.
Patch 7 is needed to make this code rerun in all the cases where we need
to rerun it, though.
Note that this means that when we start transitions, we post restyles
that are processed during the current restyling operation, rather than
in a later phase. This depends on patch 11, which makes the transition
manager skip style changes that it posts while starting transitions, to
ensure that this doesn't lead to an infinite loop. This also depends on
patch 16, which only consumes restyle data for the primary frame, to
ensure that the animation restyles posted are processed properly. It
also depends on patch 14, which makes us retain data on finished
transitions, to avoid triggering extra transitions on descendants when
both an ancestor and a descendant transition an inherited property, and
the descendant does so faster.
This fixes a known failure in layout/style/test/test_animations.html and
test_animations_omta.html (as visible in the patch). I believe this is
because this patch changes us to compute keyframe values for animations
on top of a style context *with* animation data rather than one without,
which means what we're computing them on top of changes each time. (The
purpose of patch 3 was to avoid this in the case where avoiding it
matters, i.e., implicit 0% and 100% keyframes.)
Note that this increases memory use for completed transitions since we
don't throw away the data when the transitions complete. That said,
this matches what we do for CSS Animations, and it's needed (once we
switch to the new rules for starting transitions) to maintain the
invariant that unrelated style changes don't trigger transitions.
The storage issues could be optimized in the future if it turns out to
be a problem, but I think that's unlikely, given that we'll never store
more than one for any element+property combination.
This includes removing:
* the box property directional source constants
* the CSS_PROPERTY_DIRECTIONAL_SOURCE property flag
* the CSS_PROPERTY_REPORT_OTHER_NAME property flag
* nsCSSProps::OtherNameFor
* methods on the CSS parser to parse directional box properties and set
the old *-source and *-value properties
* the resolution of logical and physical properties in nsRuleNode during
style computation, since that's now done as part of the cascade in
nsCSSExpandedDataBlock::MapRuleInfoInto
This patch (finally!) introduces the delayed start behavior. It updates
AnimationPlayer::DoPlay to put animations in the PendingPlayerTracker from
where they are triggered.
This patch also updates nsTransitionManager to set the animation's source
before calling Play as otherwise the AnimationPlayer won't be able to access
the pending player tracker (which it locates by navigating AnimationPlayer ->
Animation (source content) -> target element -> composed doc -> pending player
tracker). In future, when we support setting the AnimationPlayer.source property
we will make this more robust so that the order in which these steps are
performed doesn't matter.
This patch also updates a couple of tests to reflect the fact that
AnimationPlayer will now return the pending state.
This includes removing:
* the box property directional source constants
* the CSS_PROPERTY_DIRECTIONAL_SOURCE property flag
* the CSS_PROPERTY_REPORT_OTHER_NAME property flag
* nsCSSProps::OtherNameFor
* methods on the CSS parser to parse directional box properties and set
the old *-source and *-value properties
* the resolution of logical and physical properties in nsRuleNode during
style computation, since that's now done as part of the cascade in
nsCSSExpandedDataBlock::MapRuleInfoInto
