Simplify AreURLsInPageNavigation

content/browser/frame_host/navigation_controller_impl.cc

Index: content/browser/frame_host/navigation_controller_impl.cc
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 6d1c597d7604629df1b636ed8947c099fbc40c5c..b88bb0a1770f07edd7b13f5a32427c81320b8eba 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -104,24 +104,27 @@ void ConfigureEntriesForRestore(
   }
 }
 
-// See NavigationController::IsURLInPageNavigation for how this works and why.
+// There are two general cases where a navigation is in page:
+// 1. A fragment navigation, in which the url is kept the same except for the
+//    reference fragment.
+// 2. A history API navigation (pushState and replaceState). This case is
+//    always in-page, but the urls are not guaranteed to match excluding the
+//    fragment. The relevant spec allows pushState/replaceState to any URL on
+//    the same origin.
+// Howeer, due to reloads, even identical urls are *not* guaranteed to be

[nasko, 2014/06/06 21:09:45]

nit: "However"

+// in-page navigations, we have to trust the renderer almost entirely.
+// The one thing we do know that cross-origin navigations will *never* be

[nasko, 2014/06/06 21:09:45]

nit: "is that"

+// in-page. Therefore, trust the renderer if the URLs are on the same origin,
+// and assume the renderer is malicious if a cross-origin navigation claims to
+// be in-page.
 bool AreURLsInPageNavigation(const GURL& existing_url,
                              const GURL& new_url,
                              bool renderer_says_in_page,
-                             NavigationType navigation_type) {
-  if (existing_url.GetOrigin() == new_url.GetOrigin())
-    return renderer_says_in_page;
-
-  if (!new_url.has_ref()) {
-    // When going back from the ref URL to the non ref one the navigation type
-    // is IN_PAGE.
-    return navigation_type == NAVIGATION_TYPE_IN_PAGE;
-  }
-
-  url::Replacements<char> replacements;
-  replacements.ClearRef();
-  return existing_url.ReplaceComponents(replacements) ==
-      new_url.ReplaceComponents(replacements);
+                             RenderFrameHost* rfh) {
+  bool is_same_origin = existing_url.GetOrigin() == new_url.GetOrigin();
+  if (!is_same_origin && renderer_says_in_page)
+      rfh->GetProcess()->ReceivedBadMessage();
+  return is_same_origin && renderer_says_in_page;
 }
 
 // Determines whether or not we should be carrying over a user agent override
@@ -767,7 +770,7 @@ bool NavigationControllerImpl::RendererDidNavigate(
 
   // is_in_page must be computed before the entry gets committed.
   details->is_in_page = IsURLInPageNavigation(
-      params.url, params.was_within_same_page, details->type);
+      params.url, params.was_within_same_page, rfh);
 
   switch (details->type) {
     case NAVIGATION_TYPE_NEW_PAGE:
@@ -986,8 +989,7 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
   // navigations that don't actually navigate, but it can happen when there is
   // an encoding override (it always sends a navigation request).
   if (AreURLsInPageNavigation(existing_entry->GetURL(), params.url,
-                              params.was_within_same_page,
-                              NAVIGATION_TYPE_UNKNOWN)) {
+                              params.was_within_same_page, rfh)) {
     return NAVIGATION_TYPE_IN_PAGE;
   }
 
@@ -1253,10 +1255,10 @@ int NavigationControllerImpl::GetIndexOfEntry(
 bool NavigationControllerImpl::IsURLInPageNavigation(
     const GURL& url,
     bool renderer_says_in_page,
-    NavigationType navigation_type) const {
+    RenderFrameHost* rfh) const {
   NavigationEntry* last_committed = GetLastCommittedEntry();
   return last_committed && AreURLsInPageNavigation(
-      last_committed->GetURL(), url, renderer_says_in_page, navigation_type);
+      last_committed->GetURL(), url, renderer_says_in_page, rfh);
 }
 
 void NavigationControllerImpl::CopyStateFrom(