From 672eba29c169685bc6b08712098bed16a1eadc72 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Wed, 13 May 2009 13:22:45 +0000 Subject: 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 Review: http://codereview.chromium.org/100245 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@15950 0039d316-1c4b-4281-b951-d872f2087c98 --- .../browser/tab_contents/navigation_controller.cc | 28 ++++++++++++++-------- .../browser/tab_contents/navigation_controller.h | 6 ++--- 2 files changed, 21 insertions(+), 13 deletions(-) (limited to 'chrome/browser') 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(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--; diff --git a/chrome/browser/tab_contents/navigation_controller.h b/chrome/browser/tab_contents/navigation_controller.h index 015bdaa..83f0b8f 100644 --- a/chrome/browser/tab_contents/navigation_controller.h +++ b/chrome/browser/tab_contents/navigation_controller.h @@ -424,9 +424,9 @@ class NavigationController { // contents. void FinishRestore(int selected_index); - // Inserts an entry after the current position, removing all entries after it. - // The new entry will become the active one. - void InsertEntry(NavigationEntry* entry); + // Inserts a new entry or replaces the current entry with a new one, removing + // all entries after it. The new entry will become the active one. + void InsertOrReplaceEntry(NavigationEntry* entry, bool replace); // Discards the pending and transient entries. void DiscardNonCommittedEntriesInternal(); -- cgit v1.1