Views: Extract text selection code from Textfield.

ui/views/selection_controller.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UI_VIEWS_SELECTION_CONTROLLER_H_
+#define UI_VIEWS_SELECTION_CONTROLLER_H_
+
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+#include "ui/gfx/geometry/point.h"
+#include "ui/gfx/range/range.h"
+#include "ui/gfx/selection_model.h"
+#include "ui/views/views_export.h"
+
+namespace gfx {
+class RenderText;
+}
+
+namespace ui {
+class MouseEvent;
+}
+
+namespace views {
+class SelectionControllerDelegate;
+
+// Helper class used to facilitate mouse event handling and text selection. To
+// use, clients must implement the SelectionControllerDelegate interface.
+class VIEWS_EXPORT SelectionController {
+ public:
+  // |delegate| must be non-null.
+  explicit SelectionController(SelectionControllerDelegate* delegate);
+
+  // Handle mouse events forwarded by |delegate_|. |handled| specifies whether
+  // the event has already been handled by the |delegate_|. If |handled| is
+  // true, the mouse event is just used to update the internal state without
+  // updating the state of the associated RenderText instance.
+  bool OnMousePressed(const ui::MouseEvent& event, bool handled);

[sky, 2016/10/21 15:03:40]

I'm wondering if you can make this class an EventHandler and add it as a
pre-target handler for the textfield/label. That way you wouldn't need to change
textfield/label to explicitly call into this class.

[sky, 2016/10/21 15:03:40]

As commented over IM I think touch/gesture should be forwarded as well so that
this responds to touch too. Please add a TODO.

[karandeepb, 2016/10/25 05:30:42]

Currently Textfield::OnMousePressed first forwards the event to the
TextfieldController, then if the event is not handled, we request focus etc. if
needed and only then do we pass the event to the selection controller. 

Also, classes like OmniboxViewViews override OnMouse* handlers for Textfield
(similarly views::Link will override OnMouse* handlers for Labels) So I am not
sure if adding SelectionController as a pre-target handler is feasible with the
way things are currently implemented.

Also, I am not sure whether it is the correct thing to do, meaning when should
we handle an event in the target phase vs the pre/post target phase?

[karandeepb, 2016/10/25 05:30:42]

Done.

[sky, 2016/10/25 16:46:03]

Fair enough. If you need certain ordering than I agree EventHandler isn't the
right thing.

+  bool OnMouseDragged(const ui::MouseEvent& event);
+  void OnMouseReleased(const ui::MouseEvent& event);

[sky, 2016/10/21 15:03:40]

What happens if capture is lost? Doesn't that need to be forwarded too?

[karandeepb, 2016/10/25 05:30:42]

Thanks for the catch. Textfield wasn't handling OnMouseCaptureLost till now. So
on Windows, if during an active mouse drag doing a text selection, we were to
focus to a different window, the drag_selection_timer_ would not have stopped. 

Interestingly, on Mac and Linux, focusing to a different window during an active
selection, doesn't release capture, but it seems in accordance with the native
behavior.

+
+  // Returns the latest click location.
+  gfx::Point last_click_location() const { return last_click_location_; }

[sky, 2016/10/21 15:03:40]

const gfx::Point&

[karandeepb, 2016/10/25 05:30:42]

Done. Is it preferred that we should be returning a const reference for simple
getters? Didn't find anything in the style guides.

[sky, 2016/10/25 16:46:03]

Generally in chromium code you'll see const& return types.

+
+ private:
+  // Tracks the mouse clicks for single/double/triple clicks.
+  void TrackMouseClicks(const ui::MouseEvent& event);
+
+  // Returns the associated render text instance via the |delegate_|.
+  gfx::RenderText* GetRenderText();
+
+  // Helper function to update the selection on a mouse drag as per
+  // |last_drag_location_|. Can be called asynchronously, through a timer.
+  void SelectThroughLastDragLocation();
+
+  // 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.
+  base::TimeTicks last_click_time_;
+  gfx::Point last_click_location_;
+  size_t aggregated_clicks_;

[sky, 2016/10/21 15:03:40]

Please add a comment as to what this means. It isn't readily obvious to me.

[karandeepb, 2016/10/25 05:30:42]

Done.

+  gfx::Range double_click_word_;
+
+  // Weak pointer.
+  SelectionControllerDelegate* delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(SelectionController);
+};
+
+}  // namespace views
+
+#endif  // UI_VIEWS_SELECTION_CONTROLLER_H_