Takeover MT initiated animations from the compositor

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 76 matching lines @@
         ScrollableArea::trace(visitor);
     }
 
 private:
     explicit MockScrollableArea(bool scrollAnimatorEnabled)
         : m_scrollAnimatorEnabled(scrollAnimatorEnabled) { }
 
     bool m_scrollAnimatorEnabled;
 };
 
+class TestScrollAnimator : public ScrollAnimator {
+public:
+    explicit TestScrollAnimator(ScrollableArea* scrollableArea, WTF::TimeFunction timingFunction = WTF::monotonicallyIncreasingTime)

[jbroman, 2016/03/07 18:38:41]

This is never constructed with the default argument; why have a default?

Also, two-argument constructors need not be explicit.

[ymalik, 2016/03/07 21:26:07]

Done.

+        : ScrollAnimator(scrollableArea, timingFunction) {};
+    ~TestScrollAnimator() override {};
+
+    void setShouldSendToCompositor(bool send)
+    {
+        m_shouldSendToCompositor = send;
+    }
+
+    bool sendAnimationToCompositor() override
+    {
+        if (m_shouldSendToCompositor) {
+            m_runState = ScrollAnimatorCompositorCoordinator::RunState::RunningOnCompositor;
+            m_compositorAnimationId = 1;
+            return true;
+        }
+        return false;
+    }
+
+protected:
+    void abortAnimation() override {};

[jbroman, 2016/03/07 18:38:41]

nit: superfluous semicolon

+
+private:
+    bool m_shouldSendToCompositor = false;
+};
+
 } // namespace
 
 static void reset(ScrollAnimator& scrollAnimator)
 {
     scrollAnimator.scrollToOffsetWithoutAnimation(FloatPoint());
 }
 
 // TODO(skobes): Add unit tests for composited scrolling paths.
 
 TEST(ScrollAnimatorTest, MainThreadStates)
@@ skipping 162 matching lines @@
     EXPECT_TRUE(result.didScrollX);
     gMockedTime += 0.05;
     scrollAnimator->updateCompositorAnimations();
     EXPECT_FALSE(scrollAnimator->hasRunningAnimation());
     EXPECT_EQ(x + 100, scrollAnimator->currentPosition().x());
     EXPECT_EQ(0, scrollAnimator->currentPosition().y());
 
     reset(*scrollAnimator);
 }
 
+// Test that a smooth scroll offset animation running on the compositor is
+// completed on the main thread.
+TEST(ScrollAnimatorTest, AnimatedScrollTakeover)
+{
+    OwnPtrWillBeRawPtr<MockScrollableArea> scrollableArea =
+        MockScrollableArea::create(true);
+    OwnPtrWillBeRawPtr<TestScrollAnimator> scrollAnimator = adoptPtrWillBeNoop(
+        new TestScrollAnimator(scrollableArea.get(), getMockedTime));
+
+    EXPECT_CALL(*scrollableArea, minimumScrollPosition()).Times(AtLeast(1))
+        .WillRepeatedly(Return(IntPoint()));
+    EXPECT_CALL(*scrollableArea, maximumScrollPosition()).Times(AtLeast(1))
+        .WillRepeatedly(Return(IntPoint(1000, 1000)));
+    EXPECT_CALL(*scrollableArea, setScrollOffset(_, _)).Times(3);

[jbroman, 2016/03/07 18:38:41]

It's not entirely obvious to me why we should expect three setScrollOffsets and
four registerForAnimations. Is this a meaningful expectation, or does this just
happen to be the number of calls?

[ymalik, 2016/03/07 21:26:07]

Actually setScrollOffset is called 2 times, and registerForAnimation is called 3
times (I missed the failure earlier as apparently the test still passes if the
EXPECT_CALL expectation is unsatisfied). 

The reason for the extra registerForAnimation is that when we are running an
animation on the compositor, we deregister for service because the animation is
serviced by the compositor. Thus, we need to registerForAnimation again if the
animation is cancelled or we need to takeover.

[jbroman, 2016/03/07 21:33:17]

Yeah, you'll have to make sure your mock object is collected (if, as here, it's
garbage-collected). I think tdresser ran into a similar issue recently.
A brief comment explaining where the magic numbers come from (unless it's
obvious to everyone working on ScrollAnimator) would be nice. :)

+    EXPECT_CALL(*scrollableArea, registerForAnimation()).Times(4);
+    EXPECT_CALL(*scrollableArea, scheduleAnimation()).Times(AtLeast(1))
+        .WillRepeatedly(Return(true));
+
+    EXPECT_FALSE(scrollAnimator->hasAnimationThatRequiresService());
+
+    // Smooth scroll.
+    ScrollResult result = scrollAnimator->userScroll(ScrollByLine, FloatSize(100, 0));
+    EXPECT_TRUE(scrollAnimator->hasAnimationThatRequiresService());
+    EXPECT_TRUE(result.didScrollX);
+    EXPECT_FLOAT_EQ(0.0, result.unusedScrollDeltaX);
+    EXPECT_TRUE(scrollAnimator->hasRunningAnimation());
+
+    // Update compositor animation.
+    gMockedTime += 0.05;
+    scrollAnimator->setShouldSendToCompositor(true);
+    scrollAnimator->updateCompositorAnimations();
+    EXPECT_EQ(scrollAnimator->m_runState,
+        ScrollAnimatorCompositorCoordinator::RunState::RunningOnCompositor);
+
+    // Takeover.
+    scrollAnimator->takeoverCompositorAnimation();
+    EXPECT_EQ(scrollAnimator->m_runState,
+        ScrollAnimatorCompositorCoordinator::RunState::RunningOnCompositorButNeedsTakeover);
+
+    // Animation should now be running on the main thread.
+    scrollAnimator->setShouldSendToCompositor(false);
+    scrollAnimator->updateCompositorAnimations();
+    EXPECT_EQ(scrollAnimator->m_runState,
+        ScrollAnimatorCompositorCoordinator::RunState::RunningOnMainThread);
+    scrollAnimator->tickAnimation(getMockedTime());
+    EXPECT_NE(100, scrollAnimator->currentPosition().x());
+    EXPECT_NE(0, scrollAnimator->currentPosition().x());
+    EXPECT_EQ(0, scrollAnimator->currentPosition().y());
+    reset(*scrollAnimator);
+}
+
 TEST(ScrollAnimatorTest, Disabled)
 {
     OwnPtrWillBeRawPtr<MockScrollableArea> scrollableArea = MockScrollableArea::create(false);
     OwnPtrWillBeRawPtr<ScrollAnimator> scrollAnimator = adoptPtrWillBeNoop(new ScrollAnimator(scrollableArea.get(), getMockedTime));
 
     EXPECT_CALL(*scrollableArea, minimumScrollPosition()).Times(AtLeast(1)).WillRepeatedly(Return(IntPoint()));
     EXPECT_CALL(*scrollableArea, maximumScrollPosition()).Times(AtLeast(1)).WillRepeatedly(Return(IntPoint(1000, 1000)));
     EXPECT_CALL(*scrollableArea, setScrollOffset(_, _)).Times(8);
     EXPECT_CALL(*scrollableArea, registerForAnimation()).Times(0);
 
@@ skipping 12 matching lines @@
     EXPECT_EQ(0, scrollAnimator->currentPosition().y());
     reset(*scrollAnimator);
 
     scrollAnimator->userScroll(ScrollByPixel, FloatSize(100, 0));
     EXPECT_EQ(100, scrollAnimator->currentPosition().x());
     EXPECT_EQ(0, scrollAnimator->currentPosition().y());
     reset(*scrollAnimator);
 }
 
 } // namespace blink