Support longpress drag selection

ui/touch_selection/longpress_drag_selector.cc

Index: ui/touch_selection/longpress_drag_selector.cc
diff --git a/ui/touch_selection/longpress_drag_selector.cc b/ui/touch_selection/longpress_drag_selector.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6055d4e0057e8673f34d9685287dcb2a690d9475
--- /dev/null
+++ b/ui/touch_selection/longpress_drag_selector.cc
@@ -0,0 +1,138 @@
+// Copyright 2015 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 "ui/touch_selection/longpress_drag_selector.h"
+
+#include "ui/events/gesture_detection/motion_event.h"
+
+namespace ui {
+namespace {
+
+const int kLongPressTimeEpsilonMicros = 10;
+
+}  // namespace
+
+LongPressDragSelector::LongPressDragSelector(
+    TouchSelectionDraggableClient* client,
+    float slop_length)
+    : client_(client), slop_length_(slop_length), state_(INACTIVE) {
+}
+
+LongPressDragSelector::~LongPressDragSelector() {
+}
+
+bool LongPressDragSelector::WillHandleTouchEvent(const MotionEvent& event) {
+  switch (event.GetAction()) {
+    case MotionEvent::ACTION_DOWN:
+      touch_down_position_.SetPoint(event.GetX(), event.GetY());
+      touch_down_time_ = event.GetEventTime();
+      longpress_drag_start_position_ = gfx::PointF();
+      SetState(LONGPRESS_PENDING);
+      return false;
+
+    case MotionEvent::ACTION_UP:
+    case MotionEvent::ACTION_CANCEL:
+      SetState(INACTIVE);
+      return false;
+
+    case MotionEvent::ACTION_MOVE:
+      break;
+
+    default:
+      return false;
+  }
+
+  if (state_ != DRAG_PENDING && state_ != DRAGGING)
+    return false;
+
+  gfx::PointF position(event.GetX(), event.GetY());
+  if (state_ == DRAGGING) {
+    gfx::PointF drag_position = position + longpress_drag_selection_offset_;
+    client_->OnDragUpdate(*this, drag_position);
+    return true;
+  }
+
+  // We can't use |touch_down_position_| as the offset anchor, as
+  // showing the selection UI may have shifted the motion coordinates.
+  if (longpress_drag_start_position_.IsOrigin()) {
+    longpress_drag_start_position_ = position;
+    return true;
+  }
+
+  // Allow an additional slop affordance after the longpress occurs.
+  gfx::Vector2dF delta = position - longpress_drag_start_position_;
+  if (delta.LengthSquared() < (slop_length_ * slop_length_))
+    return true;
+
+  gfx::PointF base = selection_start_;
+  gfx::PointF extent = selection_end_;
+  if (std::abs(delta.y()) > std::abs(delta.x())) {
+    // If initial motion is upward, extend the starting selection bound.

[mfomitchev, 2015/04/23 21:05:53]

By the similar logic, wouldn't we want to extend the end selection bound if the
initial motion is downward?

[jdduke (slow), 2015/04/27 20:24:25]

We do, the exceptional case is dragging 'backward', that's why we use the swap.
I changed the logic to use a bool that gets set which then dictates the extent
value, reducing some of the verbosity.

[mfomitchev, 2015/04/28 02:17:30]

Ah, right, I misread the code.

+    if (delta.y() < 0)
+      std::swap(base, extent);
+  } else {
+    // Otherwise extend the selection bound toward which we're moving.
+    gfx::Vector2dF start_delta = selection_start_ - position;
+    if (!start_delta.IsZero())
+      start_delta.Scale(1.f / start_delta.Length());
+    gfx::Vector2dF end_delta = selection_end_ - position;
+    if (!end_delta.IsZero())
+      end_delta.Scale(1.f / start_delta.Length());
+    if (gfx::DotProduct(start_delta, delta) >

[mfomitchev, 2015/04/23 21:05:53]

Heh. Smart.
Long-press is going to select a single word, which I guess also means that both
start and end are going to be on the same line. So this should work fine. If
multiple words could be selected, then dealing with mixed RTL would be
problematic. Also selection starting in the end of one line and ending in the
beginning of the next line (so that both vectors are virtually in the same
direction) probably won't work very well.
So I wonder if it would make more sense to just make an explicit assumption that
start and end are on the same line and simplify this code by just comparing the
direction of horizontal movement to the relative x positions of start and end?

[jdduke (slow), 2015/04/27 20:24:25]

Yeah, without awareness of text direction, making the right call gets tricky.
Perhaps the best long term solution would be to preserve the RTL bits (we
already get them from WebSelection), and expose them here. Then we'd have
something like

1) If dominant motion up/down, select start/end.
2) Else if motion is left/right, select start/end (unless start is RTL, then
reverse).

That should do the "righter" thing for multiline selections.

For now, I'll make the comment that this may not be 100% accurate for mixed RTL
text and/or multiline longpress selections, and I'll remove the default
initialization.

[mfomitchev, 2015/04/28 02:17:30]

We can already make the decision based on whether the motion is left/right. We
don't need to know if the text is RTL, we can just look at whether
(selection_start.x() - selection_end.x()) is the same sign as delta.x().

Something like this:

if ((delta.x() > 0) && (selection_start.x() - selection_end.x() < 0) ||
    (delta.x() < 0) && (selection_start.x() - selection_end.x() > 0)) {
  extend_selection_start = true;
}

In practice with long press both handles are going to be on the same line, so it
seems this should work fine, and it's a lot more readable than doing vector
algebra.

[jdduke (slow), 2015/04/28 20:37:48]

Hmm, yeah, the math isn't quite as clear. 

That said, I was hoping the vector dot product might be slightly more robust to
multiline longpress selections (the common case I was thinking of is when you
longpress the end of a line, and the selection start and selection end are both
to the left of your finger, but on different lines. In that case, if you drag
left, you want the selection start to move, and if you drag right, you want the
selection end to move, and the vector math actually handles that nicely (the
angle is slightly smaller for the selection bound on the following line when you
drag right.. for RTL text that may not be right though).

+        gfx::DotProduct(end_delta, delta)) {
+      std::swap(base, extent);
+    }
+  }
+  longpress_drag_selection_offset_ = extent - position;
+  client_->OnDragBegin(*this, extent);
+  SetState(DRAGGING);
+  return true;
+}
+
+void LongPressDragSelector::OnLongPressEvent(base::TimeTicks event_time,
+                                             const gfx::PointF& position) {
+  // We have no guarantees that the current gesture stream is aligned with the
+  // observed touch stream. We only know that the gesture sequence is downstream
+  // from the touch sequence. Using a time/distance heuristic helps ensure that
+  // the observed longpress corresponds to the active touch sequence.
+  if (state_ == LONGPRESS_PENDING &&
+      // Use a small time epsilon to account for floating point time conversion.
+      (touch_down_time_ - event_time) <
+          base::TimeDelta::FromMicroseconds(kLongPressTimeEpsilonMicros) &&
+      ((touch_down_position_ - position).LengthSquared() <
+       (slop_length_ * slop_length_))) {
+    SetState(SELECTION_PENDING);
+  }
+}
+
+void LongPressDragSelector::OnSelectionActivated(const gfx::PointF& start,
+                                                 const gfx::PointF& end) {
+  if (state_ == SELECTION_PENDING) {
+    SetState(DRAG_PENDING);
+    selection_start_ = start;
+    selection_end_ = end;
+  }
+}
+
+void LongPressDragSelector::OnSelectionDeactivated() {
+  SetState(INACTIVE);
+}
+
+bool LongPressDragSelector::IsActive() const {

[mfomitchev, 2015/04/23 21:05:53]

This is similar to is_dragging() in TouchHandle. Perhaps we could use a common
name and put the method to the TouchSelectionDraggable interface?

[jdduke (slow), 2015/04/27 20:24:25]

Hmm, the current result would be slightly misleading as we may not technically
be dragging yet (i.e., we want to keep the handles hidden until the user lifts
their finger and/or ends the drag). There's no real polymorphic advantage to
doing that, but I'll make the change anyway.

I still need a separate kind of bit that triggers when we show/hide the handles,
as we want the handles to remain hidden while the user's finger is down after
the selection has occurred.

[mfomitchev, 2015/04/28 02:17:30]

Sorry, I should've clarified my comment. I was suggesting we come up with a name
that would work for both IsActive() for LongPressDragSelector (LPDS) and
is_dragging() for the TouchHandle (TH). E.g. simply using IsActive might work
okay, i.e. the handle becomes active when it you press on it, and the LPDS
becomes active when the selection appears. Note that we don't actually care if
the handle is being _dragged_ when we check is_dragging() on the handle from
TSC, we just care if it's the handle the user is currently interacting with. If
we dispatched BeginDrag in MOVE instead of DOWN, DOWN-based IsActive would still
work. I like the notion of making LPDS and TH look more alike from the outside -
I think this could make the opportunities for code reuse become more apparent,
which is why I made the comment.

If you don't like using the same name for IsActive() and is_dragging(), that's
fine. But I don't think we should add IsDragging() to LPDS - there's really no
benefit since it's only used once, inside LPDS. And I liked IsActive more than
IsDetectingDrag, since it returns true in DRAGGING state, and it's a more
appropriate level of abstraction: from TSC's POV we don't necessarily care which
state the longpress sequence is in - we just care if it's active or not.

[jdduke (slow), 2015/04/28 20:37:48]

IsActive() sounds good to me.

+  return state_ != INACTIVE && state_ != LONGPRESS_PENDING;
+}
+
+void LongPressDragSelector::SetState(SelectionState state) {
+  if (state_ == state)
+    return;
+
+  const bool was_dragging = state_ == DRAGGING;
+  state_ = state;
+
+  // TODO(jdduke): Add UMA for tracking relative longpress drag frequency.
+  if (was_dragging)
+    client_->OnDragEnd(*this);
+}
+
+}  // namespace ui