From fdcf817da49ee92fe191981f7525503444f75f83 Mon Sep 17 00:00:00 2001 From: jdduke Date: Mon, 11 May 2015 19:17:21 -0700 Subject: Be explicit about forcing TouchSelectionController updates Previously, cached values in the TouchSelectionController would be reset to force future selection updates. However, these cached values can actually be used outside of selection update calls, e.g., when force showing the selection from the current cached values. Instead of resetting the cached values, simply set a dirty bit that forces an update, avoiding issues when dealing with the reset values. BUG=393025 Review URL: https://codereview.chromium.org/1127383007 Cr-Commit-Position: refs/heads/master@{#329325} --- ui/touch_selection/touch_selection_controller.cc | 25 ++++++++++------------ ui/touch_selection/touch_selection_controller.h | 6 +++++- .../touch_selection_controller_unittest.cc | 3 +++ 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/ui/touch_selection/touch_selection_controller.cc b/ui/touch_selection/touch_selection_controller.cc index 1c8ceae..47ecf5a 100644 --- a/ui/touch_selection/touch_selection_controller.cc +++ b/ui/touch_selection/touch_selection_controller.cc @@ -68,13 +68,14 @@ TouchSelectionController::~TouchSelectionController() { void TouchSelectionController::OnSelectionBoundsChanged( const SelectionBound& start, const SelectionBound& end) { - if (start == start_ && end_ == end) + if (!force_next_update_ && start == start_ && end_ == end) return; start_ = start; end_ = end; start_orientation_ = ToTouchHandleOrientation(start_.type()); end_orientation_ = ToTouchHandleOrientation(end_.type()); + force_next_update_ = false; if (!activate_selection_automatically_ && !activate_insertion_automatically_) { @@ -155,7 +156,7 @@ void TouchSelectionController::OnLongPressEvent() { response_pending_input_event_ = LONG_PRESS; ShowSelectionHandlesAutomatically(); ShowInsertionHandleAutomatically(); - ResetCachedValuesIfInactive(); + ForceNextUpdateIfInactive(); } void TouchSelectionController::AllowShowingFromCurrentSelection() { @@ -179,7 +180,7 @@ void TouchSelectionController::OnTapEvent() { ShowInsertionHandleAutomatically(); if (selection_empty_ && !show_on_tap_for_empty_editable_) DeactivateInsertion(); - ResetCachedValuesIfInactive(); + ForceNextUpdateIfInactive(); } void TouchSelectionController::HideAndDisallowShowingAutomatically() { @@ -208,7 +209,7 @@ void TouchSelectionController::OnSelectionEditable(bool editable) { if (selection_editable_ == editable) return; selection_editable_ = editable; - ResetCachedValuesIfInactive(); + ForceNextUpdateIfInactive(); if (!selection_editable_) DeactivateInsertion(); } @@ -217,7 +218,7 @@ void TouchSelectionController::OnSelectionEmpty(bool empty) { if (selection_empty_ == empty) return; selection_empty_ = empty; - ResetCachedValuesIfInactive(); + ForceNextUpdateIfInactive(); } bool TouchSelectionController::Animate(base::TimeTicks frame_time) { @@ -340,14 +341,14 @@ void TouchSelectionController::ShowInsertionHandleAutomatically() { if (activate_insertion_automatically_) return; activate_insertion_automatically_ = true; - ResetCachedValuesIfInactive(); + ForceNextUpdateIfInactive(); } void TouchSelectionController::ShowSelectionHandlesAutomatically() { if (activate_selection_automatically_) return; activate_selection_automatically_ = true; - ResetCachedValuesIfInactive(); + ForceNextUpdateIfInactive(); } void TouchSelectionController::OnInsertionChanged() { @@ -463,13 +464,9 @@ void TouchSelectionController::DeactivateSelection() { client_->OnSelectionEvent(SELECTION_CLEARED); } -void TouchSelectionController::ResetCachedValuesIfInactive() { - if (active_status_ != INACTIVE) - return; - start_ = SelectionBound(); - end_ = SelectionBound(); - start_orientation_ = TouchHandleOrientation::UNDEFINED; - end_orientation_ = TouchHandleOrientation::UNDEFINED; +void TouchSelectionController::ForceNextUpdateIfInactive() { + if (active_status_ == INACTIVE) + force_next_update_ = true; } gfx::Vector2dF TouchSelectionController::GetStartLineOffset() const { diff --git a/ui/touch_selection/touch_selection_controller.h b/ui/touch_selection/touch_selection_controller.h index f1a434d..4f6a578 100644 --- a/ui/touch_selection/touch_selection_controller.h +++ b/ui/touch_selection/touch_selection_controller.h @@ -132,7 +132,7 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController void DeactivateInsertion(); void ActivateSelection(); void DeactivateSelection(); - void ResetCachedValuesIfInactive(); + void ForceNextUpdateIfInactive(); gfx::Vector2dF GetStartLineOffset() const; gfx::Vector2dF GetEndLineOffset() const; @@ -146,6 +146,10 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController const base::TimeDelta tap_timeout_; const float tap_slop_; + // Whether to force an update on the next selection event even if the + // cached selection matches the new selection. + bool force_next_update_; + // Controls whether an insertion handle is shown on a tap for an empty // editable text. bool show_on_tap_for_empty_editable_; diff --git a/ui/touch_selection/touch_selection_controller_unittest.cc b/ui/touch_selection/touch_selection_controller_unittest.cc index 6a22b0e..46c2f4f 100644 --- a/ui/touch_selection/touch_selection_controller_unittest.cc +++ b/ui/touch_selection/touch_selection_controller_unittest.cc @@ -946,6 +946,9 @@ TEST_F(TouchSelectionControllerTest, AllowShowingFromCurrentSelection) { ChangeSelection(start_rect, visible, end_rect, visible); EXPECT_EQ(gfx::PointF(), GetLastEventStart()); + // A longpress should have no immediate effect. + controller().OnLongPressEvent(); + // Now explicitly allow showing from the previously supplied bounds. controller().AllowShowingFromCurrentSelection(); EXPECT_THAT(GetAndResetEvents(), ElementsAre(SELECTION_SHOWN)); -- cgit v1.1