Don't use didStopLoading in failed navigation when other nav in progress

For example if a JS redirect would fail while the original navigation is
still in progress we would call didStopLoading before the original
navigation calls didFinishLoading (breaking the assumption that
didFinishLoading is always called before didStopLoading).

BUG=557100, 298193

Review URL: https://codereview.chromium.org/1440933004

Cr-Commit-Position: refs/heads/master@{#362954}

4 files changed, 108 insertions, 0 deletions

diff --git a/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java b/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
index 47d0f28..ba9385f 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
@@ -334,6 +334,8 @@ public abstract class AwContentsClient {
 
     public abstract void onPageCommitVisible(String url);
 
+    public void onFailedLoadForTesting(String url) {}
+
     public final void onReceivedError(AwWebResourceRequest request, AwWebResourceError error) {
         // Only one of these callbacks actually reaches out the client code. The first callback
         // is used on API versions up to and including L, the second on subsequent releases.
diff --git a/android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java b/android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java
index 1a04af4..d9b709a 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java
@@ -81,6 +81,7 @@ public class AwWebContentsObserver extends WebContentsObserver {
             // AwContents.InterceptNavigationDelegateImpl.shouldIgnoreNavigation instead.
             client.getCallbackHelper().postOnPageFinished(failingUrl);
         }
+        client.onFailedLoadForTesting(failingUrl);
     }
 
     @Override
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java
index acc17bf..aeaa73e 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientShouldOverrideUrlLoadingTest.java
@@ -9,6 +9,7 @@ import android.util.Pair;
 
 import org.chromium.android_webview.AwContents;
 import org.chromium.android_webview.AwContentsClient;
+import org.chromium.android_webview.test.TestAwContentsClient.OnFailedLoadHelper;
 import org.chromium.android_webview.test.util.CommonResources;
 import org.chromium.android_webview.test.util.JSUtils;
 import org.chromium.android_webview.test.util.JavascriptEventObserver;
@@ -18,6 +19,7 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.content.browser.test.util.CallbackHelper;
 import org.chromium.content.browser.test.util.DOMUtils;
 import org.chromium.content.browser.test.util.TestCallbackHelperContainer.OnEvaluateJavaScriptResultHelper;
+import org.chromium.content.browser.test.util.TestCallbackHelperContainer.OnPageFinishedHelper;
 import org.chromium.content.browser.test.util.TestCallbackHelperContainer.OnPageStartedHelper;
 import org.chromium.content.browser.test.util.TestCallbackHelperContainer.OnReceivedErrorHelper;
 import org.chromium.content_public.browser.LoadUrlParams;
@@ -37,6 +39,7 @@ public class AwContentsClientShouldOverrideUrlLoadingTest extends AwTestBase {
     private static final String DATA_URL = "data:text/html,<div/>";
     private static final String REDIRECT_TARGET_PATH = "/redirect_target.html";
     private static final String TITLE = "TITLE";
+    private static final String TAG = "AwContentsClientShouldOverrideUrlLoadingTest";
 
     private TestWebServer mWebServer;
     private TestAwContentsClient mContentsClient;
@@ -1120,4 +1123,80 @@ public class AwContentsClientShouldOverrideUrlLoadingTest extends AwTestBase {
             return mValue;
         }
     }
+
+    /**
+     * This is to test a bug where a JS redirect failing in its provisional state would prevent us
+     * from posting onPageFinished for the original page load.
+     * The original page contains an iframe so that we can commit the original load but then
+     * prevent it from finishing until the JS redirect fails by having the test server defer the
+     * response to the iframe.
+     */
+    @SmallTest
+    @Feature({"AndroidWebView"})
+    public void testOnPageFinishedOnFailedJSRedirect() throws Throwable {
+        final CountDownLatch jsRedirectSignal = new CountDownLatch(1);
+
+        final String redirectTargetPath = "/redirectTargetPath.html";
+        final String redirectTargetUrl = mWebServer.setResponse(
+                redirectTargetPath, CommonResources.makeHtmlPageFrom("", ""), null);
+
+        class DelayingOverrideClient extends TestAwContentsClient {
+            @Override
+            public boolean shouldOverrideUrlLoading(AwWebResourceRequest request) {
+                if (redirectTargetUrl.equals(request.url)) {
+                    try {
+                        // Wait here to make sure the load reaches its provisional state before we
+                        // cancel it. Waiting for a callback to the WebContentsObserver to make sure
+                        // we have reached the provisional state causes a deadlock here.
+                        Thread.sleep(Math.min(WAIT_TIMEOUT_MS / 2, 2000));
+                    } catch (InterruptedException e) {
+                    }
+                    return true;
+                }
+                return false;
+            }
+        }
+        mContentsClient = new DelayingOverrideClient();
+        setupWithProvidedContentsClient(mContentsClient);
+
+        final String redirectJs = "window.location.href='" + redirectTargetUrl + "';";
+
+        final String iframePath = "/iframePath.html";
+        final String iframeUrl = mWebServer.setResponseWithRunnableAction(
+                iframePath, CommonResources.makeHtmlPageFrom("", ""), null, new Runnable() {
+                    @Override
+                    public void run() {
+                        try {
+                            mAwContents.evaluateJavaScriptForTests(redirectJs, null);
+                            jsRedirectSignal.await();
+                        } catch (InterruptedException e) {
+                        }
+                    }
+                });
+        final String iframeJs = "<iframe src='" + iframeUrl + "'></iframe>";
+
+        String startPage = makeHtmlPageFrom("", iframeJs);
+        final String startPath = "/startPath.html";
+        final String startUrl = addPageToTestServer(startPath, startPage);
+
+        enableJavaScriptOnUiThread(mAwContents);
+
+        OnPageFinishedHelper onPageFinishedHelper = mContentsClient.getOnPageFinishedHelper();
+        int onPageFinishedCallCount = onPageFinishedHelper.getCallCount();
+
+        OnFailedLoadHelper onFailedLoadHelper = mContentsClient.getOnFailedLoadHelper();
+        int onFailedLoadCallCount = onFailedLoadHelper.getCallCount();
+
+        // load start url -> iframe -> JS redirect -> fail JS redirect -> finish start URL
+        loadUrlAsync(mAwContents, startUrl);
+
+        onFailedLoadHelper.waitForCallback(onFailedLoadCallCount);
+        assertEquals(redirectTargetUrl, onFailedLoadHelper.getUrl());
+
+        // let iframe finish
+        jsRedirectSignal.countDown();
+
+        onPageFinishedHelper.waitForCallback(onPageFinishedCallCount);
+        assertEquals(startUrl, onPageFinishedHelper.getUrl());
+    }
 }
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java b/android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java
index c701907..798a214 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/TestAwContentsClient.java
@@ -29,6 +29,7 @@ public class TestAwContentsClient extends NullContentsClient {
     private boolean mAllowSslError;
     private final OnPageStartedHelper mOnPageStartedHelper;
     private final OnPageFinishedHelper mOnPageFinishedHelper;
+    private final OnFailedLoadHelper mOnFailedLoadHelper;
     private final OnPageCommitVisibleHelper mOnPageCommitVisibleHelper;
     private final OnReceivedErrorHelper mOnReceivedErrorHelper;
     private final OnReceivedError2Helper mOnReceivedError2Helper;
@@ -49,6 +50,7 @@ public class TestAwContentsClient extends NullContentsClient {
         super(ThreadUtils.getUiThreadLooper());
         mOnPageStartedHelper = new OnPageStartedHelper();
         mOnPageFinishedHelper = new OnPageFinishedHelper();
+        mOnFailedLoadHelper = new OnFailedLoadHelper();
         mOnPageCommitVisibleHelper = new OnPageCommitVisibleHelper();
         mOnReceivedErrorHelper = new OnReceivedErrorHelper();
         mOnReceivedError2Helper = new OnReceivedError2Helper();
@@ -79,6 +81,25 @@ public class TestAwContentsClient extends NullContentsClient {
         return mOnPageFinishedHelper;
     }
 
+    /**
+     * CallbackHelper for OnFailedLoad.
+     */
+    public static class OnFailedLoadHelper extends CallbackHelper {
+        private String mUrl;
+        public void notifyCalled(String url) {
+            mUrl = url;
+            notifyCalled();
+        }
+        public String getUrl() {
+            assert getCallCount() > 0;
+            return mUrl;
+        }
+    }
+
+    public OnFailedLoadHelper getOnFailedLoadHelper() {
+        return mOnFailedLoadHelper;
+    }
+
     public OnReceivedErrorHelper getOnReceivedErrorHelper() {
         return mOnReceivedErrorHelper;
     }
@@ -201,6 +222,11 @@ public class TestAwContentsClient extends NullContentsClient {
     }
 
     @Override
+    public void onFailedLoadForTesting(String url) {
+        mOnFailedLoadHelper.notifyCalled(url);
+    }
+
+    @Override
     public void onReceivedError(int errorCode, String description, String failingUrl) {
         mOnReceivedErrorHelper.notifyCalled(errorCode, description, failingUrl);
     }