Support longpress drag selection

ui/touch_selection/touch_selection_controller.cc

Index: ui/touch_selection/touch_selection_controller.cc
diff --git a/ui/touch_selection/touch_selection_controller.cc b/ui/touch_selection/touch_selection_controller.cc
index 3d52b4c3e423f276b65541c1491b0e260411fb86..996ff9989158292d0c8ad6abc394e4d5d8d47ef3 100644
--- a/ui/touch_selection/touch_selection_controller.cc
+++ b/ui/touch_selection/touch_selection_controller.cc
@@ -59,6 +59,10 @@ TouchSelectionController::TouchSelectionController(
       selection_empty_(false),
       selection_editable_(false),
       temporarily_hidden_(false),
+      temporarily_hidden_for_longpress_drag_(false),
+      consume_remaining_motion_for_longpress_drag_(false),
+      has_begun_longpress_drag_(false),
+      has_active_touch_sequence_(false),
       selection_handle_dragged_(false) {
   DCHECK(client_);
 }
@@ -126,6 +130,9 @@ void TouchSelectionController::OnSelectionBoundsChanged(
 }
 
 bool TouchSelectionController::WillHandleTouchEvent(const MotionEvent& event) {
+  if (WillHandleTouchEventForLongPressDrag(event))
+    return true;
+
   if (is_insertion_active_) {
     DCHECK(insertion_handle_);
     return insertion_handle_->WillHandleTouchEvent(event);
@@ -151,7 +158,15 @@ bool TouchSelectionController::WillHandleTouchEvent(const MotionEvent& event) {
   return false;
 }
 
-void TouchSelectionController::OnLongPressEvent() {
+void TouchSelectionController::OnLongPressEvent(const gfx::PointF& position) {
+  // If the longpress occurs at the same location as start of the active
+  // touch sequence, allow the resulting selection to be dragged by the
+  // remainder of the active touch sequence.
+  if (has_active_touch_sequence_ &&
+      ((touch_sequence_start_position_ - position).LengthSquared() <

[mfomitchev, 2015/04/18 16:09:08]

I must be missing something, but why do we need to confirm here that we are
close to touch_sequence_start_position_? If this is resolved as a long press by
the platform, then doesn't that already mean that the current position is close
to the DOWN position (by whatever metrics are used by the platform)? Why do we
need to impose our own custom long press qualifications on top of the already
existing ones?

[jdduke (slow), 2015/04/21 19:25:57]

So, the issue right now is that touch events are fed to the TSC when they arrive
in the browser, but gesture events are fed to the TSC before they are sent to
the renderer. If a touch/gesture sequence is delayed, this means there may be
intervening gestures between the touch and gesture events being observed.

It's a rather nasty issue, which is mostly a consequence of the fact that we use
the longpress/tap signals to allow showing of the selection handles (so we want
to observe them at the very last moment before they're sent to Blink). What we
should really be doing is flipping a user gesture bit on the selection (in
Blink) that propagates back to the browser to tell us whether the selection is
caused by a user gesture, and whether we should show the handles.

The possible solutions are:

1) use the heuristic I have now, which may not be exact, but should handle the
common delayed cases
2) move touch filtering down in the pipeline, so we filter touches just before
they're sent to the renderer - this has some nasty side effects, and also
doesn't handle the case that the gesture sequence itself is delayed
3) bundle the user gesture bits with the selection, as I said before, and
instead use *scroll* gesture event filtering to drive the drag selection
(instead of touch events).

[mfomitchev, 2015/04/21 21:08:57]

If do #3, then the webpage will get all the touch events and would be able to
react to them - even if they resulted in scroll events which are only meant to
move the handles and not impact the webpage in any way. That won't be right...
And if we consume the touch events, the scroll gestures won't be generated.

I talked with tdresser briefly about this, and he didn't think the current
implementation could support generating scroll gestures while preventing touch
events from getting to the renderer (unless we host our own separate gesture
recognizer inside TSC). Do you agree with this assessment or are we missing
something? If you agree - we should probably start a discussion on how to solve
this. Incidentally we have a similar issue with GestureNav.

[jdduke (slow), 2015/04/21 21:28:33]

Indeed, we'd be at the mercy of the web page. This likely wouldn't be an issue
for most sites, but for JS-scrolling sites it might prevent longpress+drag
selection entirely.
Yup, there's no clean way to do this with the existing touch/gesture pipeline.
Note that the StylusTextSelector does in fact host its own gesture detector for
scroll/tap detection. We could do a similar thing here, but it wouldn't solve
the issue you raised with this particular code segment. We still need a way to
"hand off" control of the touch/gesture sequence.

I felt like the above heuristic was a simple compromise that should handle the
99.9% case without too much machinery/plumbing, but long term there's likely a
better fix out there.

[mfomitchev, 2015/04/21 22:20:29]

Agreed.
So.. it seems like we could get a long press during a long press drag:
- long press
- lift the finger before long press is generated/processed by the browser and
quickly press in the same spot again
- generate the second long press and start moving

--> the second long press will arrive while we are long press-dragging.. and
then there will be OnSelectionChanged().. How bad will this screw us up? Is
there any way we could look at some sort of time stamps to prevent this?

+          (tap_slop_ * tap_slop_))) {
+    SetTemporarilyHiddenForLongPressDrag(true);
+  }
   response_pending_input_event_ = LONG_PRESS;
   ShowSelectionHandlesAutomatically();
   ShowInsertionHandleAutomatically();
@@ -191,14 +206,7 @@ void TouchSelectionController::SetTemporarilyHidden(bool hidden) {
   if (temporarily_hidden_ == hidden)
     return;
   temporarily_hidden_ = hidden;
-
-  TouchHandle::AnimationStyle animation_style = GetAnimationStyle(true);
-  if (is_selection_active_) {
-    start_selection_handle_->SetVisible(GetStartVisible(), animation_style);
-    end_selection_handle_->SetVisible(GetEndVisible(), animation_style);
-  }
-  if (is_insertion_active_)
-    insertion_handle_->SetVisible(GetStartVisible(), animation_style);
+  OnHandleVisibilityOverrideChanged();
 }
 
 void TouchSelectionController::OnSelectionEditable(bool editable) {
@@ -445,6 +453,12 @@ void TouchSelectionController::ActivateSelection() {
     selection_start_time_ = base::TimeTicks::Now();
     response_pending_input_event_ = INPUT_EVENT_TYPE_NONE;
     client_->OnSelectionEvent(SELECTION_SHOWN);
+    if (temporarily_hidden_for_longpress_drag_) {
+      consume_remaining_motion_for_longpress_drag_ = true;
+      longpress_drag_selection_offset_ = gfx::Vector2dF();
+      longpress_drag_initial_position_ = gfx::PointF();
+      has_begun_longpress_drag_ = false;
+    }
   }
 }
 
@@ -456,7 +470,8 @@ void TouchSelectionController::DeactivateSelection() {
   LogSelectionEnd();
   start_selection_handle_->SetEnabled(false);
   end_selection_handle_->SetEnabled(false);
-  is_selection_active_ = false;
+  SetTemporarilyHiddenForLongPressDrag(false);
+  consume_remaining_motion_for_longpress_drag_ = false;
   client_->OnSelectionEvent(SELECTION_CLEARED);
 }
 
@@ -469,6 +484,99 @@ void TouchSelectionController::ResetCachedValuesIfInactive() {
   end_orientation_ = TouchHandleOrientation::UNDEFINED;
 }
 
+bool TouchSelectionController::WillHandleTouchEventForLongPressDrag(
+    const MotionEvent& event) {
+  if (event.GetAction() == MotionEvent::ACTION_DOWN) {
+    has_active_touch_sequence_ = true;
+    touch_sequence_start_position_.SetPoint(event.GetX(), event.GetY());
+    SetTemporarilyHiddenForLongPressDrag(false);
+    consume_remaining_motion_for_longpress_drag_ = false;
+  }
+
+  if (event.GetAction() == MotionEvent::ACTION_UP ||
+      event.GetAction() == MotionEvent::ACTION_CANCEL) {
+    has_active_touch_sequence_ = false;
+    touch_sequence_start_position_ = gfx::PointF();
+    SetTemporarilyHiddenForLongPressDrag(false);

[mfomitchev, 2015/04/18 16:09:08]

It would be nice to do UMA logging for this usecase. Ideally I think we would
want to be able to distinguish between handle drag and long press drag from UMA
events.

+    consume_remaining_motion_for_longpress_drag_ = false;

[mfomitchev, 2015/04/21 22:20:29]

When the user lifts the finger at the end of the longpress-drag - do we want to
show the handles? If not, I think subsequently tapping on the selection should
show the handles...

[jdduke (slow), 2015/04/21 22:29:34]

Yep, at least, yes if we want to match the native ANdroid implementation (as
implemented with |SetTemporarilyHiddenForLongPressDrag(false) here).

[mfomitchev, 2015/04/22 19:19:50]

Ah, right. Ok, cool.

+  }
+
+  if (!consume_remaining_motion_for_longpress_drag_)
+    return false;
+
+  if (event.GetAction() != MotionEvent::ACTION_MOVE)
+    return false;
+
+  gfx::PointF position(event.GetX(), event.GetY());
+  if (has_begun_longpress_drag_) {
+    gfx::PointF drag_position = position + longpress_drag_selection_offset_;
+    client_->MoveRangeSelectionExtent(drag_position);
+    return true;
+  }
+
+  // We can't use |touch_sequence_start_position_| as the offset anchor, as
+  // showing the selection UI may have shifted the motion coordinates.
+  if (longpress_drag_initial_position_.IsOrigin()) {
+    longpress_drag_initial_position_ = position;
+    return true;
+  }
+
+  // Allow an additional slop affordance after the longpress occurs.
+  gfx::Vector2dF delta = position - longpress_drag_initial_position_;
+  if (delta.LengthSquared() < tap_slop_ * tap_slop_)
+    return true;
+
+  has_begun_longpress_drag_ = true;
+  gfx::PointF start_position =
+      start_selection_handle_->position() + GetStartLineOffset();
+  gfx::PointF end_position =
+      end_selection_handle_->position() + GetEndLineOffset();
+  gfx::PointF base, extent;
+  if (std::abs(delta.y()) > std::abs(delta.x())) {
+    // If initial motion is vertical, anchor to the appropriate handle position.
+    if (delta.y() < 0) {
+      base = end_position;
+      extent = start_position;
+    } else {
+      base = start_position;
+      extent = end_position;
+    }
+  } else {
+    // Otherwise anchor selection to the handle away from which we're moving.
+    gfx::Vector2dF start_delta = start_position - position;
+    gfx::Vector2dF end_delta = end_position - position;
+    if (gfx::DotProduct(start_delta, delta) >
+        gfx::DotProduct(end_delta, delta)) {
+      base = end_position;
+      extent = start_position;
+    } else {
+      base = start_position;
+      extent = end_position;
+    }
+  }
+  longpress_drag_selection_offset_ = extent - position;
+  client_->SelectBetweenCoordinates(base, extent);
+  return true;
+}
+
+void TouchSelectionController::SetTemporarilyHiddenForLongPressDrag(
+    bool hidden) {
+  if (hidden == temporarily_hidden_for_longpress_drag_)
+    return;
+  temporarily_hidden_for_longpress_drag_ = hidden;
+  OnHandleVisibilityOverrideChanged();
+}
+
+void TouchSelectionController::OnHandleVisibilityOverrideChanged() {
+ TouchHandle::AnimationStyle animation_style = GetAnimationStyle(true);
+  if (is_selection_active_) {
+    start_selection_handle_->SetVisible(GetStartVisible(), animation_style);
+    end_selection_handle_->SetVisible(GetEndVisible(), animation_style);
+  }
+  if (is_insertion_active_)
+    insertion_handle_->SetVisible(GetStartVisible(), animation_style);
+}
+
 gfx::Vector2dF TouchSelectionController::GetStartLineOffset() const {
   return ComputeLineOffsetFromBottom(start_);
 }
@@ -478,11 +586,17 @@ gfx::Vector2dF TouchSelectionController::GetEndLineOffset() const {
 }
 
 bool TouchSelectionController::GetStartVisible() const {
-  return start_.visible() && !temporarily_hidden_;
+  if (!start_.visible())
+    return false;
+
+  return !temporarily_hidden_ && !temporarily_hidden_for_longpress_drag_;
 }
 
 bool TouchSelectionController::GetEndVisible() const {
-  return end_.visible() && !temporarily_hidden_;
+  if (!end_.visible())
+    return false;
+
+  return !temporarily_hidden_ && !temporarily_hidden_for_longpress_drag_;
 }
 
 TouchHandle::AnimationStyle TouchSelectionController::GetAnimationStyle(