Update touch selection to only modify one selection point at a time.

content/browser/renderer_host/input/touch_selection_controller.cc

Index: content/browser/renderer_host/input/touch_selection_controller.cc
diff --git a/content/browser/renderer_host/input/touch_selection_controller.cc b/content/browser/renderer_host/input/touch_selection_controller.cc
index e904e283ee0716401cea8556353b8eba048032d1..ee9c7fc899039f8b189db12675439ac20f521b3f 100644
--- a/content/browser/renderer_host/input/touch_selection_controller.cc
+++ b/content/browser/renderer_host/input/touch_selection_controller.cc
@@ -209,13 +209,23 @@ void TouchSelectionController::OnHandleDragBegin(const TouchHandle& handle) {
     return;
   }
 
+  gfx::PointF fixed_handle_position;
   if (&handle == start_selection_handle_.get()) {
-    fixed_handle_position_ =
+    fixed_handle_position =
         end_selection_handle_->position() + GetEndLineOffset();
   } else {
-    fixed_handle_position_ =
+    fixed_handle_position =
         start_selection_handle_->position() + GetStartLineOffset();
   }
+
+  // When moving the handle we want to move only the extent point. Before doing
+  // so we must make sure that the base point is set correctly.
+  gfx::Vector2dF line_offset = &handle == end_selection_handle_.get()

[jdduke (slow), 2014/10/15 15:46:07]

This could might be more simply folded into the above if statements, so you only
have a single branch point, e.g.,

gfx::PointF base, extent;
if (&handle == start_selection_handle_.get()) {
  base = end_selection_handle_->position() + GetEndLineOffset();
  extent = start_selection_handle_->position() + GetStartLineOffset();
} ...

[christiank, 2014/10/17 14:33:05]

Good idea!

+                                   ? GetStartLineOffset()
+                                   : GetEndLineOffset();
+  gfx::PointF handle_position = handle.position() + line_offset;
+  client_->SelectBetweenCoordinates(fixed_handle_position, handle_position);

[christiank, 2014/10/15 09:55:03]

Instead of adding a new code path for swapping the base and extent I thought I
could as well use SelectBetweenCoordinates to set the desired base and extent.
SelectBetweenCoordinates calls its parameters start and end, however they
correspond to base and extent when ultimately reaching
WebLocalFrameImpl::selectRange.

Not sure what you think about this the Jared?

I think setting both points in one message is more elegant than issuing a
"switch base and extent" message. An option could be to add a new code path that
makes it more explicit that we're setting the base and extent, not start and
end.

[jdduke (slow), 2014/10/15 15:46:07]

Ah, I think what you have is reasonable, maybe we just need to update the
start/end names in the SelectRange pipeline to read base/extent? Then I think
the intent is a bit more clear?

[christiank, 2014/10/17 14:33:05]

I agree, I'll update the names.

+
   client_->OnSelectionEvent(SELECTION_DRAG_STARTED, handle.position());
 }
 
@@ -230,7 +240,7 @@ void TouchSelectionController::OnHandleDragUpdate(const TouchHandle& handle,
   if (&handle == insertion_handle_.get()) {
     client_->MoveCaret(line_position);
   } else {
-    client_->SelectBetweenCoordinates(fixed_handle_position_, line_position);
+    client_->MoveSelectionExtent(line_position);

[jdduke (slow), 2014/10/15 15:46:07]

Could add a test (or update an existing test) in
touch_selection_controller_unittest.cc to track the SelectRange +
MoveSelectionExtent flow?

[christiank, 2014/10/17 14:33:05]

Sure, will do!

   }
 }