ElementAnimations should hold an ObservableList of AnimationPlayers.

cc/animation/animation_player.h

Index: cc/animation/animation_player.h
diff --git a/cc/animation/animation_player.h b/cc/animation/animation_player.h
index 247b24c3d23c5fcc7ec66bedd5c2c0a70519d0db..5876fd9935677809e276dd8c96be2372866aa9fe 100644
--- a/cc/animation/animation_player.h
+++ b/cc/animation/animation_player.h
@@ -31,8 +31,7 @@ class ElementAnimations;
 // same-property animations.
 // Each AnimationPlayer has its copy on the impl thread.
 // This is a CC counterpart for blink::AnimationPlayer (in 1:1 relationship).
-class CC_EXPORT AnimationPlayer : public base::RefCounted<AnimationPlayer>,
-                                  public base::LinkNode<AnimationPlayer> {
+class CC_EXPORT AnimationPlayer : public base::RefCounted<AnimationPlayer> {
  public:
   static scoped_refptr<AnimationPlayer> Create(int id);
   scoped_refptr<AnimationPlayer> CreateImplInstance() const;
@@ -75,31 +74,32 @@ class CC_EXPORT AnimationPlayer : public base::RefCounted<AnimationPlayer>,
   void PushPropertiesTo(AnimationPlayer* player_impl);
 
   // AnimationDelegate routing.
-  void NotifyAnimationStarted(base::TimeTicks monotonic_time,
-                              TargetProperty::Type target_property,
-                              int group);
-  void NotifyAnimationFinished(base::TimeTicks monotonic_time,
-                               TargetProperty::Type target_property,
-                               int group);
-  void NotifyAnimationAborted(base::TimeTicks monotonic_time,
-                              TargetProperty::Type target_property,
-                              int group);
-  void NotifyAnimationTakeover(base::TimeTicks monotonic_time,
-                               TargetProperty::Type target_property,
-                               double animation_start_time,
-                               std::unique_ptr<AnimationCurve> curve);
+  void OnAnimationStarted(base::TimeTicks monotonic_time,

[loyso (OOO), 2016/08/05 00:13:25]

Any rationale for renaming? The comment "AnimationDelegate routing" means that
there is a chain of methods (ElementAnimations::NotifyPlayersAnimationStarted ->
AnimationPlayer::NotifyAnimationStarted -> AnimationDelegate::
NotifyAnimationStarted). No reason to introduce inconsistency in this chain
IMHO. Or you can rename it as a whole :)

[ymalik, 2016/08/05 00:54:04]

I did it because most uses of ObserverList had "on" as a prefix. No real reason,
you're right that it should probably remain notify.

+                          TargetProperty::Type target_property,
+                          int group);
+  void OnAnimationFinished(base::TimeTicks monotonic_time,
+                           TargetProperty::Type target_property,
+                           int group);
+  void OnAnimationAborted(base::TimeTicks monotonic_time,
+                          TargetProperty::Type target_property,
+                          int group);
+  void OnAnimationTakeover(base::TimeTicks monotonic_time,
+                           TargetProperty::Type target_property,
+                           double animation_start_time,
+                           std::unique_ptr<AnimationCurve> curve);
 
   // Whether this player has animations waiting to get sent to ElementAnimations
   bool has_pending_animations_for_testing() const {
     return !animations_.empty();
   }
 
+ protected:

[loyso (OOO), 2016/08/05 00:13:25]

No reason for private section now. This is a final class.

[ymalik, 2016/08/05 00:54:04]

You're right. Done.

+  explicit AnimationPlayer(int id);
+  virtual ~AnimationPlayer();
+
  private:
   friend class base::RefCounted<AnimationPlayer>;
 
-  explicit AnimationPlayer(int id);
-  ~AnimationPlayer();
-
   void SetNeedsCommit();
 
   void RegisterPlayer();