Merge 249142 "[android_webview] Call onPageFinished for anchor n..."

> [android_webview] Call onPageFinished for anchor navigations.
> 
> This makes the android_webview call onPageFinished for navigations
> which only result in the hash fragment being changed. This bug was
> introduced when we switched from didStopLoading to didFinishLoad.
> 
> BUG=341085, 291747
> 
> Review URL: https://codereview.chromium.org/139493008

BUG=341085
TBR=mkosiba@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/branches/1750/src@249413 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 132 insertions, 25 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 599ab8e..e3876cd 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
@@ -83,6 +83,16 @@ public abstract class AwContentsClient {
         }
 
         @Override
+        public void didNavigateMainFrame(String url, String baseUrl,
+                boolean isNavigationToDifferentPage, boolean isNavigationInPage) {
+            // This is here to emulate the Classic WebView firing onPageFinished for main frame
+            // navigations where only the hash fragment changes.
+            if (isNavigationInPage) {
+                AwContentsClient.this.onPageFinished(url);
+            }
+        }
+
+        @Override
         public void didNavigateAnyFrame(String url, String baseUrl, boolean isReload) {
             AwContentsClient.this.doUpdateVisitedHistory(url, isReload);
         }
@@ -119,6 +129,9 @@ public abstract class AwContentsClient {
     //             WebView specific methods that map directly to WebViewClient / WebChromeClient
     //--------------------------------------------------------------------------------------------
 
+    /**
+     * Parameters for the {@link AwContentsClient#showFileChooser} method.
+     */
     public static class FileChooserParams {
         public int mode;
         public String acceptTypes;
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 6bd5df1..7d467b8 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
@@ -336,6 +336,16 @@ public class AwContentsClientShouldOverrideUrlLoadingTest extends AwTestBase {
     @SmallTest
     @Feature({"AndroidWebView", "Navigation"})
     public void testNotCalledForAnchorNavigations() throws Throwable {
+        doTestNotCalledForAnchorNavigations(false);
+    }
+
+    @SmallTest
+    @Feature({"AndroidWebView", "Navigation"})
+    public void testNotCalledForAnchorNavigationsWithNonHierarchicalScheme() throws Throwable {
+        doTestNotCalledForAnchorNavigations(true);
+    }
+
+    private void doTestNotCalledForAnchorNavigations(boolean useLoadData) throws Throwable {
         final TestAwContentsClient contentsClient = new TestAwContentsClient();
         final AwTestContainerView testContainerView =
             createAwTestContainerViewOnMainSync(contentsClient);
@@ -348,7 +358,12 @@ public class AwContentsClientShouldOverrideUrlLoadingTest extends AwTestBase {
         addPageToTestServer(mWebServer, anchorLinkPath,
                 getHtmlForPageWithSimpleLinkTo(anchorLinkUrl + "#anchor"));
 
-        loadUrlSync(awContents, contentsClient.getOnPageFinishedHelper(), anchorLinkUrl);
+        if (useLoadData) {
+            loadDataSync(awContents, contentsClient.getOnPageFinishedHelper(),
+                    getHtmlForPageWithSimpleLinkTo("#anchor"), "text/html", false);
+        } else {
+            loadUrlSync(awContents, contentsClient.getOnPageFinishedHelper(), anchorLinkUrl);
+        }
 
         final int shouldOverrideUrlLoadingCallCount =
             shouldOverrideUrlLoadingHelper.getCallCount();
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
index dba2b7b..8a782c6 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
@@ -211,6 +211,28 @@ public class AwTestBase
         });
     }
 
+    protected void loadDataWithBaseUrlSync(final AwContents awContents,
+            CallbackHelper onPageFinishedHelper, final String data, final String mimeType,
+            final boolean isBase64Encoded, final String baseUrl,
+            final String historyUrl) throws Throwable {
+        int currentCallCount = onPageFinishedHelper.getCallCount();
+        loadDataWithBaseUrlAsync(awContents, data, mimeType, isBase64Encoded, baseUrl, historyUrl);
+        onPageFinishedHelper.waitForCallback(currentCallCount, 1, WAIT_TIMEOUT_MS,
+                TimeUnit.MILLISECONDS);
+    }
+
+    protected void loadDataWithBaseUrlAsync(final AwContents awContents,
+            final String data, final String mimeType, final boolean isBase64Encoded,
+            final String baseUrl, final String historyUrl) throws Throwable {
+        runTestOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                awContents.loadUrl(LoadUrlParams.createLoadDataParamsWithBaseUrl(
+                        data, mimeType, isBase64Encoded, baseUrl, historyUrl));
+            }
+        });
+    }
+
     /**
      * Reloads the current page synchronously.
      */
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 14a7001..aaaa6cc 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
@@ -7,6 +7,8 @@ package org.chromium.android_webview.test;
 import android.test.suitebuilder.annotation.MediumTest;
 
 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.Feature;
 import org.chromium.content.browser.test.util.TestCallbackHelperContainer;
 import org.chromium.net.test.util.TestWebServer;
@@ -153,4 +155,61 @@ public class ClientOnPageFinishedTest extends AwTestBase {
             if (webServer != null) webServer.shutdown();
         }
     }
+
+    @MediumTest
+    @Feature({"AndroidWebView"})
+    public void testOnPageFinishedCalledForHrefNavigations() throws Throwable {
+        doTestOnPageFinishedCalledForHrefNavigations(false);
+    }
+
+    @MediumTest
+    @Feature({"AndroidWebView"})
+    public void testOnPageFinishedCalledForHrefNavigationsWithBaseUrl() throws Throwable {
+        doTestOnPageFinishedCalledForHrefNavigations(true);
+    }
+
+    private void doTestOnPageFinishedCalledForHrefNavigations(boolean useBaseUrl) throws Throwable {
+        TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
+                mContentsClient.getOnPageFinishedHelper();
+        TestCallbackHelperContainer.OnPageStartedHelper onPageStartedHelper =
+                mContentsClient.getOnPageStartedHelper();
+        enableJavaScriptOnUiThread(mAwContents);
+
+        TestWebServer webServer = null;
+        try {
+            webServer = new TestWebServer(false);
+
+            final String testHtml = CommonResources.makeHtmlPageFrom("",
+                    "<a href=\"#anchor\" id=\"link\">anchor</a>");
+            final String testPath = "/test.html";
+            final String testUrl = webServer.setResponse(testPath, testHtml, null);
+
+            if (useBaseUrl) {
+                loadDataWithBaseUrlSync(mAwContents, onPageFinishedHelper,
+                        testHtml, "text/html", false, webServer.getBaseUrl(), null);
+            } else {
+                loadUrlSync(mAwContents, onPageFinishedHelper, testUrl);
+            }
+
+            int onPageFinishedCallCount = onPageFinishedHelper.getCallCount();
+            int onPageStartedCallCount = onPageStartedHelper.getCallCount();
+
+            JSUtils.clickOnLinkUsingJs(this, mAwContents,
+                    mContentsClient.getOnEvaluateJavaScriptResultHelper(), "link");
+
+            onPageFinishedHelper.waitForCallback(onPageFinishedCallCount);
+            assertEquals(onPageStartedCallCount, onPageStartedHelper.getCallCount());
+
+            onPageFinishedCallCount = onPageFinishedHelper.getCallCount();
+            onPageStartedCallCount = onPageStartedHelper.getCallCount();
+
+            executeJavaScriptAndWaitForResult(mAwContents, mContentsClient,
+                    "window.history.go(-1)");
+
+            onPageFinishedHelper.waitForCallback(onPageFinishedCallCount);
+            assertEquals(onPageStartedCallCount, onPageStartedHelper.getCallCount());
+        } finally {
+            if (webServer != null) webServer.shutdown();
+        }
+    }
 }
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java
index 1b75c12..077fa0e 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/LoadDataWithBaseUrlTest.java
@@ -13,7 +13,6 @@ import org.chromium.android_webview.test.util.CommonResources;
 import org.chromium.base.test.util.DisabledTest;
 import org.chromium.base.test.util.Feature;
 import org.chromium.content.browser.ContentViewCore;
-import org.chromium.content.browser.LoadUrlParams;
 import org.chromium.content.browser.test.util.Criteria;
 import org.chromium.content.browser.test.util.CriteriaHelper;
 import org.chromium.content.browser.test.util.HistoryUtils;
@@ -22,7 +21,6 @@ import org.chromium.net.test.util.TestWebServer;
 
 import java.io.File;
 import java.io.FileOutputStream;
-import java.util.concurrent.TimeUnit;
 
 /**
  * Tests for the {@link android.webkit.WebView#loadDataWithBaseURL(String, String, String, String,
@@ -47,24 +45,8 @@ public class LoadDataWithBaseUrlTest extends AwTestBase {
     protected void loadDataWithBaseUrlSync(
         final String data, final String mimeType, final boolean isBase64Encoded,
         final String baseUrl, final String historyUrl) throws Throwable {
-        TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
-                mContentsClient.getOnPageFinishedHelper();
-        int currentCallCount = onPageFinishedHelper.getCallCount();
-        loadDataWithBaseUrlAsync(data, mimeType, isBase64Encoded, baseUrl, historyUrl);
-        onPageFinishedHelper.waitForCallback(currentCallCount, 1, WAIT_TIMEOUT_MS,
-                TimeUnit.MILLISECONDS);
-    }
-
-    protected void loadDataWithBaseUrlAsync(
-        final String data, final String mimeType, final boolean isBase64Encoded,
-        final String baseUrl, final String historyUrl) throws Throwable {
-        runTestOnUiThread(new Runnable() {
-            @Override
-            public void run() {
-                mAwContents.loadUrl(LoadUrlParams.createLoadDataParamsWithBaseUrl(
-                        data, mimeType, isBase64Encoded, baseUrl, historyUrl));
-            }
-        });
+        loadDataWithBaseUrlSync(mAwContents, mContentsClient.getOnPageFinishedHelper(),
+                data, mimeType, isBase64Encoded, baseUrl, historyUrl);
     }
 
     private static final String SCRIPT_FILE = "/script.js";
@@ -204,8 +186,8 @@ public class LoadDataWithBaseUrlTest extends AwTestBase {
         TestCallbackHelperContainer.OnPageStartedHelper onPageStartedHelper =
                 mContentsClient.getOnPageStartedHelper();
         final int callCount = onPageStartedHelper.getCallCount();
-        loadDataWithBaseUrlAsync(CommonResources.ABOUT_HTML, "text/html", false, baseUrl,
-                "about:blank");
+        loadDataWithBaseUrlAsync(mAwContents, CommonResources.ABOUT_HTML, "text/html", false,
+                baseUrl, "about:blank");
         onPageStartedHelper.waitForCallback(callCount);
         assertEquals(baseUrl, onPageStartedHelper.getUrl());
     }
diff --git a/content/browser/android/web_contents_observer_android.cc b/content/browser/android/web_contents_observer_android.cc
index 235041b..797ea3b 100644
--- a/content/browser/android/web_contents_observer_android.cc
+++ b/content/browser/android/web_contents_observer_android.cc
@@ -130,9 +130,16 @@ void WebContentsObserverAndroid::DidNavigateMainFrame(
       ConvertUTF8ToJavaString(env, params.url.spec()));
   ScopedJavaLocalRef<jstring> jstring_base_url(
       ConvertUTF8ToJavaString(env, params.base_url.spec()));
-  Java_WebContentsObserverAndroid_didNavigateMainFrame(
+  // See http://crbug.com/251330 for why it's determined this way.
+  bool in_page_navigation =
+      details.type == NAVIGATION_TYPE_IN_PAGE || details.is_in_page;
+  // TODO(mkosiba): delete once downstream rolls.
+  Java_WebContentsObserverAndroid_didNavigateMainFrameV_JLS_JLS_Z(
       env, obj.obj(), jstring_url.obj(), jstring_base_url.obj(),
       details.is_navigation_to_different_page());
+  Java_WebContentsObserverAndroid_didNavigateMainFrameV_JLS_JLS_Z_Z(
+      env, obj.obj(), jstring_url.obj(), jstring_base_url.obj(),
+      details.is_navigation_to_different_page(), in_page_navigation);
 }
 
 void WebContentsObserverAndroid::DidNavigateAnyFrame(
diff --git a/content/public/android/java/src/org/chromium/content/browser/WebContentsObserverAndroid.java b/content/public/android/java/src/org/chromium/content/browser/WebContentsObserverAndroid.java
index 9578611..09a20ae 100644
--- a/content/public/android/java/src/org/chromium/content/browser/WebContentsObserverAndroid.java
+++ b/content/public/android/java/src/org/chromium/content/browser/WebContentsObserverAndroid.java
@@ -50,15 +50,24 @@ public abstract class WebContentsObserverAndroid {
             boolean isMainFrame, int errorCode, String description, String failingUrl) {
     }
 
+    // TODO(mkosiba): delete once downstream rolls.
+    @Deprecated
+    @CalledByNative
+    public void didNavigateMainFrame(String url, String baseUrl,
+            boolean isNavigationToDifferentPage) {
+    }
+
     /**
      * Called when the main frame of the page has committed.
      * @param url The validated url for the page.
      * @param baseUrl The validated base url for the page.
      * @param isNavigationToDifferentPage Whether the main frame navigated to a different page.
+     * @param isNavigationInPage Whether the main frame navigation did not cause changes to the
+     *                           document (for example scrolling to a named anchor or PopState).
      */
     @CalledByNative
     public void didNavigateMainFrame(String url, String baseUrl,
-            boolean isNavigationToDifferentPage) {
+            boolean isNavigationToDifferentPage, boolean isNavigationInPage) {
     }
 
     /**