Don't send touchcancel on touch scroll start

Don't send touchcancel on touch scroll start

This switches the default touch scrolling mode from 'touch cancel on scroll' to our new 'touch move absorption' behavior.  Briefly: touchmove events still aren't sent during active scrolling (for performance reasons), but will be sent when overscrolling.  See https://docs.google.com/a/chromium.org/document/d/1MGuCb0Lb7UfaDmH44bp9YKVG-CVfH6WtKTZl93BXwQw/edit# for details.

This exposes the touch scrolling mode in chrome://flags/#touch-scrolling-mode so that we can easily switch back to the old behavior for diagnostics purposes.  This will be removed after this has reached stable (crbug.com/350430).

Fixes a bug where (depending on timing) we could send one or more extra touchmove events at the start of a scroll sequence.  Now we start every scroll with moves being absorbed and switch to sending move events only after we've seen an unconsumed GestureScrollUpdate.

Fixes a bug where scrolling the top controls (Android) wouldn't mark the GestureScrollUpdate event as consumed.  Adds a unit test covering this and other basic parts of LayerTreeHostImpl's scrolling behavior with respect to top controls.

Updates some tests to support both the original "touchcancel" mode and the new "touchmove absorption" mode.  This way if we uncover problems after branch, we can simply flip the default mode (rather than reverting this entire CL).

BUG=346693
R=aelias@chromium.org, jdduke@chromium.org

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

[Rick Byers, 2014-03-07 17:05:14]

Jared, please review.

[jdduke (slow), 2014-03-07 18:03:16]

content/browser/renderer_host/input lgtm.

I guess we still have the GSU race, but it won't really hurt the interaction
experience any more than a heavily delayed gesture stream (also, since all
touches have to hit the impl thread (currently), delayed gesture handling will
also delay touch handling).

The last question I had was about the overscroll effect. How bad will the visual
collision be between the first overscroll frame when the notification triggers
the glow effect, but we transition to sending touchmove's (potentially
activating a separate effect)?

Worst case, we could hook into |RenderWidgetHostViewAndroid::FilterInputEvent()|
and disable the glow if we see that a TouchMove is being sent.  That actually
should work for all but the most janky cases...  or we might just drop the
initial overscroll notification  if it has zero velocity.

https://codereview.chromium.org/183013010/diff/70010/content/browser/renderer...
File content/browser/renderer_host/input/touch_event_queue_unittest.cc (right):

https://codereview.chromium.org/183013010/diff/70010/content/browser/renderer...
content/browser/renderer_host/input/touch_event_queue_unittest.cc:1550: }
Could you also test that absorption is reset when a new touch sequence is being
dispatched?

[Rick Byers, 2014-03-07 20:58:53]

On 2014/03/07 18:03:16, jdduke wrote:
> content/browser/renderer_host/input lgtm.

Thanks.  Are the about_flags and resource strings changes OK to you too? 
Everyone is an OWNER of them.

+aelias for OWNERS in cc/

> I guess we still have the GSU race, but it won't really hurt the interaction
> experience any more than a heavily delayed gesture stream (also, since all
> touches have to hit the impl thread (currently), delayed gesture handling will
> also delay touch handling).

Right, there's inherent timing issues with trying to synchronize work between
the impl thread and main thread.  My goal here is to ensure the common case
(impl thread scrolling without consuming any touchmove events) performs well. 
This may increase the chance of artifacts in UI effects that switch between
scrolling and main-thread driven animations, but I believe we'll ultimately need
an opt-in sync-scroll mode (eg. "scroll-delay") to fully address those
scenarios.

> The last question I had was about the overscroll effect. How bad will the
visual
> collision be between the first overscroll frame when the notification triggers
> the glow effect, but we transition to sending touchmove's (potentially
> activating a separate effect)?

I've played with this a bit in practice and it seems OK.  As we focus on
polishing a pull-to-refresh effect we might want to tweak the behavior to
minimize any risk of visual artifacts, but I think we should leave that for
future work when we've got more concrete scenarios (and test infrastructure).  I
think all the typical cases on the web will work well without visual artifacts
(eg. side-scrollers that fail to call preventDefault on the touchmove).

> Worst case, we could hook into
|RenderWidgetHostViewAndroid::FilterInputEvent()|
> and disable the glow if we see that a TouchMove is being sent.  That actually
> should work for all but the most janky cases...  or we might just drop the
> initial overscroll notification  if it has zero velocity.

touchmoves are sent throughout the overscroll glow, so we can't disable the glow
when we see a touchmove being sent.  But we might want to wait until the first
overscroll touchmove has been sent and gone unconsumed before starting the
overscroll glow.  It's not clear to me what the right trade-off is, so for now I
think it's best to err on the side of the existing scenarios/behavior and then
we can iterate.

https://codereview.chromium.org/183013010/diff/70010/content/browser/renderer...
File content/browser/renderer_host/input/touch_event_queue_unittest.cc (right):

https://codereview.chromium.org/183013010/diff/70010/content/browser/renderer...
content/browser/renderer_host/input/touch_event_queue_unittest.cc:1550: }
On 2014/03/07 18:03:16, jdduke wrote:
> Could you also test that absorption is reset when a new touch sequence is
being
> dispatched?

Done.

https://codereview.chromium.org/183013010/diff/90001/ui/events/gesture_detect...
File ui/events/gesture_detection/gesture_provider_unittest.cc (right):

https://codereview.chromium.org/183013010/diff/90001/ui/events/gesture_detect...
ui/events/gesture_detection/gesture_provider_unittest.cc:521:
TEST_F(GestureProviderTest, FractionalScroll) {
This was a case I initially thought was broken but turned out to actually be OK.
 Since it's a scenario that's not really tested (and important to touchmove
absorption) I figured I'd leave the test in (even though it means I'll need to
merge with your in-flight change to this test before I land - or try to sqeak in
before you <grin>).

[jdduke (slow), 2014-03-07 21:07:32]

Resource/flag changes look good with one exception.

https://codereview.chromium.org/183013010/diff/90001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/183013010/diff/90001/chrome/app/generated_res...
chrome/app/generated_resources.grd:6984: +      <message
name="IDS_FLAGS_TOUCH_SCROLLING_MODE_DESCRIPTION" desc="Description for the flag
to enable out of process PDF.">
Need to update the DESCRIPTION string.

https://codereview.chromium.org/183013010/diff/90001/ui/events/gesture_detect...
File ui/events/gesture_detection/gesture_provider_unittest.cc (right):

https://codereview.chromium.org/183013010/diff/90001/ui/events/gesture_detect...
ui/events/gesture_detection/gesture_provider_unittest.cc:521:
TEST_F(GestureProviderTest, FractionalScroll) {
On 2014/03/07 20:58:53, Rick Byers wrote:
> This was a case I initially thought was broken but turned out to actually be
OK.
>  Since it's a scenario that's not really tested (and important to touchmove
> absorption) I figured I'd leave the test in (even though it means I'll need to
> merge with your in-flight change to this test before I land - or try to sqeak
in
> before you <grin>).

Yeah this is great.  I can hold off with the timing patch if you like.

[aelias_OOO_until_Jul13, 2014-03-07 21:09:08]

https://codereview.chromium.org/183013010/diff/90001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/183013010/diff/90001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:2307: if (did_scroll_content) {
I think this should also have "|| did_scroll_top_controls".  SetNeedsRedraw is
needed when top controls move.

[aelias_OOO_until_Jul13, 2014-03-07 21:12:31]

lgtm for cc/

https://codereview.chromium.org/183013010/diff/90001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/183013010/diff/90001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:2307: if (did_scroll_content) {
On 2014/03/07 21:09:09, aelias wrote:
> I think this should also have "|| did_scroll_top_controls".  SetNeedsRedraw is
> needed when top controls move.

Sorry, ignore that comment.  Of course, you didn't change the behavior here, and
this is handled by DidChangeTopControlsPosition instead.

[Rick Byers, 2014-03-07 21:24:40]

Thanks guys!

https://codereview.chromium.org/183013010/diff/90001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/183013010/diff/90001/chrome/app/generated_res...
chrome/app/generated_resources.grd:6984: +      <message
name="IDS_FLAGS_TOUCH_SCROLLING_MODE_DESCRIPTION" desc="Description for the flag
to enable out of process PDF.">
On 2014/03/07 21:07:32, jdduke wrote:
> Need to update the DESCRIPTION string.

Done, thanks for catching this!

[Rick Byers, 2014-03-07 21:24:46]

The CQ bit was checked by rbyers@chromium.org

[commit-bot: I haz the power, 2014-03-07 21:26:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/183013010/110001

[commit-bot: I haz the power, 2014-03-07 23:27:26]

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

[commit-bot: I haz the power, 2014-03-07 23:27:26]

Retried try job too often on win_x64_rel for step(s) base_unittests,
chrome_elf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...

[Rick Byers, 2014-03-08 04:06:04]

The CQ bit was checked by rbyers@chromium.org

[commit-bot: I haz the power, 2014-03-08 10:59:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/183013010/150001

[commit-bot: I haz the power, 2014-03-08 12:59:20]

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

[commit-bot: I haz the power, 2014-03-08 12:59:20]

Retried try job too often on win for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[jdduke (slow), 2014-03-08 16:25:15]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-03-08 16:26:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/183013010/150001

[commit-bot: I haz the power, 2014-03-08 17:27:23]

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

[commit-bot: I haz the power, 2014-03-08 17:27:24]

Retried try job too often on mac_rel for step(s) telemetry_perf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[Rick Byers, 2014-03-08 18:03:30]

Committed patchset #9 manually as r255787 (presubmit successful).