ScrollView: Fix ScrollToPosition() to also update the ScrollBar position.

Using ScrollToPosition() can cause the ScrollView and ScrollBar states to go out
of sync, which causes strange scrolling UX behaviour.

BUG=934013

Change-Id: I4b1518c49c87431baa9d32425f9bff7fc41b4608
Reviewed-on: https://chromium-review.googlesource.com/c/1480194Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Tim Song <tengs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634883}

ash/system/message_center/unified_message_center_view_unittest.cc

@@ -395,9 +395,12 @@ TEST_F(UnifiedMessageCenterViewTest,
   message_center_view()->OnMessageCenterScrolled();
 
   EXPECT_TRUE(GetStackingCounter()->visible());
-  // The offset change matches with the scroll amount.
-  EXPECT_EQ(previous_bounds - gfx::Vector2d(0, scroll_amount),
-            GetMessageViewVisibleBounds(2));
+  // The offset change matches with the scroll amount plus the stacking bar
+  // height.
+  EXPECT_EQ(
+      previous_bounds -
+          gfx::Vector2d(0, scroll_amount + kStackingNotificationCounterHeight),
+      GetMessageViewVisibleBounds(2));
 
   GetScroller()->ScrollToPosition(GetScrollBar(), scroll_amount - 1);
   message_center_view()->OnMessageCenterScrolled();

ui/views/controls/scroll_view.cc

@@ -749,7 +749,6 @@ void ScrollView::ScrollContentsRegionToBeVisible(const gfx::Rect& rect) {
                                         contents_viewport_->height());
 
   ScrollToOffset(gfx::ScrollOffset(new_x, new_y));
-  UpdateScrollBarPositions();
 }
 
 void ScrollView::ComputeScrollBarsVisibility(const gfx::Size& vp_size,
@@ -834,6 +833,7 @@ void ScrollView::ScrollToOffset(const gfx::ScrollOffset& offset) {
     ScrollHeader();
   }
   UpdateOverflowIndicatorVisibility(offset);
+  UpdateScrollBarPositions();
 }
 
 bool ScrollView::ScrollsWithLayers() const {

ui/views/controls/scroll_view_unittest.cc

@@ -682,6 +682,34 @@ TEST_F(ScrollViewTest, HeaderScrollsWithContent) {
   EXPECT_EQ("-1,0", header->origin().ToString());
 }
 
+// Test that calling ScrollToPosition() also updates the position of the
+// corresponding ScrollBar.
+TEST_F(ScrollViewTest, ScrollToPositionUpdatesScrollBar) {
+  ScrollViewTestApi test_api(scroll_view_.get());
+  View* contents = InstallContents();
+
+  // Scroll the horizontal scrollbar, after which, the scroll bar thumb position
+  // should be updated (i.e. it should be non-zero).
+  contents->SetBounds(0, 0, 400, 50);
+  scroll_view_->Layout();
+  auto* scroll_bar = test_api.GetBaseScrollBar(HORIZONTAL);
+  ASSERT_TRUE(scroll_bar);
+  EXPECT_TRUE(scroll_bar->visible());
+  EXPECT_EQ(0, scroll_bar->GetPosition());
+  scroll_view_->ScrollToPosition(scroll_bar, 20);
+  EXPECT_GT(scroll_bar->GetPosition(), 0);
+
+  // Scroll the vertical scrollbar.
+  contents->SetBounds(0, 0, 50, 400);
+  scroll_view_->Layout();
+  scroll_bar = test_api.GetBaseScrollBar(VERTICAL);
+  ASSERT_TRUE(scroll_bar);
+  EXPECT_TRUE(scroll_bar->visible());
+  EXPECT_EQ(0, scroll_bar->GetPosition());
+  scroll_view_->ScrollToPosition(scroll_bar, 20);
+  EXPECT_GT(scroll_bar->GetPosition(), 0);
+}
+
 // Verifies ScrollRectToVisible() on the child works.
 TEST_F(ScrollViewTest, ScrollRectToVisible) {
   ScrollViewTestApi test_api(scroll_view_.get());