history.replaceState(..., location.href) clearing page-action icons.

content/browser/web_contents/navigation_controller_impl.cc

 // Copyright (c) 2012 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/web_contents/navigation_controller_impl.h"
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_number_conversions.h"  // Temporary
 #include "base/string_util.h"
@@ skipping 85 matching lines @@
     (*entries)[i]->SetTransitionType(content::PAGE_TRANSITION_RELOAD);
     (*entries)[i]->set_restore_type(from_last_session ?
         NavigationEntryImpl::RESTORE_LAST_SESSION :
         NavigationEntryImpl::RESTORE_CURRENT_SESSION);
     // NOTE(darin): This code is only needed for backwards compat.
     SetContentStateIfEmpty((*entries)[i].get());
   }
 }
 
 // See NavigationController::IsURLInPageNavigation for how this works and why.
-bool AreURLsInPageNavigation(const GURL& existing_url, const GURL& new_url) {
-  if (existing_url == new_url || !new_url.has_ref()) {
+bool AreURLsInPageNavigation(const GURL& existing_url,
+                             const GURL& new_url,
+                             bool renderer_says_in_page) {
+  if (existing_url == new_url)
+    return renderer_says_in_page;
+
+  if (!new_url.has_ref()) {
     // TODO(jcampan): what about when navigating back from a ref URL to the top
     // non ref URL? Nothing is loaded in that case but we return false here.
     // The user could also navigate from the ref URL to the non ref URL by
     // entering the non ref URL in the location bar or through a bookmark, in
     // which case there would be a load.  I am not sure if the non-load/load
     // scenarios can be differentiated with the TransitionType.

[Charlie Reis, 2012/08/28 20:06:16]

Is this going to be a problem for the original bug?  For example, if a page
navigates to #foo and then does history.back(), would you treat it as a new
navigation and lose the page-action icon?

[Tom Sepez, 2012/08/28 20:17:41]

Yep.  And my original patch tried to address this, but disrupted too many tests.

[Charlie Reis, 2012/08/28 21:48:21]

What's the plan for fixing that case?  Do we need a more specific TODO to get
back to it in a followup CL?

[Tom Sepez, 2012/08/28 22:46:58]

I'd assumed it was covered by the existing TODO().  This doesn't introduce new
TODO items.

     return false;
   }
 
   url_canon::Replacements<char> replacements;
   replacements.ClearRef();
   return existing_url.ReplaceComponents(replacements) ==
       new_url.ReplaceComponents(replacements);
 }
 
 // Determines whether or not we should be carrying over a user agent override
@@ skipping 553 matching lines @@
   DCHECK(pending_entry_index_ == -1 || pending_entry_->site_instance());
 
   // If we are doing a cross-site reload, we need to replace the existing
   // navigation entry, not add another entry to the history. This has the side
   // effect of removing forward browsing history, if such existed.
   if (pending_entry_ != NULL) {
     details->did_replace_entry = pending_entry_->is_cross_site_reload();
   }
 
   // is_in_page must be computed before the entry gets committed.
-  details->is_in_page = IsURLInPageNavigation(params.url);
+  details->is_in_page = IsURLInPageNavigation(
+      params.url, params.was_within_same_page);
 
   // Do navigation-type specific actions. These will make and commit an entry.
   details->type = ClassifyNavigation(params);
 
   switch (details->type) {
     case content::NAVIGATION_TYPE_NEW_PAGE:
       RendererDidNavigateToNewPage(params, &(details->did_replace_entry));
       break;
     case content::NAVIGATION_TYPE_EXISTING_PAGE:
       RendererDidNavigateToExistingPage(params);
@@ skipping 166 matching lines @@
     // this). If this matches the last committed entry, we want to just ignore
     // the pending entry and go back to where we were (the "existing entry").
     return content::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))
+  if (AreURLsInPageNavigation(existing_entry->GetURL(), params.url, false))
     return content::NAVIGATION_TYPE_IN_PAGE;
 
   // Since we weeded out "new" navigations above, we know this is an existing
   // (back/forward) navigation.
   return content::NAVIGATION_TYPE_EXISTING_PAGE;
 }
 
 bool NavigationControllerImpl::IsRedirect(
   const ViewHostMsg_FrameNavigate_Params& params) {
   // For main frame transition, we judge by params.transition.
@@ skipping 187 matching lines @@
 
 int NavigationControllerImpl::GetIndexOfEntry(
     const NavigationEntryImpl* entry) const {
   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) const {
+bool NavigationControllerImpl::IsURLInPageNavigation(
+    const GURL& url, bool renderer_says_in_page) const {
   NavigationEntry* last_committed = GetLastCommittedEntry();
-  if (!last_committed)
-    return false;
-  return AreURLsInPageNavigation(last_committed->GetURL(), url);
+  return last_committed && AreURLsInPageNavigation(
+      last_committed->GetURL(), url, renderer_says_in_page);
 }
 
 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 450 matching lines @@
   for (int i = 0; i < max_index; i++) {
     // When cloning a tab, copy all entries except interstitial pages
     if (source.entries_[i].get()->GetPageType() !=
         content::PAGE_TYPE_INTERSTITIAL) {
       entries_.insert(entries_.begin() + insert_index++,
                       linked_ptr<NavigationEntryImpl>(
                           new NavigationEntryImpl(*source.entries_[i])));
     }
   }
 }