AW: fix and renable onPageFinished test

No change to production logic.

This fixes a bad test and reenables it. In the test, the server and test
were deadlocked: the server was waiting for the test to finish, while
the test couldn't finish because the server wouldn't serve the second
page.

Instead, let the server complete the request shortly after we invoke
stopLoading(). This unblocks the server, but still guarantees the load
will still be provisional at the time we invoke stopLoading().

  --num_retries=0 --break-on-failure --repeat=1000 \
  -f ClientOnPageFinishedTest#testCalledAfterRedirectedUrlIsOverridden

Bug: 683384
Test: run_webview_instrumentation_test_apk \
Change-Id: I21be548fb1fd57724351fd372cc78d00062489b2
Reviewed-on: https://chromium-review.googlesource.com/1100547Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567840}

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

@@ -18,7 +18,6 @@ import org.junit.runner.RunWith;
 import org.chromium.android_webview.AwContents;
 import org.chromium.android_webview.test.util.CommonResources;
 import org.chromium.android_webview.test.util.JSUtils;
-import org.chromium.base.test.util.DisabledTest;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.FlakyTest;
 import org.chromium.content.browser.test.util.TestCallbackHelperContainer;
@@ -472,18 +471,19 @@ public class ClientOnPageFinishedTest {
      * Ensure onPageFinished is called when a provisional load is cancelled.
      */
     @Test
-    //@MediumTest
-    //@Feature({"AndroidWebView"})
-    @DisabledTest(message = "crbug.com/683384")
+    @MediumTest
+    @Feature({"AndroidWebView"})
     public void testCalledOnCancelingProvisionalLoad() throws Throwable {
         TestWebServer webServer = TestWebServer.start();
-        final CountDownLatch testDoneLatch = new CountDownLatch(1);
+        final CountDownLatch firstUrlLatch = new CountDownLatch(1);
         try {
             final String url = webServer.setResponseWithRunnableAction(
                     "/slow_page.html", "", null /* headers */, () -> {
                         try {
-                            Assert.assertTrue(testDoneLatch.await(WAIT_TIMEOUT_MS,
-                                    java.util.concurrent.TimeUnit.MILLISECONDS));
+                            // Delay the server response so that we guarantee stopLoading() comes
+                            // before the server response.
+                            Assert.assertTrue(firstUrlLatch.await(
+                                    WAIT_TIMEOUT_MS, java.util.concurrent.TimeUnit.MILLISECONDS));
                         } catch (InterruptedException e) {
                             Assert.fail("Caught InterruptedException " + e);
                         }
@@ -498,17 +498,16 @@ public class ClientOnPageFinishedTest {
 
             onPageFinishedHelper.waitForCallback(initialCallCount);
             Assert.assertEquals(url, onPageFinishedHelper.getUrl());
+            firstUrlLatch.countDown();
 
             // Load another page to ensure onPageFinished isn't called several times.
-            final String syncUrl = webServer.setResponse("/sync.html", "", null);
-            final int synchronizationPageCallCount = onPageFinishedHelper.getCallCount();
-            Assert.assertEquals(initialCallCount + 1, synchronizationPageCallCount);
-            mActivityTestRule.loadUrlAsync(mAwContents, syncUrl);
-
-            onPageFinishedHelper.waitForCallback(synchronizationPageCallCount);
+            final String syncUrl = webServer.setResponse("/sync.html", "", null /* headers */);
+            mActivityTestRule.loadUrlSync(mAwContents, onPageFinishedHelper, syncUrl);
             Assert.assertEquals(syncUrl, onPageFinishedHelper.getUrl());
+            final int finalCallCount = onPageFinishedHelper.getCallCount();
+            Assert.assertEquals(
+                    "onPageFinished should be called twice", initialCallCount + 2, finalCallCount);
         } finally {
-            testDoneLatch.countDown();
             webServer.shutdown();
         }
     }