Fix the omnibox stops at the middle while we stop at middle

When we scroll the page and stops at the middle of the omnibox, the
page should either go up or down to either show the whole omnibox or
not show the omnibox at all. When I moved the ScrollEnd function
(https://chromium-review.googlesource.com/c/chromium/src/+/1688884) in
InputHandlerProxy::HandleGestureScrollEnd to be called for only
impl thread scroll only, so the scroll snap does not happen for main
thread scroll. We should call ScrollEnd for both impl and main threads
scroll.

Bug: 1026033
Change-Id: Id15379d70e4ccaab28b578e0798735141caac627
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972460
Commit-Queue: Lan Wei <lanwei@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736057}

third_party/blink/renderer/core/exported/web_view_impl.cc

@@ -1284,7 +1284,7 @@ void WebViewImpl::UpdateBrowserControlsConstraint(
       GetBrowserControls().PermittedState();
 
   GetBrowserControls().UpdateConstraintsAndState(
-      constraint, cc::BrowserControlsState::kBoth, false);
+      constraint, cc::BrowserControlsState::kBoth);
 
   // If the controls are going from a locked hidden to unlocked state, or vice
   // versa, the ICB size needs to change but we can't rely on getting a

third_party/blink/renderer/core/frame/browser_controls.cc

@@ -96,6 +96,32 @@ FloatSize BrowserControls::ScrollBy(FloatSize pending_delta) {
   return pending_delta - applied_delta;
 }
 
+void BrowserControls::ScrollEnd() {
+  if ((top_shown_ratio_ == TopMinShownRatio() || top_shown_ratio_ == 1) &&
+      (bottom_shown_ratio_ == BottomMinShownRatio() ||
+       bottom_shown_ratio_ == 1)) {
+    return;
+  }
+
+  // Both threshold values are copied from LayerTreeSettings, which are used in
+  // BrowserControlsOffsetManager::ScrollEnd.
+  constexpr float kTopControlsShowThreshold = 0.5f;
+  constexpr float kTopControlsHideThreshold = 0.5f;
+  float normalized_top_ratio =
+      (top_shown_ratio_ - TopMinShownRatio()) / (1.f - TopMinShownRatio());
+  if (normalized_top_ratio >= 1.f - kTopControlsHideThreshold) {
+    // If we're showing so much that the hide threshold won't trigger, show.
+    UpdateConstraintsAndState(permitted_state_,
+                              cc::BrowserControlsState::kShown);
+  } else if (normalized_top_ratio < kTopControlsShowThreshold) {
+    // If we're showing so little that the show threshold won't trigger, hide.
+    UpdateConstraintsAndState(permitted_state_,
+                              cc::BrowserControlsState::kHidden);
+  } else {
+    NOTREACHED();
+  }
+}
+
 void BrowserControls::ResetBaseline() {
   accumulated_scroll_delta_ = 0;
   baseline_top_content_offset_ = ContentOffset();
@@ -133,14 +159,22 @@ void BrowserControls::SetShownRatio(float top_ratio, float bottom_ratio) {
 
 void BrowserControls::UpdateConstraintsAndState(
     cc::BrowserControlsState constraints,
-    cc::BrowserControlsState current,
-    bool animate) {
+    cc::BrowserControlsState current) {
   permitted_state_ = constraints;
 
   DCHECK(!(constraints == cc::BrowserControlsState::kShown &&
            current == cc::BrowserControlsState::kHidden));
   DCHECK(!(constraints == cc::BrowserControlsState::kHidden &&
            current == cc::BrowserControlsState::kShown));
+
+  if (current == cc::BrowserControlsState::kShown) {
+    top_shown_ratio_ = 1;
+    bottom_shown_ratio_ = 1;
+  } else if (current == cc::BrowserControlsState::kHidden) {
+    top_shown_ratio_ = TopMinShownRatio();
+    bottom_shown_ratio_ = BottomMinShownRatio();
+  }
+  page_->GetChromeClient().DidUpdateBrowserControls();
 }
 
 void BrowserControls::SetParams(cc::BrowserControlsParams params) {

third_party/blink/renderer/core/frame/browser_controls.h

@@ -55,8 +55,7 @@ class CORE_EXPORT BrowserControls final
   void SetShownRatio(float top_ratio, float bottom_ratio);
 
   void UpdateConstraintsAndState(cc::BrowserControlsState constraints,
-                                 cc::BrowserControlsState current,
-                                 bool animate);
+                                 cc::BrowserControlsState current);
 
   void ScrollBegin();
 
@@ -64,6 +63,8 @@ class CORE_EXPORT BrowserControls final
   // scroll amount.
   FloatSize ScrollBy(FloatSize scroll_delta);
 
+  void ScrollEnd();
+
   cc::BrowserControlsState PermittedState() const { return permitted_state_; }
 
  private:

third_party/blink/renderer/core/frame/browser_controls_test.cc

@@ -330,10 +330,12 @@ TEST_F(BrowserControlsTest, MAYBE(ShowBottomControlsOnScrollUp)) {
       GenerateEvent(WebInputEvent::kGestureScrollBegin));
   web_view->MainFrameWidget()->HandleInputEvent(
       GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, 25.f));
+  EXPECT_FLOAT_EQ(0.5f, web_view->GetBrowserControls().BottomShownRatio());
+
   web_view->MainFrameWidget()->HandleInputEvent(
       GenerateEvent(WebInputEvent::kGestureScrollEnd));
   EXPECT_FLOAT_EQ(0.f, web_view->GetBrowserControls().ContentOffset());
-  EXPECT_FLOAT_EQ(0.5f, web_view->GetBrowserControls().BottomShownRatio());
+  EXPECT_FLOAT_EQ(1.f, web_view->GetBrowserControls().BottomShownRatio());
   EXPECT_EQ(ScrollOffset(0, 25),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 }
@@ -548,42 +550,54 @@ TEST_F(BrowserControlsTest, MAYBE(ScrollableSubregionScrollFirst)) {
       mojom::blink::ScrollIntoViewParams::Type::kProgrammatic);
 
   // Test scroll down
-  // Scroll down should scroll the overflow div first but browser controls and
-  // main frame should not scroll.
+  // A full scroll down should scroll the overflow div first but browser
+  // controls and main frame should not scroll.
   VerticalScroll(-800.f);
   EXPECT_FLOAT_EQ(50.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 50),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 
-  // Continued scroll down should start hiding browser controls but main frame
+  // Now scroll down should start hiding browser controls but main frame
   // should not scroll.
-  VerticalScroll(-40.f);
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollBegin, 0, -40.f));
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, -40.f));
   EXPECT_FLOAT_EQ(10.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 50),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 
   // Continued scroll down should scroll down the main frame
-  VerticalScroll(-40.f);
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, -40.f));
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollEnd));
   EXPECT_FLOAT_EQ(0.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 80),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 
   // Test scroll up
-  // scroll up should scroll overflow div first
+  // A full scroll up should scroll overflow div first
   VerticalScroll(800.f);
   EXPECT_FLOAT_EQ(0.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 80),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 
-  // Continued scroll up should start showing browser controls but main frame
+  // Now scroll up should start showing browser controls but main frame
   // should not scroll.
-  VerticalScroll(40.f);
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollBegin, 0, 40.f));
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, 40.f));
   EXPECT_FLOAT_EQ(40.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 80),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 
-  // Continued scroll down up scroll up the main frame
-  VerticalScroll(40.f);
+  // Continued scroll up scroll up the main frame
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, 40.f));
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollEnd));
   EXPECT_FLOAT_EQ(50.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 50),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
@@ -600,42 +614,54 @@ TEST_F(BrowserControlsTest, MAYBE(ScrollableIframeScrollFirst)) {
       mojom::blink::ScrollIntoViewParams::Type::kProgrammatic);
 
   // Test scroll down
-  // Scroll down should scroll the iframe first but browser controls and main
-  // frame should not scroll.
+  // A full scroll down should scroll the iframe first but browser controls and
+  // main frame should not scroll.
   VerticalScroll(-800.f);
   EXPECT_FLOAT_EQ(50.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 50),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 
-  // Continued scroll down should start hiding browser controls but main frame
+  // Now scroll down should start hiding browser controls but main frame
   // should not scroll.
-  VerticalScroll(-40.f);
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollBegin, 0, -40.f));
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, -40.f));
   EXPECT_FLOAT_EQ(10.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 50),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 
   // Continued scroll down should scroll down the main frame
-  VerticalScroll(-40.f);
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, -40.f));
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollEnd));
   EXPECT_FLOAT_EQ(0.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 80),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 
   // Test scroll up
-  // scroll up should scroll iframe first
+  // A full scroll up should scroll iframe first
   VerticalScroll(800.f);
   EXPECT_FLOAT_EQ(0.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 80),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 
-  // Continued scroll up should start showing browser controls but main frame
+  // Now scroll up should start showing browser controls but main frame
   // should not scroll.
-  VerticalScroll(40.f);
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollBegin, 0, 40.f));
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, 40.f));
   EXPECT_FLOAT_EQ(40.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 80),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
 
-  // Continued scroll down up scroll up the main frame
-  VerticalScroll(40.f);
+  // Continued scroll up scroll up the main frame
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, 40.f));
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollEnd));
   EXPECT_FLOAT_EQ(50.f, web_view->GetBrowserControls().ContentOffset());
   EXPECT_EQ(ScrollOffset(0, 50),
             GetFrame()->View()->LayoutViewport()->GetScrollOffset());
@@ -706,16 +732,30 @@ TEST_F(BrowserControlsTest, MAYBE(ScrollUpPastLimitDoesNotHide)) {
   // Fully scroll frameview but visualviewport remains scrollable
   web_view->MainFrameImpl()->SetScrollOffset(WebSize(0, 10000));
   GetVisualViewport().SetLocation(FloatPoint(0, 0));
-  VerticalScroll(-10.f);
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollBegin, 0, -10.f));
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, -10.f));
   EXPECT_FLOAT_EQ(40, web_view->GetBrowserControls().ContentOffset());
 
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollEnd));
+  EXPECT_FLOAT_EQ(50, web_view->GetBrowserControls().ContentOffset());
+
   web_view->GetBrowserControls().SetShownRatio(1, 1);
   // Fully scroll visual veiwport but frameview remains scrollable
   web_view->MainFrameImpl()->SetScrollOffset(WebSize(0, 0));
   GetVisualViewport().SetLocation(FloatPoint(0, 10000));
-  VerticalScroll(-20.f);
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollBegin, 0, -20.f));
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, -20.f));
   EXPECT_FLOAT_EQ(30, web_view->GetBrowserControls().ContentOffset());
 
+  GetWebView()->MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollEnd));
+  EXPECT_FLOAT_EQ(50, web_view->GetBrowserControls().ContentOffset());
+
   web_view->GetBrowserControls().SetShownRatio(1, 1);
   // Fully scroll both frameview and visual viewport
   web_view->MainFrameImpl()->SetScrollOffset(WebSize(0, 10000));
@@ -800,12 +840,19 @@ TEST_F(BrowserControlsSimTest, MAYBE(StateConstraints)) {
             GetDocument().View()->LayoutViewport()->GetScrollOffset());
 
   // Setting permitted state to "both" should not change an in-flight offset.
-  VerticalScroll(20.f);
+  WebView().MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollBegin, 0, 20.f));
+  WebView().MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollUpdate, 0, 20.f));
   EXPECT_FLOAT_EQ(20, WebView().GetBrowserControls().ContentOffset());
+
+  WebView().MainFrameWidget()->HandleInputEvent(
+      GenerateEvent(WebInputEvent::kGestureScrollEnd));
+  EXPECT_FLOAT_EQ(0, WebView().GetBrowserControls().ContentOffset());
   Compositor().layer_tree_host().UpdateBrowserControlsState(
       cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kBoth, false);
   Compositor().BeginFrame();
-  EXPECT_FLOAT_EQ(20, WebView().GetBrowserControls().ContentOffset());
+  EXPECT_FLOAT_EQ(0, WebView().GetBrowserControls().ContentOffset());
 
   // Setting just the constraint should affect the content offset.
   Compositor().layer_tree_host().UpdateBrowserControlsState(
@@ -827,7 +874,7 @@ TEST_F(BrowserControlsTest, MAYBE(DontAffectLayoutHeight)) {
   WebViewImpl* web_view = Initialize("percent-height.html");
   web_view->ResizeWithBrowserControls(WebSize(400, 300), 100.f, 0, true);
   web_view->GetBrowserControls().UpdateConstraintsAndState(
-      cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kShown, false);
+      cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kShown);
   web_view->GetBrowserControls().SetShownRatio(1, 1);
   UpdateAllLifecyclePhases();
 
@@ -986,7 +1033,7 @@ TEST_F(BrowserControlsTest, MAYBE(DontAffectVHUnits)) {
   WebViewImpl* web_view = Initialize("vh-height.html");
   web_view->ResizeWithBrowserControls(WebSize(400, 300), 100.f, 0, true);
   web_view->GetBrowserControls().UpdateConstraintsAndState(
-      cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kShown, false);
+      cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kShown);
   web_view->GetBrowserControls().SetShownRatio(1, 1);
   UpdateAllLifecyclePhases();
 
@@ -1028,7 +1075,7 @@ TEST_F(BrowserControlsTest, MAYBE(DontAffectVHUnitsWithScale)) {
   WebViewImpl* web_view = Initialize("vh-height-width-800.html");
   web_view->ResizeWithBrowserControls(WebSize(400, 300), 100.f, 0, true);
   web_view->GetBrowserControls().UpdateConstraintsAndState(
-      cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kShown, false);
+      cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kShown);
   web_view->GetBrowserControls().SetShownRatio(1, 1);
   UpdateAllLifecyclePhases();
 
@@ -1078,7 +1125,7 @@ TEST_F(BrowserControlsTest, MAYBE(DontAffectVHUnitsUseLayoutSize)) {
   WebViewImpl* web_view = Initialize("vh-height-width-800-extra-wide.html");
   web_view->ResizeWithBrowserControls(WebSize(400, 300), 100.f, 0, true);
   web_view->GetBrowserControls().UpdateConstraintsAndState(
-      cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kShown, false);
+      cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kShown);
   web_view->GetBrowserControls().SetShownRatio(1, 1);
   UpdateAllLifecyclePhases();
 
@@ -1114,7 +1161,7 @@ TEST_F(BrowserControlsTest,
   web_view->ResizeWithBrowserControls(WebSize(800, layout_viewport_height),
                                       browser_controls_height, 0, true);
   web_view->GetBrowserControls().UpdateConstraintsAndState(
-      cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kShown, false);
+      cc::BrowserControlsState::kBoth, cc::BrowserControlsState::kShown);
   web_view->GetBrowserControls().SetShownRatio(1, 1);
   UpdateAllLifecyclePhases();
 
@@ -1323,8 +1370,7 @@ TEST_F(BrowserControlsTest, MAYBE(GrowingHeightKeepsTopControlsHidden)) {
                                       bottom_height, false);
 
   web_view->GetBrowserControls().UpdateConstraintsAndState(
-      cc::BrowserControlsState::kHidden, cc::BrowserControlsState::kHidden,
-      false);
+      cc::BrowserControlsState::kHidden, cc::BrowserControlsState::kHidden);
 
   // As we expand the top controls height while hidden, the content offset
   // shouldn't change.
@@ -1601,16 +1647,14 @@ TEST_F(BrowserControlsSimTest, ConstraintDoesntClampRatioInBlink) {
 
   {
     // Pass a hidden constraint to Blink (without going through CC). Make sure
-    // the shown ratio doesn't change since CC is repsonsible for updating the
+    // the shown ratio doesn't change since CC is responsible for updating the
     // ratio.
     WebView().GetBrowserControls().UpdateConstraintsAndState(
-        cc::BrowserControlsState::kHidden, cc::BrowserControlsState::kBoth,
-        true /* animated */);
+        cc::BrowserControlsState::kHidden, cc::BrowserControlsState::kBoth);
     EXPECT_EQ(1.f, WebView().GetBrowserControls().TopShownRatio());
     EXPECT_EQ(1.f, WebView().GetBrowserControls().BottomShownRatio());
     WebView().GetBrowserControls().UpdateConstraintsAndState(
-        cc::BrowserControlsState::kHidden, cc::BrowserControlsState::kBoth,
-        false /* animated */);
+        cc::BrowserControlsState::kHidden, cc::BrowserControlsState::kBoth);
     EXPECT_EQ(1.f, WebView().GetBrowserControls().TopShownRatio());
     EXPECT_EQ(1.f, WebView().GetBrowserControls().BottomShownRatio());
 
@@ -1629,13 +1673,11 @@ TEST_F(BrowserControlsSimTest, ConstraintDoesntClampRatioInBlink) {
     // Pass a shown constraint to Blink (without going through CC). Make sure
     // the shown ratio doesn't change.
     WebView().GetBrowserControls().UpdateConstraintsAndState(
-        cc::BrowserControlsState::kShown, cc::BrowserControlsState::kBoth,
-        true /* animated */);
+        cc::BrowserControlsState::kShown, cc::BrowserControlsState::kBoth);
     EXPECT_EQ(0.f, WebView().GetBrowserControls().TopShownRatio());
     EXPECT_EQ(0.f, WebView().GetBrowserControls().BottomShownRatio());
     WebView().GetBrowserControls().UpdateConstraintsAndState(
-        cc::BrowserControlsState::kShown, cc::BrowserControlsState::kBoth,
-        false /* animated */);
+        cc::BrowserControlsState::kShown, cc::BrowserControlsState::kBoth);
     EXPECT_EQ(0.f, WebView().GetBrowserControls().TopShownRatio());
     EXPECT_EQ(0.f, WebView().GetBrowserControls().BottomShownRatio());
 

third_party/blink/renderer/core/input/scroll_manager.cc

@@ -659,6 +659,8 @@ void ScrollManager::HandleDeferredGestureScrollEnd(
 WebInputEventResult ScrollManager::HandleGestureScrollEnd(
     const WebGestureEvent& gesture_event) {
   TRACE_EVENT0("input", "ScrollManager::handleGestureScrollEnd");
+  GetPage()->GetBrowserControls().ScrollEnd();
+
   Node* node = scroll_gesture_handling_node_;
 
   if (node && node->GetLayoutObject()) {