diff options
| author | laforge@chromium.org <laforge@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-16 20:22:11 +0000 |
|---|---|---|
| committer | laforge@chromium.org <laforge@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-16 20:22:11 +0000 |
| commit | 09b8f82f39b2e3613a4518d9390004522e432063 (patch) | |
| tree | 5d2f12f8c870fe0b222ae578bd1f8b4a1e1bd360 /chrome/browser/tab_contents/navigation_controller.cc | |
| parent | 44a96738ed0547db89b227399c0b46aa50cbd78e (diff) | |
| download | chromium_src-09b8f82f39b2e3613a4518d9390004522e432063.zip chromium_src-09b8f82f39b2e3613a4518d9390004522e432063.tar.gz chromium_src-09b8f82f39b2e3613a4518d9390004522e432063.tar.bz2 | |
Revert 18512 - Revert 18373 Consider a redirect following user gesture as userinitiated in maintaining
navigation entries. Also, ignore redirect or machineinitiated new subframe
navigations.
The current code treats all redirects as machineinitiated in processing
navigation to a new page (to fix Bugs 9663 and 10531). This is not always
appropriate, because some sites, e.g., www.google.com/ig, use redirect to
implement userinitiated navigation (Bug 11896).
This change assumes that a machineinitiated redirect happens within 300ms
since the last document load was completed, while a userinitiated one
happens later.
This assumption is not always correct, e.g., a user may cause transition within
300ms. But I cannot think of any better ways to tell if a redirect is machine
initiated or userinitiated.
I believe this change works good enough, at least better than the status quo.
Review URL: http://codereview.chromium.org/115919
TEST=Open http://www.hp.com and observe it redirects to
http://www.hp.com/#Product . Hit Back button and observe
the former URL is not visited. Open http://www.google.com/ig and
click tabs inside the page, and try hitting Back and Forward to see if the
navigation is right. Open http://www.google.com/codesearch, search for
something, click on a result item, and try hitting Back.
BUG=11896,12820
TBR=yuzo@chromium.org
Review URL: http://codereview.chromium.org/125202
TBR=laforge@chromium.org
Review URL: http://codereview.chromium.org/126221
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@18522 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/tab_contents/navigation_controller.cc')
| -rw-r--r-- | chrome/browser/tab_contents/navigation_controller.cc | 55 |
1 files changed, 50 insertions, 5 deletions
diff --git a/chrome/browser/tab_contents/navigation_controller.cc b/chrome/browser/tab_contents/navigation_controller.cc index eb6441a..b5ae80b 100644 --- a/chrome/browser/tab_contents/navigation_controller.cc +++ b/chrome/browser/tab_contents/navigation_controller.cc @@ -90,6 +90,12 @@ bool AreURLsInPageNavigation(const GURL& existing_url, const GURL& new_url) { new_url.ReplaceComponents(replacements); } +// Navigation within this limit since the last document load is considered to +// be automatic (i.e., machine-initiated) rather than user-initiated unless +// a user gesture has been observed. +const base::TimeDelta kMaxAutoNavigationTimeDelta = + base::TimeDelta::FromSeconds(5); + } // namespace // NavigationController --------------------------------------------------- @@ -424,6 +430,14 @@ const SkBitmap& NavigationController::GetLazyFavIcon() const { } } +void NavigationController::DocumentLoadedInFrame() { + last_document_loaded_ = base::TimeTicks::Now(); +} + +void NavigationController::OnUserGesture() { + user_gesture_observed_ = true; +} + bool NavigationController::RendererDidNavigate( const ViewHostMsg_FrameNavigate_Params& params, LoadCommittedDetails* details) { @@ -505,6 +519,8 @@ bool NavigationController::RendererDidNavigate( details->http_status_code = params.http_status_code; NotifyNavigationEntryCommitted(details); + user_gesture_observed_ = false; + return true; } @@ -585,6 +601,21 @@ NavigationType::Type NavigationController::ClassifyNavigation( return NavigationType::EXISTING_PAGE; } +bool NavigationController::IsRedirect( + const ViewHostMsg_FrameNavigate_Params& params) { + // For main frame transition, we judge by params.transition. + // Otherwise, by params.redirects. + if (PageTransition::IsMainFrame(params.transition)) { + return PageTransition::IsRedirect(params.transition); + } + return params.redirects.size() > 1; +} + +bool NavigationController::IsLikelyAutoNavigation(base::TimeTicks now) { + return !user_gesture_observed_ && + (now - last_document_loaded_) < kMaxAutoNavigationTimeDelta; +} + void NavigationController::RendererDidNavigateToNewPage( const ViewHostMsg_FrameNavigate_Params& params) { NavigationEntry* new_entry; @@ -611,11 +642,14 @@ void NavigationController::RendererDidNavigateToNewPage( new_entry->set_site_instance(tab_contents_->GetSiteInstance()); new_entry->set_has_post_data(params.is_post); - // 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. + // If the current entry is a redirection source and the redirection has + // occurred within kMaxAutoNavigationTimeDelta since the last document load, + // this is likely to be machine-initiated redirect and the entry 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())); + IsRedirect(params) && IsLikelyAutoNavigation(base::TimeTicks::Now())); } void NavigationController::RendererDidNavigateToExistingPage( @@ -687,11 +721,22 @@ void NavigationController::RendererDidNavigateInPage( NavigationEntry* new_entry = new NavigationEntry(*existing_entry); new_entry->set_page_id(params.page_id); new_entry->set_url(params.url); - InsertOrReplaceEntry(new_entry, false); + InsertOrReplaceEntry(new_entry, + IsRedirect(params) && IsLikelyAutoNavigation(base::TimeTicks::Now())); } void NavigationController::RendererDidNavigateNewSubframe( const ViewHostMsg_FrameNavigate_Params& params) { + if (PageTransition::StripQualifier(params.transition) == + PageTransition::AUTO_SUBFRAME) { + // This is not user-initiated. Ignore. + return; + } + if (IsRedirect(params)) { + // This is redirect. Ignore. + return; + } + // 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 |