Simplify animation testing API

Simplify animation testing API 

This merges pause{Animation|Transition}AtTimeOn[Pseudo]Element into a 
single function, pauseAnimations. The ability to pause individual
animations and transitions is not actually required for testing purposes.

This will enable further work to convert most of the animation tests to
use the pause API, and should simplify transition to the new model.

R=
BUG=235752
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=149860

[Steve Block, 2013-05-06 01:49:06]

I'm not really qualified to review this, but just spotted a few minor things
while reading through.

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/em...
File LayoutTests/animations/empty-keyframe-animation.html (right):

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/em...
LayoutTests/animations/empty-keyframe-animation.html:35:
testRunner.dumpAsText();
I know there are lots of places where this isn't followed, but I think Blink
style is to use 4-space indents for JS (but 2-space for HTML and I'm not sure
about CSS).

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/mi...
File LayoutTests/animations/missing-from-to.html (right):

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/mi...
LayoutTests/animations/missing-from-to.html:88: This test performs animations of
the left property on five boxes over 2 seconds.
s/five/four

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/re...
File LayoutTests/animations/resources/animation-test-helpers.js (right):

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/re...
LayoutTests/animations/resources/animation-test-helpers.js:21: instead of fast
forwarding using the pauseAnimations() JS API from Internals.
It looks like the animation name is still used to enforce this, but is there any
need to do so? Can we remove the name (either now, or with a FIXME)?

[dstockwell, 2013-05-06 04:01:00]

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/em...
File LayoutTests/animations/empty-keyframe-animation.html (right):

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/em...
LayoutTests/animations/empty-keyframe-animation.html:35:
testRunner.dumpAsText();
On 2013/05/06 01:49:06, Steve Block wrote:
> I know there are lots of places where this isn't followed, but I think Blink
> style is to use 4-space indents for JS (but 2-space for HTML and I'm not sure
> about CSS).

Done.

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/mi...
File LayoutTests/animations/missing-from-to.html (right):

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/mi...
LayoutTests/animations/missing-from-to.html:88: This test performs animations of
the left property on five boxes over 2 seconds.
On 2013/05/06 01:49:06, Steve Block wrote:
> s/five/four

Done.

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/re...
File LayoutTests/animations/resources/animation-test-helpers.js (right):

https://codereview.chromium.org/14556022/diff/18001/LayoutTests/animations/re...
LayoutTests/animations/resources/animation-test-helpers.js:21: instead of fast
forwarding using the pauseAnimations() JS API from Internals.
On 2013/05/06 01:49:06, Steve Block wrote:
> It looks like the animation name is still used to enforce this, but is there
any
> need to do so? Can we remove the name (either now, or with a FIXME)?

Not passing a name no longer falls back to the setTimeout version. Added a fixme
to remove this argument.

[dstockwell, 2013-05-07 01:40:56]

Eric, could you review, or suggest a reviewer for this change?

[eseidel, 2013-05-07 07:08:09]

lgtm

[commit-bot: I haz the power, 2013-05-07 07:08:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dstockwell@chromium.org/14556022/26001

[commit-bot: I haz the power, 2013-05-07 07:08:33]

Failed to apply patch for Source/core/page/animation/CompositeAnimation.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/page/animation/CompositeAnimation.cpp
  Hunk #1 FAILED at 474.
  1 out of 1 hunk FAILED -- saving rejects to file
Source/core/page/animation/CompositeAnimation.cpp.rej

Patch:       Source/core/page/animation/CompositeAnimation.cpp
Index: Source/core/page/animation/CompositeAnimation.cpp
diff --git a/Source/core/page/animation/CompositeAnimation.cpp
b/Source/core/page/animation/CompositeAnimation.cpp
index
f70d95492a45de9906e5095a5c573d0eb8162e07..810861eea32116561a219f0ca723b16bd8a86102
100644
--- a/Source/core/page/animation/CompositeAnimation.cpp
+++ b/Source/core/page/animation/CompositeAnimation.cpp
@@ -474,51 +474,31 @@ bool CompositeAnimation::isAnimatingProperty(CSSPropertyID
property, bool accele
     return false;
 }
 
-bool CompositeAnimation::pauseAnimationAtTime(const AtomicString& name, double
t)
+void CompositeAnimation::pauseAnimationsForTesting(double t)
 {
     m_keyframeAnimations.checkConsistency();
 
-    RefPtr<KeyframeAnimation> keyframeAnim =
m_keyframeAnimations.get(name.impl());
-    if (!keyframeAnim || !keyframeAnim->running())
-        return false;
+    AnimationNameMap::const_iterator animationsEnd =
m_keyframeAnimations.end();
+    for (AnimationNameMap::const_iterator it = m_keyframeAnimations.begin(); it
!= animationsEnd; ++it) {
+        RefPtr<KeyframeAnimation> keyframeAnim = it->value;
+        if (!keyframeAnim || !keyframeAnim->running())
+            continue;
 
-    double count = keyframeAnim->m_animation->iterationCount();
-    if ((t >= 0.0) && ((count == CSSAnimationData::IterationCountInfinite) ||
(t <= count * keyframeAnim->duration()))) {
-        keyframeAnim->freezeAtTime(t);
-        return true;
+        double count = keyframeAnim->m_animation->iterationCount();
+        if ((t >= 0.0) && ((count == CSSAnimationData::IterationCountInfinite)
|| (t <= count * keyframeAnim->duration())))
+            keyframeAnim->freezeAtTime(t);
     }
 
-    return false;
-}
-
-bool CompositeAnimation::pauseTransitionAtTime(CSSPropertyID property, double
t)
-{
-    if ((property < firstCSSProperty) || (property >= firstCSSProperty +
numCSSProperties))
-        return false;
-
-    ImplicitAnimation* implAnim = m_transitions.get(property).get();
-    if (!implAnim) {
-        // Check to see if this property is being animated via a shorthand.
-        // This code is only used for testing, so performance is not critical
here.
-        HashSet<CSSPropertyID> shorthandProperties =
CSSPropertyAnimation::animatableShorthandsAffectingProperty(property);
-        bool anyPaused = false;
-        HashSet<CSSPropertyID>::const_iterator end = shorthandProperties.end();
-        for (HashSet<CSSPropertyID>::const_iterator it =
shorthandProperties.begin(); it != end; ++it) {
-            if (pauseTransitionAtTime(*it, t))
-                anyPaused = true;
-        }
-        return anyPaused;
-    }
+    CSSPropertyTransitionsMap::const_iterator transitionsEnd =
m_transitions.end();
+    for (CSSPropertyTransitionsMap::const_iterator it = m_transitions.begin();
it != transitionsEnd; ++it) {
+        RefPtr<ImplicitAnimation> implAnim = it->value;
 
-    if (!implAnim->running())
-        return false;
+        if (!implAnim->running())
+            continue;
 
-    if ((t >= 0.0) && (t <= implAnim->duration())) {
-        implAnim->freezeAtTime(t);
-        return true;
+        if ((t >= 0.0) && (t <= implAnim->duration()))
+            implAnim->freezeAtTime(t);
     }
-
-    return false;
 }
 
 unsigned CompositeAnimation::numberOfActiveAnimations() const

[commit-bot: I haz the power, 2013-05-07 07:31:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dstockwell@chromium.org/14556022/33001

[commit-bot: I haz the power, 2013-05-07 09:07:22]

Change committed as 149860