Fix AnimationKeyframeEffectModel.MultipleProperties to not depend on the order of CSS prope…

Fix AnimationKeyframeEffectModel.MultipleProperties to not depend on the order of CSS property IDs

No functional change.

BUG=249451
R=jchaffraix@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164085

[Julien - ping for review, 2013-12-16 07:25:09]

https://codereview.chromium.org/111683005/diff/20001/Source/core/animation/Ke...
File Source/core/animation/KeyframeAnimationEffectTest.cpp (right):

https://codereview.chromium.org/111683005/diff/20001/Source/core/animation/Ke...
Source/core/animation/KeyframeAnimationEffectTest.cpp:330: }
Wouldn't this still pass if you have 2 'right' or 2 'left' even though that's
invalid?

[Steve Block, 2013-12-16 23:23:52]

https://codereview.chromium.org/111683005/diff/20001/Source/core/animation/Ke...
File Source/core/animation/KeyframeAnimationEffectTest.cpp (right):

https://codereview.chromium.org/111683005/diff/20001/Source/core/animation/Ke...
Source/core/animation/KeyframeAnimationEffectTest.cpp:330: }
Yes, it would. I didn't bother guarding against this because internally we use
the property IDs as keys in a map, so this shouldn't happen. I'll make the test
more robust though.

[Steve Block, 2013-12-18 04:11:45]

PTAL

[Julien - ping for review, 2013-12-18 08:59:29]

lgtm

[commit-bot: I haz the power, 2013-12-18 08:59:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/steveblock@chromium.org/111683005/30001

[commit-bot: I haz the power, 2013-12-18 08:59:43]

Failed to apply patch for Source/core/animation/KeyframeAnimationEffectTest.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: Source/core/animation/KeyframeAnimationEffectTest.cpp
  |diff --git a/Source/core/animation/KeyframeAnimationEffectTest.cpp
b/Source/core/animation/KeyframeAnimationEffectTest.cpp
  |index
4a493bd4f27d8c023989bac6690ecf9b0180b030..eb6bea7ad376934a1ee41ee86a67007ab943275f
100644
  |--- a/Source/core/animation/KeyframeAnimationEffectTest.cpp
  |+++ b/Source/core/animation/KeyframeAnimationEffectTest.cpp
  --------------------------
  No file to patch.  Skipping patch.
  2 out of 2 hunks ignored

Patch:       Source/core/animation/KeyframeAnimationEffectTest.cpp
Index: Source/core/animation/KeyframeAnimationEffectTest.cpp
diff --git a/Source/core/animation/KeyframeAnimationEffectTest.cpp
b/Source/core/animation/KeyframeAnimationEffectTest.cpp
index
4a493bd4f27d8c023989bac6690ecf9b0180b030..eb6bea7ad376934a1ee41ee86a67007ab943275f
100644
--- a/Source/core/animation/KeyframeAnimationEffectTest.cpp
+++ b/Source/core/animation/KeyframeAnimationEffectTest.cpp
@@ -75,6 +75,16 @@ void expectDoubleValue(double expectedValue,
PassRefPtr<AnimatableValue> value)
     EXPECT_FLOAT_EQ(static_cast<float>(expectedValue), actualValue);
 }
 
+const AnimationEffect::CompositableValue* findValue(const
AnimationEffect::CompositableValueList& values, CSSPropertyID id)
+{
+    for (size_t i = 0; i < values.size(); ++i) {
+        const std::pair<CSSPropertyID,
RefPtr<AnimationEffect::CompositableValue> >& value = values.at(i);
+        if (value.first == id)
+            return value.second.get();
+    }
+    return 0;
+}
+
 
 TEST(AnimationKeyframeAnimationEffectTest, BasicOperation)
 {
@@ -314,11 +324,13 @@ TEST(AnimationKeyframeAnimationEffectTest,
MultipleProperties)
 
     RefPtr<KeyframeAnimationEffect> effect =
KeyframeAnimationEffect::create(keyframes);
     OwnPtr<AnimationEffect::CompositableValueList> values = effect->sample(0,
0.6);
-    ASSERT_EQ(2UL, values->size());
-    EXPECT_TRUE(values->at(0).first == CSSPropertyLeft);
-    expectDoubleValue(5.0,
values->at(0).second->compositeOnto(unknownAnimatableValue(7.0)));
-    EXPECT_TRUE(values->at(1).first == CSSPropertyRight);
-    expectDoubleValue(6.0,
values->at(1).second->compositeOnto(unknownAnimatableValue(7.0)));
+    EXPECT_EQ(2UL, values->size());
+    const AnimationEffect::CompositableValue* leftValue =
findValue(*values.get(), CSSPropertyLeft);
+    ASSERT_TRUE(leftValue);
+    expectDoubleValue(5.0,
leftValue->compositeOnto(unknownAnimatableValue(7.0)));
+    const AnimationEffect::CompositableValue* rightValue =
findValue(*values.get(), CSSPropertyRight);
+    ASSERT_TRUE(rightValue);
+    expectDoubleValue(6.0,
rightValue->compositeOnto(unknownAnimatableValue(7.0)));
 }
 
 TEST(AnimationKeyframeAnimationEffectTest, RecompositeCompositableValue)

[commit-bot: I haz the power, 2013-12-18 11:47:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/steveblock@chromium.org/111683005/50001

[commit-bot: I haz the power, 2013-12-18 12:38:38]

Change committed as 164085