[aw] Software mode tests for fullscreen API.

android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java

Index: android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
index 840fbaa99eef92ba47d8e9086718aa6e188aa869..b03a0588209859e6054760fac0a69c640654cf21 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
@@ -10,6 +10,7 @@ import android.view.ViewGroup;
 
 import org.chromium.android_webview.test.util.JavascriptEventObserver;
 import org.chromium.base.test.util.Feature;
+import org.chromium.content.browser.ContentVideoView;
 import org.chromium.content.browser.ContentViewCore;
 import org.chromium.content.browser.test.util.Criteria;
 import org.chromium.content.browser.test.util.CriteriaHelper;
@@ -57,6 +58,32 @@ public class AwContentsClientFullScreenTest extends AwTestBase {
 
     @MediumTest
     @Feature({"AndroidWebView"})
+    @DisableHardwareAccelerationForTest
+    public void testFullscreenVideoInSoftwareModeDoesNotDeadlock() throws Throwable {
+        // Although fullscreen video is not supported without hardware acceleration
+        // we should not deadlock if apps try to use it.
+        loadTestPageAndClickFullscreen(VIDEO_TEST_URL);
+        mContentsClient.waitForCustomViewShown();
+        getInstrumentation().runOnMainSync(new Runnable() {
+            @Override
+            public void run() {
+                mContentsClient.getExitCallback().onCustomViewHidden();
+            }
+        });
+        mContentsClient.waitForCustomViewHidden();
+    }
+
+    @MediumTest
+    @Feature({"AndroidWebView"})
+    @DisableHardwareAccelerationForTest
+    public void testFullscreenForNonVideoElementIsSupportedInSoftwareMode() throws Throwable {
+        // Fullscreen for non-video elements is supported and works as expected. Note that
+        // this test is the same as testOnShowAndHideCustomViewWithCallback_videoInsideDiv below.
+        doTestOnShowAndHideCustomViewWithCallback(VIDEO_INSIDE_DIV_TEST_URL);
+    }
+
+    @MediumTest
+    @Feature({"AndroidWebView"})
     public void testOnShowAndHideCustomViewWithCallback_video() throws Throwable {
         doTestOnShowAndHideCustomViewWithCallback(VIDEO_TEST_URL);
     }
@@ -295,6 +322,47 @@ public class AwContentsClientFullScreenTest extends AwTestBase {
         return view.getKeepScreenOn();
     }
 
+    private void assertContainsContentVideoView()
+            throws InterruptedException {
+        // We need to poll because the ContentVideoView is added to the customView asynchronously
+        assertTrue(CriteriaHelper.pollForCriteria(new Criteria() {
+            @Override
+            public boolean isSatisfied() {
+                return containsVideoView(mContentsClient.getCustomView());

[boliu, 2014/10/22 02:01:02]

Need to do this on the UI thread.

[Ignacio Solla, 2014/10/22 09:32:16]

Why? I'm happy to change it but I'd like to understand the reason first.

[boliu, 2014/10/22 15:53:45]

Because AwContents/ContentViewCore, and the entire view tree is single threaded
and should only be accessed on the UI thread.

[Ignacio Solla, 2014/10/27 11:42:06]

OK done.  (I was thinking that read-only operations with polling in the
instrumentation thread in tests were ok, but I guess it is safer to read it on
the UI thread because thread-local caches in Java)

+            }
+        }));
+    }
+
+    private void assertIsFullscreen() throws InterruptedException {
+        // We need to poll because the Javascript state is updated asynchronously
+        assertTrue(CriteriaHelper.pollForCriteria(new Criteria() {
+            @Override
+            public boolean isSatisfied() {
+                try {
+                    return DOMUtils.isFullscreen(mContentViewCore.getWebContents());

[boliu, 2014/10/22 02:01:02]

I didn't find this DOMUtils method?! I assume it needs to happen on UI thread as
well.

[Ignacio Solla, 2014/10/22 09:32:16]

To avoid conflicts, I have set the diffbase of this change against:
https://codereview.chromium.org/639413002/

So crcr/639413002 needs to be landed first. That method is defined there.

[boliu, 2014/10/22 15:53:45]

Ok. isFullscreen itself looks thread safe. mContentViewCore.getWebContents() is
not!!

[Ignacio Solla, 2014/10/27 11:42:06]

Ok, I'm calling mContentViewCore.getWebContents() on the UI thread now.

Note that we cannot call DOMUtils.blah on the UI thread because
JavaScriptUtils.executeJavaScriptAndWaitForResult must be called on the
instrumentation test.

[boliu, 2014/10/27 16:49:44]

Why? I don't see it just glancing at the code. ThreadUtils.runOnUiThread calls
the Runnable immediately if you are on the UI thread already, and I think
everything else should just work?

And maybe we should fix that in a follow up if it doesn't work.

[Ignacio Solla, 2014/10/27 16:59:52]

See:
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/tes...

The comment reads:
"Calling this from the UI thread causes it to time-out: the UI thread being
blocked won't have a chance to process the JavaScript eval response)."

+                } catch (InterruptedException | TimeoutException e) {
+                    fail(e.getMessage());
+                    return false;
+                }
+            }
+        }));
+    }
+
+    private boolean containsVideoView(View view) {

[boliu, 2014/10/22 02:01:02]

nit: move this next to assertContainsContentVideoView

[Ignacio Solla, 2014/10/22 09:32:16]

Done.

+        if (view instanceof ContentVideoView) {
+            return true;
+        }
+        if (view instanceof ViewGroup) {
+            ViewGroup viewGroup = (ViewGroup) view;
+            for (int i = 0; i < viewGroup.getChildCount(); i++) {
+                if (containsVideoView(viewGroup.getChildAt(i))) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
     private JavascriptEventObserver registerObserver(final String observerName) {
         final JavascriptEventObserver observer = new JavascriptEventObserver();
         getInstrumentation().runOnMainSync(new Runnable() {
@@ -316,7 +384,12 @@ public class AwContentsClientFullScreenTest extends AwTestBase {
     private void doOnShowCustomViewTest(String videoTestUrl) throws Exception {
         loadTestPageAndClickFullscreen(videoTestUrl);
         mContentsClient.waitForCustomViewShown();
-        assertTrue(DOMUtils.isFullscreen(mContentViewCore.getWebContents()));
+        assertIsFullscreen();
+        if (videoTestUrl.equals(VIDEO_TEST_URL)) {
+            // We only create a ContentVideoView (ie. a hardware accelerated surface) when going
+            // fullscreen on a video element.
+            assertContainsContentVideoView();
+        }
     }
 
     private void loadTestPageAndClickFullscreen(String videoTestUrl) throws Exception {