Make the web-animations engine use the passed in blink::WebFrameTime values.

Source/core/animation/AnimationClockTest.cpp

 /*
  * Copyright (c) 2013, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 23 matching lines @@
 #include "wtf/OwnPtr.h"
 #include <gtest/gtest.h>
 
 using namespace WebCore;
 
 namespace {
 
 class AnimationAnimationClockTest : public ::testing::Test {
 public:
     AnimationAnimationClockTest()
-        : animationClock(mockTimeFunction)
+        : animationClock()
     { }
 protected:
     virtual void SetUp()
     {
-        mockTime = 0;
-        animationClock.resetTimeForTesting();
+        animationClock.clearTimeForTesting();
+        animationClock.updateTime(blink::WebFrameTime(-1, -1, 0, 0));

[eseidel, 2014/07/07 16:31:50]

This is extremely opaque. Can we have a helper?

WebFrameTime::fromSeconds(0)
WebFrameTime(SecondsOnly, 0) or some such?

[mithro-old, 2014/07/07 17:47:15]

As you said below, this type of creation only happens in unit tests because
AnimationClock doesn't use the first two fields (where the rest of Blink
does/will).

I created the helper function on this class.

     }
 
-    static double mockTimeFunction()
-    {
-        return mockTime;
-    }
-
-    static double mockTime;
     AnimationClock animationClock;
 };
 
-double AnimationAnimationClockTest::mockTime;
-
 TEST_F(AnimationAnimationClockTest, TimeIsGreaterThanZeroForUnitTests)
 {
     AnimationClock clock;
+    clock.resetTimeForTesting();
+
     // unit tests outside core/animation shouldn't need to do anything to get
     // a non-zero currentTime().
+    double lastTime = clock.currentTime();
     EXPECT_GT(clock.currentTime(), 0);
+
+    clock.tickTimeForTesting();
+    EXPECT_GT(clock.currentTime(), lastTime);
 }
 
 TEST_F(AnimationAnimationClockTest, TimeDoesNotChange)
 {
-    animationClock.updateTime(100);
+    animationClock.updateTime(blink::WebFrameTime(-1, -1, 100, 200));

[eseidel, 2014/07/07 16:31:50]

Or if these are only used in unittests at least a helper for this file.  This is
not readable to my eyes.

[mithro-old, 2014/07/07 17:47:15]

The only code which should be creating blink::WebFrameTime objects is up in
Chrome land which calls this code. For the unit tests I created helper function
for this. 

The first two fields of WebFrameTime are not used by the AnimationClock.

     EXPECT_EQ(100, animationClock.currentTime());
     EXPECT_EQ(100, animationClock.currentTime());
 }
 
 TEST_F(AnimationAnimationClockTest, TimeAdvancesWhenUpdated)
 {
-    animationClock.updateTime(100);
+    animationClock.updateTime(blink::WebFrameTime(-1, -1, 100, 200));
     EXPECT_EQ(100, animationClock.currentTime());
 
-    animationClock.updateTime(200);
+    animationClock.updateTime(blink::WebFrameTime(-1, -1, 200, 300));
     EXPECT_EQ(200, animationClock.currentTime());
 }
 
 TEST_F(AnimationAnimationClockTest, TimeAdvancesToTaskTime)
 {
-    animationClock.updateTime(100);
-    EXPECT_EQ(100, animationClock.currentTime());
-
-    mockTime = 150;
-    AnimationClock::notifyTaskStart();
-    EXPECT_GE(animationClock.currentTime(), mockTime);
-}
-
-TEST_F(AnimationAnimationClockTest, TimeAdvancesToTaskTimeOnlyWhenRequired)
-{
-    animationClock.updateTime(100);
+    animationClock.updateTime(blink::WebFrameTime(-1, -1, 100, 150));
     EXPECT_EQ(100, animationClock.currentTime());
 
     AnimationClock::notifyTaskStart();
-    animationClock.updateTime(125);
-    EXPECT_EQ(125, animationClock.currentTime());
+    EXPECT_EQ(animationClock.currentTime(), 150);
+
+    AnimationClock::notifyTaskStart();
+    EXPECT_EQ(animationClock.currentTime(), 150);
 }
 
 TEST_F(AnimationAnimationClockTest, UpdateTimeIsMonotonic)
 {
-    animationClock.updateTime(100);
+    animationClock.updateTime(blink::WebFrameTime(-1, -1, 100, 150));
     EXPECT_EQ(100, animationClock.currentTime());
+    EXPECT_DEATH({
+        animationClock.updateTime(blink::WebFrameTime(-1, -1, 50, 150));
+    }, "");
 
-    // Update can't go backwards.
-    animationClock.updateTime(50);
-    EXPECT_EQ(100, animationClock.currentTime());
-
-    mockTime = 50;
     AnimationClock::notifyTaskStart();
-    EXPECT_EQ(100, animationClock.currentTime());
-
-    mockTime = 150;
-    AnimationClock::notifyTaskStart();
-    EXPECT_GE(animationClock.currentTime(), mockTime);
-
-    // Update can't go backwards after advance to estimate.
-    animationClock.updateTime(100);
-    EXPECT_GE(animationClock.currentTime(), mockTime);
-}
-
-TEST_F(AnimationAnimationClockTest, CurrentTimeUpdatesTask)
-{
-    animationClock.updateTime(100);
-    EXPECT_EQ(100, animationClock.currentTime());
-
-    mockTime = 100;
-    AnimationClock::notifyTaskStart();
-    EXPECT_EQ(100, animationClock.currentTime());
-
-    mockTime = 150;
-    EXPECT_EQ(100, animationClock.currentTime());
+    EXPECT_DEATH({
+        animationClock.updateTime(blink::WebFrameTime(-1, -1, 149, 150));
+    }, "");
 }
 
 }