Fix issue with longpress drag selection motion

ui/touch_selection/longpress_drag_selector.cc

 // 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 "base/auto_reset.h"
 #include "ui/events/gesture_detection/motion_event.h"
 
 namespace ui {
+namespace {
+
+gfx::Vector2dF SafeNormalize(const gfx::Vector2dF& v) {
+  return v.IsZero() ? v : ScaleVector2d(v, 1.f / v.Length());
+}
+
+}  // namespace
 
 LongPressDragSelector::LongPressDragSelector(
     LongPressDragSelectorClient* client)
     : client_(client),
       state_(INACTIVE),
       has_longpress_drag_start_anchor_(false) {
 }
 
 LongPressDragSelector::~LongPressDragSelector() {
 }
@@ skipping 42 matching lines @@
   if (client_->IsWithinTapSlop(delta))
     return true;
 
   gfx::PointF selection_start = client_->GetSelectionStart();
   gfx::PointF selection_end = client_->GetSelectionEnd();
   bool extend_selection_start = false;
   if (std::abs(delta.y()) > std::abs(delta.x())) {
     // If initial motion is up/down, extend the start/end selection bound.
     extend_selection_start = delta.y() < 0;
   } else {
-    // Otherwise extend the selection bound toward which we're moving.
+    // Otherwise extend the selection bound toward which we're moving, or
+    // the closest bound if motion is already away from both bounds.
     // Note that, for mixed RTL text, or for multiline selections triggered
     // by longpress, this may not pick the most suitable drag target
     gfx::Vector2dF start_delta = selection_start - position;
+    gfx::Vector2dF end_delta = selection_end - position;
 
     // The vectors must be normalized to make dot product comparison meaningful.
-    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());
+    gfx::Vector2dF normalized_start_delta = SafeNormalize(start_delta);
+    gfx::Vector2dF normalized_end_delta = SafeNormalize(end_delta);
+    double start_dot_product = gfx::DotProduct(normalized_start_delta, delta);
+    double end_dot_product = gfx::DotProduct(normalized_end_delta, delta);
 
-    // The larger the dot product the more similar the direction.
-    extend_selection_start =
-        gfx::DotProduct(start_delta, delta) > gfx::DotProduct(end_delta, delta);
+    if (start_dot_product < 0 && end_dot_product < 0) {

[mfomitchev, 2015/08/04 18:31:36]

nit: It makes it a bit harder to parse when the order in which things are
described in the comment (lines 80-83) is different from the order in which
things are handled in code.

Perhaps handle the case when "start_dot_product > 0 || end_dot_product > 0"
first? Or change up the comment to mention the "moving away from both endpoints"
case before the other one?

[jdduke (slow), 2015/08/04 19:14:51]

Done.

+      // If we're already moving away from both endpoints, pick the closest.
+      extend_selection_start =
+          start_delta.LengthSquared() < end_delta.LengthSquared();
+    } else {
+      // Otherwise pick the endpoint toward we're which moving. The larger the
+      // dot product the more similar the direction.
+      extend_selection_start = start_dot_product > end_dot_product;
+    }
   }
 
   gfx::PointF extent = extend_selection_start ? selection_start : selection_end;
   longpress_drag_selection_offset_ = extent - position;
   client_->OnDragBegin(*this, extent);
   SetState(DRAGGING);
   return true;
 }
 
 bool LongPressDragSelector::IsActive() const {
@@ skipping 34 matching lines @@
 
   // TODO(jdduke): Add UMA for tracking relative longpress drag frequency.
   if (was_dragging)
     client_->OnDragEnd(*this);
 
   if (was_active != IsActive())
     client_->OnLongPressDragActiveStateChanged();
 }
 
 }  // namespace ui