Use page transition types to detect API initiated navs

This is more reliable than stashing the last navigated URL for
char-by-char comparison, and specifically deals with the case of a
second javascript: navigation racing in before the prior real navigation
has been processed.

BUG=b/9681730

Review URL: https://chromiumcodereview.appspot.com/18147012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@210238 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 49 insertions, 1 deletions

diff --git a/android_webview/java/src/org/chromium/android_webview/AwContents.java b/android_webview/java/src/org/chromium/android_webview/AwContents.java
index 5f6894c..d56c24f 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContents.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContents.java
@@ -245,8 +245,24 @@ public class AwContents {
         @Override
         public boolean shouldIgnoreNavigation(NavigationParams navigationParams) {
             final String url = navigationParams.url;
+            final int transitionType = navigationParams.pageTransitionType;
+            final boolean isLoadUrl =
+                    (transitionType & PageTransitionTypes.PAGE_TRANSITION_FROM_API) != 0;
+            final boolean isBackForward =
+                    (transitionType & PageTransitionTypes.PAGE_TRANSITION_FORWARD_BACK) != 0;
+            final boolean isReload =
+                    (transitionType & PageTransitionTypes.PAGE_TRANSITION_CORE_MASK) ==
+                    PageTransitionTypes.PAGE_TRANSITION_RELOAD;
+            final boolean isRedirect = navigationParams.isRedirect;
+
             boolean ignoreNavigation = false;
-            if (mLastLoadUrlAddress != null && mLastLoadUrlAddress.equals(url)) {
+
+            // Any navigation from loadUrl, goBack/Forward, or reload, are considered application
+            // initiated and hence will not yield a shouldOverrideUrlLoading() callback.
+            // TODO(joth): Using PageTransitionTypes should be sufficient to determine all app
+            // initiated navigations, and so mLastLoadUrlAddress should be removed.
+            if ((isLoadUrl && !isRedirect) || isBackForward || isReload ||
+                    mLastLoadUrlAddress != null && mLastLoadUrlAddress.equals(url)) {
                 // Support the case where the user clicks on a link that takes them back to the
                 // same page.
                 mLastLoadUrlAddress = null;
@@ -681,6 +697,8 @@ public class AwContents {
             params.getTransitionType() == PageTransitionTypes.PAGE_TRANSITION_LINK) {
             params.setTransitionType(PageTransitionTypes.PAGE_TRANSITION_RELOAD);
         }
+        params.setTransitionType(
+                params.getTransitionType() | PageTransitionTypes.PAGE_TRANSITION_FROM_API);
 
         // For WebView, always use the user agent override, which is set
         // every time the user agent in AwSettings is modified.
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 4a1d2fe..09d9cbe 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
@@ -16,6 +16,7 @@ import org.chromium.android_webview.test.util.CommonResources;
 import org.chromium.android_webview.test.util.JSUtils;
 import org.chromium.base.test.util.Feature;
 import org.chromium.content.browser.NavigationHistory;
+import org.chromium.content.browser.LoadUrlParams;
 import org.chromium.content.browser.test.util.CallbackHelper;
 import org.chromium.content.browser.test.util.Criteria;
 import org.chromium.content.browser.test.util.CriteriaHelper;
@@ -28,6 +29,7 @@ import java.net.URLEncoder;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.Callable;
+import java.util.concurrent.TimeUnit;
 
 /**
  * Tests for the WebViewClient.shouldOverrideUrlLoading() method.
@@ -783,4 +785,32 @@ public class AwContentsClientShouldOverrideUrlLoadingTest extends AwTestBase {
                 getHtmlForPageWithJsRedirectTo(redirectTargetUrl, "Replace", 100));
         doTestCalledOnRedirect(mWebServer, redirectUrl, redirectTargetUrl);
     }
+
+    @SmallTest
+    @Feature({"AndroidWebView", "Navigation"})
+    public void testDoubleNavigateDoesNotSuppressInitialNavigate() throws Throwable {
+        final String jsUrl = "javascript:try{console.log('processed js loadUrl');}catch(e){};";
+        final TestAwContentsClient contentsClient = new TestAwContentsClient();
+        final AwTestContainerView testContainerView =
+            createAwTestContainerViewOnMainSync(contentsClient);
+        final AwContents awContents = testContainerView.getAwContents();
+        TestAwContentsClient.ShouldOverrideUrlLoadingHelper shouldOverrideUrlLoadingHelper =
+            contentsClient.getShouldOverrideUrlLoadingHelper();
+
+        // Do a double navigagtion, the second being an effective no-op, in quick succession (i.e.
+        // without yielding the main thread inbetween).
+        int currentCallCount = contentsClient.getOnPageFinishedHelper().getCallCount();
+        getInstrumentation().runOnMainSync(new Runnable() {
+            @Override
+            public void run() {
+                awContents.loadUrl(LoadUrlParams.createLoadDataParams(
+                        getHtmlForPageWithSimpleLinkTo(DATA_URL), "text/html", false));
+                awContents.loadUrl(new LoadUrlParams(jsUrl));
+            }
+        });
+        contentsClient.getOnPageFinishedHelper().waitForCallback(currentCallCount, 1,
+                WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+
+        assertEquals(0, shouldOverrideUrlLoadingHelper.getCallCount());
+    }
 }