cc: Always commit and redraw after scrolling

During the move from WebKit::IntSize to gfx::Vector2d in the compositor
code an IntSize::isEmpty() got accidentally replaced with
Vector2d::IsZero() in LayerTreeHostImpl::scrollBy(). This caused cases
where we'd change the position of a layer during scrolling but didn't
schedule a commit or a redraw. This happened for instance when
diagonally scrolling a layer against its edge.

This patch fixes the issue by always requesting a commit/redraw if any
layer was moved while applying the scroll delta.

TEST=LayerTreeHostImplTest.scrollByReturnsCorrectValue


Review URL: https://chromiumcodereview.appspot.com/11348381

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@171069 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 9 insertions, 3 deletions

diff --git a/cc/layer_tree_host_impl.cc b/cc/layer_tree_host_impl.cc
index 1e91860..8f32526 100644
--- a/cc/layer_tree_host_impl.cc
+++ b/cc/layer_tree_host_impl.cc
@@ -1292,6 +1292,7 @@ bool LayerTreeHostImpl::scrollBy(const gfx::Point& viewportPoint,
         return false;
 
     gfx::Vector2dF pendingDelta = scrollDelta;
+    bool didScroll = false;
 
     for (LayerImpl* layerImpl = m_currentlyScrollingLayerImpl; layerImpl; layerImpl = layerImpl->parent()) {
         if (!layerImpl->scrollable())
@@ -1309,6 +1310,7 @@ bool LayerTreeHostImpl::scrollBy(const gfx::Point& viewportPoint,
         float moveThresholdSquared = 0.1f * 0.1f;
         if (appliedDelta.LengthSquared() < moveThresholdSquared)
             continue;
+        didScroll = true;
 
         // If the applied delta is within 45 degrees of the input delta, bail out to make it easier
         // to scroll just one layer in one direction without affecting any of its parents.
@@ -1327,12 +1329,11 @@ bool LayerTreeHostImpl::scrollBy(const gfx::Point& viewportPoint,
             break;
     }
 
-    if (!scrollDelta.IsZero() && gfx::ToFlooredVector2d(pendingDelta).IsZero()) {
+    if (didScroll) {
         m_client->setNeedsCommitOnImplThread();
         m_client->setNeedsRedrawOnImplThread();
-        return true;
     }
-    return false;
+    return didScroll;
 }
 
 void LayerTreeHostImpl::clearCurrentlyScrollingLayer()
diff --git a/cc/layer_tree_host_impl_unittest.cc b/cc/layer_tree_host_impl_unittest.cc
index b13b2a9..ececd8b 100644
--- a/cc/layer_tree_host_impl_unittest.cc
+++ b/cc/layer_tree_host_impl_unittest.cc
@@ -518,6 +518,11 @@ TEST_P(LayerTreeHostImplTest, scrollByReturnsCorrectValue)
     EXPECT_TRUE(m_hostImpl->scrollBy(gfx::Point(), gfx::Vector2d(0, -10)));
     EXPECT_TRUE(m_hostImpl->scrollBy(gfx::Point(), gfx::Vector2d(-10, -10)));
 
+    // Scrolling diagonally against an edge will succeed.
+    EXPECT_TRUE(m_hostImpl->scrollBy(gfx::Point(), gfx::Vector2d(10, -10)));
+    EXPECT_TRUE(m_hostImpl->scrollBy(gfx::Point(), gfx::Vector2d(-10, 0)));
+    EXPECT_TRUE(m_hostImpl->scrollBy(gfx::Point(), gfx::Vector2d(-10, 10)));
+
     // Trying to scroll more than the available space will also succeed.
     EXPECT_TRUE(m_hostImpl->scrollBy(gfx::Point(), gfx::Vector2d(5000, 5000)));
 }