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..2b09e82422a890089923e495fdb36e3670ccdb81 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,23 +76,44 @@ 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 StrategyState {
+        // Starting state.
+        // Selection was cleared and there were no extent updates since then.
+        // One an update is performed, the strategy goes into the Expanding
+        // state unless the upate shrinks the selection without changing
+        // relative base/extent order, in which case the strategy goes into the
+        // Shrinking state.
+        Cleared,
+        // Last time the selection was changed by updateExtent - it was expanded
+        // or the relative base/extent order was changed.
+        Expanding,
+        // Last time the selection was changed by updateExtent - it was shrunk
+        // (without changing relative base/extent order).
+        Shrinking
+    };
     enum class BoundAdjust {CurrentPosIfOnBound, NextBoundIfOnBound};
     enum class SearchDirection {SearchBackwards, SearchForward};
 
+    // Order is specified using the same contract as comparePositions.
+    bool arePositionsInSpecifiedOrder(const VisiblePosition& vp1, const VisiblePosition& vp2, int specifiedOrder);
+
     // 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
     // located exactly on word boundary.
     VisiblePosition nextWordBound(const VisiblePosition& /*pos*/, SearchDirection /*direction*/, BoundAdjust /*nextIfOnBound*/);
 
+    StrategyState m_state;
     // Current selection granularity being used
     TextGranularity m_granularity;
-    // 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;
+    // The vector defining the location of the extent point in contents-space

[leviw_travelin_and_unemployed, 2015/05/26 20:25:50]

What vector? This content doesn't help me understand what
m_subPositionCorrection is when looking at this header.

[mfomitchev, 2015/05/26 22:32:47]

Is there a more appropriate class than IntSize to represent a vector in Blink?
In Chromium there are separate gfx::Vector2d and gfx::Size classes, but I only
found IntSize in Blink.

This is a vector between two points. The start of the vector is the bottom-left
of the caret rect of the VisiblePosition corresponding to the selection extent
(which btw always matches either selection start or selection end). The end of
the vector is the extentPoint moved horizontally by |offset|.

I'll try to explain this better.

[leviw_travelin_and_unemployed, 2015/05/26 22:47:49]

IntSize is the most appropriate in Blink, but we don't usually refer to these
solely as a "vector," because of the Vector<> container.

[mfomitchev, 2015/05/27 17:24:29]

Acknowledged.

[mfomitchev, 2015/06/03 18:57:54]

Done.

+    // coordinates relative to the extent's VisiblePosition.
+    IntSize m_subPositionCorrection;
+    // Offset applied to the extent point.

[leviw_travelin_and_unemployed, 2015/05/26 20:25:50]

This is always going to be an x offset, right? Not width, or Y, or anything with
transforms? And you did say it's in pixels, right? Could be be clearer than just
"offset"?

[mfomitchev, 2015/05/26 22:32:47]

Acknowledged.

[mfomitchev, 2015/06/03 18:57:54]

Done.

+    int m_offset;
 };
 
 } // namespace blink