Correctly handle cancelling compositor animations initiated from main thread.

third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp

Index: third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp
diff --git a/third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp b/third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp
index de0000b4a8f5a948cb00f564d08a62014024fb01..9024ccc84ca536d98be100bbd5ea8e5da9d740af 100644
--- a/third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp
+++ b/third_party/WebKit/Source/platform/scroll/ScrollAnimator.cpp
@@ -110,6 +110,13 @@ ScrollResultOneDimensional ScrollAnimator::userScroll(
     FloatPoint targetPos = desiredTargetPosition();
     targetPos.moveBy(pixelDelta);

[ymalik, 2016/02/07 21:23:37]

@ajuma

This is speculatively fix up the assert failure on L128. Presumably what's
happening is that it's in state WaitingToCancelOnCompositor. I suspect that a
user scroll is cancelled and another user scroll happens before
UpdateCompositorState can clear the animation. This is possible right?

[ajuma, 2016/02/08 15:05:35]

Assuming that userScroll can get called at arbitrary times, yes.

 
+    // Reset animation state in case we get a scroll between when an animation
+    // was asked to be cancelled and updateCompositorAnimations is called.
+    if (m_runState == RunState::WaitingToCancelOnCompositor) {
+        abortAnimation();

[ajuma, 2016/02/08 15:05:35]

It looks like this could touch compositing state in a situation where the
document lifecycle state isn't CompositingClean. There's probably an assert
disabler somewhere that's preventing an ASSERT from triggering for this. (And
there's another abortAnimation call above that's similarly problematic.)
Anything involving updating things on the compositor should happen inside of
::updateCompositorAnimations.

[ymalik, 2016/02/08 18:41:23]

Ah right. That's why we have the whole state machine to begin with. Handling
this and the above case from ::updateCompositorAnimations.

+        resetAnimationState();
+    }
+
     if (m_animationCurve) {
         if ((targetPos - m_targetOffset).isZero()) {
             // Report unused delta only if there is no animation running. See
@@ -189,23 +196,11 @@ void ScrollAnimator::tickAnimation(double monotonicTime)
 
 void ScrollAnimator::updateCompositorAnimations()
 {

[ymalik, 2016/02/07 21:23:37]

I don't think we should ever be in a state where we have
m_compositorAnimationId, and are not running on the compositor. I copied this
over from programmatic scroll animator at the time. In the case of a
programmatic scroll, we could be in WaitingToSendToCompositor if another
programmatic scroll happens while we're running on the compositor. In this case,
we either update the target right from SA::UserScroll or go into state
RunningOnCompositorButNeedsUpdate.

WDYT?

[ajuma, 2016/02/08 15:05:35]

Sounds reasonable.

[ymalik, 2016/02/08 18:41:23]

Based on your feedback above, I moved the call to abortAnimation into
::updateComposiorAnimations. Now we can be in a state when
m_compositorAnimationId exists, and the state is WaitingToSendToCompositor
(We're in WaitingToCancelOnCompositor and another user scroll arrives).

Thus, this change should be undone. 

-    if (m_compositorAnimationId && m_runState != RunState::RunningOnCompositor
-        && m_runState != RunState::RunningOnCompositorButNeedsUpdate) {
-        // If the current run state is WaitingToSendToCompositor but we have a
-        // non-zero compositor animation id, there's a currently running
-        // compositor animation that needs to be removed here before the new
-        // animation is added below.
-        ASSERT(m_runState == RunState::WaitingToCancelOnCompositor
-            || m_runState == RunState::WaitingToSendToCompositor);
-
+    if (m_runState == RunState::WaitingToCancelOnCompositor) {
+        ASSERT(m_compositorAnimationId);
         abortAnimation();
-
-        m_compositorAnimationId = 0;
-        m_compositorAnimationGroupId = 0;
-        if (m_runState == RunState::WaitingToCancelOnCompositor) {
-            resetAnimationState();
-            return;
-        }
+        resetAnimationState();
+        return;
     }
 
     if (m_runState == RunState::WaitingToSendToCompositor