Early terminate flings when scrolling impossible

Early terminate flings when scrolling impossible

Previously, flings were terminated early only if the root layer overscrolled.
Now, flings will terminate if the fling target's |scrollBy()| method returns
false, providing consistency between layer types and saving battery on
sites with scrolling sublayers.

Also move |DidOverscrollParams| into content/ in anticipation of using it
directly in the overscroll IPC message.

Corresponding Blink patch:
https://codereview.chromium.org/139493003/

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

[jdduke (slow), 2014-03-28 21:03:13]

aelias@: I've been sitting on this patch for a while, might as well push it out
for review.  A lot of the small changes are just plumbing tweaks from moving
|DidOverscrollParams| out of cc/ into content/ (planning to use it as the sole
overscroll notification argument, and eventually in the event ack).

I'll add some tests if you think this is a worthwhile change, and there may be
some corner cases I need to address.

[jdduke (slow), 2014-03-28 21:05:42]

On 2014/03/28 21:03:13, jdduke wrote:
> aelias@: I've been sitting on this patch for a while, might as well push it
out
> for review.  A lot of the small changes are just plumbing tweaks from moving
> |DidOverscrollParams| out of cc/ into content/ (planning to use it as the sole
> overscroll notification argument, and eventually in the event ack).
> 
> I'll add some tests if you think this is a worthwhile change, and there may be
> some corner cases I need to address.

Hmm, I realize the key change here might be somewhat muddied by the refactoring
of velocity notifications.  The key change is to the return value of
|WebGestureCurveTarget::scrollBy|, where returning false will now terminate the
fling.

[aelias_OOO_until_Jul13, 2014-03-28 23:55:08]

lgtm

[jdduke (slow), 2014-03-29 14:07:22]

jochen@: Owner review for content/child and content/test?  Thanks.

[jochen (gone - plz use gerrit), 2014-03-31 09:59:10]

lgtm with question

https://codereview.chromium.org/136173004/diff/120001/content/child/fling_ani...
File content/child/fling_animator_impl_android.cc (right):

https://codereview.chromium.org/136173004/diff/120001/content/child/fling_ani...
content/child/fling_animator_impl_android.cc:19: const char* kCurveName =
"FlingAnimatorImpl";
why do you declare a variable for this but none of the other string constants?

[jdduke (slow), 2014-03-31 15:41:43]

https://codereview.chromium.org/136173004/diff/120001/content/child/fling_ani...
File content/child/fling_animator_impl_android.cc (right):

https://codereview.chromium.org/136173004/diff/120001/content/child/fling_ani...
content/child/fling_animator_impl_android.cc:19: const char* kCurveName =
"FlingAnimatorImpl";
On 2014/03/31 09:59:10, jochen wrote:
> why do you declare a variable for this but none of the other string constants?

I added the trace to mimic what was done in touch_fling_gesture_curve.cc, where
they also declare the string constant as a variable.  However, it appears we
already have an async fling trace sequence in input_handler_proxy.cc, so this is
redundant and I see no reason to have this, I'll remove.

[jdduke (slow), 2014-03-31 15:41:57]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-03-31 15:44:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/136173004/180001

[commit-bot: I haz the power, 2014-03-31 20:24:47]

Change committed as 260631

[tkent, 2014-04-01 04:16:32]

A revert of this CL has been created in
https://codereview.chromium.org/213743004/ by tkent@chromium.org.

The reason for reverting is: Broke a layout test,
fast/events/touch/gesture/pad-gesture-fling.html

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40...
.

[jdduke (slow), 2014-04-01 04:40:43]

On 2014/04/01 04:16:32, tkent wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/213743004/ by mailto:tkent@chromium.org.
> 
> The reason for reverting is: Broke a layout test,
> fast/events/touch/gesture/pad-gesture-fling.html
> 
>
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40...
> .

Hmm, looks like this test needs to re-baselined.