Improving direction-based selection strategy.

Source/core/editing/GranularityStrategy.h

Index: Source/core/editing/GranularityStrategy.h
diff --git a/Source/core/editing/GranularityStrategy.h b/Source/core/editing/GranularityStrategy.h
index 9d31bc84eb0ad41f48f7a04ea2fa49f2e994b75c..51b657a1634c2b61c5a50388bbab9d65d36ad441 100644
--- a/Source/core/editing/GranularityStrategy.h
+++ b/Source/core/editing/GranularityStrategy.h
@@ -16,9 +16,9 @@ public:
     virtual SelectionStrategy GetType() const = 0;
     virtual void Clear() = 0;
 
-    // Calculates and returns the new selection based on the updated user
-    // selection extent |extentPosition| and the granularity strategy.
-    virtual VisibleSelection updateExtent(const VisiblePosition& extentPosition, const VisibleSelection&) = 0;
+    // Calculates and returns the new selection based on the updated extent
+    // location in contents-space coordinates.
+    virtual VisibleSelection updateExtent(const IntPoint&, LocalFrame*) = 0;
 
 protected:
     GranularityStrategy();
@@ -33,13 +33,41 @@ public:
     // GranularityStrategy:
     SelectionStrategy GetType() const final;
     void Clear() final;
-    VisibleSelection updateExtent(const VisiblePosition& extentPosition, const VisibleSelection&) final;
+    VisibleSelection updateExtent(const IntPoint&, LocalFrame*) final;
 };
 
 // "Expand by word, shrink by character" selection strategy.
 // Uses character granularity when selection is shrinking. If the selection is
 // expanding, granularity doesn't change until a word boundary is passed, after
 // which the granularity switches to "word".
+// In word granularity, the word is not selected until the extent passes the
+// middle of the word. If the selection is shrunk when there is a distance
+// between the extent and the selection end on extent's side of the base, then
+// this distance is applied to the extent as an offset for the purpose of
+// determining the selection bound. This prevents the selection edge from
+// "jumping" back to extent position when it is shrunk after extending. This
+// offset can be reduced or set to 0 if the extent is moved in the opposite
+// direction.
+//
+// Example:
+// ^ marks base, | marks extent passed in updateExtent, > marks selection end:
+// Lorem ip^sum|> dolor sit amet, consectetur
+//
+// Move extent over the middle of "dolor", granularity should change to word
+// granularity and the selection end should jump to the end of the word.
+// Lorem ip^sum dolo|r> sit amet, consectetur
+//
+// Move extent back one character. Granularity changes to "character". The
+// selection end should move back one character as well. Note an offset between
+// the extent and the selection end.
+// Lorem ip^sum dol|o>r sit amet, consectetur
+//
+// Move extent forward one character. The offset is reduced to 0. Selection end
+// doesn't change.
+// Lorem ip^sum dolo|>r sit amet, consectetur
+//
+// Move forward one character. End moves with extent in character granularity.
+// Lorem ip^sum dolor|> sit amet, consectetur
 class DirectionGranularityStrategy final : public GranularityStrategy {
 public:
     DirectionGranularityStrategy();
@@ -48,12 +76,16 @@ public:
     // GranularityStrategy:
     SelectionStrategy GetType() const final;
     void Clear() final;
-    VisibleSelection updateExtent(const VisiblePosition&, const VisibleSelection&) final;
+    VisibleSelection updateExtent(const IntPoint&, LocalFrame*) final;
 
 private:
     enum class BoundAdjust {CurrentPosIfOnBound, NextBoundIfOnBound};
     enum class SearchDirection {SearchBackwards, SearchForward};
 
+    // Returns true if the two supplies positions are in the same order as
+    // the order defined by the supplied int based on camparePositions.
+    bool arePositionsInOrder(const VisiblePosition&, const VisiblePosition&, int);
+
     // Returns the next word boundary starting from |pos|. |direction| specifies
     // the direction in which to search for the next bound. nextIfOnBound
     // controls whether |pos| or the next boundary is returned when |pos| is
@@ -65,6 +97,13 @@ private:
     // Set to true if the selection was shrunk (without changing relative
     // base/extent order) as a result of the most recent updateExtent call.
     bool m_lastMoveShrunkSelection;
+    // Coordinates of the previous extent in contents-space.
+    IntPoint m_extentPoint;

[yosin_UTC9, 2015/05/18 07:47:03]

Contents-Space point is fragile and not tracked, e.g. changing content before
extent or rotating screen changes point for extent. If we really need point, we
should calculate on demand.

[mfomitchev, 2015/05/19 00:50:57]

How can we calculate it? This IntPoint corresponds to the extentPoint from the
previous updateExtent call, so we can't determine it based on the current state.
Also saving VisiblePosition wouldn't have enough accuracy since there are
several pixels corresponding to the same position and we need pixel accuracy.

Note that in practice all these state variables are only used during a single
handle drag. Every time a new handle drag is started, the selection is reset
(https://code.google.com/p/chromium/codesearch#chromium/src/ui/touch_selection...),
which clears the strategy's state
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...).
A rotation will interrupt the drag, as would many other user actions. If JS
modifies the content while we are dragging the handle, then the stored state
could get invalid, but that should be pretty unusual. Even then, the
consequences of invalid state are not severe - selection can be set with an
incorrect granularity, but it is easy to recover by moving the handle.

+    // Offset applied to the extent.
+    int m_offset;
+    // True if there were no extent updates since the strategy was last cleared.
+    // Initialized to true on creation.
+    bool m_cleared;

[yosin_UTC9, 2015/05/18 07:47:03]

Can we introduce |enum State| or another name to unify
|m_lastMoveShrunkSelection| and |m_cleared| for describing behavior in state
transition?

[mfomitchev, 2015/05/19 00:50:57]

Done.

 };
 
 } // namespace blink