Fix tests and enable virtual viewport on Android WebView.

Fix tests and enable virtual viewport on Android WebView.

In pinch virtual viewport mode, the viewport visible to Javascript is sized
to the width of the content and its height is that width multiplied by the
aspect ratio of the true viewport.  So I changed the Android WebView tests
to only do vertical scrolling, and also changed the width of the test page
to not be larger than the viewport (so that vertical scrolling also behaves as expected).

NOTRY=true
BUG=148816
Committed: https://crrev.com/879b23ff025295c97d950a2917d67f6714f17bd5
Cr-Commit-Position: refs/heads/master@{#302712}

[aelias_OOO_until_Jul13, 2014-11-03 09:04:58]

aelias@chromium.org changed reviewers:
+ bokan@chromium.org, mkosiba@chromium.org

[aelias_OOO_until_Jul13, 2014-11-03 09:12:05]

Hi Marcin,

This should fix the remaining Webview-specific failures for pinch viewport.  I'd
like to get it enabled on Webview before M40 branch point to get on track to
release this mode and delete the old way.  If we find out from manual testing
that real-world apps actually rely on the specific behavior that the previous
version of the test specified, then we can revert the flag flip and test
changes, then work on adding a WebView quirk for this before we turn the flag
back on again.  But we probably won't discover these example apps without going
ahead and enabling this now.

[mkosiba (inactive), 2014-11-03 11:38:59]

Hi Alex,
  It seems like none of the tests scroll horizontally now? I don't think
impossible for JS to see a != 0 horizontal scroll offset though. I would prefer
to at least have 1 test that does a horizontal scroll (I can code this up) -
would it be sufficient to have a page of width 2*w and a viewport of width w for
that to work?
  Let's give it a shot for M40. Also, now that the jumping around bug has been
fixed I'll once more go through the apps that were problematic (wrt scrolling)
in the past and see if we get any more issues.

https://codereview.chromium.org/694113002/diff/20001/android_webview/javatest...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
(left):

https://codereview.chromium.org/694113002/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java:582:
getInstrumentation().runOnMainSync(new Runnable() {
If you're removing this bit then the rest of the test doesn't make much sense
(it's now asserting that if you call window.scrollTo(0, some large value); you
scroll somewhere...

Can we make any predictions about the vertical scroll offset? For example - is
it guaranteed to be within 2*(testContainerView.getHeight()) ? If not then let's
take the entire test out.

https://codereview.chromium.org/694113002/diff/20001/android_webview/javatest...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
(right):

https://codereview.chromium.org/694113002/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java:420:
assert (maxScrollXPix % dragStepSize) != 0;
you'll need to take this out.

https://codereview.chromium.org/694113002/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java:620:
testContainerView.getAwContents().flingScroll(0, 1000);
there is a CTS test identical test to this. I'll need to remember to update it.

[bokan, 2014-11-03 13:39:19]

On 2014/11/03 11:38:59, mkosiba wrote:
> Hi Alex,
>   It seems like none of the tests scroll horizontally now? I don't think
> impossible for JS to see a != 0 horizontal scroll offset though. I would
prefer
> to at least have 1 test that does a horizontal scroll (I can code this up) -
> would it be sufficient to have a page of width 2*w and a viewport of width w
for
> that to work?

The way to do this would be to give the page a minimum scale. In that case, the
main frame (outer viewport) size will be set to pinch viewport size * minimum
scale. Therefore, if you set the min-scale to 1.0 the outer and inner viewports
should be the same size.

[aelias_OOO_until_Jul13, 2014-11-04 06:32:05]

OK, thanks to bokan@ for the tip, I had forgotten about that behavior.  With
minimum-scale=1, all of the horizontal scrolling tests pass without any other
change.  The scroll-to-bottom-at-0.6-scale test still fails because the outer
viewport still cannot be smaller than the device width; I deleted it.

https://codereview.chromium.org/694113002/diff/20001/android_webview/javatest...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java
(left):

https://codereview.chromium.org/694113002/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidScrollIntegrationTest.java:582:
getInstrumentation().runOnMainSync(new Runnable() {
On 2014/11/03 at 11:38:58, mkosiba wrote:
> If you're removing this bit then the rest of the test doesn't make much sense
(it's now asserting that if you call window.scrollTo(0, some large value); you
scroll somewhere...
> 
> Can we make any predictions about the vertical scroll offset? For example - is
it guaranteed to be within 2*(testContainerView.getHeight()) ? If not then let's
take the entire test out.

We can make a loose prediction, but I don't think it would make this test cover
any behavior that's particularly useful to anybody, so I deleted it.

[mkosiba (inactive), 2014-11-04 16:56:29]

lgtm

[mkosiba (inactive), 2014-11-04 17:12:48]

hmm, so the testPageDown failure is a bit weird.. it's only interacting with the
Java-side scrolling. The only thing I can think of is that the Java side has
gotten confused about what the maximal scroll offset is...

[aelias_OOO_until_Jul13, 2014-11-04 19:28:59]

I'll try to get to the bottom of why testPageDown is timing out.  It was passing
locally for me.

[boliu, 2014-11-04 20:15:11]

drive-by blind suggestion: If adding @DisableHardwareAccelerationForTest to the
test makes it pass on the bot, just add it and file a bug against me

[mkosiba (inactive), 2014-11-04 21:20:02]

On 2014/11/04 20:15:11, boliu wrote:
> drive-by blind suggestion: If adding @DisableHardwareAccelerationForTest to
the
> test makes it pass on the bot, just add it and file a bug against me

Ah, since this test is using a Java-driven animation maybe it's flakey
because of https://codereview.chromium.org/665233004 not being landed yet?

[aelias_OOO_until_Jul13, 2014-11-04 21:21:21]

On 2014/11/04 at 21:20:02, mkosiba wrote:
> On 2014/11/04 20:15:11, boliu wrote:
> > drive-by blind suggestion: If adding @DisableHardwareAccelerationForTest to
the
> > test makes it pass on the bot, just add it and file a bug against me
> 
> Ah, since this test is using a Java-driven animation maybe it's flakey
> because of https://codereview.chromium.org/665233004 not being landed yet?

That was my first suspicion too, but it seems DisableHardwareAccelerationForTest
actually made it go green.  I'll run another tryjob just to make sure.

[boliu, 2014-11-04 23:10:07]

On 2014/11/04 21:21:21, aelias wrote:
> On 2014/11/04 at 21:20:02, mkosiba wrote:
> > On 2014/11/04 20:15:11, boliu wrote:
> > > drive-by blind suggestion: If adding @DisableHardwareAccelerationForTest
to
> the
> > > test makes it pass on the bot, just add it and file a bug against me
> > 
> > Ah, since this test is using a Java-driven animation maybe it's flakey
> > because of https://codereview.chromium.org/665233004 not being landed yet?
> 
> That was my first suspicion too, but it seems
DisableHardwareAccelerationForTest
> actually made it go green.  I'll run another tryjob just to make sure.

If DisableHardwareAccelerationForTest fixes this it's probably the same cause as
crbug.com/428355. And the fix for that I just uploaded at
https://codereview.chromium.org/689473005/

Also you might find this useful. I made a hack to repeat an instrumentation run
until there's a failure. Can use it with -f parameter to repeat/stress a single
flaky tests: https://codereview.chromium.org/701943002/ It should really be an
existing feature in the test script..

[aelias_OOO_until_Jul13, 2014-11-04 23:52:04]

OK, let me land this with @DisableHardwareAcceleration for now.  The other
change just landed too so hopefully it won't flake on the waterfall.

[aelias_OOO_until_Jul13, 2014-11-04 23:52:30]

The CQ bit was checked by aelias@chromium.org

[commit-bot: I haz the power, 2014-11-04 23:53:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/694113002/100001

[commit-bot: I haz the power, 2014-11-04 23:54:36]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2014-11-04 23:55:21]

Patchset 6 (id:??) landed as
https://crrev.com/879b23ff025295c97d950a2917d67f6714f17bd5
Cr-Commit-Position: refs/heads/master@{#302712}

[boliu, 2014-11-05 01:34:48]

Locally confirmed the test is the same issue, so also run it in hardware mode in
https://codereview.chromium.org/689473005/