Make Textfield scroll continuously when dragging beyond edges.

Make Textfield scroll continuously when dragging beyond edges.

Add a timer, point, and function drag selection helpers.
Respect ScopedAnimationDurationScaleMode for TextfieldTest.DragToSelect.
Comment and function order cleanup; fix timer.h indentation.

TODO(followup): Make the scrolling "animation" smoother.

BUG=373886
TEST=Dragging to select beyond the left or right ends of the omnibox, find bar textfield, etc. continuously scrolls and modifies the selection.
R=pkasting@chromium.org,thakis@chromium.org,sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272591

[msw, 2014-05-21 22:33:32]

Hey guys, please take a look; thanks!
Peter: textfield.* changes.
Nico: timer.h (indentation fix).

[Nico, 2014-05-21 22:35:44]

timer.h lgtm :-)

[Peter Kasting, 2014-05-21 22:44:27]

https://codereview.chromium.org/290733007/diff/40001/ui/views/controls/textfi...
File ui/views/controls/textfield/textfield.cc (right):

https://codereview.chromium.org/290733007/diff/40001/ui/views/controls/textfi...
ui/views/controls/textfield/textfield.cc:1543:
model_->MoveCursorTo(last_drag_location_, true);
How does this work with off-Textfield cursor locations?  Does it pretend the
textfield is infinitely wide, and then select to "where the mouse would be"?  If
so, doesn't this mean that the further off the side the user places the mouse
cursor, the more text will be selected on every timer tick?

It seems like, ideally, we'd have a high-framerate (60 Hz) animation that
scrolled-in-and-selected a small amount of new text each frame, regardless of
how far off the side the mouse cursor is.

[msw, 2014-05-21 23:15:10]

https://codereview.chromium.org/290733007/diff/40001/ui/views/controls/textfi...
File ui/views/controls/textfield/textfield.cc (right):

https://codereview.chromium.org/290733007/diff/40001/ui/views/controls/textfi...
ui/views/controls/textfield/textfield.cc:1543:
model_->MoveCursorTo(last_drag_location_, true);
On 2014/05/21 22:44:28, Peter Kasting wrote:
> How does this work with off-Textfield cursor locations?  Does it pretend the
> textfield is infinitely wide, and then select to "where the mouse would be"? 
If
> so, doesn't this mean that the further off the side the user places the mouse
> cursor, the more text will be selected on every timer tick?

Yes, that's how it works currently, and I think it's reasonable. That's how
Blink and Mac/Win/GTK native textfields work, except that for some of those
(like the Win native textfield), the selection is updated on drag events, and
not on a regularly occurring timer.

> It seems like, ideally, we'd have a high-framerate (60 Hz) animation that
> scrolled-in-and-selected a small amount of new text each frame, regardless of
> how far off the side the mouse cursor is.

I agree that we ultimately want a smoother animation, but think we'd want to
increase the rate of scrolling as the user moves the mouse further beyond the
edge (providing the same correlation as Blink and Mac/Win/GTK native textfields
and this behavior I'm adding).

However, the smoother animation would require non-trivial changes to RenderText
and Textfield (we'd need to update the display offset disjoint from the cursor
position), which might open a bigger can of worms. I think the current behavior
is straightforward and a good V1 compromise, and I'll leave the bug open for
animation smoothing.

[Peter Kasting, 2014-05-21 23:18:43]

All right, LGTM

[msw, 2014-05-21 23:23:58]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-05-21 23:30:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/msw@chromium.org/290733007/40001

[commit-bot: I haz the power, 2014-05-22 08:21:43]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-05-22 09:34:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-22 09:34:55]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[msw, 2014-05-22 19:38:21]

Hey Scott, is the use of ScopedAnimationDurationScaleMode to fix
TextfieldTest.DragToSelect in the latest patch set appropriate? It seems like
many places have ad-hoc flags to prevent animation, but this seems like a better
shared pattern.

[sky, 2014-05-22 20:10:11]

LGTM - seems like it would be nice to make the default ZERO_DURATION for tests
and let people reset to other values that need it.

[msw, 2014-05-22 21:01:02]

On 2014/05/22 20:10:11, sky wrote:
> LGTM - seems like it would be nice to make the default ZERO_DURATION for tests
> and let people reset to other values that need it.

Good news, it's already set by AuraTestHelper via ViewsTestHelper[Aura] via
ViewsTestBase::SetUp. I removed the unnecessary explicit setting in textfield
test code; landing.

[msw, 2014-05-22 21:01:11]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-05-22 21:01:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/msw@chromium.org/290733007/90001

[commit-bot: I haz the power, 2014-05-23 00:29:29]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-23 04:29:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-23 04:29:16]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[msw, 2014-05-23 06:30:49]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-05-23 06:32:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/msw@chromium.org/290733007/90001

[commit-bot: I haz the power, 2014-05-23 12:16:41]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-23 14:52:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-23 14:52:01]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[msw, 2014-05-23 17:17:48]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2014-05-23 17:19:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/msw@chromium.org/290733007/90001

[commit-bot: I haz the power, 2014-05-23 19:01:02]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-23 21:09:22]

Change committed as 272591