Move setNonDirectionalSelectionIfNeeded() to SelectionController from FrameSelection

This patch moves |setNonDirectionalSelectionIfNeeded()| to |SelectionController|
from |FrameSelection| since it is used only in |SelectionController| to simplify
|FrameSelection| for improving code health.

This patch also gets rid of |originalBase()| which had been used in
|setNonDirectionalSelectionIfNeeded()|, but it has never been used.

BUG=n/a
TEST=n/a; no behavior changes

Review-Url: https://codereview.chromium.org/2441573002
Cr-Commit-Position: refs/heads/master@{#427032}

third_party/WebKit/Source/core/dom/Document.cpp

@@ -120,6 +120,7 @@
 #include "core/editing/Editor.h"
 #include "core/editing/FrameSelection.h"
 #include "core/editing/InputMethodController.h"
+#include "core/editing/SelectionController.h"
 #include "core/editing/markers/DocumentMarkerController.h"
 #include "core/editing/serializers/Serialization.h"
 #include "core/editing/spellcheck/SpellChecker.h"
@@ -2354,6 +2355,7 @@ void Document::shutdown() {
   frameHost()->eventHandlerRegistry().documentDetached(*this);
 
   m_frame->selection().documentDetached(*this);
+  m_frame->eventHandler().selectionController().documentDetached();
   m_frame->inputMethodController().documentDetached();
 
   // If this Document is associated with a live DocumentLoader, the

third_party/WebKit/Source/core/editing/FrameSelection.cpp

@@ -223,7 +223,9 @@ static void adjustEndpointsAtBidiBoundary(
   }
 }
 
-void FrameSelection::setNonDirectionalSelectionIfNeeded(
+// TODO(yosin): We should move |setNonDirectionalSelectionIfNeeded()| to
+// "SelectionController.cpp"
+void SelectionController::setNonDirectionalSelectionIfNeeded(
     const VisibleSelectionInFlatTree& passedNewSelection,
     TextGranularity granularity,
     EndPointsAdjustmentMode endpointsAdjustmentMode) {
@@ -256,7 +258,7 @@ void FrameSelection::setNonDirectionalSelectionIfNeeded(
     newSelection.setBase(newBase);
     newSelection.setExtent(newExtent);
   } else if (originalBase.isNotNull()) {
-    if (visibleSelection<EditingInFlatTreeStrategy>().base() ==
+    if (selection().visibleSelection<EditingInFlatTreeStrategy>().base() ==
         newSelection.base())
       newSelection.setBase(originalBase);
     m_originalBaseInFlatTree = VisiblePositionInFlatTree();
@@ -264,12 +266,13 @@ void FrameSelection::setNonDirectionalSelectionIfNeeded(
 
   // Adjusting base and extent will make newSelection always directional
   newSelection.setIsDirectional(isDirectional);
-  if (visibleSelection<EditingInFlatTreeStrategy>() == newSelection)
+  if (selection().visibleSelection<EditingInFlatTreeStrategy>() == newSelection)
     return;
 
-  const SetSelectionOptions options = CloseTyping | ClearTypingStyle;
-  setSelection(newSelection, options, CursorAlignOnScroll::IfNeeded,
-               granularity);
+  const FrameSelection::SetSelectionOptions options =
+      FrameSelection::CloseTyping | FrameSelection::ClearTypingStyle;
+  selection().setSelection(newSelection, options, CursorAlignOnScroll::IfNeeded,
+                           granularity);
 }
 
 template <typename Strategy>
@@ -813,7 +816,6 @@ void FrameSelection::documentAttached(Document* document) {
 void FrameSelection::documentDetached(const Document& document) {
   DCHECK_EQ(m_document, document);
   m_document = nullptr;
-  m_originalBaseInFlatTree = VisiblePositionInFlatTree();
   m_granularity = CharacterGranularity;
 
   LayoutViewItem view = m_frame->contentLayoutItem();
@@ -823,6 +825,7 @@ void FrameSelection::documentDetached(const Document& document) {
   clearTypingStyle();
   m_selectionEditor->documentDetached(document);
   m_frameCaret->documentDetached();
+  m_frame->eventHandler().selectionController().documentDetached();
 }
 
 LayoutBlock* FrameSelection::caretLayoutObject() const {
@@ -1373,7 +1376,6 @@ DEFINE_TRACE(FrameSelection) {
   visitor->trace(m_frame);
   visitor->trace(m_pendingSelection);
   visitor->trace(m_selectionEditor);
-  visitor->trace(m_originalBaseInFlatTree);
   visitor->trace(m_typingStyle);
   visitor->trace(m_frameCaret);
 }

third_party/WebKit/Source/core/editing/FrameSelection.h

@@ -249,14 +249,6 @@ class CORE_EXPORT FrameSelection final
   void showTreeForThis() const;
 #endif
 
-  enum EndPointsAdjustmentMode {
-    AdjustEndpointsAtBidiBoundary,
-    DoNotAdjustEndpoints
-  };
-  void setNonDirectionalSelectionIfNeeded(
-      const VisibleSelectionInFlatTree&,
-      TextGranularity,
-      EndPointsAdjustmentMode = DoNotAdjustEndpoints);
   void setFocusedNodeIfNeeded();
   void notifyLayoutObjectOfSelectionChange(EUserTriggered);
 
@@ -295,6 +287,7 @@ class CORE_EXPORT FrameSelection final
  private:
   friend class FrameSelectionTest;
   friend class PaintControllerPaintTestForSlimmingPaintV1AndV2;
+  friend class SelectionControllerTest;
   FRIEND_TEST_ALL_PREFIXES(PaintControllerPaintTestForSlimmingPaintV1AndV2,
                            FullDocumentPaintingWithCaret);
 
@@ -304,11 +297,6 @@ class CORE_EXPORT FrameSelection final
   // use |visibleSelection<EditingInFlatTreeStrategy>()|.
   const VisibleSelectionInFlatTree& selectionInFlatTree() const;
 
-  template <typename Strategy>
-  VisiblePositionTemplate<Strategy> originalBase() const;
-  void setOriginalBase(const VisiblePosition&);
-  void setOriginalBase(const VisiblePositionInFlatTree&);
-
   template <typename Strategy>
   void setSelectionAlgorithm(const VisibleSelectionTemplate<Strategy>&,
                              SetSelectionOptions,
@@ -345,8 +333,6 @@ class CORE_EXPORT FrameSelection final
   const Member<PendingSelection> m_pendingSelection;
   const Member<SelectionEditor> m_selectionEditor;
 
-  // Used to store base before the adjustment at bidi boundary
-  VisiblePositionInFlatTree m_originalBaseInFlatTree;
   TextGranularity m_granularity;
   LayoutUnit m_xPosForVerticalArrowNavigation;
 

third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp

@@ -10,8 +10,10 @@
 #include "core/dom/Text.h"
 #include "core/editing/EditingTestBase.h"
 #include "core/editing/FrameCaret.h"
+#include "core/editing/SelectionController.h"
 #include "core/frame/FrameView.h"
 #include "core/html/HTMLBodyElement.h"
+#include "core/input/EventHandler.h"
 #include "core/layout/LayoutView.h"
 #include "core/paint/PaintInfo.h"
 #include "core/paint/PaintLayer.h"
@@ -273,7 +275,40 @@ TEST_F(FrameSelectionTest, MoveRangeSelectionTest) {
   EXPECT_EQ_SELECTED_TEXT("Foo Bar");
 }
 
-TEST_F(FrameSelectionTest, setNonDirectionalSelectionIfNeeded) {
+// TODO(yosin): We should move |SelectionControllerTest" to
+// "SelectionControllerTest.cpp"
+class SelectionControllerTest : public EditingTestBase {
+ protected:
+  SelectionControllerTest() = default;
+
+  const VisibleSelection& visibleSelectionInDOMTree() const {
+    return selection().selection();
+  }
+
+  const VisibleSelectionInFlatTree& visibleSelectionInFlatTree() const {
+    return selection().selectionInFlatTree();
+  }
+
+  void setNonDirectionalSelectionIfNeeded(const VisibleSelectionInFlatTree&,
+                                          TextGranularity);
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SelectionControllerTest);
+};
+
+// TODO(yosin): We should move this function to "SelectionControllerTest.cpp"
+void SelectionControllerTest::setNonDirectionalSelectionIfNeeded(
+    const VisibleSelectionInFlatTree& newSelection,
+    TextGranularity granularity) {
+  frame()
+      .eventHandler()
+      .selectionController()
+      .setNonDirectionalSelectionIfNeeded(
+          newSelection, granularity, SelectionController::DoNotAdjustEndpoints);
+}
+
+// TODO(yosin): We should move this test to "SelectionControllerTest.cpp"
+TEST_F(SelectionControllerTest, setNonDirectionalSelectionIfNeeded) {
   const char* bodyContent = "<span id=top>top</span><span id=host></span>";
   const char* shadowContent = "<span id=bottom>bottom</span>";
   setBodyContent(bodyContent);
@@ -284,7 +319,7 @@ TEST_F(FrameSelectionTest, setNonDirectionalSelectionIfNeeded) {
   Node* host = document().getElementById("host");
 
   // top to bottom
-  selection().setNonDirectionalSelectionIfNeeded(
+  setNonDirectionalSelectionIfNeeded(
       createVisibleSelection(SelectionInFlatTree::Builder()
                                  .collapse(PositionInFlatTree(top, 1))
                                  .extend(PositionInFlatTree(bottom, 3))
@@ -302,7 +337,7 @@ TEST_F(FrameSelectionTest, setNonDirectionalSelectionIfNeeded) {
   EXPECT_EQ(PositionInFlatTree(bottom, 3), visibleSelectionInFlatTree().end());
 
   // bottom to top
-  selection().setNonDirectionalSelectionIfNeeded(
+  setNonDirectionalSelectionIfNeeded(
       createVisibleSelection(SelectionInFlatTree::Builder()
                                  .collapse(PositionInFlatTree(bottom, 3))
                                  .extend(PositionInFlatTree(top, 1))

third_party/WebKit/Source/core/editing/SelectionController.cpp

@@ -61,6 +61,7 @@ SelectionController::SelectionController(LocalFrame& frame)
 
 DEFINE_TRACE(SelectionController) {
   visitor->trace(m_frame);
+  visitor->trace(m_originalBaseInFlatTree);
 }
 
 namespace {
@@ -123,6 +124,15 @@ VisiblePositionInFlatTree visiblePositionOfHitTestResult(
 
 }  // namespace
 
+Document& SelectionController::document() const {
+  DCHECK(m_frame->document());
+  return *m_frame->document();
+}
+
+void SelectionController::documentDetached() {
+  m_originalBaseInFlatTree = VisiblePositionInFlatTree();
+}
+
 bool SelectionController::handleMousePressEventSingleClick(
     const MouseEventWithHitTestResults& event) {
   TRACE_EVENT0("blink",
@@ -332,9 +342,8 @@ void SelectionController::updateSelectionForMouseDrag(
   if (selection().granularity() != CharacterGranularity)
     newSelection.expandUsingGranularity(selection().granularity());
 
-  selection().setNonDirectionalSelectionIfNeeded(
-      newSelection, selection().granularity(),
-      FrameSelection::AdjustEndpointsAtBidiBoundary);
+  setNonDirectionalSelectionIfNeeded(newSelection, selection().granularity(),
+                                     AdjustEndpointsAtBidiBoundary);
 }
 
 bool SelectionController::updateSelectionForMouseDownDispatchingSelectStart(
@@ -362,7 +371,8 @@ bool SelectionController::updateSelectionForMouseDownDispatchingSelectStart(
     m_selectionState = SelectionState::PlacedCaret;
   }
 
-  this->selection().setNonDirectionalSelectionIfNeeded(selection, granularity);
+  setNonDirectionalSelectionIfNeeded(selection, granularity,
+                                     DoNotAdjustEndpoints);
 
   return true;
 }

third_party/WebKit/Source/core/editing/SelectionController.h

@@ -39,13 +39,15 @@ class FrameSelection;
 class HitTestResult;
 class LocalFrame;
 
-class SelectionController final : public GarbageCollected<SelectionController> {
+class CORE_EXPORT SelectionController final
+    : public GarbageCollectedFinalized<SelectionController> {
   WTF_MAKE_NONCOPYABLE(SelectionController);
 
  public:
   static SelectionController* create(LocalFrame&);
   DECLARE_TRACE();
 
+  void documentDetached();
   void handleMousePressEvent(const MouseEventWithHitTestResults&);
   bool handleMousePressEventSingleClick(const MouseEventWithHitTestResults&);
   bool handleMousePressEventDoubleClick(const MouseEventWithHitTestResults&);
@@ -80,10 +82,18 @@ class SelectionController final : public GarbageCollected<SelectionController> {
   }
 
  private:
+  friend class SelectionControllerTest;
+
   explicit SelectionController(LocalFrame&);
 
   enum class AppendTrailingWhitespace { ShouldAppend, DontAppend };
   enum class SelectInputEventType { Touch, Mouse };
+  enum EndPointsAdjustmentMode {
+    AdjustEndpointsAtBidiBoundary,
+    DoNotAdjustEndpoints
+  };
+
+  Document& document() const;
 
   void selectClosestWordFromHitTestResult(const HitTestResult&,
                                           AppendTrailingWhitespace,
@@ -95,6 +105,9 @@ class SelectionController final : public GarbageCollected<SelectionController> {
       const MouseEventWithHitTestResults&);
   void selectClosestWordOrLinkFromMouseEvent(
       const MouseEventWithHitTestResults&);
+  void setNonDirectionalSelectionIfNeeded(const VisibleSelectionInFlatTree&,
+                                          TextGranularity,
+                                          EndPointsAdjustmentMode);
   bool updateSelectionForMouseDownDispatchingSelectStart(
       Node*,
       const VisibleSelectionInFlatTree&,
@@ -103,6 +116,10 @@ class SelectionController final : public GarbageCollected<SelectionController> {
   FrameSelection& selection() const;
 
   Member<LocalFrame> const m_frame;
+  // TODO(yosin): We should use |PositionWIthAffinityInFlatTree| since we
+  // should reduce usage of |VisibleSelectionInFlatTree|.
+  // Used to store base before the adjustment at bidi boundary
+  VisiblePositionInFlatTree m_originalBaseInFlatTree;
   bool m_mouseDownMayStartSelect;
   bool m_mouseDownWasSingleClickInSelection;
   bool m_mouseDownAllowsMultiClick;