Refactor VR instrumentation test waits

Refactors waitOnJavaScriptStep to be more strict. The behavior should
be identical to the old implementation except:
1. Attempting to use waitOnJavaScriptStep without the proper JavaScript
    code to do so results in a failure.
2. JavaScript test failures that occur while waiting for a step are
    caught and reported immediately instead of whenever Java calls
    endTest.

Bug: 
Change-Id: I2ce955f31c6c90355400f42b2f66c5ff4b042538
Reviewed-on: https://chromium-review.googlesource.com/809386Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521802}

chrome/android/BUILD.gn

@@ -645,6 +645,7 @@ if (enable_vr) {
       "//third_party/android_support_test_runner:rules_java",
       "//third_party/android_support_test_runner:runner_java",
       "//third_party/android_tools:android_arch_lifecycle_common_java",
+      "//third_party/android_tools:android_support_annotations_java",
       "//third_party/android_tools:android_support_v7_appcompat_java",
       "//third_party/android_tools:android_support_v7_recyclerview_java",
       "//third_party/custom_tabs_client:custom_tabs_support_java",
@@ -744,7 +745,9 @@ template("chrome_shared_library") {
     if (enable_vr) {
       # Ensure libgvr static library appears before gcc library in linking order.
       # See https://crbug.com/704305 for details.
-      libs = [ "//third_party/gvr-android-sdk/libgvr_shim_static_${current_cpu}.a" ]
+      libs = [
+        "//third_party/gvr-android-sdk/libgvr_shim_static_${current_cpu}.a",
+      ]
     }
 
     if (use_order_profiling) {
@@ -960,7 +963,9 @@ if (!android_64bit_target_cpu ||
     if (enable_vr) {
       # Ensure libgvr static library appears before gcc library in linking order.
       # See https://crbug.com/704305 for details.
-      libs = [ "//third_party/gvr-android-sdk/libgvr_shim_static_${current_cpu}.a" ]
+      libs = [
+        "//third_party/gvr-android-sdk/libgvr_shim_static_${current_cpu}.a",
+      ]
     }
 
     configs -= [ "//build/config/android:hide_all_but_jni_onload" ]

chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrTestFramework.java

@@ -4,6 +4,8 @@
 
 package org.chromium.chrome.browser.vr_shell;
 
+import android.support.annotation.IntDef;
+
 import org.junit.Assert;
 
 import org.chromium.base.Log;
@@ -15,6 +17,8 @@ import org.chromium.content.browser.test.util.CriteriaHelper;
 import org.chromium.content.browser.test.util.JavaScriptUtils;
 import org.chromium.content_public.browser.WebContents;
 
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -49,6 +53,14 @@ public class VrTestFramework {
     private static final String TAG = "VrTestFramework";
     static final String TEST_DIR = "chrome/test/data/android/webvr_instrumentation";
 
+    // Test status enum
+    private static final int STATUS_RUNNING = 0;
+    private static final int STATUS_PASSED = 1;
+    private static final int STATUS_FAILED = 2;
+    @Retention(RetentionPolicy.SOURCE)
+    @IntDef({STATUS_RUNNING, STATUS_PASSED, STATUS_FAILED})
+    private @interface TestStatus {}
+
     private ChromeActivityTestRule mRule;
     private WebContents mFirstTabWebContents;
     private ContentViewCore mFirstTabCvc;
@@ -131,15 +143,24 @@ public class VrTestFramework {
     }
 
     /**
-     * Ends the test harness test and checks whether there it passed.
-     * @param webContents The WebContents for the tab to check results in.
-     * @return "Passed" if test passed, String with failure reason otherwise.
+     * Retrieves the current status of the JavaScript test and returns an enum corresponding to it.
+     * @param webContents The WebContents for the tab to check the status in.
+     * @return A TestStatus integer corresponding to the current state of the JavaScript test
      */
-    public static String checkResults(WebContents webContents) {
-        if (runJavaScriptOrFail("testPassed", POLL_TIMEOUT_SHORT_MS, webContents).equals("true")) {
-            return "Passed";
+    @TestStatus
+    public static int checkTestStatus(WebContents webContents) {
+        String resultString =
+                runJavaScriptOrFail("resultString", POLL_TIMEOUT_SHORT_MS, webContents);
+        boolean testPassed = Boolean.parseBoolean(
+                runJavaScriptOrFail("testPassed", POLL_TIMEOUT_SHORT_MS, webContents));
+        if (testPassed) {
+            return STATUS_PASSED;
+        } else if (!testPassed && resultString.equals("\"\"")) {
+            return STATUS_RUNNING;
+        } else {
+            // !testPassed && !resultString.equals("\"\"")
+            return STATUS_FAILED;
         }
-        return runJavaScriptOrFail("resultString", POLL_TIMEOUT_SHORT_MS, webContents);
     }
 
     /**
@@ -148,7 +169,20 @@ public class VrTestFramework {
      * @param webContents The WebContents for the tab to check test results in.
      */
     public static void endTest(WebContents webContents) {
-        Assert.assertEquals("Passed", checkResults(webContents));
+        switch (checkTestStatus(webContents)) {
+            case STATUS_PASSED:
+                break;
+            case STATUS_FAILED:
+                String resultString =
+                        runJavaScriptOrFail("resultString", POLL_TIMEOUT_SHORT_MS, webContents);
+                Assert.fail("JavaScript testharness failed with result: " + resultString);
+                break;
+            case STATUS_RUNNING:
+                Assert.fail("Attempted to end test in Java without finishing in JavaScript.");
+                break;
+            default:
+                Assert.fail("Received unknown test status.");
+        }
     }
 
     /**
@@ -188,17 +222,39 @@ public class VrTestFramework {
      * @param webContents The WebContents for the tab the JavaScript step is in.
      */
     public static void waitOnJavaScriptStep(WebContents webContents) {
-        if (!pollJavaScriptBoolean("javascriptDone", POLL_TIMEOUT_LONG_MS, webContents)) {
-            String reason = "Polling JavaScript boolean javascriptDone timed out.";
+        // Make sure we aren't trying to wait on a JavaScript test step without the code to do so.
+        Assert.assertTrue("Attempted to wait on a JavaScript step without the code to do so. You "
+                        + "either forgot to import webvr_e2e.js or are incorrectly using a Java "
+                        + "method.",
+                Boolean.parseBoolean(runJavaScriptOrFail("typeof javascriptDone !== 'undefined'",
+                        POLL_TIMEOUT_SHORT_MS, webContents)));
+
+        // Actually wait for the step to finish
+        boolean success =
+                pollJavaScriptBoolean("javascriptDone", POLL_TIMEOUT_LONG_MS, webContents);
+
+        // Check what state we're in to make sure javascriptDone wasn't called because the test
+        // failed.
+        int testStatus = checkTestStatus(webContents);
+        if (!success || testStatus == STATUS_FAILED) {
+            // Failure states: Either polling failed or polling succeeded, but because the test
+            // failed.
+            String reason;
+            if (!success) {
+                reason = "Polling JavaScript boolean javascriptDone timed out.";
+            } else {
+                reason = "Polling JavaScript boolean javascriptDone succeeded, but test failed.";
+            }
             String resultString =
                     runJavaScriptOrFail("resultString", POLL_TIMEOUT_SHORT_MS, webContents);
-            if (resultString.equals("")) {
+            if (resultString.equals("\"\"")) {
                 reason += " Did not obtain specific reason from testharness.";
             } else {
                 reason += " Testharness reported failure: " + resultString;
             }
             Assert.fail(reason);
         }
+
         // Reset the synchronization boolean
         runJavaScriptOrFail("javascriptDone = false", POLL_TIMEOUT_SHORT_MS, webContents);
     }