[Android] Select text when stylus first button is pressed.

content/browser/renderer_host/input/gesture_text_selector.cc

+// Copyright (c) 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/renderer_host/input/gesture_text_selector.h"
+
+#include "ui/events/event_constants.h"
+#include "ui/events/gesture_detection/gesture_event_data.h"
+#include "ui/events/gesture_detection/motion_event.h"
+
+namespace content {
+
+GestureTextSelector::GestureTextSelector(GestureTextSelectorClient* client)
+    : client_(client),
+      text_selection_triggered_(false),
+      anchor_x_(0.0f),
+      anchor_y_(0.0f) {
+}
+
+GestureTextSelector::~GestureTextSelector() {
+}
+
+bool GestureTextSelector::OnTouchEvent(const ui::MotionEvent& event) {
+  if (event.GetAction() == ui::MotionEvent::ACTION_DOWN)
+    text_selection_triggered_ = ShouldStartTextSelection(event);
+
+  if (text_selection_triggered_) {

[jdduke (slow), 2014/06/24 15:38:36]

Nit: To avoid nesting invert the condition and early return, i.e.,

if (!text_selection_triggered_)
  return false;

...

[Changwan Ryu, 2014/06/25 07:26:20]

Done.

+    client_->ShowSelectionHandlesAutomatically();

[jdduke (slow), 2014/06/24 15:38:36]

We should probably only call this upon ACTION_DOWN.

[Changwan Ryu, 2014/06/25 07:26:20]

Done.

+    switch (event.GetAction()) {
+      case ui::MotionEvent::ACTION_DOWN:
+        client_->Unselect();
+        anchor_x_ = event.GetX();
+        anchor_y_ = event.GetY();
+        break;
+      case ui::MotionEvent::ACTION_MOVE:
+      case ui::MotionEvent::ACTION_UP:
+        if (text_selection_triggered_)
+          client_->SelectRange(anchor_x_, anchor_y_,

[jdduke (slow), 2014/06/24 15:38:36]

Hmm, we shouldn't use SelectRange with both ACTION_DOWN and SCROLL_UPDATE, it
should only be the latter (with SCROLL_UPDATE).

[Changwan Ryu, 2014/06/25 07:26:20]

Done.

+                               event.GetX(), event.GetY());
+        break;
+      case ui::MotionEvent::ACTION_CANCEL:
+        text_selection_triggered_ = false;

[jdduke (slow), 2014/06/24 15:38:37]

ACTION_CANCEL shouldn't reset the state, as the cancel event may trigger some
"cleanup" gestures that would then get forwarded. 

[Changwan Ryu, 2014/06/25 07:26:20]

Done.

+        client_->Unselect();
+        break;
+      default:
+        break;
+    }
+    return true;
+  }
+  return false;
+}
+
+bool GestureTextSelector::OnGestureEvent(const ui::GestureEventData& gesture) {
+  if (text_selection_triggered_) {

[jdduke (slow), 2014/06/24 15:38:37]

Nit:

if (!text_selection_triggered_)
  return false;

if (drop_remaining_sequence_)
  return true;

...

[Changwan Ryu, 2014/06/25 07:26:20]

Done.

+    switch (gesture.type()) {
+      case ui::ET_GESTURE_LONG_PRESS: {
+        client_->SelectWord(gesture.x, gesture.y);
+        // Exits the text selection mode until ACTION_DOWN resets it.
+        // Long press is likely to be followed by ACTION_MOVE, and we don't want
+        // the selected text to be unselected or changed right afterwards.
+        text_selection_triggered_ = false;

[jdduke (slow), 2014/06/24 15:38:37]

The problem with assigning this to false is that then we'll start allowing
touches through to the renderer, when it didn't get the first part of the
sequence.

You'll probably need another condition, |drop_remaining_sequence_| or
|long_press_triggered_|, in which case we drop all remaining gestures and
touches until ACTION_DOWN (resetting the condition).  It might be easier to
formulate this as a 3-state enum, but either way I think you need another bit
here.

[Changwan Ryu, 2014/06/25 07:26:20]

Added drop_remaining_sequence_

+        break;
+      }
+      case ui::ET_GESTURE_SCROLL_BEGIN: {
+        anchor_x_ = gesture.x;
+        anchor_y_ = gesture.y;
+        break;
+      }
+      case ui::ET_GESTURE_SCROLL_UPDATE:
+      case ui::ET_GESTURE_SCROLL_END: {

[jdduke (slow), 2014/06/24 15:38:36]

No need to listen to SCROLL_END, SCROLL_UPDATE should be sufficient.

[Changwan Ryu, 2014/06/25 07:26:20]

Done.

+        client_->SelectRange(anchor_x_, anchor_y_, gesture.x, gesture.y);
+        break;
+      }
+      default:
+        // Suppress all other gestures when we are selecting text.
+        break;
+    }
+    return true;
+  }
+  return false;
+}
+
+// static
+bool GestureTextSelector::ShouldStartTextSelection(
+    const ui::MotionEvent& event) {
+  DCHECK(event.GetPointerCount() > 0);

[jdduke (slow), 2014/06/24 15:38:37]

Nit: DCHECK_GT(event.GetPointerCount(), 0);

[Changwan Ryu, 2014/06/25 07:26:20]

Done.

+  // Currently we are supporting stylus-only cases.
+  const bool is_stylus =
+      event.GetToolType(0) == ui::MotionEvent::TOOL_TYPE_STYLUS;
+  const bool is_only_secondary_button_pressed =
+      event.GetButtonState() == ui::MotionEvent::BUTTON_SECONDARY;
+  return is_stylus && is_only_secondary_button_pressed;
+}
+
+}  // namespace content