Simplify AreURLsInPageNavigation

content/browser/frame_host/navigation_controller_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/navigation_controller_impl.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
@@ skipping 86 matching lines @@
   for (size_t i = 0; i < entries->size(); ++i) {
     // Use a transition type of reload so that we don't incorrectly increase
     // the typed count.
     (*entries)[i]->SetTransitionType(PAGE_TRANSITION_RELOAD);
     (*entries)[i]->set_restore_type(ControllerRestoreTypeToEntryType(type));
     // NOTE(darin): This code is only needed for backwards compat.
     SetPageStateIfEmpty((*entries)[i].get());
   }
 }
 
-// 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.
+// However, due to reloads, even identical urls are *not* guaranteed to be
+// in-page navigations, we have to trust the renderer almost entirely.
+// The one thing we do know is that cross-origin navigations will *never* be
+// 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.is_empty() ||

[Nate Chapin, 2014/06/06 22:45:23]

Declaring a navigation from an empty url same-origin handles the case of
history.pushState in a frame's initial empty document. blink allows this, though
it's not clear to me whether that's correct.

[nasko, 2014/06/09 18:49:43]

Isn't the initial empty document with "about:blank" URL (vs empty string)?

[Nate Chapin, 2014/06/09 18:53:49]

It is (most of the time, there's a special case in blink's Document
constructor), but I don't think blink reports the initial empty document's
commit to the the browser process.

[nasko, 2014/06/09 18:59:24]

Ok, though I think we would have to start reporting it when session history
moves to the browser.

[Nate Chapin, 2014/06/09 19:02:54]

Huh, interesting. I would have thought that the initial empty document wouldn't
necessarily need to be declared to the browser process, though if it was
scripted or navigated to about:blank, then it would.

[nasko, 2014/06/09 20:41:28]

When I discussed this with Charlie, we found that having all cases reported to
the browser process might make the logic more streamlined. In any case, I
mentioned it as a FYI, no action item here for this CL.

+                        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
 // between two NavigationEntries.
 bool ShouldKeepOverride(const NavigationEntry* last_entry) {
   return last_entry && last_entry->GetIsOverridingUserAgent();
 }
 
 }  // namespace
 
@@ skipping 624 matching lines @@
   // effect of removing forward browsing history, if such existed.
   // Or if we are doing a cross-site redirect navigation,
   // we will do a similar thing.
   details->did_replace_entry =
       pending_entry_ && pending_entry_->should_replace_entry();
 
   // Do navigation-type specific actions. These will make and commit an entry.
   details->type = ClassifyNavigation(rfh, params);
 
   // 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);
+  details->is_in_page = AreURLsInPageNavigation(rfh->GetLastCommittedURL(),

[Nate Chapin, 2014/06/06 22:45:23]

Unlike other uses of the in-page logic, this callsite does not guarantee that it
was the main frame that was navigated. Use the RenderFrameHost's state to
determine the existing url to ensure we correctly handle subframe navigations.

[nasko, 2014/06/09 18:49:43]

GetLastCommittedURL on RFH worries me a bit. It comes from the frame tree node,
which we save for estimating process allocation and overhead of site isolation.
It looks we always set it on commit, but it was never used for such decisions
before.

[Nate Chapin, 2014/06/09 18:53:49]

Fair enough. Is there an alternate way to get the committing frame's previous
url without adding it to the DidCommitProvisionaLoad IPC?

[nasko, 2014/06/09 18:59:24]

It is already there:
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/fra...

[Nate Chapin, 2014/06/09 19:02:54]

I see the URL being navigated to, the base url, and the start of the redirect
chain, but not the url that we're leaving. Am I missing something?

[nasko, 2014/06/09 20:41:28]

Duh, I misread what you meant. You are correct. Looking at it, we do set it
always on commit, so I think it is fine to use and we can revisit this if needed
in the future.

+      params.url, params.was_within_same_page, rfh);
 
   switch (details->type) {
     case NAVIGATION_TYPE_NEW_PAGE:
       RendererDidNavigateToNewPage(rfh, params, details->did_replace_entry);
       break;
     case NAVIGATION_TYPE_EXISTING_PAGE:
       RendererDidNavigateToExistingPage(rfh, params);
       break;
     case NAVIGATION_TYPE_SAME_PAGE:
       RendererDidNavigateToSamePage(rfh, params);
@@ skipping 198 matching lines @@
     // the pending entry and go back to where we were (the "existing entry").
     return NAVIGATION_TYPE_SAME_PAGE;
   }
 
   // Any toplevel navigations with the same base (minus the reference fragment)
   // are in-page navigations. We weeded out subframe navigations above. Most of
   // the time this doesn't matter since WebKit doesn't tell us about subframe
   // 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;
   }
 
   // Since we weeded out "new" navigations above, we know this is an existing
   // (back/forward) navigation.
   return NAVIGATION_TYPE_EXISTING_PAGE;
 }
 
 void NavigationControllerImpl::RendererDidNavigateToNewPage(
     RenderFrameHost* rfh,
@@ skipping 245 matching lines @@
   const NavigationEntries::const_iterator i(std::find(
       entries_.begin(),
       entries_.end(),
       entry));
   return (i == entries_.end()) ? -1 : static_cast<int>(i - entries_.begin());
 }
 
 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(
     const NavigationController& temp) {
   const NavigationControllerImpl& source =
       static_cast<const NavigationControllerImpl&>(temp);
   // Verify that we look new.
   DCHECK(GetEntryCount() == 0 && !GetPendingEntry());
 
   if (source.GetEntryCount() == 0)
@@ skipping 500 matching lines @@
     }
   }
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content