Web Animations CSS: Start running animations on the compositor

Source/core/animation/DocumentTimeline.cpp

Index: Source/core/animation/DocumentTimeline.cpp
diff --git a/Source/core/animation/DocumentTimeline.cpp b/Source/core/animation/DocumentTimeline.cpp
index b3b7d08b93dcba629baae80b9b1073f6d829363f..29da7dd8ab6c7d9d504e4e006359b14756192d98 100644
--- a/Source/core/animation/DocumentTimeline.cpp
+++ b/Source/core/animation/DocumentTimeline.cpp
@@ -69,14 +69,20 @@ void DocumentTimeline::add(Player* player)
         m_timing->serviceOnNextFrame();
 }
 
-PassRefPtr<Player> DocumentTimeline::play(TimedItem* child)
+PassRefPtr<Player> DocumentTimeline::createPlayer(TimedItem* child)

[Steve Block, 2013/11/18 05:03:03]

This should return a raw pointer, not a PRP. This object holds a ref to the
newly created Player, so we can't know that the caller wants to take a ref.
timeline.createPlayer()->pause() is valid and shouldn't churn the ref count.

Same is true of play().

Could you pull the addition of this method (and fixing the PRP) into a
prerequisite patch?

[dstockwell, 2013/11/18 06:11:20]

Hmm, maybe I'm confused about how this works. I thought that since we have a
RefPtr here, doing a release() is essentially the same cost?

Either a deref happens in the caller (if we release here) or here (if we return
a raw pointer).

Will split this out after discussing.

 {
     RefPtr<Player> player = Player::create(*this, child);
-    player->setStartTime(currentTime());
     add(player.get());
     return player.release();
 }
 
+PassRefPtr<Player> DocumentTimeline::play(TimedItem* child)
+{
+    RefPtr<Player> player = createPlayer(child);
+    player->setStartTime(currentTime());
+    return player.release();
+}
+
 void DocumentTimeline::wake()
 {
     m_timing->serviceOnNextFrame();
@@ -156,9 +162,16 @@ void DocumentTimeline::dispatchEvents()
 
 size_t DocumentTimeline::numberOfActiveAnimationsForTesting() const
 {
+    if (isNull(m_zeroTime))
+        return 0;
     // Includes all players whose directly associated timed items
     // are current or in effect.
-    return isNull(m_zeroTime) ? 0 : m_players.size();
+    size_t started = 0;
+    for (size_t i = 0; i < m_players.size(); ++i) {
+        if (m_players[i]->hasStartTime())
+            ++started;

[Steve Block, 2013/11/18 05:03:03]

I think this is wrong, but it was before too. See
https://codereview.chromium.org/66953009

[dstockwell, 2013/11/18 06:11:20]

Will wait and rebase.

+    }
+    return started;
 }
 
 } // namespace