Commit da232ee4 authored by jdduke's avatar jdduke Committed by Commit bot

Reland "Be explicit about forcing TouchSelectionController updates"

This change landed in r329325 but was reverted due to an
uninitialized member variable. The issue has been fixed.

Original description: ----------------------------

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@{#329422}
parent 88221a5c
...@@ -48,6 +48,7 @@ TouchSelectionController::TouchSelectionController( ...@@ -48,6 +48,7 @@ TouchSelectionController::TouchSelectionController(
: client_(client), : client_(client),
tap_timeout_(tap_timeout), tap_timeout_(tap_timeout),
tap_slop_(tap_slop), tap_slop_(tap_slop),
force_next_update_(false),
show_on_tap_for_empty_editable_(show_on_tap_for_empty_editable), show_on_tap_for_empty_editable_(show_on_tap_for_empty_editable),
response_pending_input_event_(INPUT_EVENT_TYPE_NONE), response_pending_input_event_(INPUT_EVENT_TYPE_NONE),
start_orientation_(TouchHandleOrientation::UNDEFINED), start_orientation_(TouchHandleOrientation::UNDEFINED),
...@@ -68,13 +69,14 @@ TouchSelectionController::~TouchSelectionController() { ...@@ -68,13 +69,14 @@ TouchSelectionController::~TouchSelectionController() {
void TouchSelectionController::OnSelectionBoundsChanged( void TouchSelectionController::OnSelectionBoundsChanged(
const SelectionBound& start, const SelectionBound& start,
const SelectionBound& end) { const SelectionBound& end) {
if (start == start_ && end_ == end) if (!force_next_update_ && start == start_ && end_ == end)
return; return;
start_ = start; start_ = start;
end_ = end; end_ = end;
start_orientation_ = ToTouchHandleOrientation(start_.type()); start_orientation_ = ToTouchHandleOrientation(start_.type());
end_orientation_ = ToTouchHandleOrientation(end_.type()); end_orientation_ = ToTouchHandleOrientation(end_.type());
force_next_update_ = false;
if (!activate_selection_automatically_ && if (!activate_selection_automatically_ &&
!activate_insertion_automatically_) { !activate_insertion_automatically_) {
...@@ -155,7 +157,7 @@ void TouchSelectionController::OnLongPressEvent() { ...@@ -155,7 +157,7 @@ void TouchSelectionController::OnLongPressEvent() {
response_pending_input_event_ = LONG_PRESS; response_pending_input_event_ = LONG_PRESS;
ShowSelectionHandlesAutomatically(); ShowSelectionHandlesAutomatically();
ShowInsertionHandleAutomatically(); ShowInsertionHandleAutomatically();
ResetCachedValuesIfInactive(); ForceNextUpdateIfInactive();
} }
void TouchSelectionController::AllowShowingFromCurrentSelection() { void TouchSelectionController::AllowShowingFromCurrentSelection() {
...@@ -179,7 +181,7 @@ void TouchSelectionController::OnTapEvent() { ...@@ -179,7 +181,7 @@ void TouchSelectionController::OnTapEvent() {
ShowInsertionHandleAutomatically(); ShowInsertionHandleAutomatically();
if (selection_empty_ && !show_on_tap_for_empty_editable_) if (selection_empty_ && !show_on_tap_for_empty_editable_)
DeactivateInsertion(); DeactivateInsertion();
ResetCachedValuesIfInactive(); ForceNextUpdateIfInactive();
} }
void TouchSelectionController::HideAndDisallowShowingAutomatically() { void TouchSelectionController::HideAndDisallowShowingAutomatically() {
...@@ -208,7 +210,7 @@ void TouchSelectionController::OnSelectionEditable(bool editable) { ...@@ -208,7 +210,7 @@ void TouchSelectionController::OnSelectionEditable(bool editable) {
if (selection_editable_ == editable) if (selection_editable_ == editable)
return; return;
selection_editable_ = editable; selection_editable_ = editable;
ResetCachedValuesIfInactive(); ForceNextUpdateIfInactive();
if (!selection_editable_) if (!selection_editable_)
DeactivateInsertion(); DeactivateInsertion();
} }
...@@ -217,7 +219,7 @@ void TouchSelectionController::OnSelectionEmpty(bool empty) { ...@@ -217,7 +219,7 @@ void TouchSelectionController::OnSelectionEmpty(bool empty) {
if (selection_empty_ == empty) if (selection_empty_ == empty)
return; return;
selection_empty_ = empty; selection_empty_ = empty;
ResetCachedValuesIfInactive(); ForceNextUpdateIfInactive();
} }
bool TouchSelectionController::Animate(base::TimeTicks frame_time) { bool TouchSelectionController::Animate(base::TimeTicks frame_time) {
...@@ -340,14 +342,14 @@ void TouchSelectionController::ShowInsertionHandleAutomatically() { ...@@ -340,14 +342,14 @@ void TouchSelectionController::ShowInsertionHandleAutomatically() {
if (activate_insertion_automatically_) if (activate_insertion_automatically_)
return; return;
activate_insertion_automatically_ = true; activate_insertion_automatically_ = true;
ResetCachedValuesIfInactive(); ForceNextUpdateIfInactive();
} }
void TouchSelectionController::ShowSelectionHandlesAutomatically() { void TouchSelectionController::ShowSelectionHandlesAutomatically() {
if (activate_selection_automatically_) if (activate_selection_automatically_)
return; return;
activate_selection_automatically_ = true; activate_selection_automatically_ = true;
ResetCachedValuesIfInactive(); ForceNextUpdateIfInactive();
} }
void TouchSelectionController::OnInsertionChanged() { void TouchSelectionController::OnInsertionChanged() {
...@@ -463,13 +465,9 @@ void TouchSelectionController::DeactivateSelection() { ...@@ -463,13 +465,9 @@ void TouchSelectionController::DeactivateSelection() {
client_->OnSelectionEvent(SELECTION_CLEARED); client_->OnSelectionEvent(SELECTION_CLEARED);
} }
void TouchSelectionController::ResetCachedValuesIfInactive() { void TouchSelectionController::ForceNextUpdateIfInactive() {
if (active_status_ != INACTIVE) if (active_status_ == INACTIVE)
return; force_next_update_ = true;
start_ = SelectionBound();
end_ = SelectionBound();
start_orientation_ = TouchHandleOrientation::UNDEFINED;
end_orientation_ = TouchHandleOrientation::UNDEFINED;
} }
gfx::Vector2dF TouchSelectionController::GetStartLineOffset() const { gfx::Vector2dF TouchSelectionController::GetStartLineOffset() const {
......
...@@ -132,7 +132,7 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController ...@@ -132,7 +132,7 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController
void DeactivateInsertion(); void DeactivateInsertion();
void ActivateSelection(); void ActivateSelection();
void DeactivateSelection(); void DeactivateSelection();
void ResetCachedValuesIfInactive(); void ForceNextUpdateIfInactive();
gfx::Vector2dF GetStartLineOffset() const; gfx::Vector2dF GetStartLineOffset() const;
gfx::Vector2dF GetEndLineOffset() const; gfx::Vector2dF GetEndLineOffset() const;
...@@ -146,6 +146,10 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController ...@@ -146,6 +146,10 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController
const base::TimeDelta tap_timeout_; const base::TimeDelta tap_timeout_;
const float tap_slop_; 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 // Controls whether an insertion handle is shown on a tap for an empty
// editable text. // editable text.
bool show_on_tap_for_empty_editable_; bool show_on_tap_for_empty_editable_;
......
...@@ -946,6 +946,9 @@ TEST_F(TouchSelectionControllerTest, AllowShowingFromCurrentSelection) { ...@@ -946,6 +946,9 @@ TEST_F(TouchSelectionControllerTest, AllowShowingFromCurrentSelection) {
ChangeSelection(start_rect, visible, end_rect, visible); ChangeSelection(start_rect, visible, end_rect, visible);
EXPECT_EQ(gfx::PointF(), GetLastEventStart()); EXPECT_EQ(gfx::PointF(), GetLastEventStart());
// A longpress should have no immediate effect.
controller().OnLongPressEvent();
// Now explicitly allow showing from the previously supplied bounds. // Now explicitly allow showing from the previously supplied bounds.
controller().AllowShowingFromCurrentSelection(); controller().AllowShowingFromCurrentSelection();
EXPECT_THAT(GetAndResetEvents(), ElementsAre(SELECTION_SHOWN)); EXPECT_THAT(GetAndResetEvents(), ElementsAre(SELECTION_SHOWN));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment