Web Animations CSS: Start running animations on the compositor

Source/core/animation/DocumentTimeline.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 51 matching lines @@
 }
 
 void DocumentTimeline::add(Player* player)
 {
     m_players.append(player);
 
     if (m_document->view())
         m_timing->serviceOnNextFrame();
 }
 
+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);
+    add(player.get());
+    return player.release();
+}
+
 PassRefPtr<Player> DocumentTimeline::play(TimedItem* child)
 {
-    RefPtr<Player> player = Player::create(*this, child);
+    RefPtr<Player> player = createPlayer(child);
     player->setStartTime(currentTime());
-    add(player.get());
     return player.release();
 }
 
 void DocumentTimeline::wake()
 {
     m_timing->serviceOnNextFrame();
 }
 
 bool DocumentTimeline::serviceAnimations()
 {
@@ skipping 62 matching lines @@
 void DocumentTimeline::dispatchEvents()
 {
     Vector<EventToDispatch> events = m_events;
     m_events.clear();
     for (size_t i = 0; i < events.size(); i++)
         events[i].target->dispatchEvent(events[i].event.release());
 }
 
 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