Communicate MT changes to impl-only scroll offset animations

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

Index: third_party/WebKit/Source/platform/scroll/ScrollAnimatorTest.cpp
diff --git a/third_party/WebKit/Source/platform/scroll/ScrollAnimatorTest.cpp b/third_party/WebKit/Source/platform/scroll/ScrollAnimatorTest.cpp
index 679f5a6e598f7cd7b292fba1d710d8fb06348147..7116bc797d9363eea635377ba762a69379e3b673 100644
--- a/third_party/WebKit/Source/platform/scroll/ScrollAnimatorTest.cpp
+++ b/third_party/WebKit/Source/platform/scroll/ScrollAnimatorTest.cpp
@@ -519,4 +519,33 @@ TEST(ScrollAnimatorTest, CancellingCompositorAnimation)
     ThreadHeap::collectAllGarbage();
 }
 
+// This test verifies that ImplOnlyAnimationUpdates get cleared once they're
+// pushed to compositor animation host.
+TEST(ScrollAnimatorTest, ImplOnlyAnimationUpdatesCleared)
+{
+    MockScrollableArea* scrollableArea = MockScrollableArea::create(true);
+    TestScrollAnimator* scrollAnimator = new TestScrollAnimator(scrollableArea, getMockedTime);
+
+    EXPECT_CALL(*scrollableArea, registerForAnimation()).Times(2);
+
+    EXPECT_EQ(scrollAnimator->m_runState, ScrollAnimatorCompositorCoordinator::RunState::Idle);
+    EXPECT_FALSE(scrollAnimator->hasAnimationThatRequiresService());
+    EXPECT_TRUE(scrollAnimator->m_implOnlyAnimationAdjustments.empty());
+
+    scrollAnimator->updateImplOnlyScrollOffsetAnimation(FloatSize(100.f, 100.f));
+    scrollAnimator->updateImplOnlyScrollOffsetAnimation(FloatSize(10.f, -10.f));
+
+    EXPECT_TRUE(scrollAnimator->hasAnimationThatRequiresService());
+    EXPECT_EQ(2U, scrollAnimator->m_implOnlyAnimationAdjustments.size());

[jbroman, 2016/05/16 19:15:11]

Accessing the private members of an object is unfortunate. I'd rather test the
client-visible behaviour if it's not too inconvenient (failing that, a
-ForTesting accessor -- though that ship may have sailed already with
m_runState).

Why is it important that the updates be cleared once they're pushed to the host?
Is it possible to test for whatever bad behaviour that would cause, or is that
difficult?

[ymalik, 2016/05/16 20:52:35]

The correct way to test the bad behavior would require a test suite that
connects the TestScrollAnimator to the scroll animation logic in CC, which is a
lot of work.

I added a ForTesting accessor and a TODO for m_runState.

+
+    scrollAnimator->updateCompositorAnimations();
+
+    EXPECT_EQ(scrollAnimator->m_runState, ScrollAnimatorCompositorCoordinator::RunState::Idle);
+    EXPECT_FALSE(scrollAnimator->hasAnimationThatRequiresService());
+    EXPECT_TRUE(scrollAnimator->m_implOnlyAnimationAdjustments.empty());
+
+    // Forced GC in order to finalize objects depending on the mock object.
+    ThreadHeap::collectAllGarbage();
+}
+
 } // namespace blink