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
This patch also tightens up a one or two references to 'target effect' replacing
them with just 'effect'. This is because 'target effect' is longer and easily
confused with 'target element'. 'effect' should be sufficient. 'target element'
is a term from the Web Animations specification and in that context, simply
referring to the 'effect' would sound a little odd.
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
Most of this is fairly obvious. However, the addition of 'override' to
ElementPropertyTransition::Name() is not strictly necessary. It was simply added
because while making these changes I accidentally dropped the 'virtual' keyword
from the method in the superclass and the compiler didn't complain. Adding this
will hopefully make it harder to create the same bug in the future.
--HG--
rename : dom/animation/test/css-animations/test_animation-effect-name.html => dom/animation/test/css-animations/test_animation-name.html
rename : dom/animation/test/css-transitions/test_animation-effect-name.html => dom/animation/test/css-transitions/test_animation-name.html
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.
The tests in dom/animation/tests/ use an old version of idlharness.js that
doesn't support inherited interfaces. As discussed in bug 1152619 we're not
looking at updating these old tests (under dom/imptests) at the moment which
means we won't be able to update the IDL tests in dom/animation/tests/ to
continue passing once we introduce DocumentTimeline as a subinterface of
AnimationTimeline.
As a result, this patch simply the removes the IDL tests for this interface from
dom/animation/tests. However, we have a test for this interface in
web-platform-tests where I've set up a pull request to apply the required
renaming so we should eventually get test coverage for this renaming.
https://github.com/w3c/web-platform-tests/pull/1748
In the long run, all the tests in dom/animation/tests should end up in
web-platform-tests. The main reason they aren't there yet is that most of them
test the mapping between the Web Animations API and CSS and there's currently no
spec defining that so there's no place to put them in the web-platform-tests
repository.
There are a few tests for animation timeline which could be landed in
web-platform-tests (and then removed from dom/animation/tests) but we need to
discuss with Google if this is the desired behavior or not first. For the time
being I have a branch setup for that and I'm leaving the tests in
dom/animation/tests so we continue to test what *we* think the behavior should
be in the meantime. That branch is here:
https://github.com/birtles/web-platform-tests/compare/rename-animation-timeline...birtles:add-hidden-iframe-tests
(I don't understand the motivation for the changes to how it was set,
but this just switches to having two variables for the two uses.)
This fixes the test failures in test_animations_omta.html with OMT
animations enabled.
This patch (finally) puts pausing animations in the pending player map so that
they are resolved asynchronously.
Since this changes the pausing behavior this patch updates a number of tests so
that they continue to pass.
When a pending pause operation is interrupted by a play operation we should
preserve the original start time of the animation so that it appears to continue
moving uninterrupted. At the same time, however, for consistency with other
calls to play(), the operation should complete asynchronously.
This patch adds the method that is called when an asynchronous pause operation
has completed. It is not used yet, however, since we don't yet put
AnimationPlayer objects in the pause-pending map.
This patch (after stepping through the call graph) affects the following
places:
* CommonAnimationManager::GetAnimationsForCompositor, which is used
only by nsDisplayListBuilder::AddAnimationsAndTransitionsToLayer,
which already checks the individual animations (so really no change)
* AnimationPlayerCollection::CanThrottleAnimation
* ActiveLayerTracker::IsStyleAnimated
* nsLayoutUtils::HasAnimationsForCompositor
* nsLayoutUtils::HasAnimations (which is used only to check whether we
can make the 0-opacity optimization)
I believe it makes sense to change all of these locations (although in
the long term we want to throttle (or similar) more animations).
Without this patch, I believe we're forcing the creation of an opacity
layer because we think we have animations to send to it.
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.
This patch renames the confusing IsRunning method since IsRunning() is *not*
the same as (PlayState() == AnimationPlayState::Running). It also removes
the old definition to make better re-use of PlayState() and IsInPlay().
This patch adds a method for testing if an animation is "in play" which is
a term defined in the Web Animations spec. This is in preparation for removing
some slightly redundant code in IsRunning and aligning better with the spec.
In preparation for introducing IsInPlay (where "in play" is a term in the Web
Animations spec), this patch aligns the existing IsCurrent with the definition
in the spec that says an animation effect is only current if it is attached
to an animation (player in our current naming) that is not finished. In order
to ensure that we need to pass the animation/player into the method.
This actually changes the behavior of IsCurrent since now we will return false
for animations that are finished. As far as I can tell, all the call sites that
are requesting current animations should only be concerned with animations that
are actually running. If not, they need to be adjusted to look for animations
that are either current or in effect.
This patch simply updates the method that cancels pending plays to also cancel
pending pauses. As it stands, for some of places where this is called it might
not be appropriate to cancel pending pauses but we will adjust each of these
call sites one-by-one in subsequent patches in this series.
IsPaused is used in nsAnimationManager to detect if a newly created animation
should be paused. It is also used inside AnimationPlayer::IsRunning which is
used to determine what animations to send to the compositor (we don't send
paused animations to the compositor). In all these cases we want to treat paused
animations and pause-pending animations alike.
This patch updates IsPaused to include also pause-pending animations. At the
same time it renames IsPaused to IsPausedOrPausing to reflect the change in
behavior.
This patch also adjusts a few nearby one-line functions to put the opening brace
on a new line since apparently this is what the coding style says to do.
This won't actually do anything yet because:
(a) We don't yet add pause-pending players to the PendingPlayerTracker
(b) We never mark pausing players as pending so
AnimationPlayer::TriggerOnNextTick will just ignore them.
Brian Birtles
Jonathan Watt
L. David Baron
Markus Stange
Cameron McCormack
Tom Tromey