Make forward/backward navigation work even when redirection is involved.

chrome/browser/tab_contents/navigation_controller.cc

 // Copyright (c) 2006-2008 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 "chrome/browser/tab_contents/navigation_controller.h"
 
 #include "app/resource_bundle.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_util.h"
@@ skipping 587 matching lines @@
     new_entry = new NavigationEntry;
   }
 
   new_entry->set_url(params.url);
   new_entry->set_referrer(params.referrer);
   new_entry->set_page_id(params.page_id);
   new_entry->set_transition_type(params.transition);
   new_entry->set_site_instance(tab_contents_->GetSiteInstance());
   new_entry->set_has_post_data(params.is_post);
 
-  InsertEntry(new_entry);
+  // If the current entry is a redirection source, it needs to be replaced with
+  // the new entry to avoid unwanted redirections in navigating backward /
+  // forward. Otherwise, just insert the new entry.
+  InsertOrReplaceEntry(new_entry,
+      PageTransition::IsRedirect(new_entry->transition_type()));
 }
 
 void NavigationController::RendererDidNavigateToExistingPage(
     const ViewHostMsg_FrameNavigate_Params& params) {
   // We should only get here for main frame navigations.
   DCHECK(PageTransition::IsMainFrame(params.transition));
 
   // This is a back/forward navigation. The existing page for the ID is
   // guaranteed to exist by ClassifyNavigation, and we just need to update it
   // with new information from the renderer.
@@ skipping 48 matching lines @@
   NavigationEntry* existing_entry = GetEntryWithPageID(
       tab_contents_->GetSiteInstance(),
       params.page_id);
 
   // Reference fragment navigation. We're guaranteed to have the last_committed
   // entry and it will be the same page as the new navigation (minus the
   // reference fragments, of course).
   NavigationEntry* new_entry = new NavigationEntry(*existing_entry);
   new_entry->set_page_id(params.page_id);
   new_entry->set_url(params.url);
-  InsertEntry(new_entry);
+  InsertOrReplaceEntry(new_entry, false);
 }
 
 void NavigationController::RendererDidNavigateNewSubframe(
     const ViewHostMsg_FrameNavigate_Params& params) {
   // Manual subframe navigations just get the current entry cloned so the user
   // can go back or forward to it. The actual subframe information will be
   // stored in the page state for each of those entries. This happens out of
   // band with the actual navigations.
   DCHECK(GetLastCommittedEntry()) << "ClassifyNavigation should guarantee "
                                   << "that a last committed entry exists.";
   NavigationEntry* new_entry = new NavigationEntry(*GetLastCommittedEntry());
   new_entry->set_page_id(params.page_id);
-  InsertEntry(new_entry);
+  InsertOrReplaceEntry(new_entry, false);
 }
 
 bool NavigationController::RendererDidNavigateAutoSubframe(
     const ViewHostMsg_FrameNavigate_Params& params) {
   // We're guaranteed to have a previously committed entry, and we now need to
   // handle navigation inside of a subframe in it without creating a new entry.
   DCHECK(GetLastCommittedEntry());
 
   // Handle the case where we're navigating back/forward to a previous subframe
   // navigation entry. This is case "2." in NAV_AUTO_SUBFRAME comment in the
@@ skipping 35 matching lines @@
     // This is a previous navigation (back/forward) that we're just now
     // committing. Just mark it as committed.
     details.type = NavigationType::EXISTING_PAGE;
     int new_entry_index = pending_entry_index_;
     DiscardNonCommittedEntriesInternal();
 
     // Mark that entry as committed.
     last_committed_entry_index_ = new_entry_index;
   } else {
     // This is a new navigation. It's easiest to just copy the entry and insert
-    // it new again, since InsertEntry expects to take ownership and also
-    // discard the pending entry. We also need to synthesize a page ID. We can
-    // only do this because this function will only be called by our custom
+    // it new again, since InsertOrReplaceEntry expects to take ownership and
+    // also discard the pending entry. We also need to synthesize a page ID. We
+    // can only do this because this function will only be called by our custom
     // TabContents types. For TabContents, the IDs are generated by the
     // renderer, so we can't do this.
     details.type = NavigationType::NEW_PAGE;
     pending_entry_->set_page_id(tab_contents_->GetMaxPageID() + 1);
     tab_contents_->UpdateMaxPageID(pending_entry_->page_id());
-    InsertEntry(new NavigationEntry(*pending_entry_));
+    InsertOrReplaceEntry(new NavigationEntry(*pending_entry_), false);
   }
 
   // Broadcast the notification of the navigation.
   details.entry = GetActiveEntry();
   details.is_auto = false;
   details.is_in_page = AreURLsInPageNavigation(details.previous_url,
                                                details.entry->url());
   details.is_main_frame = true;
   NotifyNavigationEntryCommitted(&details);
 }
@@ skipping 35 matching lines @@
   DiscardNonCommittedEntriesInternal();
 
   // If there was a transient entry, invalidate everything so the new active
   // entry state is shown.
   if (transient) {
     tab_contents_->NotifyNavigationStateChanged(
         TabContents::INVALIDATE_EVERYTHING);
   }
 }
 
-void NavigationController::InsertEntry(NavigationEntry* entry) {
+void NavigationController::InsertOrReplaceEntry(NavigationEntry* entry,
+                                                bool replace) {
   DCHECK(entry->transition_type() != PageTransition::AUTO_SUBFRAME);
 
   // Copy the pending entry's unique ID to the committed entry.
   // I don't know if pending_entry_index_ can be other than -1 here.
   const NavigationEntry* const pending_entry = (pending_entry_index_ == -1) ?
       pending_entry_ : entries_[pending_entry_index_].get();
   if (pending_entry)
     entry->set_unique_id(pending_entry->unique_id());
 
   DiscardNonCommittedEntriesInternal();
 
   int current_size = static_cast<int>(entries_.size());
 
-  // Prune any entries which are in front of the current entry.
   if (current_size > 0) {
+    // Prune any entries which are in front of the current entry.
+    // Also prune the current entry if we are to replace the current entry.
+    int prune_up_to = replace ? last_committed_entry_index_ - 1
+                              : last_committed_entry_index_;
     int num_pruned = 0;
-    while (last_committed_entry_index_ < (current_size - 1)) {
+    while (prune_up_to < (current_size - 1)) {
       num_pruned++;
       entries_.pop_back();
       current_size--;
     }
     if (num_pruned > 0)  // Only notify if we did prune something.
       NotifyPrunedEntries(this, false, num_pruned);
   }
 
   if (entries_.size() >= max_entry_count_) {
     RemoveEntryAtIndex(0, GURL());
@@ skipping 119 matching lines @@
       return i;
   }
   return -1;
 }
 
 NavigationEntry* NavigationController::GetTransientEntry() const {
   if (transient_entry_index_ == -1)
     return NULL;
   return entries_[transient_entry_index_].get();
 }