Handle root layer fling Java-side in android_webview.

Handle root layer fling Java-side in android_webview.

The android_webview implementation needs to handle root layer fling
Java-side. This is required for proper integration with the android
view system which requires the fling scroll offset update to be
performed as a result of the computeScroll method.

BUG=None
TEST=AndroidWebViewTest

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

[mkosiba (inactive), 2013-07-31 17:30:14]

Alex - PTAL @ cc/ and setting plumbing.
Joth - PTAL at the rest.

I'll add the remaining OWNERS for rubber stamps later on.

https://codereview.chromium.org/20990009/diff/1/android_webview/javatests/src...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwScrollOffsetManagerTest.java
(right):

https://codereview.chromium.org/20990009/diff/1/android_webview/javatests/src...
android_webview/javatests/src/org/chromium/android_webview/test/AwScrollOffsetManagerTest.java:96:
private static class TestOverScroller extends OverScroller {
I'm actually a bit stuck on how to add any reasonable tests here. The only one I
could think of is that 'in computeScroll, if the overScroller says the new Y is
+10 px overScrollBy is called with a delta of 10 px'. I'll add that test case
tomorrow.

[aelias_OOO_until_Jul13, 2013-07-31 17:43:06]

cc/ lgtm

[joth, 2013-07-31 17:45:28]

aw/ bits look good, only minor (non-function) suggestions.

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:404:
public void onFlingStartGestureStart(int vx, int vy) {
velocityX, Y ?

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1305: if
(event.getActionMasked() == MotionEvent.ACTION_DOWN) {
we have a block just above for ACTION_DOWN, I think this go there? if not,
comment why it needs to happen after the releaseAll (and why the rest can't move
down to here)

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
File
android_webview/java/src/org/chromium/android_webview/AwScrollOffsetManager.java
(right):

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwScrollOffsetManager.java:61:
private int mLastFlingVy;
VelocityX,Y

(or is the VX terminology completely ingrained in platform? if so, nvrmd)

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwScrollOffsetManager.java:235:
public void onUnhandledFlingStartEvent() {
I've been leaning towards making public methods on public classes by stuff that
is good to call from the embedding layer. Unit testing complicates it. Maybe
this whole class should be @VisibleForTesting?

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwScrollOffsetManager.java:249:
if (!mScroller.computeScrollOffset()) {
nit:

bool stillAnimating = mScroller.computeScrollOffset();
if (!stillAnimating) return;

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwScrollOffsetManager.java:266:
overScrollBy(x - oldX, y - oldY);
maybe comment that this will generally not result in an overscroll if there
still scroll range remaining? (IIUC)

https://codereview.chromium.org/20990009/diff/1/content/renderer/gpu/render_w...
File content/renderer/gpu/render_widget_compositor.cc (right):

https://codereview.chromium.org/20990009/diff/1/content/renderer/gpu/render_w...
content/renderer/gpu/render_widget_compositor.cc:283:
cmd->HasSwitch(switches::kIgnoreRootLayerFlings);
We already use widget->UsingSynchronousRendererCompositor(); to set a bunch of
LTH settings up top of this method. Could just add this to that set and avoid
another Command line switch?

[mkosiba (inactive), 2013-08-01 15:59:59]

Thanks!

Joth, I refactored AwScrollOffsetManager a bit and have it cancel the fling
whenever the GestureHandler tells us to (which de-dupes the 'cancel fling
onTouchDown' logic).

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
File android_webview/java/src/org/chromium/android_webview/AwContents.java
(right):

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:404:
public void onFlingStartGestureStart(int vx, int vy) {
On 2013/07/31 17:45:29, joth wrote:
> velocityX, Y ?

Done.

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:1305: if
(event.getActionMasked() == MotionEvent.ACTION_DOWN) {
On 2013/07/31 17:45:29, joth wrote:
> we have a block just above for ACTION_DOWN, I think this go there? if not,
> comment why it needs to happen after the releaseAll (and why the rest can't
move
> down to here)

there is no ordering requirement, I simply didn't notice the other block

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
File
android_webview/java/src/org/chromium/android_webview/AwScrollOffsetManager.java
(right):

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwScrollOffsetManager.java:61:
private int mLastFlingVy;
On 2013/07/31 17:45:29, joth wrote:
> VelocityX,Y
> 
> (or is the VX terminology completely ingrained in platform? if so, nvrmd)

Done.

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwScrollOffsetManager.java:235:
public void onUnhandledFlingStartEvent() {
On 2013/07/31 17:45:29, joth wrote:
> I've been leaning towards making public methods on public classes by stuff
that
> is good to call from the embedding layer. Unit testing complicates it. Maybe
> this whole class should be @VisibleForTesting?

Done. Marked as @VisibleForTesting

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwScrollOffsetManager.java:249:
if (!mScroller.computeScrollOffset()) {
On 2013/07/31 17:45:29, joth wrote:
> nit:
> 
> bool stillAnimating = mScroller.computeScrollOffset();
> if (!stillAnimating) return;

Done.

https://codereview.chromium.org/20990009/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwScrollOffsetManager.java:266:
overScrollBy(x - oldX, y - oldY);
On 2013/07/31 17:45:29, joth wrote:
> maybe comment that this will generally not result in an overscroll if there
> still scroll range remaining? (IIUC)

Done.

https://codereview.chromium.org/20990009/diff/1/android_webview/javatests/src...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwScrollOffsetManagerTest.java
(right):

https://codereview.chromium.org/20990009/diff/1/android_webview/javatests/src...
android_webview/javatests/src/org/chromium/android_webview/test/AwScrollOffsetManagerTest.java:96:
private static class TestOverScroller extends OverScroller {
On 2013/07/31 17:30:14, mkosiba wrote:
> I'm actually a bit stuck on how to add any reasonable tests here. The only one
I
> could think of is that 'in computeScroll, if the overScroller says the new Y
is
> +10 px overScrollBy is called with a delta of 10 px'. I'll add that test case
> tomorrow.

actually adding tests is proving to be a pain since all of the 'fun'
OverScroller methods are final.

https://codereview.chromium.org/20990009/diff/1/content/renderer/gpu/render_w...
File content/renderer/gpu/render_widget_compositor.cc (right):

https://codereview.chromium.org/20990009/diff/1/content/renderer/gpu/render_w...
content/renderer/gpu/render_widget_compositor.cc:283:
cmd->HasSwitch(switches::kIgnoreRootLayerFlings);
On 2013/07/31 17:45:29, joth wrote:
> We already use widget->UsingSynchronousRendererCompositor(); to set a bunch of
> LTH settings up top of this method. Could just add this to that set and avoid
> another Command line switch?

Done.

[mkosiba (inactive), 2013-08-01 16:05:05]

+ joi@ for the content API modification

+ jochen@ for content/browser/web_contents rubber-stamp

+ kbr@ for content/renderer/gpu rubber-stamp

[mkosiba (inactive), 2013-08-01 16:05:58]

+ avi@ for the content API modification

+ kbr@ for content/renderer/gpu rubber-stamp

[Ken Russell (switch to Gerrit), 2013-08-01 18:00:10]

content/renderer/gpu LGTM

[jochen (gone - plz use gerrit), 2013-08-01 20:32:56]

content/browser/web_contents lgtm

[Avi (use Gerrit), 2013-08-01 20:37:35]

LGTM

[joth, 2013-08-01 21:21:18]

lgtm

[commit-bot: I haz the power, 2013-08-02 10:38:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/20990009/16001

[commit-bot: I haz the power, 2013-08-02 16:27:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/20990009/34001

[commit-bot: I haz the power, 2013-08-02 19:11:50]

Change committed as 215339