Views: Extract text selection code from Textfield.

ui/views/controls/textfield/textfield.h

Index: ui/views/controls/textfield/textfield.h
diff --git a/ui/views/controls/textfield/textfield.h b/ui/views/controls/textfield/textfield.h
index c6dd5c11eaa2def01c015a62a224062858bb2549..0f61c44d3f403d7fe3d1bed0a016a1471c8cca15 100644
--- a/ui/views/controls/textfield/textfield.h
+++ b/ui/views/controls/textfield/textfield.h
@@ -29,6 +29,7 @@
 #include "ui/views/context_menu_controller.h"
 #include "ui/views/controls/textfield/textfield_model.h"
 #include "ui/views/drag_controller.h"
+#include "ui/views/selection_controller_host.h"
 #include "ui/views/view.h"
 
 namespace views {
@@ -36,12 +37,14 @@ namespace views {
 class MenuRunner;
 class Painter;
 class TextfieldController;
+class SelectionController;
 
 // A views/skia textfield implementation. No platform-specific code is used.
 class VIEWS_EXPORT Textfield : public View,
                                public TextfieldModel::Delegate,
                                public ContextMenuController,
                                public DragController,
+                               public SelectionControllerHost,
                                public ui::TouchEditable,
                                public ui::TextInputClient {
  public:
@@ -93,17 +96,6 @@ class VIEWS_EXPORT Textfield : public View,
   // not reveal the text for a password textfield.
   base::string16 GetSelectedText() const;
 
-  // Select the entire text range. If |reversed| is true, the range will end at
-  // the logical beginning of the text; this generally shows the leading portion
-  // of text that overflows its display area.
-  void SelectAll(bool reversed);
-
-  // A convenience method to select the word closest to |point|.
-  void SelectWordAt(const gfx::Point& point);
-
-  // Clears the selection within the edit field and sets the caret to the end.
-  void ClearSelection();
-
   // Checks if there is any selected text.
   bool HasSelection() const;
 
@@ -172,9 +164,6 @@ class VIEWS_EXPORT Textfield : public View,
   // Gets the selected logical text range.
   const gfx::Range& GetSelectedRange() const;
 
-  // Selects the specified logical text range.
-  void SelectRange(const gfx::Range& range);
-
   // Gets the text selection model.
   const gfx::SelectionModel& GetSelectionModel() const;
 
@@ -201,9 +190,6 @@ class VIEWS_EXPORT Textfield : public View,
   // Set the accessible name of the text field.
   void SetAccessibleName(const base::string16& name);
 
-  // Returns whether there is a drag operation originating from the textfield.
-  bool HasTextBeingDragged();
-
   // View overrides:
   gfx::Insets GetInsets() const override;
   int GetBaseline() const override;
@@ -254,6 +240,12 @@ class VIEWS_EXPORT Textfield : public View,
                            const gfx::Point& press_pt,
                            const gfx::Point& p) override;
 
+  // SelectionControllerHost overrides:
+  bool HasTextBeingDragged() const override;
+  void SelectAll(bool reversed) override;

[karandeepb, 2016/10/11 03:43:56]

I think it's a bit problematic that these overridden functions are also part of
the public Textfield interface. Since these can be called by external callers,
we can't for example DCHECK(performing_mouse_action_) in them. 

Do you think it's a good idea to have a nested class implement
SelectionControllerHost instead. This way these overridden functions which are
merely an implementation detail of supporting selection, wouldn't be exposed as
part of the public Textfield interface. However, it will lead to some more
boilerplate code.

[tapted, 2016/10/11 04:42:09]

I think it's reasonable for classes outside to be able to consider a TextField
as a thing-that-can-have-a-selection, and for them to manipulate that.

But I also like the idea of encapsulating more things into a nested class. E.g.
something that just wraps a RenderText, plus helpers to manipulate its
selection. Maybe this should be owned by TextfieldModel though. E.g. split out
the selection/non-editing parts from TextfieldModel, or have TextfieldModel
inherit from a `TextSelectionModel` and then extend it with editing
capabilities.

[karandeepb, 2016/10/11 05:01:30]

Yeah we definitely need these on the public interface. I meant having a
SelectionControllerHostImpl inner class. Then Textfield can have a
SelectionControllerHostImpl as a member. This has the advantage that
Textfield::SelectRange will be decoupled from
SelectionControllerHost::SelectRange, and the methods on the
SelectionControllerHost interface wouldn't be exposed through Textfield. But
it'll probably add unnecessary boiler plate code.

+  void SelectRange(const gfx::Range& range) override;
+  void ClearSelection() override;
+
   // ui::TouchEditable overrides:
   void SelectRect(const gfx::Point& start, const gfx::Point& end) override;
   void MoveCaretTo(const gfx::Point& point) override;
@@ -311,7 +303,7 @@ class VIEWS_EXPORT Textfield : public View,
   // Returns the TextfieldModel's text/cursor/selection rendering model.
   gfx::RenderText* GetRenderText() const;
 
-  gfx::Point last_click_location() const { return last_click_location_; }
+  gfx::Point last_click_location() const;
 
   // Get the text from the selection clipboard.
   virtual base::string16 GetSelectionClipboardText() const;
@@ -329,6 +321,22 @@ class VIEWS_EXPORT Textfield : public View,
   bool OnKeyPressed(const ui::KeyEvent& event) final;
   bool OnKeyReleased(const ui::KeyEvent& event) final;
 
+  // SelectionControllerHost overrides:
+  gfx::RenderText* GetRenderTextForSelection() override;
+  bool IsReadOnly() const override;
+  void SetTextBeingDragged(bool value) override;
+  int GetViewHeight() const override;
+  int GetViewWidth() const override;
+  int GetDragSelectionDelay() const override;
+  void OnBeforeMouseAction() override;
+  void OnAfterMouseAction(bool text_changed, bool selection_changed) override;
+  void SelectWordAt(const gfx::Point& point) override;
+  void SelectWord() override;
+  void MoveCursorTo(const gfx::Point& point, bool select) override;
+  void SelectTillEdge(gfx::VisualCursorDirection direction) override;
+  void PasteSelectionClipboard() override;
+  void UpdateSelectionClipboard() override;
+
   // Handles a request to change the value of this text field from software
   // using an accessibility API (typically automation software, screen readers
   // don't normally use this). Sets the value and clears the selection.
@@ -337,7 +345,9 @@ class VIEWS_EXPORT Textfield : public View,
   // Updates the painted background color.
   void UpdateBackgroundColor();
 
-  // Does necessary updates when the text and/or cursor position changes.
+  // Does necessary updates when the text and/or cursor position changes. NO-OP
+  // if |performing_mouse_action_| is true to ensure updates are only made when
+  // the mouse action completes i.e. on the call to OnAfterMouseAction.
   void UpdateAfterChange(bool text_changed, bool cursor_changed);
 
   // A callback function to periodically update the cursor state.
@@ -348,12 +358,6 @@ class VIEWS_EXPORT Textfield : public View,
 
   void PaintTextAndCursor(gfx::Canvas* canvas);
 
-  // Helper function to call MoveCursorTo on the TextfieldModel.
-  void MoveCursorTo(const gfx::Point& point, bool select);
-
-  // Helper function to update the selection on a mouse drag.
-  void SelectThroughLastDragLocation();
-
   // Convenience method to notify the InputMethod and TouchSelectionController.
   void OnCaretBoundsChanged();
 
@@ -376,9 +380,6 @@ class VIEWS_EXPORT Textfield : public View,
   // Utility function to prepare the context menu.
   void UpdateContextMenu();
 
-  // Tracks the mouse clicks for single/double/triple clicks.
-  void TrackMouseClicks(const ui::MouseEvent& event);
-
   // Returns true if the current text input type allows access by the IME.
   bool ImeEditingAllowed() const;
 
@@ -388,13 +389,6 @@ class VIEWS_EXPORT Textfield : public View,
 
   void CreateTouchSelectionControllerAndNotifyIt();
 
-  // Updates the selection clipboard to any non-empty text selection for a non-
-  // password textfield.
-  void UpdateSelectionClipboard() const;
-
-  // Pastes the selection clipboard for the specified mouse event.
-  void PasteSelectionClipboard(const ui::MouseEvent& event);
-
   // Called whenever a keypress is unhandled for any reason, including failing
   // to insert text into a readonly text field.
   void OnKeypressUnhandled();
@@ -469,6 +463,10 @@ class VIEWS_EXPORT Textfield : public View,
   // has been called, but OnAfterUserAction() has not yet been called.
   bool performing_user_action_;
 
+  // Tracks whether a mouse action is being performed i.e. OnBeforeMouseAction()
+  // has been called, but OnAfterMouseAction() has not yet been called.
+  bool performing_mouse_action_;
+
   // True if InputMethod::CancelComposition() should not be called.
   bool skip_input_method_cancel_composition_;
 
@@ -482,19 +480,11 @@ class VIEWS_EXPORT Textfield : public View,
   // Is the user potentially dragging and dropping from this view?
   bool initiating_drag_;
 
-  // A timer and point used to modify the selection when dragging.
-  base::RepeatingTimer drag_selection_timer_;
-  gfx::Point last_drag_location_;
-
-  // State variables used to track double and triple clicks.
-  size_t aggregated_clicks_;
-  base::TimeTicks last_click_time_;
-  gfx::Point last_click_location_;
-  gfx::Range double_click_word_;
-
   std::unique_ptr<ui::TouchEditingControllerDeprecated>
       touch_selection_controller_;
 
+  std::unique_ptr<SelectionController> selection_controller_;
+
   // Used to track touch drag starting location and offset to enable touch
   // scrolling.
   gfx::Point drag_start_location_;