Avoid conditional Animation prefinalizers.

Recast the conditionally-eager finalization of Animation objects - only
needed if the Animation object has a CompositorAnimationPlayer attached -
wrapping instead the player object inside an eagerly-finalized object.

By doing so, we remove the need to support explicit prefinalizer
registration.

R=haraken
BUG=673645

Review-Url: https://codereview.chromium.org/2570503002
Cr-Commit-Position: refs/heads/master@{#438089}

third_party/WebKit/Source/core/animation/Animation.cpp

@@ -99,7 +99,6 @@ Animation::Animation(ExecutionContext* executionContext,
       m_compositorState(nullptr),
       m_compositorPending(false),
       m_compositorGroup(0),
-      m_preFinalizerRegistered(false),
       m_currentTimePending(false),
       m_stateIsBeingUpdated(false),
       m_effectSuppressed(false) {
@@ -115,7 +114,8 @@ Animation::Animation(ExecutionContext* executionContext,
 }
 
 Animation::~Animation() {
-  destroyCompositorPlayer();
+  // Verify that m_compositorPlayer has been disposed of.
+  DCHECK(!m_compositorPlayer);
 }
 
 void Animation::dispose() {
@@ -910,17 +910,9 @@ void Animation::endUpdatingState() {
 void Animation::createCompositorPlayer() {
   if (Platform::current()->isThreadedAnimationEnabled() &&
       !m_compositorPlayer) {
-    // We only need to pre-finalize if we are running animations on the
-    // compositor.
-    if (!m_preFinalizerRegistered) {
-      ThreadState::current()->registerPreFinalizer(this);
-      m_preFinalizerRegistered = true;
-    }
-
     DCHECK(Platform::current()->compositorSupport());
-    m_compositorPlayer = CompositorAnimationPlayer::create();
+    m_compositorPlayer = CompositorAnimationPlayerHolder::create(this);
     DCHECK(m_compositorPlayer);
-    m_compositorPlayer->setAnimationDelegate(this);
     attachCompositorTimeline();
   }
 
@@ -932,8 +924,8 @@ void Animation::destroyCompositorPlayer() {
 
   if (m_compositorPlayer) {
     detachCompositorTimeline();
-    m_compositorPlayer->setAnimationDelegate(nullptr);
-    m_compositorPlayer.reset();
+    m_compositorPlayer->detach();
+    m_compositorPlayer = nullptr;
   }
 }
 
@@ -966,8 +958,8 @@ void Animation::attachCompositedLayers() {
 }
 
 void Animation::detachCompositedLayers() {
-  if (m_compositorPlayer && m_compositorPlayer->isElementAttached())
-    m_compositorPlayer->detachElement();
+  if (m_compositorPlayer && m_compositorPlayer->player()->isElementAttached())
+    m_compositorPlayer->player()->detachElement();
 }
 
 void Animation::notifyAnimationStarted(double monotonicTime, int group) {
@@ -1126,8 +1118,36 @@ DEFINE_TRACE(Animation) {
   visitor->trace(m_pendingCancelledEvent);
   visitor->trace(m_finishedPromise);
   visitor->trace(m_readyPromise);
+  visitor->trace(m_compositorPlayer);
   EventTargetWithInlineData::trace(visitor);
   SuspendableObject::trace(visitor);
 }
 
+Animation::CompositorAnimationPlayerHolder*
+Animation::CompositorAnimationPlayerHolder::create(Animation* animation) {
+  return new CompositorAnimationPlayerHolder(animation);
+}
+
+Animation::CompositorAnimationPlayerHolder::CompositorAnimationPlayerHolder(
+    Animation* animation)
+    : m_animation(animation) {
+  ThreadState::current()->registerPreFinalizer(this);
+  m_compositorPlayer = CompositorAnimationPlayer::create();
+  m_compositorPlayer->setAnimationDelegate(m_animation);
+}
+
+void Animation::CompositorAnimationPlayerHolder::dispose() {
+  if (!m_animation)
+    return;
+  m_animation->dispose();
+  DCHECK(!m_animation);
+  DCHECK(!m_compositorPlayer);
+}
+
+void Animation::CompositorAnimationPlayerHolder::detach() {
+  DCHECK(m_compositorPlayer);
+  m_compositorPlayer->setAnimationDelegate(nullptr);
+  m_animation = nullptr;
+  m_compositorPlayer.reset();
+}
 }  // namespace blink

third_party/WebKit/Source/core/animation/Animation.h

@@ -62,7 +62,6 @@ class CORE_EXPORT Animation final : public EventTargetWithInlineData,
                                     public CompositorAnimationPlayerClient {
   DEFINE_WRAPPERTYPEINFO();
   USING_GARBAGE_COLLECTED_MIXIN(Animation);
-  USING_PRE_FINALIZER(Animation, dispose);
 
  public:
   enum AnimationPlayState { Unset, Idle, Pending, Running, Paused, Finished };
@@ -162,7 +161,7 @@ class CORE_EXPORT Animation final : public EventTargetWithInlineData,
   void notifyStartTime(double timelineTime);
   // CompositorAnimationPlayerClient implementation.
   CompositorAnimationPlayer* compositorPlayer() const override {
-    return m_compositorPlayer.get();
+    return m_compositorPlayer ? m_compositorPlayer->player() : nullptr;
   }
 
   bool affects(const Element&, CSSPropertyID) const;
@@ -300,6 +299,33 @@ class CORE_EXPORT Animation final : public EventTargetWithInlineData,
     CompositorPendingChange m_compositorPendingChange;
   };
 
+  // CompositorAnimationPlayer objects need to eagerly sever
+  // their connection to their Animation delegate; use a separate
+  // 'holder' on-heap object to accomplish that.
+  class CompositorAnimationPlayerHolder
+      : public GarbageCollectedFinalized<CompositorAnimationPlayerHolder> {
+    USING_PRE_FINALIZER(CompositorAnimationPlayerHolder, dispose);
+
+   public:
+    static CompositorAnimationPlayerHolder* create(Animation*);
+
+    void detach();
+
+    DEFINE_INLINE_TRACE() { visitor->trace(m_animation); }
+
+    CompositorAnimationPlayer* player() const {
+      return m_compositorPlayer.get();
+    }
+
+   private:
+    explicit CompositorAnimationPlayerHolder(Animation*);
+
+    void dispose();
+
+    std::unique_ptr<CompositorAnimationPlayer> m_compositorPlayer;
+    Member<Animation> m_animation;
+  };
+
   // This mirrors the known compositor state. It is created when a compositor
   // animation is started. Updated once the start time is known and each time
   // modifications are pushed to the compositor.
@@ -307,8 +333,7 @@ class CORE_EXPORT Animation final : public EventTargetWithInlineData,
   bool m_compositorPending;
   int m_compositorGroup;
 
-  std::unique_ptr<CompositorAnimationPlayer> m_compositorPlayer;
-  bool m_preFinalizerRegistered;
+  Member<CompositorAnimationPlayerHolder> m_compositorPlayer;
 
   bool m_currentTimePending;
   bool m_stateIsBeingUpdated;