Call WebViewClient#onPageFinished when a main frame fails to load

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

Index: android_webview/javatests/src/org/chromium/android_webview/test/ClientOnPageFinishedTest.java
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/ClientOnPageFinishedTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/ClientOnPageFinishedTest.java
index aaaa6cc5860a6e385f7a1200c59f7d26345145a0..3679daf1a4c1d5b6166c8cf1dcd13fd12f5a09e4 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/ClientOnPageFinishedTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/ClientOnPageFinishedTest.java
@@ -13,8 +13,6 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.content.browser.test.util.TestCallbackHelperContainer;
 import org.chromium.net.test.util.TestWebServer;
 
-import java.util.concurrent.TimeUnit;
-
 /**
  * Tests for the ContentViewClient.onPageFinished() method.
  */
@@ -26,7 +24,11 @@ public class ClientOnPageFinishedTest extends AwTestBase {
     @Override
     public void setUp() throws Exception {
         super.setUp();
-        mContentsClient = new TestAwContentsClient();
+        setTestAwContentsClient(new TestAwContentsClient());
+    }
+
+    private void setTestAwContentsClient(TestAwContentsClient contentsClient) throws Exception {
+        mContentsClient = contentsClient;
         final AwTestContainerView testContainerView =
                 createAwTestContainerViewOnMainSync(mContentsClient);
         mAwContents = testContainerView.getAwContents();
@@ -49,6 +51,25 @@ public class ClientOnPageFinishedTest extends AwTestBase {
     @MediumTest
     @Feature({"AndroidWebView"})
     public void testOnPageFinishedCalledAfterError() throws Throwable {
+        setTestAwContentsClient(new TestAwContentsClient() {
+            private boolean isOnReceivedErrorCalled = false;
+            private boolean isOnPageFinishedCalled = false;
+
+            @Override
+            public void onReceivedError(int errorCode, String description, String failingUrl) {
+                isOnReceivedErrorCalled = true;
+                // Make sure onReceivedError is called before onPageFinished
+                assertEquals(false, isOnPageFinishedCalled);
+                super.onReceivedError(errorCode, description, failingUrl);
+            }
+
+            @Override
+            public void onPageFinished(String url) {
+                isOnPageFinishedCalled = true;
+                super.onPageFinished(url);

[mkosiba (inactive), 2014/02/13 17:21:50]

maybe assert onReceivedError was called?

+            }
+        });
+
         TestCallbackHelperContainer.OnReceivedErrorHelper onReceivedErrorHelper =
                 mContentsClient.getOnReceivedErrorHelper();
         TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
@@ -56,20 +77,53 @@ public class ClientOnPageFinishedTest extends AwTestBase {
 
         assertEquals(0, onReceivedErrorHelper.getCallCount());

[mkosiba (inactive), 2014/02/13 17:21:50]

I don't think this is needed.

 
-        String url = "http://localhost:7/non_existent";
+        String invalidUrl = "http://localhost:7/non_existent";
         int onReceivedErrorCallCount = onReceivedErrorHelper.getCallCount();
         int onPageFinishedCallCount = onPageFinishedHelper.getCallCount();
-        loadUrlAsync(mAwContents, url);
-
-        onReceivedErrorHelper.waitForCallback(onReceivedErrorCallCount,
-                                              1 /* numberOfCallsToWaitFor */,
-                                              WAIT_TIMEOUT_MS,
-                                              TimeUnit.MILLISECONDS);
-        onPageFinishedHelper.waitForCallback(onPageFinishedCallCount,
-                                             1 /* numberOfCallsToWaitFor */,
-                                             WAIT_TIMEOUT_MS,
-                                             TimeUnit.MILLISECONDS);
-        assertEquals(1, onReceivedErrorHelper.getCallCount());
+        loadUrlSync(mAwContents, onPageFinishedHelper, invalidUrl);
+
+        assertEquals(invalidUrl, onReceivedErrorHelper.getFailingUrl());
+        assertEquals(invalidUrl, onPageFinishedHelper.getUrl());
+    }
+
+    @MediumTest
+    @Feature({"AndroidWebView"})
+    public void testOnPageFinishedCalledAfterRedirectedUrlIsOverridden() throws Throwable {
+        /*
+         * If url1 is redirected url2, and url2 load is overridden, onPageFinished should still be
+         * called for url2.
+         * Steps:
+         * 1. load url1. url1 onPageStarted
+         * 2. server redirects url1 to url2. url2 onPageStarted
+         * 3. shouldOverridedUrlLoading called for url2 and returns true
+         * 4. url2 onPageFinishedCalled
+         */
+
+        TestWebServer webServer = null;
+        try {
+            webServer = new TestWebServer(false);
+            final String redirectTargetPath = "/redirect_target.html";
+            final String redirectTargetUrl = webServer.setResponse(redirectTargetPath,
+                    "<html><body>hello world</body></html>", null);
+            final String redirectUrl = webServer.setRedirect("/302.html", redirectTargetUrl);
+
+            final TestAwContentsClient.ShouldOverrideUrlLoadingHelper urlOverrideHelper =
+                    mContentsClient.getShouldOverrideUrlLoadingHelper();
+            // Override the load of redirectTargetUrl
+            urlOverrideHelper.setShouldOverrideUrlLoadingReturnValue(true);
+
+            TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
+                mContentsClient.getOnPageFinishedHelper();
+
+            final int currentOnPageFinishedCallCount = onPageFinishedHelper.getCallCount();
+            loadUrlAsync(mAwContents, redirectUrl);
+
+            onPageFinishedHelper.waitForCallback(currentOnPageFinishedCallCount);
+            // onPageFinished needs to be called for redirectTargetUrl, but not for redirectUrl
+            assertEquals(redirectTargetUrl, onPageFinishedHelper.getUrl());
+        } finally {
+            if (webServer != null) webServer.shutdown();
+        }
     }
 
     @MediumTest