Fix touch editing handles not shown after trying overscroll

Because for each overscroll session, OverscrollStarted() and
OverscrollCompleted() (in TouchEditableImplAura) are not necessarily
called the same number of times, TouchEditableImplAura could be stuck in
a state of thinking that there is always an scroll in progress which
prevents handles from appearing after a long-press.

BUG=430176

Review URL: https://codereview.chromium.org/963103003

Cr-Commit-Position: refs/heads/master@{#319761}

content/browser/web_contents/touch_editable_impl_aura.cc

@@ -62,11 +62,12 @@ void TouchEditableImplAura::UpdateEditingController() {
 }
 
 void TouchEditableImplAura::OverscrollStarted() {
-  scrolls_in_progress_++;
+  overscroll_in_progress_ = true;
 }
 
 void TouchEditableImplAura::OverscrollCompleted() {
-  ScrollEnded();
+  overscroll_in_progress_ = false;
+  StartTouchEditingIfNecessary();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -106,8 +107,8 @@ void TouchEditableImplAura::OnSelectionOrCursorChanged(
   // If touch editing handles were not visible, we bring them up only if the
   // current event is a gesture event, no scroll/fling/overscoll is in progress,
   // and there is non-zero selection on the page
-  if (selection_gesture_in_process_ && !scrolls_in_progress_ &&
-      selection_anchor_ != selection_focus_) {
+  if (selection_gesture_in_process_ && !scroll_in_progress_ &&
+      !overscroll_in_progress_ && selection_anchor_ != selection_focus_) {
     StartTouchEditing();
     selection_gesture_in_process_ = false;
   }
@@ -158,7 +159,7 @@ bool TouchEditableImplAura::HandleInputEvent(const ui::Event* event) {
       selection_gesture_in_process_ = true;
       break;
     case ui::ET_GESTURE_SCROLL_BEGIN:
-      scrolls_in_progress_++;
+      scroll_in_progress_ = true;;
       // We need to hide selection handles during scroll (including fling and
       // overscroll), but they should be re-activated after scrolling if:
       //  - an existing scroll decided that handles should be shown after
@@ -172,7 +173,8 @@ bool TouchEditableImplAura::HandleInputEvent(const ui::Event* event) {
       EndTouchEditing(true);
       break;
     case ui::ET_GESTURE_SCROLL_END:
-      ScrollEnded();
+      scroll_in_progress_ = false;
+      StartTouchEditingIfNecessary();
       break;
     default:
       break;
@@ -191,7 +193,8 @@ void TouchEditableImplAura::GestureEventAck(int gesture_event_type) {
 }
 
 void TouchEditableImplAura::DidStopFlinging() {
-  ScrollEnded();
+  scroll_in_progress_ = false;
+  StartTouchEditingIfNecessary();
 }
 
 void TouchEditableImplAura::OnViewDestroyed() {
@@ -348,15 +351,16 @@ TouchEditableImplAura::TouchEditableImplAura()
       rwhva_(NULL),
       selection_gesture_in_process_(false),
       handles_hidden_due_to_scroll_(false),
-      scrolls_in_progress_(0),
+      scroll_in_progress_(false),
+      overscroll_in_progress_(false),
       textfield_was_focused_on_tap_(false) {
 }
 
-void TouchEditableImplAura::ScrollEnded() {
-  scrolls_in_progress_--;
+void TouchEditableImplAura::StartTouchEditingIfNecessary() {
   // If there is no scrolling left in progress, show selection handles if they
   // were hidden due to scroll and there is a selection.
-  if (!scrolls_in_progress_ && handles_hidden_due_to_scroll_ &&
+  if (!scroll_in_progress_ && !overscroll_in_progress_ &&
+      handles_hidden_due_to_scroll_ &&
       (selection_anchor_ != selection_focus_ ||
           text_input_type_ != ui::TEXT_INPUT_TYPE_NONE)) {
     StartTouchEditing();
@@ -374,7 +378,8 @@ void TouchEditableImplAura::Cleanup() {
   EndTouchEditing(true);
   selection_gesture_in_process_ = false;
   handles_hidden_due_to_scroll_ = false;
-  scrolls_in_progress_ = 0;
+  scroll_in_progress_ = false;
+  overscroll_in_progress_ = false;
 }
 
 }  // namespace content

content/browser/web_contents/touch_editable_impl_aura.h

@@ -78,7 +78,7 @@ class CONTENT_EXPORT TouchEditableImplAura
 
   // A convenience function that is called after scroll/fling/overscroll ends to
   // re-activate touch selection if necessary.
-  void ScrollEnded();
+  void StartTouchEditingIfNecessary();
 
   void Cleanup();
 
@@ -100,8 +100,9 @@ class CONTENT_EXPORT TouchEditableImplAura
   // whether to re-show handles after a scrolling session.
   bool handles_hidden_due_to_scroll_;
 
-  // Keeps track of number of scrolls/flings/overscrolls in progress.
-  int scrolls_in_progress_;
+  // Keep track of scrolls/overscrolls in progress.
+  bool scroll_in_progress_;
+  bool overscroll_in_progress_;
 
   // Used to track if a textfield was focused when the current tap gesture
   // happened.

content/browser/web_contents/touch_editable_impl_aura_browsertest.cc

@@ -337,6 +337,75 @@ IN_PROC_BROWSER_TEST_F(TouchEditableImplAuraTest,
   EXPECT_TRUE(GetTouchSelectionController(touch_editable));
 }
 
+IN_PROC_BROWSER_TEST_F(TouchEditableImplAuraTest,
+                       TestTouchSelectionWhenOverscrolling) {
+  ASSERT_NO_FATAL_FAILURE(StartTestWithPage("files/touch_selection.html"));
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  RenderFrameHost* main_frame = web_contents->GetMainFrame();
+  WebContentsViewAura* view_aura = static_cast<WebContentsViewAura*>(
+      web_contents->GetView());
+  TestTouchEditableImplAura* touch_editable = new TestTouchEditableImplAura;
+  view_aura->SetTouchEditableForTest(touch_editable);
+  RenderWidgetHostViewAura* rwhva = static_cast<RenderWidgetHostViewAura*>(
+      web_contents->GetRenderWidgetHostView());
+  EXPECT_EQ(GetRenderWidgetHostViewAura(touch_editable), rwhva);
+
+  // Long press to select word.
+  ui::GestureEvent long_press(
+      10,
+      10,
+      0,
+      ui::EventTimeForNow(),
+      ui::GestureEventDetails(ui::ET_GESTURE_LONG_PRESS));
+  touch_editable->Reset();
+  rwhva->OnGestureEvent(&long_press);
+  touch_editable->WaitForSelectionChangeCallback();
+
+  // Check if selection handles are showing.
+  EXPECT_TRUE(GetTouchSelectionController(touch_editable));
+
+  scoped_ptr<base::Value> value =
+      content::ExecuteScriptAndGetValue(main_frame, "get_selection()");
+  std::string selection;
+  value->GetAsString(&selection);
+  EXPECT_STREQ("Some", selection.c_str());
+
+  // Overscroll is preceded with a scroll. Handles should get hidden.
+  ui::GestureEvent scroll_begin(
+      10,
+      10,
+      0,
+      ui::EventTimeForNow(),
+      ui::GestureEventDetails(ui::ET_GESTURE_SCROLL_BEGIN, 0, 0));
+  rwhva->OnGestureEvent(&scroll_begin);
+  EXPECT_FALSE(GetTouchSelectionController(touch_editable));
+
+  // Then overscroll itself starts. Handles should remain hidden.
+  touch_editable->OverscrollStarted();
+  EXPECT_FALSE(GetTouchSelectionController(touch_editable));
+
+  // We might have multiple overscroll-starts in one overscroll session. Handles
+  // should still remain hidden.
+  touch_editable->OverscrollStarted();
+  EXPECT_FALSE(GetTouchSelectionController(touch_editable));
+
+  // An overscroll session ends with a single overscroll-complete.  Handles
+  // should still remain hidden as the scroll is still in progress.
+  touch_editable->OverscrollCompleted();
+  EXPECT_FALSE(GetTouchSelectionController(touch_editable));
+
+  // And, finally an scroll-end. Handles should come back.
+  ui::GestureEvent scroll_end(
+      10,
+      10,
+      0,
+      ui::EventTimeForNow(),
+      ui::GestureEventDetails(ui::ET_GESTURE_SCROLL_END));
+  rwhva->OnGestureEvent(&scroll_end);
+  EXPECT_TRUE(GetTouchSelectionController(touch_editable));
+}
+
 IN_PROC_BROWSER_TEST_F(TouchEditableImplAuraTest,
                        TouchSelectionOnLongPressTest) {
   ASSERT_NO_FATAL_FAILURE(StartTestWithPage("files/touch_selection.html"));