Reorder "show browser controls" animation after gesture scrolls.

If chrome starts an animation to show browser controls that has
either (a) not completed before a user gesture scroll starts, or
(b) starts during the user gesture scroll, then restart the
animation after the scroll completes.  This makes it unambiguous
to the user that the controls can be present, since the scroll
will cancel the animation otherwise.  We don't want a small
scroll, which is easy to generate accidentally, to have the large
effect of preventing the browser controls from becoming visible.

For simplicity, this CL doesn't try to differentiate a small
scroll from any other; any scroll that overlaps with an animation
to show the browser controls results in the controls becoming
visible when the scroll completes.

Bug: 1341541
Change-Id: I58950b78bd02cb84989290a45cf5f67626508c61
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4136461
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1089760}

cc/input/browser_controls_offset_manager.cc

@@ -195,6 +195,13 @@ void BrowserControlsOffsetManager::UpdateBrowserControlsState(
 
   ResetAnimations();
 
+  // If we're about to animate the controls in, then restart the animation after
+  // the scroll completes.  We don't know if a scroll is in progress, but that's
+  // okay; the flag will be reset when a scroll starts next in that case.
+  if (animate && direction == AnimationDirection::SHOWING_CONTROLS) {
+    show_controls_when_scroll_completes_ = true;
+  }
+
   if (animate)
     SetupAnimation(direction);
   else
@@ -387,6 +394,11 @@ void BrowserControlsOffsetManager::ScrollBegin() {
   if (pinch_gesture_active_)
     return;
 
+  // If an animation to show the controls is in progress, re-order the animation
+  // to start after the scroll completes.  This ensures that the user doesn't
+  // accidentally hide the controls with a gesture that would not normally be
+  // enough to hide them.
+  show_controls_when_scroll_completes_ = IsAnimatingToShowControls();
   ResetAnimations();
   ResetBaseline();
 }
@@ -459,8 +471,13 @@ gfx::Vector2dF BrowserControlsOffsetManager::ScrollBy(
 
   // If the controls are fully visible, treat the current position as the
   // new baseline even if the gesture didn't end.
-  if (TopControlsShownRatio() == 1.f && BottomControlsShownRatio() == 1.f)
+  if (TopControlsShownRatio() == 1.f && BottomControlsShownRatio() == 1.f) {
     ResetBaseline();
+    // Once the controls are fully visible, then any cancelled animation to show
+    // them isn't relevant; the user definitely sees the controls and can decide
+    // if they'd like to keep them.
+    show_controls_when_scroll_completes_ = false;
+  }
 
   ResetAnimations();
 
@@ -476,6 +493,13 @@ void BrowserControlsOffsetManager::ScrollEnd() {
   if (pinch_gesture_active_)
     return;
 
+  // See if we should animate the top bar in, in case there was a race between
+  // chrome showing the controls and the user performing a scroll.
+  if (show_controls_when_scroll_completes_) {
+    SetupAnimation(AnimationDirection::SHOWING_CONTROLS);
+    return;
+  }
+
   StartAnimationIfNecessary();
 }
 

cc/input/browser_controls_offset_manager.h

@@ -81,6 +81,11 @@ class CC_EXPORT BrowserControlsOffsetManager {
   std::pair<float, float> BottomControlsShownRatioRange();
 
   bool HasAnimation();
+  bool IsAnimatingToShowControls() const {
+    return top_controls_animation_.IsInitialized() &&
+           top_controls_animation_.Direction() ==
+               AnimationDirection::SHOWING_CONTROLS;
+  }
 
   void UpdateBrowserControlsState(BrowserControlsState constraints,
                                   BrowserControlsState current,
@@ -177,15 +182,24 @@ class CC_EXPORT BrowserControlsOffsetManager {
   absl::optional<std::pair<float, float>>
       bottom_min_height_offset_animation_range_;
 
+  // Should ScrollEnd() animate the controls into view?  This is used if there's
+  // a race between chrome starting an animation to show the controls while the
+  // user is doing a scroll gesture, which would cancel animations.  We want to
+  // err on the side of showing the controls, so that the user realizes that
+  // they're an option. If we have started, but not yet completed an animation
+  // to show the controls when the scroll starts, or if one starts during the
+  // gesture, then we reorder the animation until after the scroll.
+  bool show_controls_when_scroll_completes_ = false;
+
   // Class that holds and manages the state of the controls animations.
   class Animation {
    public:
     Animation();
 
     // Whether the animation is initialized with a direction and start and stop
     // values.
-    bool IsInitialized() { return initialized_; }
-    AnimationDirection Direction() { return direction_; }
+    bool IsInitialized() const { return initialized_; }
+    AnimationDirection Direction() const { return direction_; }
     void Initialize(AnimationDirection direction,
                     float start_value,
                     float stop_value,

cc/input/browser_controls_offset_manager_unittest.cc

@@ -1299,5 +1299,65 @@ TEST(BrowserControlsOffsetManagerTest,
   EXPECT_FLOAT_EQ(20.f, manager->TopControlsMinHeightOffset());
 }
 
+// Tests that a "show" animation that's interrupted by a scroll is restarted
+// when the gesture completes.
+TEST(BrowserControlsOffsetManagerTest,
+     InterruptedShowAnimationsAreRestartedAfterScroll) {
+  MockBrowserControlsOffsetManagerClient client(100.f, 0.5f, 0.5f);
+  BrowserControlsOffsetManager* manager = client.manager();
+  // Start off with the controls mostly hidden, so that they will, by default,
+  // try to fully hide at the end of a scroll.
+  client.SetCurrentBrowserControlsShownRatio(/*top_ratio=*/0.2,
+                                             /*bottom_ratio=*/0.2);
+
+  EXPECT_FALSE(manager->HasAnimation());
+
+  // Start an animation to show the controls
+  manager->UpdateBrowserControlsState(BrowserControlsState::kBoth,
+                                      BrowserControlsState::kShown, true);
+  EXPECT_TRUE(manager->IsAnimatingToShowControls());
+
+  // Start a scroll, which should cancel the animation.
+  manager->ScrollBegin();
+  EXPECT_FALSE(manager->HasAnimation());
+
+  // Finish the scroll, which should restart the show animation.  Since the
+  // animation didn't run yet, the controls would auto-hide otherwise since they
+  // started almost hidden.
+  manager->ScrollEnd();
+  EXPECT_TRUE(manager->IsAnimatingToShowControls());
+}
+
+// If chrome tries to animate in browser controls during a scroll gesture, it
+// should animate them in after the scroll completes.
+TEST(BrowserControlsOffsetManagerTest,
+     ShowingControlsDuringScrollStartsAnimationAfterScroll) {
+  MockBrowserControlsOffsetManagerClient client(100.f, 0.5f, 0.5f);
+  BrowserControlsOffsetManager* manager = client.manager();
+  // Start off with the controls mostly hidden, so that they will, by default,
+  // try to fully hide at the end of a scroll.
+  client.SetCurrentBrowserControlsShownRatio(/*top_ratio=*/0.2,
+                                             /*bottom_ratio=*/0.2);
+
+  EXPECT_FALSE(manager->HasAnimation());
+
+  // Start a scroll. Make sure that there's no animation running, else we're
+  // testing the wrong case.
+  ASSERT_FALSE(manager->HasAnimation());
+  manager->ScrollBegin();
+  EXPECT_FALSE(manager->HasAnimation());
+
+  // Start an animation to show the controls.
+  manager->UpdateBrowserControlsState(BrowserControlsState::kBoth,
+                                      BrowserControlsState::kShown, true);
+  EXPECT_TRUE(manager->IsAnimatingToShowControls());
+
+  // Finish the scroll, and the animation should still be in progress and/or
+  // restarted.  We don't really care which, as long as it wasn't cancelled and
+  // is trying to show the controls.
+  manager->ScrollEnd();
+  EXPECT_TRUE(manager->IsAnimatingToShowControls());
+}
+
 }  // namespace
 }  // namespace cc