Implementing directional selection strategy in Blink.

Source/core/editing/FrameSelection.cpp

Index: Source/core/editing/FrameSelection.cpp
diff --git a/Source/core/editing/FrameSelection.cpp b/Source/core/editing/FrameSelection.cpp
index 8fbfe7e993caa533506be2cf5dc1eb312796dfc9..1d2237ba54aa4bded1a6cdfa92019b2c5fb5650a 100644
--- a/Source/core/editing/FrameSelection.cpp
+++ b/Source/core/editing/FrameSelection.cpp
@@ -92,6 +92,118 @@ static inline bool shouldAlwaysUseDirectionalSelection(LocalFrame* frame)
     return !frame || frame->editor().behavior().shouldConsiderSelectionAsDirectional();
 }
 
+class GranularityStrategy {
+public:
+    virtual ~GranularityStrategy() { };
+    virtual void Clear();
+    // Returns the selection extent based on the strategy.
+    virtual VisiblePosition moveRangeSelectionExtent(const VisiblePosition&, FrameSelection*);

[yoichio, 2015/03/16 04:49:25]

Could you use |const VisibleSelection& currentSelection| instead of
FrameSelection*?

[mfomitchev, 2015/03/16 17:14:12]

Done.

+protected:
+    GranularityStrategy() { };
+};
+
+class DefaultGranularityStrategy : public GranularityStrategy {
+public:
+    DefaultGranularityStrategy() { };
+    ~DefaultGranularityStrategy() override { };
+
+    void Clear() override { };
+    VisiblePosition moveRangeSelectionExtent(const VisiblePosition& extentPosition, FrameSelection*) override
+    {
+        return extentPosition;
+    };
+};
+
+// "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 the word boundary is passed, after which the granularity
+// switches to "word".
+class DirectionGranularityStrategy : public GranularityStrategy {
+public:
+    DirectionGranularityStrategy();
+    ~DirectionGranularityStrategy() override { };
+
+    void Clear() override;
+    VisiblePosition moveRangeSelectionExtent(const VisiblePosition&, FrameSelection*) override;
+private:
+    // 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*/, int /*direction*/, bool /*nextIfOnBound*/);
+
+    // Current selection granularity being used
+    TextGranularity m_granularity;
+    // If the selection extent is extended beyond this threshold -
+    // the granularity will be changed to WordGranularity
+    VisiblePosition m_wordBoundary;

[yoichio, 2015/03/16 04:49:25]

Please initialize these variables.

[mfomitchev, 2015/03/16 17:14:12]

We do initialize m_granularity in the constructor. Are you suggesting we also
initialize m_wordBoundary and m_extentPosition? Default constructor works just
fine for them, and we don't usually initialize object fields explicitly when the
default constructor does the job - e.g. m_originalBase, m_selection in
FrameSelection.

+    // Cached extentPosition passed in moveRangeSelectionExtent
+    VisiblePosition m_extentPosition;
+};
+
+DirectionGranularityStrategy::DirectionGranularityStrategy()
+    : m_granularity(CharacterGranularity) { }
+
+void DirectionGranularityStrategy::Clear()
+{
+    m_wordBoundary.clear();
+    m_extentPosition.clear();
+    m_granularity = CharacterGranularity;
+}
+
+VisiblePosition DirectionGranularityStrategy::nextWordBound(
+    const VisiblePosition& pos,
+    int direction,
+    bool nextIfOnBound)
+{
+    return direction > 0 ? endOfWord(pos, nextIfOnBound ? RightWordIfOnBoundary : LeftWordIfOnBoundary)
+        : startOfWord(pos, nextIfOnBound ? LeftWordIfOnBoundary : RightWordIfOnBoundary);
+}
+
+VisiblePosition DirectionGranularityStrategy::moveRangeSelectionExtent(const VisiblePosition& extentPosition, FrameSelection* fs)
+{
+    const VisibleSelection& oldSel = fs->selection();
+    const VisiblePosition& base = oldSel.visibleBase();
+    const VisiblePosition& oldExtent = oldSel.visibleExtent();
+
+    int extentBaseOrder = comparePositions(extentPosition, base);
+    int oldExtentBaseOrder = comparePositions(oldExtent, base);
+    ASSERT(extentBaseOrder != 0);
+
+    bool extentBaseOrderChanged = extentBaseOrder * oldExtentBaseOrder < 0;
+
+    if (m_wordBoundary.isNull()) {
+        ASSERT(m_extentPosition.isNull());
+        ASSERT(m_granularity == CharacterGranularity);
+        m_wordBoundary = nextWordBound(oldExtent, extentBaseOrder, false);
+        m_extentPosition = extentPosition;
+    }
+    if (extentBaseOrderChanged) {
+        m_wordBoundary = nextWordBound(base, extentBaseOrder, true);
+        m_granularity = CharacterGranularity;
+    }
+    bool beyondWordBoundary = extentBaseOrder > 0 ? comparePositions(extentPosition, m_wordBoundary) > 0
+        : comparePositions(extentPosition, m_wordBoundary) < 0;
+    if (beyondWordBoundary) {
+        if (m_granularity == CharacterGranularity)
+            m_granularity = WordGranularity;
+        // The value passed for |nextIfOnBound| doesn't matter here.
+        m_wordBoundary = nextWordBound(extentPosition, extentBaseOrder, true);
+    } else if (!extentBaseOrderChanged) {
+        // If selection was shrunk - switch to character granularity
+        if (extentBaseOrder * comparePositions(extentPosition, m_extentPosition) < 0) {
+            m_granularity = CharacterGranularity;
+            m_wordBoundary = nextWordBound(extentPosition, extentBaseOrder, true);
+        }
+    }
+    m_extentPosition = extentPosition;

[yoichio, 2015/03/16 04:49:25]

I guess we need to update m_wordBoundary, m_granularity and m_extentPosition at
most once
 but code flow doesn't looks.

[mfomitchev, 2015/03/16 17:14:12]

Ok, updated the code so that we only do it once. See if you like the new
structure better.

+
+    VisiblePosition visibleExtent = extentPosition;
+    if (m_granularity == WordGranularity)
+        visibleExtent = nextWordBound(extentPosition, extentBaseOrder, false);
+
+    return visibleExtent;
+}
+
 FrameSelection::FrameSelection(LocalFrame* frame)
     : m_frame(frame)
     , m_xPosForVerticalArrowNavigation(NoXPosForVerticalArrowNavigation())
@@ -221,6 +333,8 @@ void FrameSelection::setNonDirectionalSelectionIfNeeded(const VisibleSelection&
 
 void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity)
 {
+    if ((options & FrameSelection::DoNotClearStrategy) == 0)
+        granularityStrategy().Clear();
     bool closeTyping = options & CloseTyping;
     bool shouldClearTypingStyle = options & ClearTypingStyle;
     EUserTriggered userTriggered = selectionOptionsToUserTriggered(options);
@@ -1173,6 +1287,7 @@ LayoutUnit FrameSelection::lineDirectionPointForBlockDirectionNavigation(EPositi
 void FrameSelection::clear()
 {
     m_granularity = CharacterGranularity;
+    granularityStrategy().Clear();
     setSelection(VisibleSelection());
 }
 
@@ -1912,27 +2027,34 @@ bool FrameSelection::selectWordAroundPosition(const VisiblePosition& position)
     return false;
 }
 
-void FrameSelection::moveRangeSelectionExtent(const VisiblePosition& extentPosition, TextGranularity granularity)
+GranularityStrategy& FrameSelection::granularityStrategy()
 {
-    if (isNone())
-        return;
+    if (m_granularityStrategy)
+        return *m_granularityStrategy;
+    // We do lazy initalization for m_granularityStrategy, because if we initialize it
+    // right in the constructor - the correct settings may not be set yet.
+    Settings* settings = m_frame ? m_frame->settings() : 0;
+    if (settings && settings->selectionStrategy() == StrategyDirection)
+        m_granularityStrategy = adoptPtr(new DirectionGranularityStrategy());
+    else
+        m_granularityStrategy = adoptPtr(new DefaultGranularityStrategy());
+    return *m_granularityStrategy;
+}
 
-    const VisiblePosition basePosition = m_selection.isBaseFirst() ?
-        m_selection.visibleStart() : m_selection.visibleEnd();
+void FrameSelection::moveRangeSelectionExtent(const VisiblePosition& extentPosition)
+{
+    const VisiblePosition base = m_selection.visibleBase();
 
-    int order = comparePositions(basePosition, extentPosition);
-    if (!order)
+    if (isNone() || comparePositions(base, extentPosition) == 0)
         return;
 
-    // Currently we support only CharaterGranularity and WordGranurarity.
-    // If |granurarity| is not of them, we fall back it to
-    // CharacterGranularity.
-    VisiblePosition newExtentPosition = extentPosition;
-    if (granularity == WordGranularity)
-        newExtentPosition = order < 0 ? endOfWord(extentPosition) : startOfWord(extentPosition);
-
-    VisibleSelection newSelection = VisibleSelection(basePosition, newExtentPosition);
-    setSelection(newSelection, FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle | UserTriggered, FrameSelection::AlignCursorOnScrollIfNeeded, granularity);
+    VisiblePosition newExtentPosition = granularityStrategy().moveRangeSelectionExtent(extentPosition, this);
+    VisibleSelection newSelection(base, newExtentPosition);
+    setSelection(
+        newSelection,
+        FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle | FrameSelection::DoNotClearStrategy | UserTriggered,
+        FrameSelection::AlignCursorOnScrollIfNeeded,
+        CharacterGranularity);
 }
 
 void FrameSelection::moveRangeSelection(const VisiblePosition& basePosition, const VisiblePosition& extentPosition, TextGranularity granularity)