diff options
| -rw-r--r-- | chrome/browser/native_ui_contents.cc | 5 | ||||
| -rw-r--r-- | chrome/browser/navigation_controller.cc | 23 | ||||
| -rw-r--r-- | chrome/browser/navigation_controller.h | 54 | ||||
| -rw-r--r-- | chrome/browser/safe_browsing/safe_browsing_blocking_page.cc | 5 | ||||
| -rw-r--r-- | chrome/browser/ssl_blocking_page.cc | 5 | ||||
| -rw-r--r-- | chrome/browser/tab_contents.cc | 15 | ||||
| -rw-r--r-- | chrome/browser/tab_contents.h | 7 | ||||
| -rw-r--r-- | chrome/browser/views/info_bar_view.cc | 96 | ||||
| -rw-r--r-- | chrome/browser/views/info_bar_view.h | 23 | ||||
| -rw-r--r-- | chrome/browser/web_contents.cc | 290 | ||||
| -rw-r--r-- | chrome/browser/web_contents.h | 28 | ||||
| -rw-r--r-- | chrome/common/notification_types.h | 3 |
12 files changed, 299 insertions, 255 deletions
diff --git a/chrome/browser/native_ui_contents.cc b/chrome/browser/native_ui_contents.cc index 2046ba0..73cd614 100644 --- a/chrome/browser/native_ui_contents.cc +++ b/chrome/browser/native_ui_contents.cc @@ -269,7 +269,10 @@ bool NativeUIContents::Navigate(const NavigationEntry& entry, bool reload) { state_->GetByteRepresentation(&content_state); new_entry->set_content_state(content_state); const int32 page_id = new_entry->page_id(); - DidNavigateToEntry(new_entry); + + // The default details is "new navigation", and that's OK with us. + NavigationController::LoadCommittedDetails details; + DidNavigateToEntry(new_entry, &details); // This is not a WebContents, so we use a NULL SiteInstance. controller()->NotifyEntryChangedByPageID(type(), NULL, page_id); return true; diff --git a/chrome/browser/navigation_controller.cc b/chrome/browser/navigation_controller.cc index 715e33f7..c944d99 100644 --- a/chrome/browser/navigation_controller.cc +++ b/chrome/browser/navigation_controller.cc @@ -493,7 +493,8 @@ void NavigationController::SetAlternateNavURLFetcher( alternate_nav_url_fetcher_entry_unique_id_ = pending_entry_->unique_id(); } -void NavigationController::DidNavigateToEntry(NavigationEntry* entry) { +void NavigationController::DidNavigateToEntry(NavigationEntry* entry, + LoadCommittedDetails* details) { DCHECK(active_contents_); DCHECK(entry->tab_type() == active_contents_->type()); @@ -501,6 +502,11 @@ void NavigationController::DidNavigateToEntry(NavigationEntry* entry) { entry->set_restored(false); + // Update the details to list the last URL. Later, we'll update the current + // entry (after it's committed) and the details will be complete. + if (GetLastCommittedEntry()) + details->previous_url = GetLastCommittedEntry()->url(); + // If the entry is that of a page with PageID larger than any this Tab has // seen before, then consider it a new navigation. Note that if the entry // has a SiteInstance, it should be the same as the SiteInstance of the @@ -508,7 +514,7 @@ void NavigationController::DidNavigateToEntry(NavigationEntry* entry) { DCHECK(entry->page_id() >= 0) << "Page ID must be set before calling us."; if (entry->page_id() > GetMaxPageID()) { InsertEntry(entry); - NotifyNavigationEntryCommitted(); + NotifyNavigationEntryCommitted(details); // It is now a safe time to schedule collection for any tab contents of a // different type, because a navigation is necessary to get back to them. ScheduleTabContentsCollectionForInactiveTabs(); @@ -566,7 +572,7 @@ void NavigationController::DidNavigateToEntry(NavigationEntry* entry) { } delete entry; - NotifyNavigationEntryCommitted(); + NotifyNavigationEntryCommitted(details); if (alternate_nav_url_fetcher_.get()) { // Because this call may synchronously show an infobar, we do it last, to @@ -715,7 +721,8 @@ void NavigationController::NavigateToPendingEntry(bool reload) { DiscardPendingEntry(); } -void NavigationController::NotifyNavigationEntryCommitted() { +void NavigationController::NotifyNavigationEntryCommitted( + LoadCommittedDetails* details) { // Reset the Alternate Nav URL Fetcher if we're loading some page it doesn't // care about. We must do this before calling Notify() below as that may // result in the creation of a new fetcher. @@ -735,9 +742,11 @@ void NavigationController::NotifyNavigationEntryCommitted() { active_contents_->NotifyNavigationStateChanged( TabContents::INVALIDATE_EVERYTHING); - NotificationService::current()->Notify(NOTIFY_NAV_ENTRY_COMMITTED, - Source<NavigationController>(this), - NotificationService::NoDetails()); + details->entry = GetActiveEntry(); + NotificationService::current()->Notify( + NOTIFY_NAV_ENTRY_COMMITTED, + Source<NavigationController>(this), + Details<LoadCommittedDetails>(details)); } void NavigationController::NotifyPrunedEntries() { diff --git a/chrome/browser/navigation_controller.h b/chrome/browser/navigation_controller.h index c2c517d..e43bfe1 100644 --- a/chrome/browser/navigation_controller.h +++ b/chrome/browser/navigation_controller.h @@ -48,6 +48,46 @@ class NavigationController { int index; }; + struct LoadCommittedDetails { + // By default, the entry will be filled according to a new main frame + // navigation. + LoadCommittedDetails() + : entry(NULL), + is_auto(false), + is_in_page(false), + is_main_frame(true) { + } + + // The committed entry. This will be the active entry in the controller. + NavigationEntry* entry; + + // The previous URL that the user was on. This may be empty if none. + GURL previous_url; + + // True when this load was non-user initated. This corresponds to a + // a NavigationGestureAuto call from WebKit (see webview_delegate.h). + // We also count reloads and meta-refreshes as "auto" to account for the + // fact that WebKit doesn't always set the user gesture properly in these + // cases (see bug 1051891). + bool is_auto; + + // True if the navigation was in-page. This means that the active entry's + // URL and the |previous_url| are the same except for reference fragments. + bool is_in_page; + + // True when the main frame was navigated. False means the navigation was a + // sub-frame. + bool is_main_frame; + + // Returns whether the user probably felt like they navigated somewhere new. + // We often need this logic for showing or hiding something, and this + // returns true only for main frame loads that the user initiated, that go + // to a new page. + bool is_user_initiated_main_frame_load() const { + return !is_auto && !is_in_page && is_main_frame; + } + }; + NavigationController(TabContents* initial_contents, Profile* profile); // Creates a NavigationController from the specified history. Processing // for this is asynchronous and handled via the RestoreHelper (in @@ -208,7 +248,15 @@ class NavigationController { // for a tab. The controller takes ownership of the entry. Any entry located // forward to the current entry will be deleted. The new entry becomes the // current entry. - void DidNavigateToEntry(NavigationEntry* entry); + // + // The details are populated by the caller except for the new NavigationEntry + // pointer and the previous URL. We will fill these in before using it to + // broadcast notifications, so it can also be used by the caller. + // + // TODO(brettw) bug 1343146: The NavigationController should internally make + // the entry and the notification details. + void DidNavigateToEntry(NavigationEntry* entry, + LoadCommittedDetails* details); // Calling this may cause the active tab contents to switch if the current // entry corresponds to a different tab contents type. @@ -284,8 +332,8 @@ class NavigationController { void NavigateToPendingEntry(bool reload); // Allows the derived class to issue notifications that a load has been - // committed. - void NotifyNavigationEntryCommitted(); + // committed. This will fill in the active entry to the details structure. + void NotifyNavigationEntryCommitted(LoadCommittedDetails* details); // Invoked when entries have been pruned, or removed. For example, if the // current entries are [google, digg, yahoo], with the current entry google, diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc index c495952..8d39465 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc @@ -176,7 +176,10 @@ void SafeBrowsingBlockingPage::DisplayBlockingPage() { nav_entry->set_page_id(tab_->GetMaxPageID() + 1); nav_entry->set_page_type(NavigationEntry::INTERSTITIAL_PAGE); nav_entry->set_url(url_); - tab_->controller()->DidNavigateToEntry(nav_entry); + + // The default details is "new navigation", and that's OK with us. + NavigationController::LoadCommittedDetails details; + tab_->controller()->DidNavigateToEntry(nav_entry, &details); created_temporary_entry_ = true; } diff --git a/chrome/browser/ssl_blocking_page.cc b/chrome/browser/ssl_blocking_page.cc index db0f684..a8f5152 100644 --- a/chrome/browser/ssl_blocking_page.cc +++ b/chrome/browser/ssl_blocking_page.cc @@ -153,7 +153,10 @@ void SSLBlockingPage::Show() { nav_entry->ssl().set_cert_status(ssl_info.cert_status); nav_entry->ssl().set_security_bits(ssl_info.security_bits); // The controller will own the entry. - tab_->controller()->DidNavigateToEntry(nav_entry); + + // The default details is "new navigation", and that's OK with us. + NavigationController::LoadCommittedDetails details; + tab_->controller()->DidNavigateToEntry(nav_entry, &details); tab->ShowInterstitialPage(html_text, NULL); } diff --git a/chrome/browser/tab_contents.cc b/chrome/browser/tab_contents.cc index 27d7a0a..503c99e 100644 --- a/chrome/browser/tab_contents.cc +++ b/chrome/browser/tab_contents.cc @@ -256,11 +256,13 @@ void TabContents::SetIsLoading(bool is_loading, NotificationService::NoDetails()); } -void TabContents::DidNavigateToEntry(NavigationEntry* entry) { +void TabContents::DidNavigateToEntry( + NavigationEntry* entry, + NavigationController::LoadCommittedDetails* details) { // The entry may be deleted by DidNavigateToEntry... int new_page_id = entry->page_id(); - controller_->DidNavigateToEntry(entry); + controller_->DidNavigateToEntry(entry, details); // update after informing the navigation controller so it can check the // previous value of the max page id. @@ -275,7 +277,14 @@ bool TabContents::Navigate(const NavigationEntry& entry, bool reload) { new_entry->set_page_id(0); new_entry->set_title(GetDefaultTitle()); } - DidNavigateToEntry(new_entry); + + // When we're commanded to navigate like this, it's always a new main frame + // navigation (which is the default for the details). + NavigationController::LoadCommittedDetails details; + if (controller()->GetLastCommittedEntry()) + details.previous_url = controller()->GetLastCommittedEntry()->url(); + + DidNavigateToEntry(new_entry, &details); return true; } diff --git a/chrome/browser/tab_contents.h b/chrome/browser/tab_contents.h index 9d117a5..7d0f784 100644 --- a/chrome/browser/tab_contents.h +++ b/chrome/browser/tab_contents.h @@ -489,8 +489,11 @@ class TabContents : public PageNavigator, void SetIsLoading(bool is_loading, LoadNotificationDetails* details); // Called by subclasses when a navigation occurs. Ownership of the entry - // object is passed to this method. - void DidNavigateToEntry(NavigationEntry* entry); + // object is passed to this method. The details object should be filled in + // *except* for the entry (which the NavigationController will set). This + // behaves the same as NavigationController::DidNavigate, see that for more. + void DidNavigateToEntry(NavigationEntry* entry, + NavigationController::LoadCommittedDetails* details); // Called by a derived class when the TabContents is resized, causing // suppressed constrained web popups to be repositioned to the new bounds diff --git a/chrome/browser/views/info_bar_view.cc b/chrome/browser/views/info_bar_view.cc index c10dfa9..56c2ecc 100644 --- a/chrome/browser/views/info_bar_view.cc +++ b/chrome/browser/views/info_bar_view.cc @@ -5,6 +5,7 @@ #include "chrome/browser/views/info_bar_view.h" #include "base/logging.h" +#include "chrome/browser/navigation_controller.h" #include "chrome/browser/navigation_entry.h" #include "chrome/browser/tab_contents_delegate.h" #include "chrome/browser/web_contents.h" @@ -31,9 +32,16 @@ static const int kSeparatorHeight = 1; InfoBarView::InfoBarView(WebContents* web_contents) : web_contents_(web_contents) { Init(); + NotificationService::current()->AddObserver( + this, NOTIFY_NAV_ENTRY_COMMITTED, + Source<NavigationController>(web_contents_->controller())); } -InfoBarView::~InfoBarView() {} +InfoBarView::~InfoBarView() { + NotificationService::current()->RemoveObserver( + this, NOTIFY_NAV_ENTRY_COMMITTED, + Source<NavigationController>(web_contents_->controller())); +} void InfoBarView::AppendInfoBarItem(ChromeViews::View* view, bool auto_expire) { // AddChildView adds an entry to expire_map_ for view. @@ -107,43 +115,6 @@ void InfoBarView::ChildAnimationEnded() { web_contents_->ToolbarSizeChanged(false); } -void InfoBarView::DidNavigate(NavigationEntry* entry) { - // Determine the views to remove first. - std::vector<ChromeViews::View*> to_remove; - NavigationEntry* pending_entry = - web_contents_->controller()->GetPendingEntry(); - const int active_id = pending_entry ? pending_entry->unique_id() : - entry->unique_id(); - for (std::map<View*,int>::iterator i = expire_map_.begin(); - i != expire_map_.end(); ++i) { - if ((pending_entry && - pending_entry->transition_type() == PageTransition::RELOAD) || - i->second != active_id) - to_remove.push_back(i->first); - } - - if (to_remove.empty()) - return; - - // Remove the views. - for (std::vector<ChromeViews::View*>::iterator i = to_remove.begin(); - i != to_remove.end(); ++i) { - // RemoveChildView takes care of removing from expire_map for us. - RemoveChildView(*i); - - // We own the child and we're removing it, need to delete it. - delete *i; - } - - if (GetChildViewCount() == 0) { - // All our views have been removed, no need to stay visible. - web_contents_->SetInfoBarVisible(false); - } else if (web_contents_) { - // This triggers a layout. - web_contents_->ToolbarSizeChanged(false); - } -} - void InfoBarView::ViewHierarchyChanged(bool is_add, View *parent, View *child) { if (parent == this && child->GetParent() == this) { @@ -220,3 +191,52 @@ void InfoBarView::PaintSeparator(ChromeCanvas* canvas, GetWidth(), kSeparatorHeight); } + +void InfoBarView::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& in_details) { + // We should get only commit notifications from our controller. + DCHECK(type == NOTIFY_NAV_ENTRY_COMMITTED); + DCHECK(web_contents_->controller() == + Source<NavigationController>(source).ptr()); + + NavigationController::LoadCommittedDetails& details = + *(Details<NavigationController::LoadCommittedDetails>(in_details).ptr()); + + // Only hide infobars when the user has done something that makes the main + // frame load. We don't want various automatic or subframe navigations making + // it disappear. + if (!details.is_user_initiated_main_frame_load()) + return; + + // Determine the views to remove first. + std::vector<ChromeViews::View*> to_remove; + for (std::map<View*,int>::iterator i = expire_map_.begin(); + i != expire_map_.end(); ++i) { + if (PageTransition::StripQualifier(details.entry->transition_type()) == + PageTransition::RELOAD || + i->second != details.entry->unique_id()) + to_remove.push_back(i->first); + } + + if (to_remove.empty()) + return; + + // Remove the views. + for (std::vector<ChromeViews::View*>::iterator i = to_remove.begin(); + i != to_remove.end(); ++i) { + // RemoveChildView takes care of removing from expire_map for us. + RemoveChildView(*i); + + // We own the child and we're removing it, need to delete it. + delete *i; + } + + if (GetChildViewCount() == 0) { + // All our views have been removed, no need to stay visible. + web_contents_->SetInfoBarVisible(false); + } else if (web_contents_) { + // This triggers a layout. + web_contents_->ToolbarSizeChanged(false); + } +} diff --git a/chrome/browser/views/info_bar_view.h b/chrome/browser/views/info_bar_view.h index 8234c0a..0ff9b94 100644 --- a/chrome/browser/views/info_bar_view.h +++ b/chrome/browser/views/info_bar_view.h @@ -2,11 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_VIEWS_INFO_BAR_VIEW_H__ -#define CHROME_BROWSER_VIEWS_INFO_BAR_VIEW_H__ +#ifndef CHROME_BROWSER_VIEWS_INFO_BAR_VIEW_H_ +#define CHROME_BROWSER_VIEWS_INFO_BAR_VIEW_H_ #include <map> +#include "chrome/common/notification_service.h" #include "chrome/views/view.h" class NavigationEntry; @@ -16,7 +17,8 @@ class WebContents; // // It will paint all of its children vertically, with the most recently // added child displayed at the top of the info bar. -class InfoBarView : public ChromeViews::View { +class InfoBarView : public ChromeViews::View, + public NotificationObserver { public: explicit InfoBarView(WebContents* web_contents); @@ -42,12 +44,6 @@ class InfoBarView : public ChromeViews::View { virtual void DidChangeBounds(const CRect& previous, const CRect& current); - // Notification that the user has done a navigation. Removes child views that - // are set to be removed after a certain number of navigations and - // potentially hides the info bar. |entry| is the new entry that we are - // navigating to. - void DidNavigate(NavigationEntry* entry); - WebContents* web_contents() { return web_contents_; } protected: @@ -74,14 +70,19 @@ class InfoBarView : public ChromeViews::View { ChromeViews::View* v1, ChromeViews::View* v2); + // NotificationObserver implementation. + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + WebContents* web_contents_; // Map from view to number of navigations before it is removed. If a child // doesn't have an entry in here, it is NOT removed on navigations. std::map<View*,int> expire_map_; - DISALLOW_EVIL_CONSTRUCTORS(InfoBarView); + DISALLOW_COPY_AND_ASSIGN(InfoBarView); }; -#endif // CHROME_BROWSER_VIEWS_INFO_BAR_VIEW_H__ +#endif // CHROME_BROWSER_VIEWS_INFO_BAR_VIEW_H_ diff --git a/chrome/browser/web_contents.cc b/chrome/browser/web_contents.cc index 45eec91..2367a9b 100644 --- a/chrome/browser/web_contents.cc +++ b/chrome/browser/web_contents.cc @@ -567,7 +567,7 @@ void WebContents::SavePage(const std::wstring& main_file, // the ResourceDispatcherHost, who unpauses the response. Data is then sent // to the pending RVH. // - The pending renderer sends a FrameNavigate message that invokes the -// WebContents::DidNavigate method. This replaces the current RVH with the +// DidNavigate method. This replaces the current RVH with the // pending RVH and goes back to the NORMAL RendererState. bool WebContents::Navigate(const NavigationEntry& entry, bool reload) { @@ -1194,46 +1194,59 @@ void WebContents::DidNavigate(RenderViewHost* rvh, return; } + // Generate the details for the notification. + // TODO(brettw) bug 1343146: Move into the NavigationController. + NavigationController::LoadCommittedDetails details; + // WebKit doesn't set the "auto" transition on meta refreshes properly (bug + // 1051891) so we manually set it for redirects which we normally treat as + // "non-user-gestures" where we want to update stuff after navigations. + // + // Note that the redirect check also checks for a pending entry to + // differentiate real redirects from browser initiated navigations to a + // redirected entry. This happens when you hit back to go to a page that was + // the destination of a redirect, we don't want to treat it as a redirect + // even though that's what its transition will be. See bug 1117048. + // + // TODO(brettw) write a test for this complicated logic. + details.is_auto = (PageTransition::IsRedirect(params.transition) && + !controller()->GetPendingEntry()) || + params.gesture == NavigationGestureAuto; + details.is_in_page = IsInPageNavigation(params.url); + details.is_main_frame = PageTransition::IsMainFrame(params.transition); + // DO NOT ADD MORE STUFF TO THIS FUNCTION! Don't make me come over there! // ======================================================================= - // Add your code to DidNavigateAnyFramePreCommit if you have a helper object + // Add your code to DidNavigateAnyFramePostCommit if you have a helper object // that needs to know about all navigations. If it needs to do it only for // main frame or sub-frame navigations, add your code to - // DidNavigateMainFrame or DidNavigateSubFrame. If you need to run it after - // the navigation has been committed, put it in a *PostCommit version. + // DidNavigate*PostCommit. // Create the new navigation entry for this navigation and do work specific to // this frame type. The main frame / sub frame functions will do additional // updates to the NavigationEntry appropriate for the navigation type (in // addition to a lot of other stuff). scoped_ptr<NavigationEntry> entry(CreateNavigationEntryForCommit(params)); - if (PageTransition::IsMainFrame(params.transition)) - DidNavigateMainFramePreCommit(params, entry.get()); - else - DidNavigateSubFramePreCommit(params, entry.get()); - - // Now we do non-frame-specific work in *AnyFramePreCommit (this depends on - // the entry being completed appropriately in the frame-specific versions - // above before the call). - DidNavigateAnyFramePreCommit(params, entry.get()); // Commit the entry to the navigation controller. - DidNavigateToEntry(entry.release()); + DidNavigateToEntry(entry.release(), &details); // WARNING: NavigationController will have taken ownership of entry at this // point, and may have deleted it. As such, do NOT use entry after this. // Run post-commit tasks. - if (PageTransition::IsMainFrame(params.transition)) - DidNavigateMainFramePostCommit(params); - DidNavigateAnyFramePostCommit(rvh, params); + if (details.is_main_frame) + DidNavigateMainFramePostCommit(details, params); + DidNavigateAnyFramePostCommit(rvh, details, params); } +// TODO(brettw) bug 1343146: This logic should be moved to NavigationController. NavigationEntry* WebContents::CreateNavigationEntryForCommit( const ViewHostMsg_FrameNavigate_Params& params) { // This new navigation entry will represent the navigation. Note that we // don't set the URL. This will happen in the DidNavigateMainFrame/SubFrame // because the entry's URL should represent the toplevel frame only. NavigationEntry* entry = new NavigationEntry(type()); + if (PageTransition::IsMainFrame(params.transition)) + entry->set_url(params.url); entry->set_page_id(params.page_id); entry->set_transition_type(params.transition); entry->set_site_instance(site_instance()); @@ -1273,97 +1286,103 @@ NavigationEntry* WebContents::CreateNavigationEntryForCommit( } } + if (PageTransition::IsMainFrame(params.transition)) { + NavigationEntry* pending = controller()->GetPendingEntry(); + if (pending) { + // Copy fields from the pending NavigationEntry into the actual + // NavigationEntry that we're committing to. + entry->set_user_typed_url(pending->user_typed_url()); + if (pending->has_display_url()) + entry->set_display_url(pending->display_url()); + if (pending->url().SchemeIsFile()) + entry->set_title(pending->title()); + entry->set_content_state(pending->content_state()); + } + entry->set_has_post_data(params.is_post); + } else { + NavigationEntry* last_committed = controller()->GetLastCommittedEntry(); + if (last_committed) { + // In the case of a sub-frame navigation within a window that was created + // without an URL (window.open), we may not have a committed entry yet! + + // Reset entry state to match that of the pending entry. + entry->set_unique_id(last_committed->unique_id()); + entry->set_url(last_committed->url()); + entry->set_transition_type(last_committed->transition_type()); + entry->set_user_typed_url(last_committed->user_typed_url()); + entry->set_content_state(last_committed->content_state()); + + // TODO(jcampan): when navigating to an insecure/unsafe inner frame, the + // main entry is the one that gets notified of the mixed/unsafe contents + // (see SSLPolicy::OnRequestStarted()). Here we are just transferring + // that state. We should find a better way to do this. Note that it is OK + // that the mixed/unsafe contents is set on the wrong navigation entry, as + // that state is reset when navigating back to it. + DCHECK(last_committed->ssl().content_status() == 0) << "We should never " + "be setting the status bits from 1 to 0 on navigate."; + entry->ssl() = last_committed->ssl(); + } + } + return entry; } -void WebContents::DidNavigateMainFramePreCommit( - const ViewHostMsg_FrameNavigate_Params& params, - NavigationEntry* entry) { - // Update contents MIME type of the main webframe. - contents_mime_type_ = params.contents_mime_type; - - entry->set_url(params.url); - - NavigationEntry* pending = controller()->GetPendingEntry(); - if (pending) { - // Copy fields from the pending NavigationEntry into the actual - // NavigationEntry that we're committing to. - entry->set_user_typed_url(pending->user_typed_url()); - if (pending->has_display_url()) - entry->set_display_url(pending->display_url()); - if (pending->url().SchemeIsFile()) - entry->set_title(pending->title()); - entry->set_content_state(pending->content_state()); +void WebContents::DidNavigateMainFramePostCommit( + const NavigationController::LoadCommittedDetails& details, + const ViewHostMsg_FrameNavigate_Params& params) { + // Hide the download shelf if all the following conditions are true: + // - there are no active downloads. + // - this is a navigation to a different TLD. + // - at least 5 seconds have elapsed since the download shelf was shown. + // TODO(jcampan): bug 1156075 when user gestures are reliable, they should + // be used to ensure we are hiding only on user initiated + // navigations. + DownloadManager* download_manager = profile()->GetDownloadManager(); + // download_manager can be NULL in unit test context. + if (download_manager && download_manager->in_progress_count() == 0 && + !details.previous_url.is_empty() && + !net::RegistryControlledDomainService::SameDomainOrHost( + details.previous_url, details.entry->url())) { + TimeDelta time_delta( + TimeTicks::Now() - last_download_shelf_show_); + if (time_delta > + TimeDelta::FromMilliseconds(kDownloadShelfHideDelay)) { + SetDownloadShelfVisible(false); + } } - // We no longer know the title after this navigation. - has_page_title_ = false; - - // Reset the starred button to false by default, but also request from - // history whether it's actually starred. - // - // Only save the URL in the entry for top-level frames. This will appear in - // the UI for the page, so we always want to use the toplevel URL. - // - // The |user_initiated_big_change| flag indicates whether we can tell the - // infobar/password manager about this navigation. True for non-redirect, - // non-in-page user initiated navigations; assume this is true and set false - // below if not. - // - // TODO(pkasting): http://b/1048012 We should notify based on whether the - // navigation was triggered by a user action rather than most of our current - // heuristics. Be careful with SSL infobars, though. - // - // See bug 1051891 for reasons why we need both a redirect check and gesture - // check; basically gesture checking is not always accurate. - // - // Note that the redirect check also checks for a pending entry to - // differentiate real redirects from browser initiated navigations to a - // redirected entry (like when you hit back to go to a page that was the - // destination of a redirect, we don't want to treat it as a redirect - // even though that's what its transition will be) http://b/1117048. - bool user_initiated_big_change = true; - if ((PageTransition::IsRedirect(entry->transition_type()) && - !controller()->GetPendingEntry()) || - (params.gesture == NavigationGestureAuto) || - IsInPageNavigation(params.url)) { - user_initiated_big_change = false; - } else { + if (details.is_user_initiated_main_frame_load()) { // Clear the status bubble. This is a workaround for a bug where WebKit // doesn't let us know that the cursor left an element during a // transition (this is also why the mouse cursor remains as a hand after // clicking on a link); see bugs 1184641 and 980803. We don't want to // clear the bubble when a user navigates to a named anchor in the same // page. - UpdateTargetURL(params.page_id, GURL()); - } + UpdateTargetURL(details.entry->page_id(), GURL()); - // Let the infobar know about the navigation to give the infobar a chance - // to remove any views on navigating. Only do so if this navigation was - // initiated by the user, and we are not simply following a fragment to - // relocate within the current page. - // - // We must do this after calling DidNavigateToEntry(), since the info bar - // view checks the controller's active entry to determine whether to auto- - // expire any children. - if (user_initiated_big_change && IsInfoBarVisible()) { - InfoBarView* info_bar = GetInfoBarView(); - DCHECK(info_bar); - info_bar->DidNavigate(entry); + // UpdateHelpersForDidNavigate will handle the case where the password_form + // origin is valid. + // TODO(brettw) bug 1343111: Password manager stuff in here needs to be + // cleaned up and covered by tests. + if (!params.password_form.origin.is_valid()) + GetPasswordManager()->DidNavigate(); } - // UpdateHelpersForDidNavigate will handle the case where the password_form - // origin is valid. - if (user_initiated_big_change && !params.password_form.origin.is_valid()) - GetPasswordManager()->DidNavigate(); - + // The keyword generator uses the navigation entries, so must be called after + // the commit. GenerateKeywordIfNecessary(params); - // Close constrained popups if necessary. - MaybeCloseChildWindows(params); + // We no longer know the title after this navigation. + has_page_title_ = false; + + // Update contents MIME type of the main webframe. + contents_mime_type_ = params.contents_mime_type; // Get the favicon, either from history or request it from the net. - fav_icon_helper_.FetchFavIcon(entry->url()); + fav_icon_helper_.FetchFavIcon(details.entry->url()); + + // Close constrained popups if necessary. + MaybeCloseChildWindows(params); // We hide the FindInPage window when the user navigates away, except on // reload. @@ -1371,66 +1390,14 @@ void WebContents::DidNavigateMainFramePreCommit( PageTransition::RELOAD) SetFindInPageVisible(false); - entry->set_has_post_data(params.is_post); -} - -void WebContents::DidNavigateSubFramePreCommit( - const ViewHostMsg_FrameNavigate_Params& params, - NavigationEntry* entry) { - NavigationEntry* last_committed = controller()->GetLastCommittedEntry(); - if (!last_committed) { - // In the case of a sub-frame navigation within a window that was created - // without an URL (via window.open), we may not have a committed entry yet! - return; - } - - // Reset entry state to match that of the pending entry. - entry->set_unique_id(last_committed->unique_id()); - entry->set_url(last_committed->url()); - entry->set_transition_type(last_committed->transition_type()); - entry->set_user_typed_url(last_committed->user_typed_url()); - entry->set_content_state(last_committed->content_state()); - - // TODO(jcampan): when navigating to an insecure/unsafe inner frame, the - // main entry is the one that gets notified of the mixed/unsafe contents - // (see SSLPolicy::OnRequestStarted()). Here we are just transferring that - // state. We should find a better way to do this. - // Note that it is OK that the mixed/unsafe contents is set on the wrong - // navigation entry, as that state is reset when navigating back to it. - DCHECK(last_committed->ssl().content_status() == 0) << "We should never be " - "setting the status bits from 1 to 0 on navigate."; - entry->ssl() = last_committed->ssl(); + // Update the starred state. + UpdateStarredStateForCurrentURL(); } -void WebContents::DidNavigateAnyFramePreCommit( - const ViewHostMsg_FrameNavigate_Params& params, - NavigationEntry* entry) { - - // Hide the download shelf if all the following conditions are true: - // - there are no active downloads. - // - this is a navigation to a different TLD. - // - at least 5 seconds have elapsed since the download shelf was shown. - // TODO(jcampan): bug 1156075 when user gestures are reliable, they should - // be used to ensure we are hiding only on user initiated - // navigations. - NavigationEntry* current_entry = controller()->GetLastCommittedEntry(); - DownloadManager* download_manager = profile()->GetDownloadManager(); - // download_manager can be NULL in unit test context. - if (download_manager && download_manager ->in_progress_count() == 0 && - current_entry && !net::RegistryControlledDomainService::SameDomainOrHost( - current_entry->url(), entry->url())) { - TimeDelta time_delta( - TimeTicks::Now() - last_download_shelf_show_); - if (time_delta > - TimeDelta::FromMilliseconds(kDownloadShelfHideDelay)) { - SetDownloadShelfVisible(false); - } - } - - // Notify the password manager of the navigation or form submit. - if (params.password_form.origin.is_valid()) - GetPasswordManager()->ProvisionallySavePassword(params.password_form); - +void WebContents::DidNavigateAnyFramePostCommit( + RenderViewHost* render_view_host, + const NavigationController::LoadCommittedDetails& details, + const ViewHostMsg_FrameNavigate_Params& params) { // If we navigate, start showing messages again. This does nothing to prevent // a malicious script from spamming messages, since the script could just // reload the page to stop blocking. @@ -1441,27 +1408,20 @@ void WebContents::DidNavigateAnyFramePreCommit( if (params.should_update_history) { // Most of the time, the displayURL matches the loaded URL, but for about: // URLs, we use a data: URL as the real value. We actually want to save - // the about: URL to the history db and keep the data: URL hidden. - UpdateHistoryForNavigation(entry->display_url(), params); + // the about: URL to the history db and keep the data: URL hidden. This is + // what the TabContents' URL getter does. + UpdateHistoryForNavigation(GetURL(), params); } -} -void WebContents::DidNavigateMainFramePostCommit( - const ViewHostMsg_FrameNavigate_Params& params) { - // The keyword generator uses the navigation entries, so must be called after - // the commit. - GenerateKeywordIfNecessary(params); - - // Update the starred state. - UpdateStarredStateForCurrentURL(); -} - -void WebContents::DidNavigateAnyFramePostCommit( - RenderViewHost* render_view_host, - const ViewHostMsg_FrameNavigate_Params& params) { // Have the controller save the current session. controller()->NotifyEntryChangedByPageID(type(), site_instance(), - params.page_id); + details.entry->page_id()); + + // Notify the password manager of the navigation or form submit. + // TODO(brettw) bug 1343111: Password manager stuff in here needs to be + // cleaned up and covered by tests. + if (params.password_form.origin.is_valid()) + GetPasswordManager()->ProvisionallySavePassword(params.password_form); BroadcastProvisionalLoadCommit(render_view_host, params); } diff --git a/chrome/browser/web_contents.h b/chrome/browser/web_contents.h index 87acbd5..43ba2e8 100644 --- a/chrome/browser/web_contents.h +++ b/chrome/browser/web_contents.h @@ -555,34 +555,16 @@ class WebContents : public TabContents, NavigationEntry* CreateNavigationEntryForCommit( const ViewHostMsg_FrameNavigate_Params& params); - // Handles post-navigation tasks specific to some set of frames. DidNavigate() - // calls these with newly created navigation entry for this navigation BEFORE - // that entry has been committed to the navigation controller. The functions - // can update the entry as needed. - // - // First the frame-specific version (main or sub) will be called to update the - // entry as needed after it was created by CreateNavigationEntryForCommit. - // - // Then DidNavigateAnyFramePreCommit will be called with the now-complete - // entry for further processing that is not specific to the type of frame. - void DidNavigateMainFramePreCommit( - const ViewHostMsg_FrameNavigate_Params& params, - NavigationEntry* entry); - void DidNavigateSubFramePreCommit( - const ViewHostMsg_FrameNavigate_Params& params, - NavigationEntry* entry); - void DidNavigateAnyFramePreCommit( - const ViewHostMsg_FrameNavigate_Params& params, - NavigationEntry* entry); - // Handles post-navigation tasks in DidNavigate AFTER the entry has been - // committed to the navigation controller. See WillNavigate* above. Note that - // the navigation entry is not provided since it may be invalid/changed after - // being committed. + // committed to the navigation controller. Note that the navigation entry is + // not provided since it may be invalid/changed after being committed. The + // current navigation entry is in the NavigationController at this point. void DidNavigateMainFramePostCommit( + const NavigationController::LoadCommittedDetails& details, const ViewHostMsg_FrameNavigate_Params& params); void DidNavigateAnyFramePostCommit( RenderViewHost* render_view_host, + const NavigationController::LoadCommittedDetails& details, const ViewHostMsg_FrameNavigate_Params& params); // Called when navigating the main frame to close all child windows if the diff --git a/chrome/common/notification_types.h b/chrome/common/notification_types.h index 7bd9bb1..cff0655 100644 --- a/chrome/common/notification_types.h +++ b/chrome/common/notification_types.h @@ -43,6 +43,9 @@ enum NotificationType { // A new non-pending navigation entry has been created. This will correspond // to one NavigationController entry being created (in the case of new // navigations) or renavigated to (for back/forward navigations). + // + // The source will be the navigation controller doing the commit. The details + // will be NavigationController::LoadCommittedDetails. NOTIFY_NAV_ENTRY_COMMITTED, // Indicates that the NavigationController given in the Source has decreased |