[aw] Fullscreen tests for non-media elements.

[aw] Fullscreen tests for non-media elements.

It is very common to go fullscreen on a div
containing a video element with custom html controls,
so we test that scenario.

We also rename AwContentsClientFullScreenVideoTest to
AwContentsClientFullScreenTest as this class is now testing
the fullscreen API for all types of elements.

This change depends on:
https://codereview.chromium.org/618013003/

BUG=398485
Committed: https://crrev.com/2d50461c7f3a529c880bdab509d9d560a047890c
Cr-Commit-Position: refs/heads/master@{#300110}

[Ignacio Solla, 2014-10-08 17:00:37]

igsolla@chromium.org changed reviewers:
+ mkosiba@chromium.org

[mkosiba (inactive), 2014-10-09 11:47:51]

https://codereview.chromium.org/639923003/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenVideoInsideDivTest.java
(right):

https://codereview.chromium.org/639923003/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenVideoInsideDivTest.java:19:
* div elements, in this case a div containing a video.
Unfortunately experience shows that reusing test code via inheritance does not
work.

The way we've been handling this kind of problem till now is by extracting the
common code to a

    private void doTest...(args) {...}

method and then having either multiple test methods call that method with
different params or (less preferred) having one test method call all of the
doTest variants with different params.

It's not very elegant and does create some duplication in test method
declarations. I'm open to other suggestions that don't involve inheritance :)

[Ignacio Solla, 2014-10-09 15:12:26]

Please not that I have also renamed AwContentsClientFullScreenVideoTest to
AwContentsClientFullScreenTest as this class is now testing the fullscreen API
for all types of elements, not just video.

https://codereview.chromium.org/639923003/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenVideoInsideDivTest.java
(right):

https://codereview.chromium.org/639923003/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenVideoInsideDivTest.java:19:
* div elements, in this case a div containing a video.
On 2014/10/09 11:47:51, mkosiba wrote:
> Unfortunately experience shows that reusing test code via inheritance does not
> work.
> 
> The way we've been handling this kind of problem till now is by extracting the
> common code to a
> 
>     private void doTest...(args) {...}
> 
> method and then having either multiple test methods call that method with
> different params or (less preferred) having one test method call all of the
> doTest variants with different params.
> 
> It's not very elegant and does create some duplication in test method
> declarations. I'm open to other suggestions that don't involve inheritance :)

Ok, as discussed offline I have chosen the doTest approach.

[mkosiba (inactive), 2014-10-09 15:18:54]

thanks! LGTM

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

https://codereview.chromium.org/639923003/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:117:
assertTrue(onPlayObserver.waitForEvent(2000));
any reason not to use AwTestBase.WAIT_TIMEOUT_MS? (it's 15000 ms)
If you need a different constant please wrap it in scaleTimeout so that using
ASan doesn't cause timeouts

https://codereview.chromium.org/639923003/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:140:
assertTrue(fullScreenErrorObserver.waitForEvent(500));
same here.

https://codereview.chromium.org/639923003/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:167:
assertTrue(onEnterFullscreenObserver.waitForEvent(2000));
and here

[Ignacio Solla, 2014-10-10 15:34:20]

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

https://codereview.chromium.org/639923003/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:117:
assertTrue(onPlayObserver.waitForEvent(2000));
On 2014/10/09 15:18:54, mkosiba wrote:
> any reason not to use AwTestBase.WAIT_TIMEOUT_MS? (it's 15000 ms)
> If you need a different constant please wrap it in scaleTimeout so that using
> ASan doesn't cause timeouts

Changed to WAIT_TIMEOUT_MS.

https://codereview.chromium.org/639923003/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:140:
assertTrue(fullScreenErrorObserver.waitForEvent(500));
On 2014/10/09 15:18:54, mkosiba wrote:
> same here.

Done.

https://codereview.chromium.org/639923003/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java:167:
assertTrue(onEnterFullscreenObserver.waitForEvent(2000));
On 2014/10/09 15:18:54, mkosiba wrote:
> and here

Done.

[Ignacio Solla, 2014-10-16 15:52:31]

The CQ bit was checked by igsolla@chromium.org

[commit-bot: I haz the power, 2014-10-16 15:53:30]

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

[commit-bot: I haz the power, 2014-10-16 16:01:29]

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

[commit-bot: I haz the power, 2014-10-16 16:01:30]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ignacio Solla, 2014-10-17 14:47:22]

The CQ bit was checked by igsolla@chromium.org

[commit-bot: I haz the power, 2014-10-17 14:47:52]

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

[commit-bot: I haz the power, 2014-10-17 15:11:32]

Committed patchset #5 (id:110001)

[commit-bot: I haz the power, 2014-10-17 15:12:20]

Patchset 5 (id:??) landed as
https://crrev.com/2d50461c7f3a529c880bdab509d9d560a047890c
Cr-Commit-Position: refs/heads/master@{#300110}