Deflake AwContentsClientFullScreenTest

Header's content length should be set in
ResourceMultiBufferDataProvider::DidReceiveResponse()
and copied to url_data_ such that MultibufferDataSource::StartCallback()
later determines the success of the reading.

Then why is this flaky?
ResourceMultiBufferDataProvider::DidFinishLoading() also copies the url
data, so depending on the timing, StartCallback() could pick up the
correct url data length, and the test passes when this happens.

The expected content length information seems missing in
AndroidStreamReaderUrlLoader.

Also, when the test fails, readyState doesn't seem to change from
0 to 4 in blink::WebMediaPlayerImpl. I'm not entirely sure if this
is needed for all the tests that require media playback, but this
seems to make it more obvious where it fails.

Bug: 936757
Change-Id: I62245808876385a05c8e011882e2122622ff5c40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1548579Reviewed-by: Tim Volodine <timvolodine@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Changwan Ryu <changwan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649630}

android_webview/browser/network_service/android_stream_reader_url_loader.cc

@@ -223,6 +223,7 @@ void AndroidStreamReaderURLLoader::HeadersComplete(
       content_length_header.append(
           base::NumberToString(expected_content_size_));
       head.headers->AddHeader(content_length_header);
+      head.content_length = expected_content_size_;
     }
 
     std::string mime_type;

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

@@ -198,7 +198,7 @@ public class AwContentsClientFullScreenTest {
         doOnShowCustomViewTest(videoTestUrl);
         Assert.assertTrue(DOMUtils.isMediaPaused(getWebContentsOnUiThread(), VIDEO_ID));
 
-        tapPlayButton();
+        playVideo();
         DOMUtils.waitForMediaPlay(getWebContentsOnUiThread(), VIDEO_ID);
     }
 
@@ -252,7 +252,7 @@ public class AwContentsClientFullScreenTest {
         assertKeepScreenOnActive(customView, false);
 
         // Play and verify that there is an active power save blocker.
-        tapPlayButton();
+        playVideo();
         assertWaitForKeepScreenOnActive(customView, true);
 
         // Stop the video and verify that the power save blocker is gone.
@@ -271,7 +271,7 @@ public class AwContentsClientFullScreenTest {
         assertKeepScreenOnActive(mTestContainerView, false);
 
         // Play and verify that there is an active power save blocker.
-        tapPlayButton();
+        playVideoOnEmbedderView();
         assertWaitForKeepScreenOnActive(mTestContainerView, true);
 
         // Stop the video and verify that the power save blocker is gone.
@@ -287,7 +287,7 @@ public class AwContentsClientFullScreenTest {
         loadTestPage(VIDEO_INSIDE_DIV_TEST_URL);
 
         // Play and verify that there is an active power save blocker.
-        tapPlayButton();
+        playVideoOnEmbedderView();
         assertWaitForKeepScreenOnActive(mTestContainerView, true);
 
         // Enter fullscreen and verify that the power save blocker is
@@ -317,7 +317,7 @@ public class AwContentsClientFullScreenTest {
 
         // Play and verify that there is an active power save blocker
         // in fullscreen.
-        tapPlayButton();
+        playVideoOnEmbedderView();
         assertWaitForKeepScreenOnActive(customView, true);
 
         // Exit fullscreen and verify that the power save blocker is
@@ -327,23 +327,59 @@ public class AwContentsClientFullScreenTest {
         assertKeepScreenOnActive(customView, true);
     }
 
-    private void tapPlayButton() throws Exception {
+    private boolean shouldPlayOnFullScreenView() throws Exception {
         String testUrl = mTestContainerView.getAwContents().getUrl();
-        if (VIDEO_TEST_URL.equals(testUrl)
-                && DOMUtils.isFullscreen(getWebContentsOnUiThread())) {
-            // The VIDEO_TEST_URL page goes fullscreen on the <video> element. In fullscreen
-            // the button with id CUSTOM_PLAY_CONTROL_ID will not be visible, but the standard
-            // html5 video controls are. The standard html5 controls are shadow html elements
-            // without any ids so it is difficult to retrieve its precise location. However,
-            // a large play control is rendered in the center of the custom view
-            // (containing the fullscreen <video>) so we just rely on that fact here.
-            TouchCommon.singleClickView(mContentsClient.getCustomView());
+        return VIDEO_TEST_URL.equals(testUrl) && DOMUtils.isFullscreen(getWebContentsOnUiThread());
+    }
+
+    private void playVideo() throws Exception {
+        if (shouldPlayOnFullScreenView()) {
+            playVideoOnFullScreenView();
         } else {
-            JSUtils.clickNodeWithUserGesture(
-                    mTestContainerView.getWebContents(), CUSTOM_PLAY_CONTROL_ID);
+            playVideoOnEmbedderView();
         }
     }
 
+    private void playVideoOnEmbedderView() throws Exception {
+        Assert.assertFalse(shouldPlayOnFullScreenView());
+        waitUntilHaveEnoughDataForPlay();
+        JSUtils.clickNodeWithUserGesture(
+                mTestContainerView.getWebContents(), CUSTOM_PLAY_CONTROL_ID);
+    }
+
+    private void waitUntilHaveEnoughDataForPlay() throws Exception {
+        // crbug.com/936757: you are expected to wait before media playback is ready.
+        CriteriaHelper.pollInstrumentationThread(new Criteria() {
+            @Override
+            public boolean isSatisfied() {
+                try {
+                    // Checking HTMLMediaElement.readyState == 4 (HAVE_ENOUGH_DATA).
+                    int readyState = DOMUtils.getNodeField(
+                            "readyState", getWebContentsOnUiThread(), VIDEO_ID, Integer.class);
+                    updateFailureReason(
+                            "Expected readyState == 4, but timed out when readyState == "
+                            + readyState);
+                    return readyState == 4; // HAVE_ENOUGH_DATA
+                } catch (Exception e) {
+                    throw new RuntimeException(e);
+                }
+            }
+        });
+    }
+
+    private void playVideoOnFullScreenView() throws Exception {
+        Assert.assertTrue(shouldPlayOnFullScreenView());
+        waitUntilHaveEnoughDataForPlay();
+        // JSUtils.clickNodeWithUserGesture(getWebContentsOnUiThread(), nodeId);
+        // The VIDEO_TEST_URL page goes fullscreen on the <video> element. In fullscreen
+        // the button with id CUSTOM_PLAY_CONTROL_ID will not be visible, but the standard
+        // html5 video controls are. The standard html5 controls are shadow html elements
+        // without any ids so it is difficult to retrieve its precise location. However,
+        // a large play control is rendered in the center of the custom view
+        // (containing the fullscreen <video>) so we just rely on that fact here.
+        TouchCommon.singleClickView(mContentsClient.getCustomView());
+    }
+
     /**
      * Asserts that the keep screen on property in the given {@code view} is active as
      * {@code expected}. It also verifies that it is only active when the video is playing.

content/public/test/android/javatests/src/org/chromium/content_public/browser/test/util/DOMUtils.java

@@ -483,8 +483,8 @@ public class DOMUtils {
      * @param valueType The type of the value to read.
      * @return the field's value.
      */
-    private static <T> T getNodeField(String fieldName, final WebContents webContents,
-            String nodeId, Class<T> valueType) throws InterruptedException, TimeoutException {
+    public static <T> T getNodeField(String fieldName, final WebContents webContents, String nodeId,
+            Class<T> valueType) throws InterruptedException, TimeoutException {
         StringBuilder sb = new StringBuilder();
         sb.append("(function() {");
         sb.append("  var node = document.getElementById('" + nodeId + "');");

testing/buildbot/filters/mojo.fyi.network_webview_instrumentation_test_apk.filter

@@ -9,12 +9,4 @@
 # support running NS OOP, https://crbug.com/882650
 
 # https://crbug.com/893575
--org.chromium.android_webview.test.CookieManagerStartupTest.testStartup
-
-# Flaky tests on android_mojo and android_mojo_rel bots
-# https://crbug.com/936757, https://crbug.com/939355
--org.chromium.android_webview.test.AwContentsClientFullScreenTest.testOnShowCustomViewAndPlayWithHtmlControl_videoInsideDiv
--org.chromium.android_webview.test.AwContentsClientFullScreenTest.testPowerSaveBlockerIsEnabledDuringEmbeddedPlayback
--org.chromium.android_webview.test.AwContentsClientFullScreenTest.testPowerSaveBlockerIsTransferredToEmbedded
--org.chromium.android_webview.test.AwContentsClientFullScreenTest.testPowerSaveBlockerIsTransferredToFullscreen
--org.chromium.android_webview.test.AwContentsClientFullScreenTest.testPowerSaveBlockerIsEnabledDuringFullscreenPlayback_videoInsideDiv
+-org.chromium.android_webview.test.CookieManagerStartupTest.testStartup
\ No newline at end of file