Trust the renderer's same-document navigation flag if it is a same-origin nav.

Currently in AreURLsInPageNavigation, we only trust renderer_says_in_page if
the before and after urls are identical. This prevents us from correctly
classifying history.pushState and history.replaceState navigations as in-page.
Navigations via the history API are required to be same-origin, but can differ
by more than just the ref component, so we get the correct behavior without
the renderer process being able to lie about a cross-origin navigation.

BUG=138324
TEST=Added cases to NavigationControllerTest.IsInPageNavigation

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

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

12 files changed, 72 insertions, 34 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 e5ce027..61486bb 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
@@ -89,10 +89,10 @@ public abstract class AwContentsClient {
 
         @Override
         public void didNavigateMainFrame(String url, String baseUrl,
-                boolean isNavigationToDifferentPage, boolean isNavigationInPage) {
+                boolean isNavigationToDifferentPage, boolean isFragmentNavigation) {
             // This is here to emulate the Classic WebView firing onPageFinished for main frame
             // navigations where only the hash fragment changes.
-            if (isNavigationInPage) {
+            if (isFragmentNavigation) {
                 AwContentsClient.this.onPageFinished(url);
             }
         }
diff --git a/chrome/browser/extensions/active_script_controller_unittest.cc b/chrome/browser/extensions/active_script_controller_unittest.cc
index a670a53f..0b40328 100644
--- a/chrome/browser/extensions/active_script_controller_unittest.cc
+++ b/chrome/browser/extensions/active_script_controller_unittest.cc
@@ -219,9 +219,9 @@ TEST_F(ActiveScriptControllerUnitTest, PendingInjectionsRemovedAtNavigation) {
   EXPECT_TRUE(controller()->GetActionForExtension(extension));
   EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
 
-  // Navigate away. This should remove the pending injection, and we should not
+  // Reload. This should remove the pending injection, and we should not
   // execute anything.
-  NavigateAndCommit(GURL("https://www.google.com"));
+  Reload();
   EXPECT_FALSE(controller()->GetActionForExtension(extension));
   EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
 
@@ -281,7 +281,7 @@ TEST_F(ActiveScriptControllerUnitTest, ActiveScriptsUseActiveTabPermissions) {
   EXPECT_FALSE(controller()->RequiresUserConsentForScriptInjection(extension));
 
   // Also test that granting active tab runs any pending tasks.
-  NavigateAndCommit(GURL("https://www.google.com"));
+  Reload();
   // Navigating should mean we need permission again.
   EXPECT_TRUE(controller()->RequiresUserConsentForScriptInjection(extension));
 
diff --git a/chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc b/chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc
index 4ca5415..57d8050 100644
--- a/chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc
+++ b/chrome/browser/geolocation/chrome_geolocation_permission_context_unittest.cc
@@ -291,7 +291,7 @@ TEST_F(GeolocationPermissionContextTests, GeolocationEnabledDisabled) {
   base::string16 text_0 = infobar_delegate_0->GetButtonLabel(
       ConfirmInfoBarDelegate::BUTTON_OK);
 
-  NavigateAndCommit(requesting_frame);
+  Reload();
   MockGoogleLocationSettingsHelper::SetLocationStatus(true, false);
   EXPECT_EQ(0U, infobar_service()->infobar_count());
   RequestGeolocationPermission(web_contents(), RequestID(0), requesting_frame);
@@ -303,7 +303,7 @@ TEST_F(GeolocationPermissionContextTests, GeolocationEnabledDisabled) {
       ConfirmInfoBarDelegate::BUTTON_OK);
   EXPECT_NE(text_0, text_1);
 
-  NavigateAndCommit(requesting_frame);
+  Reload();
   MockGoogleLocationSettingsHelper::SetLocationStatus(false, false);
   EXPECT_EQ(0U, infobar_service()->infobar_count());
   RequestGeolocationPermission(web_contents(), RequestID(0), requesting_frame);
diff --git a/chrome/browser/translate/translate_manager_render_view_host_unittest.cc b/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
index 8e9d444..fadcd1b 100644
--- a/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
+++ b/chrome/browser/translate/translate_manager_render_view_host_unittest.cc
@@ -94,7 +94,10 @@ class TranslateManagerRenderViewHostTest
   void SimulateNavigation(const GURL& url,
                           const std::string& lang,
                           bool page_translatable) {
-    NavigateAndCommit(url);
+    if (rvh()->GetMainFrame()->GetLastCommittedURL() == url)
+      Reload();
+    else
+      NavigateAndCommit(url);
     SimulateOnTranslateLanguageDetermined(lang, page_translatable);
   }
 
@@ -794,7 +797,8 @@ TEST_F(TranslateManagerRenderViewHostTest, ReloadFromLocationBar) {
   NavEntryCommittedObserver nav_observer(web_contents());
   web_contents()->GetController().LoadURL(
       url, content::Referrer(), content::PAGE_TRANSITION_TYPED, std::string());
-  rvh_tester()->SendNavigate(0, url);
+  rvh_tester()->SendNavigateWithTransition(
+      0, url, content::PAGE_TRANSITION_TYPED);
 
   // Test that we are really getting a same page navigation, the test would be
   // useless if it was not the case.
diff --git a/content/browser/android/web_contents_observer_android.cc b/content/browser/android/web_contents_observer_android.cc
index 72b2241..19a622a 100644
--- a/content/browser/android/web_contents_observer_android.cc
+++ b/content/browser/android/web_contents_observer_android.cc
@@ -129,12 +129,23 @@ void WebContentsObserverAndroid::DidNavigateMainFrame(
       ConvertUTF8ToJavaString(env, params.url.spec()));
   ScopedJavaLocalRef<jstring> jstring_base_url(
       ConvertUTF8ToJavaString(env, params.base_url.spec()));
+
   // 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;
+  url::Replacements<char> replacements;
+  replacements.ClearRef();
+  bool urls_same_ignoring_fragment =
+      params.url.ReplaceComponents(replacements) ==
+      details.previous_url.ReplaceComponents(replacements);
+
+  // is_fragment_navigation is indicative of the intent of this variable.
+  // However, there isn't sufficient information here to determine whether this
+  // is actually a fragment navigation, or a history API navigation to a URL
+  // that would also be valid for a fragment navigation.
+  bool is_fragment_navigation = urls_same_ignoring_fragment &&
+      (details.type == NAVIGATION_TYPE_IN_PAGE || details.is_in_page);
   Java_WebContentsObserverAndroid_didNavigateMainFrame(
       env, obj.obj(), jstring_url.obj(), jstring_base_url.obj(),
-      details.is_navigation_to_different_page(), in_page_navigation);
+      details.is_navigation_to_different_page(), is_fragment_navigation);
 }
 
 void WebContentsObserverAndroid::DidNavigateAnyFrame(
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index bcb3702..6d1c597 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -109,7 +109,7 @@ bool AreURLsInPageNavigation(const GURL& existing_url,
                              const GURL& new_url,
                              bool renderer_says_in_page,
                              NavigationType navigation_type) {
-  if (existing_url == new_url)
+  if (existing_url.GetOrigin() == new_url.GetOrigin())
     return renderer_says_in_page;
 
   if (!new_url.has_ref()) {
diff --git a/content/browser/frame_host/navigation_controller_impl.h b/content/browser/frame_host/navigation_controller_impl.h
index 7ec82f3..7a06ba66 100644
--- a/content/browser/frame_host/navigation_controller_impl.h
+++ b/content/browser/frame_host/navigation_controller_impl.h
@@ -159,15 +159,11 @@ class CONTENT_EXPORT NavigationControllerImpl
   // whether a navigation happened without loading anything, the same URL could
   // be a reload, while only a different ref would be in-page (pages can't clear
   // refs without reload, only change to "#" which we don't count as empty).
-  bool IsURLInPageNavigation(const GURL& url) const {
-    return IsURLInPageNavigation(url, false, NAVIGATION_TYPE_UNKNOWN);
-  }
-
+  //
   // The situation is made murkier by history.replaceState(), which could
   // provide the same URL as part of an in-page navigation, not a reload. So
-  // we need this form which lets the (untrustworthy) renderer resolve the
-  // ambiguity, but only when the URLs are equal. This should be safe since the
-  // origin isn't changing.
+  // we need to let the (untrustworthy) renderer resolve the ambiguity, but
+  // only when the URLs are on the same origin.
   bool IsURLInPageNavigation(
       const GURL& url,
       bool renderer_says_in_page,
diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc
index 9bde501..6bb3d0e 100644
--- a/content/browser/frame_host/navigation_controller_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc
@@ -2163,6 +2163,7 @@ TEST_F(NavigationControllerTest, InPage_Replace) {
   params.gesture = NavigationGestureUser;
   params.is_post = false;
   params.page_state = PageState::CreateFromURL(url2);
+  params.was_within_same_page = true;
 
   // This should NOT generate a new entry, nor prune the list.
   LoadCommittedDetails details;
@@ -2214,6 +2215,7 @@ TEST_F(NavigationControllerTest, ClientRedirectAfterInPageNavigation) {
     params.gesture = NavigationGestureUnknown;
     params.is_post = false;
     params.page_state = PageState::CreateFromURL(url);
+    params.was_within_same_page = true;
 
     // This should NOT generate a new entry, nor prune the list.
     LoadCommittedDetails details;
@@ -3077,21 +3079,28 @@ TEST_F(NavigationControllerTest, IsInPageNavigation) {
   main_test_rfh()->SendNavigate(0, url);
 
   // Reloading the page is not an in-page navigation.
-  EXPECT_FALSE(controller.IsURLInPageNavigation(url));
+  EXPECT_FALSE(controller.IsURLInPageNavigation(url, false,
+      NAVIGATION_TYPE_UNKNOWN));
   const GURL other_url("http://www.google.com/add.html");
-  EXPECT_FALSE(controller.IsURLInPageNavigation(other_url));
+  EXPECT_FALSE(controller.IsURLInPageNavigation(other_url, false,
+      NAVIGATION_TYPE_UNKNOWN));
   const GURL url_with_ref("http://www.google.com/home.html#my_ref");
-  EXPECT_TRUE(controller.IsURLInPageNavigation(url_with_ref));
+  EXPECT_TRUE(controller.IsURLInPageNavigation(url_with_ref, true,
+      NAVIGATION_TYPE_UNKNOWN));
 
   // Navigate to URL with refs.
   main_test_rfh()->SendNavigate(1, url_with_ref);
 
   // Reloading the page is not an in-page navigation.
-  EXPECT_FALSE(controller.IsURLInPageNavigation(url_with_ref));
-  EXPECT_FALSE(controller.IsURLInPageNavigation(url));
-  EXPECT_FALSE(controller.IsURLInPageNavigation(other_url));
+  EXPECT_FALSE(controller.IsURLInPageNavigation(url_with_ref, false,
+      NAVIGATION_TYPE_UNKNOWN));
+  EXPECT_FALSE(controller.IsURLInPageNavigation(url, false,
+      NAVIGATION_TYPE_UNKNOWN));
+  EXPECT_FALSE(controller.IsURLInPageNavigation(other_url, false,
+      NAVIGATION_TYPE_UNKNOWN));
   const GURL other_url_with_ref("http://www.google.com/home.html#my_other_ref");
-  EXPECT_TRUE(controller.IsURLInPageNavigation(other_url_with_ref));
+  EXPECT_TRUE(controller.IsURLInPageNavigation(other_url_with_ref, true,
+      NAVIGATION_TYPE_UNKNOWN));
 
   // Going to the same url again will be considered in-page
   // if the renderer says it is even if the navigation type isn't IN_PAGE.
@@ -3102,6 +3111,17 @@ TEST_F(NavigationControllerTest, IsInPageNavigation) {
   // type is IN_PAGE.
   EXPECT_TRUE(controller.IsURLInPageNavigation(url, true,
       NAVIGATION_TYPE_IN_PAGE));
+
+  // If the renderer says this is a same-origin in-page navigation, believe it.
+  // This is the pushState/replaceState case.
+  EXPECT_TRUE(controller.IsURLInPageNavigation(other_url, true,
+      NAVIGATION_TYPE_UNKNOWN));
+
+  // Don't believe the renderer if it claims a cross-origin navigation is
+  // in-page.
+  const GURL different_origin_url("http://www.example.com");
+  EXPECT_FALSE(controller.IsURLInPageNavigation(different_origin_url, true,
+      NAVIGATION_TYPE_UNKNOWN));
 }
 
 // Some pages can have subframes with the same base URL (minus the reference) as
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
index 054c6e8..bd39f04 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
@@ -619,7 +619,7 @@ public class ContentViewCore
         mWebContentsObserver = new WebContentsObserverAndroid(this) {
             @Override
             public void didNavigateMainFrame(String url, String baseUrl,
-                    boolean isNavigationToDifferentPage, boolean isNavigationInPage) {
+                    boolean isNavigationToDifferentPage, boolean isFragmentNavigation) {
                 if (!isNavigationToDifferentPage) return;
                 hidePopups();
                 resetScrollInProgress();
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 17081e5..66e84ad 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
@@ -55,12 +55,12 @@ public abstract class WebContentsObserverAndroid {
      * @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).
+     * @param isFragmentNavigation 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 isNavigationInPage) {
+            boolean isNavigationToDifferentPage, boolean isFragmentNavigation) {
     }
 
     /**
diff --git a/content/public/test/test_renderer_host.cc b/content/public/test/test_renderer_host.cc
index 45e8306..ca18244 100644
--- a/content/public/test/test_renderer_host.cc
+++ b/content/public/test/test_renderer_host.cc
@@ -150,7 +150,8 @@ void RenderViewHostTestHarness::Reload() {
   DCHECK(entry);
   controller().Reload(false);
   static_cast<TestRenderViewHost*>(
-      rvh())->SendNavigate(entry->GetPageID(), entry->GetURL());
+      rvh())->SendNavigateWithTransition(
+          entry->GetPageID(), entry->GetURL(), PAGE_TRANSITION_RELOAD);
 }
 
 void RenderViewHostTestHarness::FailedReload() {
diff --git a/content/test/test_render_frame_host.cc b/content/test/test_render_frame_host.cc
index 9eb1eaa..98408fb 100644
--- a/content/test/test_render_frame_host.cc
+++ b/content/test/test_render_frame_host.cc
@@ -46,7 +46,7 @@ void TestRenderFrameHost::SendNavigateWithTransition(
 
 void TestRenderFrameHost::SendFailedNavigate(int page_id, const GURL& url) {
   SendNavigateWithTransitionAndResponseCode(
-      page_id, url, PAGE_TRANSITION_LINK, 500);
+      page_id, url, PAGE_TRANSITION_RELOAD, 500);
 }
 
 void TestRenderFrameHost::SendNavigateWithTransitionAndResponseCode(
@@ -113,13 +113,19 @@ void TestRenderFrameHost::SendNavigateWithParameters(
   params.gesture = NavigationGestureUser;
   params.contents_mime_type = contents_mime_type_;
   params.is_post = false;
-  params.was_within_same_page = false;
   params.http_status_code = response_code;
   params.socket_address.set_host("2001:db8::1");
   params.socket_address.set_port(80);
   params.history_list_was_cleared = simulate_history_list_was_cleared_;
   params.original_request_url = original_request_url;
 
+  url::Replacements<char> replacements;
+  replacements.ClearRef();
+  params.was_within_same_page = transition != PAGE_TRANSITION_RELOAD &&
+      transition != PAGE_TRANSITION_TYPED &&
+      url.ReplaceComponents(replacements) ==
+          GetLastCommittedURL().ReplaceComponents(replacements);
+
   params.page_state = PageState::CreateForTesting(
       url,
       false,