Communicate MT changes to impl-only scroll offset animations

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

 /*
  * Copyright (C) 2010 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:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 501 matching lines @@
     EXPECT_EQ(scrollAnimator->m_runState,
         ScrollAnimatorCompositorCoordinator::RunState::RunningOnCompositorButNeedsUpdate);
     EXPECT_EQ(250, scrollAnimator->desiredTargetPosition().x());
     EXPECT_EQ(0, scrollAnimator->desiredTargetPosition().y());
     reset(*scrollAnimator);
 
     // Forced GC in order to finalize objects depending on the mock object.
     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