This is the successor to http://codereview.chromium.org/67150

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

Currently, Chrome tries to go back to the page immediately before
the current one. This doesn't work if the current page was visited
by redirection; redirection just occurs again.

With this change, Chrome first tries to find the redirection source
of the current page and then to go back to the page before the source.

BUG=9663,10531

Tested: unit_tests, ui_tests, manually.

Patch contributed by Yuzo Fujishima <yuzo@google.com>

Review: http://codereview.chromium.org/100245

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

1 files changed, 18 insertions, 10 deletions

diff --git a/chrome/browser/tab_contents/navigation_controller.cc b/chrome/browser/tab_contents/navigation_controller.cc
index 4b3e748..a097455 100644
--- a/chrome/browser/tab_contents/navigation_controller.cc
+++ b/chrome/browser/tab_contents/navigation_controller.cc
@@ -605,7 +605,11 @@ void NavigationController::RendererDidNavigateToNewPage(
   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(
@@ -674,7 +678,7 @@ void NavigationController::RendererDidNavigateInPage(
   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(
@@ -687,7 +691,7 @@ void NavigationController::RendererDidNavigateNewSubframe(
                                   << "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(
@@ -743,15 +747,15 @@ void NavigationController::CommitPendingEntry() {
     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.
@@ -807,7 +811,8 @@ void NavigationController::DiscardNonCommittedEntries() {
   }
 }
 
-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.
@@ -821,10 +826,13 @@ void NavigationController::InsertEntry(NavigationEntry* entry) {
 
   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--;