[WebLayer] Attempt to de-flake instrumentation tests

ExecuteScriptTest.testBasicScript was flaking because it seems the page
was not completely loaded when the script was run. Changing the wait for
navigation code to wait for the navigationComplete and load stop events
hopefully will fix this.

Change-Id: I91ca7b68b242d36b547072bddef336933d388c57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869849Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707861}

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/BrowserObserverTest.java

@@ -90,16 +90,13 @@ public class BrowserObserverTest {
     public void testLoadEvents() {
         String startupUrl = "about:blank";
         WebLayerShellActivity activity = mActivityTestRule.launchShellWithUrl(startupUrl);
-        Assert.assertNotNull(activity);
-        mActivityTestRule.waitForNavigation(startupUrl);
 
         Observer observer = new Observer();
         TestThreadUtils.runOnUiThreadBlocking(
                 () -> { activity.getBrowserController().addObserver(observer); });
 
         String url = "data:text,foo";
-        mActivityTestRule.loadUrl(url);
-        mActivityTestRule.waitForNavigation(url);
+        mActivityTestRule.navigateAndWait(url);
 
         /* Verify that the visible URL changes to the target. */
         observer.visibleUrlChangedCallback.waitUntilValueObserved(url);

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/FragmentRestoreTest.java

@@ -25,13 +25,11 @@ public class FragmentRestoreTest {
         mActivityTestRule.launchShellWithUrl("about:blank");
 
         String url = "data:text,foo";
-        mActivityTestRule.loadUrl(url);
-        mActivityTestRule.waitForNavigation(url);
+        mActivityTestRule.navigateAndWait(url);
 
         mActivityTestRule.rotateActivity();
 
         url = "data:text,bar";
-        mActivityTestRule.loadUrl(url);
-        mActivityTestRule.waitForNavigation(url);
+        mActivityTestRule.navigateAndWait(url);
     }
 }

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/NavigationTest.java

@@ -6,7 +6,6 @@ package org.chromium.weblayer.test;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import static org.chromium.content_public.browser.test.util.TestThreadUtils.runOnUiThreadBlocking;
@@ -77,29 +76,17 @@ public class NavigationTest {
 
     private final Observer mObserver = new Observer();
 
-    @Test
-    @SmallTest
-    public void testBaseStartup() {
-        WebLayerShellActivity activity = mActivityTestRule.launchShellWithUrl(URL1);
-
-        assertNotNull(activity);
-
-        mActivityTestRule.waitForNavigation(URL1);
-    }
-
     @Test
     @SmallTest
     public void testNavigationEvents() throws Exception {
         WebLayerShellActivity activity = mActivityTestRule.launchShellWithUrl(URL1);
-        mActivityTestRule.waitForNavigation(URL1);
 
         setNavigationObserver(activity);
         int curStartedCount = mObserver.onStartedCallback.getCallCount();
         int curCommittedCount = mObserver.onCommittedCallback.getCallCount();
         int curCompletedCount = mObserver.onCompletedCallback.getCallCount();
 
-        mActivityTestRule.loadUrl(URL2);
-        mActivityTestRule.waitForNavigation(URL2);
+        mActivityTestRule.navigateAndWait(URL2);
 
         mObserver.onStartedCallback.assertCalledWith(curStartedCount, URL2);
         mObserver.onCommittedCallback.assertCalledWith(curCommittedCount, URL2);
@@ -110,13 +97,10 @@ public class NavigationTest {
     @SmallTest
     public void testGoBackAndForward() throws Exception {
         WebLayerShellActivity activity = mActivityTestRule.launchShellWithUrl(URL1);
-        mActivityTestRule.waitForNavigation(URL1);
         setNavigationObserver(activity);
 
-        mActivityTestRule.loadUrl(URL2);
-        mActivityTestRule.waitForNavigation(URL2);
-        mActivityTestRule.loadUrl(URL3);
-        mActivityTestRule.waitForNavigation(URL3);
+        mActivityTestRule.navigateAndWait(URL2);
+        mActivityTestRule.navigateAndWait(URL3);
 
         NavigationController navigationController =
                     activity.getBrowserController().getNavigationController();

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/RenderingTest.java

@@ -26,7 +26,6 @@ public class RenderingTest {
     @SmallTest
     public void testSetSupportEmbeddingFromCallback() {
         WebLayerShellActivity activity = mActivityTestRule.launchShellWithUrl("about:blank");
-        Assert.assertNotNull(activity);
 
         CountDownLatch latch = new CountDownLatch(1);
         String url = "data:text,foo";
@@ -38,7 +37,6 @@ public class RenderingTest {
                         activity.getBrowserFragmentController().setSupportsEmbedding(false)
                                 .addCallback((Boolean result2) -> {
                                     Assert.assertTrue(result2);
-                                    mActivityTestRule.loadUrl(url);
                                     latch.countDown();
                                 });
                     });
@@ -49,14 +47,13 @@ public class RenderingTest {
         } catch (InterruptedException e) {
             Assert.fail(e.toString());
         }
-        mActivityTestRule.waitForNavigation(url);
+        mActivityTestRule.navigateAndWait(url);
     }
 
     @Test
     @SmallTest
     public void testRepeatSetSupportEmbeddingGeneratesCallback() {
         WebLayerShellActivity activity = mActivityTestRule.launchShellWithUrl("about:blank");
-        Assert.assertNotNull(activity);
 
         CountDownLatch latch = new CountDownLatch(2);
         String url = "data:text,foo";

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/SmokeTest.java

@@ -26,7 +26,6 @@ public class SmokeTest {
     @SmallTest
     public void testSetSupportEmbedding() {
         WebLayerShellActivity activity = mActivityTestRule.launchShellWithUrl("about:blank");
-        Assert.assertNotNull(activity);
 
         TestThreadUtils.runOnUiThreadBlocking(
                 () -> { activity.getBrowserFragmentController().setSupportsEmbedding(true); });
@@ -38,7 +37,6 @@ public class SmokeTest {
             activity.getBrowserFragmentController().setSupportsEmbedding(true).addCallback(
                     (Boolean result) -> {
                         Assert.assertTrue(result);
-                        mActivityTestRule.loadUrl(url);
                         latch.countDown();
                     });
         });
@@ -48,6 +46,6 @@ public class SmokeTest {
         } catch (InterruptedException e) {
             Assert.fail(e.toString());
         }
-        mActivityTestRule.waitForNavigation(url);
+        mActivityTestRule.navigateAndWait(url);
     }
 }

weblayer/shell/android/javatests/src/org/chromium/weblayer/test/WebLayerShellActivityTestRule.java

@@ -14,13 +14,19 @@ import android.net.Uri;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.rule.ActivityTestRule;
 
-import org.chromium.content_public.browser.test.util.Criteria;
+import org.junit.Assert;
+
+import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
-import org.chromium.weblayer.NavigationController;
+import org.chromium.weblayer.BrowserController;
+import org.chromium.weblayer.BrowserObserver;
+import org.chromium.weblayer.Navigation;
+import org.chromium.weblayer.NavigationObserver;
 import org.chromium.weblayer.shell.WebLayerShellActivity;
 
 import java.lang.reflect.Field;
+import java.util.concurrent.TimeoutException;
 
 /**
  * ActivityTestRule for WebLayerShellActivity.
@@ -28,7 +34,59 @@ import java.lang.reflect.Field;
  * Test can use this ActivityTestRule to launch or get WebLayerShellActivity.
  */
 public class WebLayerShellActivityTestRule extends ActivityTestRule<WebLayerShellActivity> {
-    private static final long WAIT_FOR_NAVIGATION_TIMEOUT = 10000L;
+    private static final class NavigationWaiter {
+        private String mUrl;
+        private BrowserController mController;
+        private boolean mNavigationComplete;
+        private boolean mDoneLoading;
+        private CallbackHelper mCallbackHelper = new CallbackHelper();
+
+        private NavigationObserver mNavigationObserver = new NavigationObserver() {
+            @Override
+            public void navigationCompleted(Navigation navigation) {
+                if (navigation.getUri().toString().equals(mUrl)) {
+                    mNavigationComplete = true;
+                    checkComplete();
+                }
+            }
+        };
+
+        private BrowserObserver mBrowserObserver = new BrowserObserver() {
+            @Override
+            public void loadingStateChanged(boolean isLoading, boolean toDifferentDocument) {
+                mDoneLoading = !isLoading;
+                checkComplete();
+            }
+        };
+
+        public NavigationWaiter(String url, BrowserController controller) {
+            mUrl = url;
+            mController = controller;
+        }
+
+        public void navigateAndWait() {
+            TestThreadUtils.runOnUiThreadBlocking(() -> {
+                mController.addObserver(mBrowserObserver);
+                mController.getNavigationController().addObserver(mNavigationObserver);
+                mController.getNavigationController().navigate(Uri.parse(mUrl));
+            });
+            try {
+                mCallbackHelper.waitForCallback(0);
+            } catch (TimeoutException e) {
+                throw new RuntimeException(e);
+            }
+            TestThreadUtils.runOnUiThreadBlocking(() -> {
+                mController.removeObserver(mBrowserObserver);
+                mController.getNavigationController().removeObserver(mNavigationObserver);
+            });
+        }
+
+        private void checkComplete() {
+            if (mNavigationComplete && mDoneLoading) {
+                mCallbackHelper.notifyCalled();
+            }
+        }
+    }
 
     public WebLayerShellActivityTestRule() {
         super(WebLayerShellActivity.class, false, false);
@@ -41,34 +99,23 @@ public class WebLayerShellActivityTestRule extends ActivityTestRule<WebLayerShel
         Intent intent = new Intent(Intent.ACTION_MAIN);
         intent.addCategory(Intent.CATEGORY_LAUNCHER);
         intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
-        if (url != null) intent.setData(Uri.parse(url));
+        // Prevent URL from being loaded on start.
+        intent.putExtra(WebLayerShellActivity.EXTRA_NO_LOAD, true);
         intent.setComponent(
                 new ComponentName(InstrumentationRegistry.getInstrumentation().getTargetContext(),
                         WebLayerShellActivity.class));
-        return launchActivity(intent);
-    }
-
-    /**
-     * Waits for the shell to navigate to the given URI.
-     */
-    public void waitForNavigation(String uri) {
-        CriteriaHelper.pollUiThread(new Criteria() {
-            @Override
-            public boolean isSatisfied() {
-                NavigationController navigationController =
-                        getActivity().getBrowserController().getNavigationController();
-                Uri currentUri = navigationController.getNavigationEntryDisplayUri(
-                        navigationController.getNavigationListCurrentIndex());
-                return currentUri.toString().equals(uri);
-            }
-        }, WAIT_FOR_NAVIGATION_TIMEOUT, CriteriaHelper.DEFAULT_POLLING_INTERVAL);
+        WebLayerShellActivity activity = launchActivity(intent);
+        Assert.assertNotNull(activity);
+        navigateAndWait(url);
+        return activity;
     }
 
     /**
      * Loads the given URL in the shell.
      */
-    public void loadUrl(String url) {
-        TestThreadUtils.runOnUiThreadBlocking(() -> { getActivity().loadUrl(url); });
+    public void navigateAndWait(String url) {
+        NavigationWaiter waiter = new NavigationWaiter(url, getActivity().getBrowserController());
+        waiter.navigateAndWait();
     }
 
     /**

weblayer/shell/android/shell_apk/src/org/chromium/weblayer/shell/WebLayerShellActivity.java

@@ -45,6 +45,8 @@ import java.util.List;
  * Activity for managing the Demo Shell.
  */
 public class WebLayerShellActivity extends FragmentActivity {
+    public static final String EXTRA_NO_LOAD = "extra_no_load";
+
     private static final String TAG = "WebLayerShell";
     private static final String KEY_MAIN_VIEW_ID = "mainViewId";
 
@@ -176,11 +178,15 @@ public class WebLayerShellActivity extends FragmentActivity {
         mBrowserFragmentController.setTopView(mTopContentsContainer);
 
         mBrowserController = mBrowserFragmentController.getBrowserController();
-        String startupUrl = getUrlFromIntent(getIntent());
-        if (TextUtils.isEmpty(startupUrl)) {
-            startupUrl = "http://google.com";
+        boolean blockFirstLoad = getIntent().getExtras() != null
+                && getIntent().getExtras().getBoolean(EXTRA_NO_LOAD, false);
+        if (!blockFirstLoad) {
+            String startupUrl = getUrlFromIntent(getIntent());
+            if (TextUtils.isEmpty(startupUrl)) {
+                startupUrl = "http://google.com";
+            }
+            loadUrl(startupUrl);
         }
-        loadUrl(startupUrl);
         mBrowserController.addObserver(new BrowserObserver() {
             @Override
             public void visibleUrlChanged(Uri uri) {