diff options
24 files changed, 1304 insertions, 990 deletions
diff --git a/chrome/browser/back_forward_menu_model_unittest.cc b/chrome/browser/back_forward_menu_model_unittest.cc index e2e04835a..f91081d 100644 --- a/chrome/browser/back_forward_menu_model_unittest.cc +++ b/chrome/browser/back_forward_menu_model_unittest.cc @@ -29,30 +29,20 @@ class BackFwdMenuModelTestTabContents : public TabContents { BackFwdMenuModelTestTabContents() : TabContents(kHTTPTabContentsType) { } - bool Navigate(const NavigationEntry& entry, bool reload) { - NavigationEntry* pending_entry = new NavigationEntry(entry); - if (pending_entry->page_id() == -1) { - pending_entry->set_page_id(g_page_id_++); - } - NavigationController::LoadCommittedDetails details; - DidNavigateToEntry(pending_entry, &details); + // We do the same thing as the TabContents one (just commit the navigation) + // but we *don't* want to reset the title since the test looks for this. + virtual bool NavigateToPendingEntry(bool reload) { + controller()->CommitPendingEntry(); return true; } void UpdateState(const std::wstring& title) { NavigationEntry* entry = - controller()->GetEntryWithPageID(type(), NULL, g_page_id_ - 1); + controller()->GetEntryWithPageID(type(), NULL, GetMaxPageID()); entry->set_title(title); } - - private: - // We need to use valid, incrementing page ids otherwise the TabContents - // and NavController will not play nice when we try to go back and forward. - static int g_page_id_; }; -int BackFwdMenuModelTestTabContents::g_page_id_ = 0; - // This constructs our fake TabContents. class BackFwdMenuModelTestTabContentsFactory : public TabContentsFactory { public: diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index b01c2ce..235edbf 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -540,7 +540,7 @@ void Browser::OpenURLFromTab(TabContents* source, if (web_contents) { const GURL& current_url = web_contents->GetURL(); if (SiteInstance::IsSameWebSite(current_url, url)) - instance = web_contents->site_instance(); + instance = web_contents->GetSiteInstance(); } } } diff --git a/chrome/browser/dom_ui/new_tab_ui.cc b/chrome/browser/dom_ui/new_tab_ui.cc index b5cdf52..3d0cdf1 100644 --- a/chrome/browser/dom_ui/new_tab_ui.cc +++ b/chrome/browser/dom_ui/new_tab_ui.cc @@ -797,15 +797,9 @@ void NewTabUIContents::AttachMessageHandlers() { } } -bool NewTabUIContents::Navigate(const NavigationEntry& entry, bool reload) { - const bool result = WebContents::Navigate(entry, reload); - - // Force the title to say 'New tab', even when loading. The supplied entry is - // also the pending entry. - NavigationEntry* pending_entry = controller()->GetPendingEntry(); - DCHECK(pending_entry && pending_entry == &entry); - pending_entry->set_title(forced_title_); - +bool NewTabUIContents::NavigateToPendingEntry(bool reload) { + const bool result = WebContents::NavigateToPendingEntry(reload); + controller()->GetPendingEntry()->set_title(forced_title_); return result; } diff --git a/chrome/browser/dom_ui/new_tab_ui.h b/chrome/browser/dom_ui/new_tab_ui.h index 3584954..092cdb4 100644 --- a/chrome/browser/dom_ui/new_tab_ui.h +++ b/chrome/browser/dom_ui/new_tab_ui.h @@ -290,7 +290,7 @@ class NewTabUIContents : public DOMUIHost { // WebContents overrides. // Overriden to force the title of the page to forced_title_. - virtual bool Navigate(const NavigationEntry& entry, bool reload); + virtual bool NavigateToPendingEntry(bool reload); // We don't want a favicon on the new tab page. virtual bool ShouldDisplayFavIcon() { return false; } // The bookmark bar is always visible on the new tab. diff --git a/chrome/browser/native_ui_contents.cc b/chrome/browser/native_ui_contents.cc index 4c967a2..e4dce38 100644 --- a/chrome/browser/native_ui_contents.cc +++ b/chrome/browser/native_ui_contents.cc @@ -210,20 +210,20 @@ void NativeUIContents::SetPageState(PageState* page_state) { state_.reset(page_state); NavigationController* ctrl = controller(); if (ctrl) { - NavigationEntry* ne = ctrl->GetLastCommittedEntry(); + int ne_index = ctrl->GetLastCommittedEntryIndex(); + NavigationEntry* ne = ctrl->GetEntryAtIndex(ne_index); if (ne) { // NavigationEntry is null if we're being restored. DCHECK(ne); std::string rep; state_->GetByteRepresentation(&rep); ne->set_content_state(rep); - // This is not a WebContents, so we use a NULL SiteInstance. - ctrl->NotifyEntryChangedByPageID(type(), NULL, ne->page_id()); + ctrl->NotifyEntryChanged(ne, ne_index); } } } -bool NativeUIContents::Navigate(const NavigationEntry& entry, bool reload) { +bool NativeUIContents::NavigateToPendingEntry(bool reload) { ChromeViews::RootView* root_view = GetRootView(); DCHECK(root_view); @@ -234,7 +234,8 @@ bool NativeUIContents::Navigate(const NavigationEntry& entry, bool reload) { current_view_ = NULL; } - NativeUI* new_ui = GetNativeUIForURL(entry.url()); + NavigationEntry* pending_entry = controller()->GetPendingEntry(); + NativeUI* new_ui = GetNativeUIForURL(pending_entry->url()); if (new_ui) { current_ui_ = new_ui; is_visible_ = true; @@ -242,9 +243,9 @@ bool NativeUIContents::Navigate(const NavigationEntry& entry, bool reload) { current_view_ = new_ui->GetView(); root_view->AddChildView(current_view_); - std::string s = entry.content_state(); + std::string s = pending_entry->content_state(); if (s.empty()) - state_->InitWithURL(entry.url()); + state_->InitWithURL(pending_entry->url()); else state_->InitWithBytes(s); @@ -252,29 +253,32 @@ bool NativeUIContents::Navigate(const NavigationEntry& entry, bool reload) { Layout(); } - NavigationEntry* new_entry = new NavigationEntry(entry); - if (new_entry->page_id() == -1) - new_entry->set_page_id(++g_next_page_id); - new_entry->set_title(GetDefaultTitle()); - new_entry->favicon().set_bitmap(GetFavIcon()); - new_entry->favicon().set_is_valid(true); + // Commit the new load in the navigation controller. If the ID of the + // NavigationEntry we were given was -1, that means this is a new load, so + // we have to generate a new ID. + controller()->CommitPendingEntry(); + + // Populate the committed entry. + NavigationEntry* committed_entry = controller()->GetLastCommittedEntry(); + committed_entry->set_title(GetDefaultTitle()); + committed_entry->favicon().set_bitmap(GetFavIcon()); + committed_entry->favicon().set_is_valid(true); if (new_ui) { // Strip out the query params, they should have moved to state. // TODO(sky): use GURL methods for replacements once bug is fixed. size_t scheme_end, host_end; - GetSchemeAndHostEnd(entry.url(), &scheme_end, &host_end); - new_entry->set_url(GURL(entry.url().spec().substr(0, host_end))); + GetSchemeAndHostEnd(committed_entry->url(), &scheme_end, &host_end); + committed_entry->set_url( + GURL(committed_entry->url().spec().substr(0, host_end))); } std::string content_state; state_->GetByteRepresentation(&content_state); - new_entry->set_content_state(content_state); - const int32 page_id = new_entry->page_id(); - - // 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); + committed_entry->set_content_state(content_state); + + // Broadcast the fact that we just updated all that crap. + controller()->NotifyEntryChanged( + committed_entry, + controller()->GetIndexOfEntry(committed_entry)); return true; } diff --git a/chrome/browser/native_ui_contents.h b/chrome/browser/native_ui_contents.h index 50407b2..443e3be 100644 --- a/chrome/browser/native_ui_contents.h +++ b/chrome/browser/native_ui_contents.h @@ -52,7 +52,7 @@ class NativeUIContents : public TabContents, // // TabContents implementation // - virtual bool Navigate(const NavigationEntry& entry, bool reload); + virtual bool NavigateToPendingEntry(bool reload); virtual const std::wstring GetDefaultTitle() const; virtual SkBitmap GetFavIcon() const; virtual bool ShouldDisplayURL() { return false; } diff --git a/chrome/browser/navigation_controller.cc b/chrome/browser/navigation_controller.cc index df55f9b..d294ace 100644 --- a/chrome/browser/navigation_controller.cc +++ b/chrome/browser/navigation_controller.cc @@ -23,6 +23,63 @@ #include "net/base/net_util.h" #include "webkit/glue/webkit_glue.h" +namespace { + +// Invoked when entries have been pruned, or removed. For example, if the +// current entries are [google, digg, yahoo], with the current entry google, +// and the user types in cnet, then digg and yahoo are pruned. +void NotifyPrunedEntries(NavigationController* nav_controller) { + NotificationService::current()->Notify( + NOTIFY_NAV_LIST_PRUNED, + Source<NavigationController>(nav_controller), + NotificationService::NoDetails()); +} + +// Ensure the given NavigationEntry has a valid state, so that WebKit does not +// get confused if we navigate back to it. +// +// An empty state is treated as a new navigation by WebKit, which would mean +// losing the navigation entries and generating a new navigation entry after +// this one. We don't want that. To avoid this we create a valid state which +// WebKit will not treat as a new navigation. +void SetContentStateIfEmpty(NavigationEntry* entry) { + if (entry->content_state().empty() && + (entry->tab_type() == TAB_CONTENTS_WEB || + entry->tab_type() == TAB_CONTENTS_NEW_TAB_UI || + entry->tab_type() == TAB_CONTENTS_ABOUT_UI || + entry->tab_type() == TAB_CONTENTS_HTML_DIALOG)) { + entry->set_content_state( + webkit_glue::CreateHistoryStateForURL(entry->url())); + } +} + +// Configure all the NavigationEntries in entries for restore. This resets +// the transition type to reload and makes sure the content state isn't empty. +void ConfigureEntriesForRestore( + std::vector<linked_ptr<NavigationEntry> >* entries) { + for (size_t i = 0; i < entries->size(); ++i) { + // Use a transition type of reload so that we don't incorrectly increase + // the typed count. + (*entries)[i]->set_transition_type(PageTransition::RELOAD); + (*entries)[i]->set_restored(true); + // NOTE(darin): This code is only needed for backwards compat. + SetContentStateIfEmpty((*entries)[i].get()); + } +} + +// See NavigationController::IsURLInPageNavigation for how this works and why. +bool AreURLsInPageNavigation(const GURL& existing_url, const GURL& new_url) { + if (existing_url == new_url || !new_url.has_ref()) + return false; + + url_canon::Replacements<char> replacements; + replacements.ClearRef(); + return existing_url.ReplaceComponents(replacements) == + new_url.ReplaceComponents(replacements); +} + +} // namespace + // TabContentsCollector --------------------------------------------------- // We never destroy a TabContents synchronously because there are some @@ -101,20 +158,6 @@ static void CreateNavigationEntriesFromTabNavigations( } } -// Configure all the NavigationEntries in entries for restore. This resets -// the transition type to reload and makes sure the content state isn't empty. -static void ConfigureEntriesForRestore( - std::vector<linked_ptr<NavigationEntry> >* entries) { - for (size_t i = 0, count = entries->size(); i < count; ++i) { - // Use a transition type of reload so that we don't incorrectly increase - // the typed count. - (*entries)[i]->set_transition_type(PageTransition::RELOAD); - (*entries)[i]->set_restored(true); - // NOTE(darin): This code is only needed for backwards compat. - NavigationController::SetContentStateIfEmpty((*entries)[i].get()); - } -} - NavigationController::NavigationController(TabContents* contents, Profile* profile) : profile_(profile), @@ -182,7 +225,6 @@ TabContents* NavigationController::GetTabContents(TabContentsType t) { } void NavigationController::Reload() { - // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()? DiscardPendingEntryInternal(); int current_index = GetCurrentEntryIndex(); if (check_for_repost_ && current_index != -1 && @@ -204,7 +246,6 @@ void NavigationController::Reload() { if (current_index == -1) return; - // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()? DiscardPendingEntryInternal(); pending_entry_index_ = current_index; @@ -223,8 +264,6 @@ void NavigationController::LoadEntry(NavigationEntry* entry) { // When navigating to a new page, we don't know for sure if we will actually // end up leaving the current page. The new page load could for example // result in a download or a 'no content' response (e.g., a mailto: URL). - - // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()? DiscardPendingEntryInternal(); pending_entry_ = entry; NotificationService::current()->Notify( @@ -234,24 +273,6 @@ void NavigationController::LoadEntry(NavigationEntry* entry) { NavigateToPendingEntry(false); } -/* static */ -void NavigationController::SetContentStateIfEmpty( - NavigationEntry* entry) { - if (entry->content_state().empty() && - (entry->tab_type() == TAB_CONTENTS_WEB || - entry->tab_type() == TAB_CONTENTS_NEW_TAB_UI || - entry->tab_type() == TAB_CONTENTS_ABOUT_UI || - entry->tab_type() == TAB_CONTENTS_HTML_DIALOG)) { - // The state is empty and the url will be rendered by WebKit. An empty - // state is treated as a new navigation by WebKit, which would mean - // losing the navigation entries and generating a new navigation - // entry after this one. We don't want that. To avoid this we create - // a valid state which WebKit will not treat as a new navigation. - entry->set_content_state( - webkit_glue::CreateHistoryStateForURL(entry->url())); - } -} - NavigationEntry* NavigationController::GetActiveEntry() const { NavigationEntry* entry = pending_entry_; if (!entry) @@ -279,11 +300,6 @@ NavigationEntry* NavigationController::GetEntryAtOffset(int offset) const { return entries_[index].get(); } -bool NavigationController::CanStop() const { - // TODO(darin): do we have something pending that we can stop? - return false; -} - bool NavigationController::CanGoBack() const { return entries_.size() > 1 && GetCurrentEntryIndex() > 0; } @@ -343,14 +359,6 @@ void NavigationController::GoToOffset(int offset) { GoToIndex(index); } -void NavigationController::Stop() { - DCHECK(CanStop()); - - // TODO(darin): we probably want to just call Stop on the active tab - // contents, but should we also call DiscardPendingEntry? - NOTREACHED() << "implement me"; -} - void NavigationController::ReloadDontCheckForRepost() { Reload(); } @@ -459,7 +467,6 @@ void NavigationController::LoadURLLazily(const GURL& url, if (icon) entry->favicon().set_bitmap(*icon); - // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()? DiscardPendingEntryInternal(); pending_entry_ = entry; load_pending_entry_when_active_ = true; @@ -493,105 +500,351 @@ void NavigationController::SetAlternateNavURLFetcher( alternate_nav_url_fetcher_entry_unique_id_ = pending_entry_->unique_id(); } -void NavigationController::DidNavigateToEntry(NavigationEntry* entry, - LoadCommittedDetails* details) { - DCHECK(active_contents_); - DCHECK(entry->tab_type() == active_contents_->type()); +bool NavigationController::RendererDidNavigate( + const ViewHostMsg_FrameNavigate_Params& params, + bool is_interstitial, + LoadCommittedDetails* details) { + // Save the previous URL before we clobber it. + if (GetLastCommittedEntry()) + details->previous_url = GetLastCommittedEntry()->url(); - SetContentStateIfEmpty(entry); + // Assign the current site instance to any pending entry, so we can find it + // later by calling GetEntryIndexWithPageID. We only care about this if the + // pending entry is an existing navigation and not a new one (or else we + // wouldn't care about finding it with GetEntryIndexWithPageID). + // + // TODO(brettw) this seems slightly bogus as we don't really know if the + // pending entry is what this navigation is for. There is a similar TODO + // w.r.t. the pending entry in RendererDidNavigateToNewPage. + if (pending_entry_index_ >= 0) + pending_entry_->set_site_instance(active_contents_->GetSiteInstance()); + + // Do navigation-type specific actions. These will make and commit an entry. + switch (ClassifyNavigation(params)) { + case NAV_NEW_PAGE: + RendererDidNavigateToNewPage(params); + break; + case NAV_EXISTING_PAGE: + RendererDidNavigateToExistingPage(params); + break; + case NAV_SAME_PAGE: + RendererDidNavigateToSamePage(params); + break; + case NAV_IN_PAGE: + RendererDidNavigateInPage(params); + break; + case NAV_NEW_SUBFRAME: + RendererDidNavigateNewSubframe(params); + break; + case NAV_AUTO_SUBFRAME: + if (!RendererDidNavigateAutoSubframe(params)) + return false; + break; + case NAV_IGNORE: + // There is nothing we can do with this navigation, so we just return to + // the caller that nothing has happened. + return false; + default: + NOTREACHED(); + } - entry->set_restored(false); + // All committed entries should have nonempty content state so WebKit doesn't + // get confused when we go back to them (see the function for details). + SetContentStateIfEmpty(GetActiveEntry()); - // 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(); + // 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) && + !GetPendingEntry()) || + params.gesture == NavigationGestureAuto; - // 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 - // active WebContents, because we have just navigated to it. - DCHECK(entry->page_id() >= 0) << "Page ID must be set before calling us."; - if (entry->page_id() > GetMaxPageID()) { - InsertEntry(entry); - 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(); - return; + // Now prep the rest of the details for the notification and broadcast. + details->entry = GetActiveEntry(); + details->is_in_page = IsURLInPageNavigation(params.url); + details->is_main_frame = PageTransition::IsMainFrame(params.transition); + NotifyNavigationEntryCommitted(details); + + // Because this call may synchronously show an infobar, we do it last, to + // make sure all other state is stable and the infobar won't get blown away + // by some transition. + // + // TODO(brettw) bug 1324500: This logic should be moved out of here, it should + // listen for the notification instead. + if (alternate_nav_url_fetcher_.get()) + alternate_nav_url_fetcher_->OnNavigatedToEntry(); + + // Broadcast the NOTIFY_FRAME_PROVISIONAL_LOAD_COMMITTED notification for use + // by the SSL manager. + // + // TODO(brettw) bug 1352803: this information should be combined with + // NOTIFY_NAV_ENTRY_COMMITTED so this one can be deleted. + ProvisionalLoadDetails provisional_details(details->is_main_frame, + is_interstitial, + details->is_in_page, + params.url, + params.security_info); + NotificationService::current()-> + Notify(NOTIFY_FRAME_PROVISIONAL_LOAD_COMMITTED, + Source<NavigationController>(this), + Details<ProvisionalLoadDetails>(&provisional_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(); + return true; +} + +NavigationController::NavClass NavigationController::ClassifyNavigation( + const ViewHostMsg_FrameNavigate_Params& params) const { + // If a page makes a popup navigated to about blank, and then writes stuff + // like a subframe navigated to a real site, we'll get a notification with an + // invalid page ID. There's nothing we can do with these, so just ignore them. + if (params.page_id == -1) { + DCHECK(!GetActiveEntry()) << "Got an invalid page ID but we seem to be " + " navigated to a valid page. This should be impossible."; + return NAV_IGNORE; + } + + if (params.page_id > active_contents_->GetMaxPageID()) { + // Greater page IDs than we've ever seen before are new pages. We may or may + // not have a pending entry for the page, and this may or may not be the + // main frame. + if (PageTransition::IsMainFrame(params.transition)) + return NAV_NEW_PAGE; + return NAV_NEW_SUBFRAME; } - // Otherwise, we just need to update an existing entry with matching PageID. - // If the existing entry corresponds to the entry which is pending, then we - // must update the current entry index accordingly. When navigating to the - // same URL, a new PageID is not created. - - int existing_entry_index = GetEntryIndexWithPageID(entry->tab_type(), - entry->site_instance(), - entry->page_id()); - NavigationEntry* existing_entry = (existing_entry_index != -1) ? - entries_[existing_entry_index].get() : NULL; - if (!existing_entry) { - // No existing entry, then simply ignore this navigation! - DLOG(WARNING) << "ignoring navigation for page: " << entry->page_id(); - } else if ((existing_entry != pending_entry_) && pending_entry_ && - (pending_entry_->page_id() == -1) && - (pending_entry_->url() == existing_entry->url())) { + // Now we know that the notification is for an existing page. Find that entry. + int existing_entry_index = GetEntryIndexWithPageID( + active_contents_->type(), + active_contents_->GetSiteInstance(), + params.page_id); + if (existing_entry_index == -1) { + // The page was not found. It could have been pruned because of the limit on + // back/forward entries (not likely since we'll usually tell it to navigate + // to such entries). It could also mean that the renderer is smoking crack. + NOTREACHED(); + return NAV_IGNORE; + } + NavigationEntry* existing_entry = entries_[existing_entry_index].get(); + + if (pending_entry_ && + pending_entry_->url() == params.url && + existing_entry != pending_entry_ && + pending_entry_->page_id() == -1 && + pending_entry_->url() == existing_entry->url()) { // In this case, we have a pending entry for a URL but WebCore didn't do a // new navigation. This happens when you press enter in the URL bar to - // reload. We will create a pending entry, but WebCore will convert it to + // reload. We will create a pending entry, but WebKit will convert it to // a reload since it's the same page and not create a new entry for it // (the user doesn't want to have a new back/forward entry when they do - // this). In this case, we want to just ignore the pending entry and go back - // to where we were. - existing_entry->set_unique_id(pending_entry_->unique_id()); - DiscardPendingEntry(); - } else { - DCHECK(existing_entry != entry); - // The given entry might provide a new URL... e.g., navigating back to a - // page in session history could have resulted in a new client redirect. - // The given entry might also provide a new title (typically an empty title - // to overwrite the existing title). - existing_entry->set_url(entry->url()); - existing_entry->set_title(entry->title()); - existing_entry->favicon() = entry->favicon(); - existing_entry->set_content_state(entry->content_state()); - - // TODO(brettw) why only copy the security style and no other SSL stuff? - existing_entry->ssl().set_security_style(entry->ssl().security_style()); - - const int prev_entry_index = last_committed_entry_index_; - if (existing_entry == pending_entry_) { - DCHECK(pending_entry_index_ != -1); - last_committed_entry_index_ = pending_entry_index_; - // TODO(pkasting): http://b/1113085 Should this use DiscardPendingEntry()? - DiscardPendingEntryInternal(); - } else { - // NOTE: Do not update the unique ID here, as we don't want infobars etc. - // to dismiss. + // this). In this case, we want to just ignore the pending entry and go + // back to where we were (the "existing entry"). + return NAV_SAME_PAGE; + } - // The navigation could have been issued by the renderer, so be sure that - // we update our current index. - last_committed_entry_index_ = existing_entry_index; - } - IndexOfActiveEntryChanged(prev_entry_index); + if (AreURLsInPageNavigation(existing_entry->url(), params.url)) + return NAV_IN_PAGE; + + if (!PageTransition::IsMainFrame(params.transition)) + return NAV_AUTO_SUBFRAME; // All manual subframes would get new IDs and + // were handled above. + // Since we weeded out "new" navigations above, we know this is an existing + // navigation. + return NAV_EXISTING_PAGE; +} + +void NavigationController::RendererDidNavigateToNewPage( + const ViewHostMsg_FrameNavigate_Params& params) { + NavigationEntry* new_entry; + if (pending_entry_) { + // TODO(brettw) this assumes that the pending entry is appropriate for the + // new page that was just loaded. I don't think this is necessarily the + // case! We should have some more tracking to know for sure. This goes along + // with a similar TODO at the top of RendererDidNavigate where we blindly + // set the site instance on the pending entry. + new_entry = new NavigationEntry(*pending_entry_); + + // Don't use the page type from the pending entry. Some interstitial page + // may have set the type to interstitial. Once we commit, however, the page + // type must always be normal. + new_entry->set_page_type(NavigationEntry::NORMAL_PAGE); + } else { + new_entry = new NavigationEntry(active_contents_->type()); } - delete entry; - NotifyNavigationEntryCommitted(details); + new_entry->set_url(params.url); + new_entry->set_page_id(params.page_id); + new_entry->set_transition_type(params.transition); + new_entry->set_site_instance(active_contents_->GetSiteInstance()); + new_entry->set_has_post_data(params.is_post); - if (alternate_nav_url_fetcher_.get()) { - // Because this call may synchronously show an infobar, we do it last, to - // make sure all other state is stable and the infobar won't get blown away - // by some transition. - alternate_nav_url_fetcher_->OnNavigatedToEntry(); + InsertEntry(new_entry); +} + +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, and we just need to update it with new information + // from the renderer. + int entry_index = GetEntryIndexWithPageID( + active_contents_->type(), + active_contents_->GetSiteInstance(), + params.page_id); + DCHECK(entry_index >= 0 && + entry_index < static_cast<int>(entries_.size())); + NavigationEntry* entry = entries_[entry_index].get(); + + // The URL may have changed due to redirects. The site instance will normally + // be the same except during session restore, when no site instance will be + // assigned. + entry->set_url(params.url); + DCHECK(entry->site_instance() == NULL || + entry->site_instance() == active_contents_->GetSiteInstance()); + entry->set_site_instance(active_contents_->GetSiteInstance()); + + // The entry we found in the list might be pending if the user hit + // back/forward/reload. This load should commit it (since it's already in the + // list, we can just discard the pending pointer). + // + // Note that we need to use the "internal" version since we don't want to + // actually change any other state, just kill the pointer. + if (entry == pending_entry_) + DiscardPendingEntryInternal(); + + int old_committed_entry_index = last_committed_entry_index_; + last_committed_entry_index_ = entry_index; + IndexOfActiveEntryChanged(old_committed_entry_index); +} + +void NavigationController::RendererDidNavigateToSamePage( + const ViewHostMsg_FrameNavigate_Params& params) { + // This mode implies we have a pending entry that's the same as an existing + // entry for this page ID. All we need to do is update the existing entry. + NavigationEntry* existing_entry = GetEntryWithPageID( + active_contents_->type(), + active_contents_->GetSiteInstance(), + params.page_id); + + // We assign the entry's unique ID to be that of the new one. Since this is + // always the result of a user action, we want to dismiss infobars, etc. like + // a regular user-initiated navigation. + existing_entry->set_unique_id(pending_entry_->unique_id()); + + DiscardPendingEntry(); +} + +void NavigationController::RendererDidNavigateInPage( + const ViewHostMsg_FrameNavigate_Params& params) { + DCHECK(PageTransition::IsMainFrame(params.transition)) << + "WebKit should only tell us about in-page navs for the main frame."; + // We're guaranteed to have an entry for this one. + NavigationEntry* existing_entry = GetEntryWithPageID( + active_contents_->type(), + active_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); +} + +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()); + NavigationEntry* new_entry = new NavigationEntry(*GetLastCommittedEntry()); + new_entry->set_page_id(params.page_id); + InsertEntry(new_entry); +} + +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 + // header file. In case "1." this will be a NOP. + int entry_index = GetEntryIndexWithPageID( + active_contents_->type(), + active_contents_->GetSiteInstance(), + params.page_id); + if (entry_index < 0 || + entry_index >= static_cast<int>(entries_.size())) { + NOTREACHED(); + return false; } - // 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(); + // Update the current navigation entry in case we're going back/forward. + if (entry_index != last_committed_entry_index_) { + int old_committed_entry_index = last_committed_entry_index_; + last_committed_entry_index_ = entry_index; + IndexOfActiveEntryChanged(old_committed_entry_index); + return true; + } + return false; } +void NavigationController::CommitPendingEntry() { + if (!GetPendingEntry()) + return; // Nothing to do. + + // Need to save the previous URL for the notification. + LoadCommittedDetails details; + if (GetLastCommittedEntry()) + details.previous_url = GetLastCommittedEntry()->url(); + + if (pending_entry_index_ >= 0) { + // This is a previous navigation (back/forward) that we're just now + // committing. Just mark it as committed. + int new_entry_index = pending_entry_index_; + DiscardPendingEntryInternal(); + + // Mark that entry as committed. + int old_committed_entry_index = last_committed_entry_index_; + last_committed_entry_index_ = new_entry_index; + IndexOfActiveEntryChanged(old_committed_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 + // TabContents types. For WebContents, the IDs are generated by the + // renderer, so we can't do this. + pending_entry_->set_page_id(active_contents_->GetMaxPageID() + 1); + active_contents_->UpdateMaxPageID(pending_entry_->page_id()); + InsertEntry(new NavigationEntry(*pending_entry_)); + } + + // 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); +} int NavigationController::GetIndexOfEntry( const NavigationEntry* entry) const { @@ -602,7 +855,7 @@ int NavigationController::GetIndexOfEntry( return (i == entries_.end()) ? -1 : static_cast<int>(i - entries_.begin()); } -void NavigationController::RemoveLastEntry() { +void NavigationController::RemoveLastEntryForInterstitial() { int current_size = static_cast<int>(entries_.size()); if (current_size > 0) { @@ -612,13 +865,51 @@ void NavigationController::RemoveLastEntry() { entries_.pop_back(); - if (last_committed_entry_index_ >= current_size - 1) + if (last_committed_entry_index_ >= current_size - 1) { last_committed_entry_index_ = current_size - 2; - NotifyPrunedEntries(); + // Broadcast the notification of the navigation. This is kind of a hack, + // since the navigation wasn't actually committed. But this function is + // used for interstital pages, and the UI needs to get updated when the + // interstitial page + LoadCommittedDetails details; + details.entry = GetActiveEntry(); + details.is_auto = false; + details.is_in_page = false; + details.is_main_frame = true; + NotifyNavigationEntryCommitted(&details); + } + + NotifyPrunedEntries(this); } } +void NavigationController::AddDummyEntryForInterstitial( + const NavigationEntry& clone_me) { + // We need to send a commit notification for this transition. + LoadCommittedDetails details; + if (GetLastCommittedEntry()) + details.previous_url = GetLastCommittedEntry()->url(); + + NavigationEntry* new_entry = new NavigationEntry(clone_me); + InsertEntry(new_entry); + // Watch out, don't use clone_me after this. The caller may have passed in a + // reference to our pending entry, which means it would have been destroyed. + + details.is_auto = false; + details.entry = GetActiveEntry(); + details.is_in_page = false; + details.is_main_frame = true; + NotifyNavigationEntryCommitted(&details); +} + +bool NavigationController::IsURLInPageNavigation(const GURL& url) const { + NavigationEntry* last_committed = GetLastCommittedEntry(); + if (!last_committed) + return false; + return AreURLsInPageNavigation(last_committed->url(), url); +} + void NavigationController::DiscardPendingEntry() { DiscardPendingEntryInternal(); @@ -675,7 +966,7 @@ void NavigationController::InsertEntry(NavigationEntry* entry) { current_size--; } if (pruned) // Only notify if we did prune something. - NotifyPrunedEntries(); + NotifyPrunedEntries(this); } if (entries_.size() >= max_entry_count_) @@ -683,7 +974,12 @@ void NavigationController::InsertEntry(NavigationEntry* entry) { entries_.push_back(linked_ptr<NavigationEntry>(entry)); last_committed_entry_index_ = static_cast<int>(entries_.size()) - 1; + + // This is a new page ID, so we need everybody to know about it. + active_contents_->UpdateMaxPageID(entry->page_id()); + // TODO(brettw) this seems bogus. The tab contents can listen for the + // notification or use the details that we pass back to it. active_contents_->NotifyDidNavigate(NAVIGATION_NEW, 0); } @@ -723,7 +1019,8 @@ void NavigationController::NavigateToPendingEntry(bool reload) { from_contents->delegate()->ReplaceContents(from_contents, contents); } - if (!contents->Navigate(*pending_entry_, reload)) + NavigationEntry temp_entry(*pending_entry_); + if (!contents->NavigateToPendingEntry(reload)) DiscardPendingEntry(); } @@ -755,20 +1052,16 @@ void NavigationController::NotifyNavigationEntryCommitted( Details<LoadCommittedDetails>(details)); } -void NavigationController::NotifyPrunedEntries() { - NotificationService::current()->Notify(NOTIFY_NAV_LIST_PRUNED, - Source<NavigationController>(this), - NotificationService::NoDetails()); -} - -void NavigationController::IndexOfActiveEntryChanged( - int prev_committed_index) { +void NavigationController::IndexOfActiveEntryChanged(int prev_committed_index) { NavigationType nav_type = NAVIGATION_BACK_FORWARD; int relative_navigation_offset = GetLastCommittedEntryIndex() - prev_committed_index; - if (relative_navigation_offset == 0) { + if (relative_navigation_offset == 0) nav_type = NAVIGATION_REPLACE; - } + + // TODO(brettw) I don't think this call should be necessary. There is already + // a notification of this event that could be used, or maybe all the tab + // contents' know when we navigate (WebContents does). active_contents_->NotifyDidNavigate(nav_type, relative_navigation_offset); } @@ -819,15 +1112,6 @@ void NavigationController::RegisterTabContents(TabContents* some_contents) { some_contents->AsDOMUIHost()->AttachMessageHandlers(); } -void NavigationController::NotifyEntryChangedByPageID( - TabContentsType type, - SiteInstance *instance, - int32 page_id) { - int index = GetEntryIndexWithPageID(type, instance, page_id); - if (index != -1) - NotifyEntryChanged(entries_[index].get(), index); -} - // static void NavigationController::DisablePromptOnRepost() { check_for_repost_ = false; @@ -888,10 +1172,6 @@ void NavigationController::RemoveEntryAtIndex(int index) { // session service can stay in sync. } -int NavigationController::GetMaxPageID() const { - return active_contents_->GetMaxPageID(); -} - NavigationController* NavigationController::Clone(HWND parent_hwnd) { NavigationController* nc = new NavigationController(NULL, profile_); @@ -974,18 +1254,6 @@ void NavigationController::DiscardPendingEntryInternal() { int NavigationController::GetEntryIndexWithPageID( TabContentsType type, SiteInstance* instance, int32 page_id) const { - // The instance should only be specified for contents displaying web pages. - // TODO(evanm): checking against NEW_TAB_UI and HTML_DLG here is lame. - // It'd be nice for DomUIHost to just use SiteInstances for keeping content - // separated properly. - if (type != TAB_CONTENTS_WEB && - type != TAB_CONTENTS_NEW_TAB_UI && - type != TAB_CONTENTS_ABOUT_UI && - type != TAB_CONTENTS_HTML_DIALOG && - type != TAB_CONTENTS_VIEW_SOURCE && - type != TAB_CONTENTS_DEBUGGER) - DCHECK(instance == NULL); - for (int i = static_cast<int>(entries_.size()) - 1; i >= 0; --i) { if ((entries_[i]->tab_type() == type) && (entries_[i]->site_instance() == instance) && diff --git a/chrome/browser/navigation_controller.h b/chrome/browser/navigation_controller.h index 976313e..f099704 100644 --- a/chrome/browser/navigation_controller.h +++ b/chrome/browser/navigation_controller.h @@ -5,7 +5,8 @@ #ifndef CHROME_BROWSER_NAVIGATION_CONTROLLER_H_ #define CHROME_BROWSER_NAVIGATION_CONTROLLER_H_ -#include "base/hash_tables.h" +#include <map> + #include "base/linked_ptr.h" #include "base/ref_counted.h" #include "chrome/browser/alternate_nav_url_fetcher.h" @@ -20,24 +21,17 @@ class TabContents; class WebContents; class TabContentsCollector; struct TabNavigation; +struct ViewHostMsg_FrameNavigate_Params; -namespace printing { -class PrintViewManager; -} - -//////////////////////////////////////////////////////////////////////////////// -// -// NavigationController class -// -// A NavigationController maintains navigation data. We have one -// NavigationController instance per tab. +// A NavigationController maintains the back-forward list for a single tab and +// manages all navigation within that list. // // The NavigationController also owns all TabContents for the tab. This is to // make sure that we have at most one TabContents instance per type. -// -//////////////////////////////////////////////////////////////////////////////// class NavigationController { public: + // Notification details ------------------------------------------------------ + // Provides the details for a NOTIFY_NAV_ENTRY_CHANGED notification. struct EntryChangedDetails { // The changed navigation entry after it has been updated. @@ -47,6 +41,7 @@ class NavigationController { int index; }; + // Provides the details for a NOTIFY_NAV_ENTRY_COMMITTED notification. struct LoadCommittedDetails { // By default, the entry will be filled according to a new main frame // navigation. @@ -87,7 +82,10 @@ class NavigationController { } }; + // --------------------------------------------------------------------------- + NavigationController(TabContents* initial_contents, Profile* profile); + // Creates a NavigationController from the specified history. Processing // for this is asynchronous and handled via the RestoreHelper (in // navigation_controller.cc). @@ -98,8 +96,24 @@ class NavigationController { HWND parent); ~NavigationController(); - // Same as Reload, but doesn't check if current entry has POST data. - void ReloadDontCheckForRepost(); + // Begin the destruction sequence for this NavigationController and all its + // registered tabs. The sequence is as follows: + // 1. All tabs are asked to Destroy themselves. + // 2. When each tab is finished Destroying, it will notify the controller. + // 3. Once all tabs are Destroyed, the NavigationController deletes itself. + // This ensures that all the TabContents outlive the NavigationController. + void Destroy(); + + // Clone the receiving navigation controller. Only the active tab contents is + // duplicated. It is created as a child of the provided HWND. + NavigationController* Clone(HWND hwnd); + + // Returns the profile for this controller. It can never be NULL. + Profile* profile() const { + return profile_; + } + + // Active entry -------------------------------------------------------------- // Returns the active entry, which is the pending entry if a navigation is in // progress or the last committed entry otherwise. NOTE: This can be NULL!! @@ -114,19 +128,7 @@ class NavigationController { // it is the pending_entry_index_. int GetCurrentEntryIndex() const; - // Returns the pending entry corresponding to the navigation that is - // currently in progress, or null if there is none. - NavigationEntry* GetPendingEntry() const { - return pending_entry_; - } - - // Returns the index of the pending entry or -1 if the pending entry - // corresponds to a new navigation (created via LoadURL). - int GetPendingEntryIndex() const { - return pending_entry_index_; - } - - // Returns the last committed entry, which may be null if there are no + // Returns the last committed entry, which may be null if there are no // committed entries. NavigationEntry* GetLastCommittedEntry() const; @@ -135,6 +137,8 @@ class NavigationController { return last_committed_entry_index_; } + // Navigation list ----------------------------------------------------------- + // Returns the number of entries in the NavigationControllerBase, excluding // the pending entry if there is one. int GetEntryCount() const { @@ -149,85 +153,155 @@ class NavigationController { // if out of bounds. NavigationEntry* GetEntryAtOffset(int offset) const; - bool CanStop() const; + // Returns the index of the specified entry, or -1 if entry is not contained + // in this NavigationControllerBase. + int GetIndexOfEntry(const NavigationEntry* entry) const; - // Return whether this controller can go back. - bool CanGoBack() const; + // Return the index of the entry with the corresponding type, instance, and + // page_id, or -1 if not found. Use a NULL instance if the type is not + // TAB_CONTENTS_WEB. + int GetEntryIndexWithPageID(TabContentsType type, + SiteInstance* instance, + int32 page_id) const; - // Return whether this controller can go forward. - bool CanGoForward() const; + // Return the entry with the corresponding type, instance, and page_id, or + // NULL if not found. Use a NULL instance if the type is not + // TAB_CONTENTS_WEB. + NavigationEntry* GetEntryWithPageID(TabContentsType type, + SiteInstance* instance, + int32 page_id) const; - // Causes the controller to go back. - void GoBack(); + // Pending entry ------------------------------------------------------------- + + // Commits the current pending entry and issues the NOTIFY_NAV_ENTRY_COMMIT + // notification. No changes are made to the entry during this process, it is + // just moved from pending to committed. This is an alternative to + // RendererDidNavigate for simple TabContents types. + // + // When the pending entry is a new navigation, it will have a page ID of -1. + // The caller should leave this as-is. CommitPendingEntry will generate a + // new page ID for you and update the TabContents with that ID. + void CommitPendingEntry(); + + // Calling this may cause the active tab contents to switch if the current + // entry corresponds to a different tab contents type. + void DiscardPendingEntry(); + + // Returns the pending entry corresponding to the navigation that is + // currently in progress, or null if there is none. + NavigationEntry* GetPendingEntry() const { + return pending_entry_; + } + + // Returns the index of the pending entry or -1 if the pending entry + // corresponds to a new navigation (created via LoadURL). + int GetPendingEntryIndex() const { + return pending_entry_index_; + } + + // New navigations ----------------------------------------------------------- + + // Loads the specified URL. + void LoadURL(const GURL& url, PageTransition::Type type); - // Causes the controller to go forward. + // Load the specified URL the next time it becomes active. + void LoadURLLazily(const GURL& url, PageTransition::Type type, + const std::wstring& title, SkBitmap* icon); + + // Loads the current page if this NavigationController was restored from + // history and the current page has not loaded yet. + void LoadIfNecessary(); + + // Renavigation -------------------------------------------------------------- + + // Navigation relative to the "current entry" + bool CanGoBack() const; + bool CanGoForward() const; + void GoBack(); void GoForward(); - // Causes the controller to go to the specified index. + // Navigates to the specified absolute index. void GoToIndex(int index); - // Causes the controller to go to the specified offset from current. Does - // nothing if out of bounds. + // Navigates to the specified offset from the "current entry". Does nothing if + // the offset is out of bounds. void GoToOffset(int offset); - // Causes the controller to stop a pending navigation if any. - void Stop(); - - // Causes the controller to reload the current entry. Will prompt the user if - // reloading a URL with POST data and the active WebContents isn't showing the - // POST interstitial page. + // Reloads the current entry. The user will be prompted if the URL has POST + // data and the active WebContents isn't showing the POST interstitial page. void Reload(); - // Return the entry with the corresponding type, instance, and page_id, or - // NULL if not found. Use a NULL instance if the type is not - // TAB_CONTENTS_WEB. - NavigationEntry* GetEntryWithPageID(TabContentsType type, - SiteInstance* instance, - int32 page_id) const; - - // Causes the controller to load the specified entry. The controller - // assumes ownership of the entry. - // NOTE: Do not pass an entry that the controller already owns! - void LoadEntry(NavigationEntry* entry); - - // Ensure the given NavigationEntry has a valid state, so that WebKit does - // not get confused. - static void SetContentStateIfEmpty(NavigationEntry* entry); + // Same as Reload, but doesn't check if current entry has POST data. + void ReloadDontCheckForRepost(); - // Begin the destruction sequence for this NavigationController and all its - // registered tabs. The sequence is as follows: - // 1. All tabs are asked to Destroy themselves. - // 2. When each tab is finished Destroying, it will notify the controller. - // 3. Once all tabs are Destroyed, the NavigationController deletes itself. - // This ensures that all the TabContents outlive the NavigationController. - void Destroy(); + // TabContents --------------------------------------------------------------- // Notifies the controller that a TabContents that it owns has been destroyed. // This is part of the NavigationController's Destroy sequence. void TabContentsWasDestroyed(TabContentsType type); + // Returns the TabContents cached on this controller for the given type or + // NULL if there is none. + TabContents* GetTabContents(TabContentsType type); + // Returns the currently-active TabContents associated with this controller. // You should use GetActiveEntry instead of this in most cases. TabContents* active_contents() const { return active_contents_; } - // This can never be null. - Profile* profile() const { - return profile_; - } + // For use by TabContents ---------------------------------------------------- - // Returns the TabContents cached on this controller for the given type or - // NULL if there is none. - TabContents* GetTabContents(TabContentsType type); + // Handles updating the navigation state after the renderer has navigated. + // This is used by the WebContents. Simpler tab contents types can use + // CommitPendingEntry below. + // + // If a new entry is created, it will return true and will have filled the + // given details structure and broadcast the NOTIFY_NAV_ENTRY_COMMITTED + // notification. The caller can then use the details without worrying about + // listening for the notification. + // + // In the case that nothing has changed, the details structure is undefined + // and it will return false. + bool RendererDidNavigate(const ViewHostMsg_FrameNavigate_Params& params, + bool is_interstitial, + LoadCommittedDetails* details); + + // Inserts a new entry by making a copy of the given navigation entry. This is + // used by interstitials to create dummy entries that they will be in charge + // of removing later. + void AddDummyEntryForInterstitial(const NavigationEntry& clone_me); + + // Removes the last entry in the list. This is used by the interstitial code + // to delete the dummy entry created by AddDummyEntryForInterstitial. If the + // last entry is the currently committed one, a ENTRY_COMMITTED notification + // will be broadcast. + void RemoveLastEntryForInterstitial(); + + // Notifies us that we just became active. This is used by the TabContents + // so that we know to load URLs that were pending as "lazy" loads. + void SetActive(bool is_active); - // Causes the controller to load the specified URL. - void LoadURL(const GURL& url, PageTransition::Type type); + // Broadcasts the NOTIFY_NAV_ENTRY_CHANGED notification for the given entry + // (which must be at the given index). This will keep things in sync like + // the saved session. + void NotifyEntryChanged(const NavigationEntry* entry, int index); - // Causes the controller to load the specified URL the next time it becomes - // active. - void LoadURLLazily(const GURL& url, PageTransition::Type type, - const std::wstring& title, SkBitmap* icon); + // Returns true if the given URL would be an in-page navigation (i.e. only + // the reference fragment is different) from the "last committed entry". We do + // not compare it against the "active entry" since the active entry can be + // pending and in page navigations only happen on committed pages. If there + // is no last committed entry, then nothing will be in-page. + // + // Special note: if the URLs are the same, it does NOT count as an in-page + // navigation. Neither does an input URL that has no ref, even if the rest is + // the same. This may seem weird, but when we're considering whether a + // navigation happened without loading anything, the same URL would be a + // reload, while only a different ref would be in-page (pages can't clear + // refs without reload, only change to "#" which we don't count as empty). + bool IsURLInPageNavigation(const GURL& url) const; + + // Random data --------------------------------------------------------------- // Returns true if this NavigationController is is configured to load a URL // lazily. If true, use GetLazyTitle() and GetLazyFavIcon() to discover the @@ -237,34 +311,10 @@ class NavigationController { const std::wstring& GetLazyTitle() const; const SkBitmap& GetLazyFavIcon() const; + // TODO(brettw) bug 1324500: move this out of here. void SetAlternateNavURLFetcher( AlternateNavURLFetcher* alternate_nav_url_fetcher); - // -------------------------------------------------------------------------- - // For use by TabContents implementors: - - // Used to inform the NavigationControllerBase of a navigation being committed - // 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. - // - // 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. - void DiscardPendingEntry(); - - // Inserts an entry after the current position, removing all entries after it. - // The new entry will become the active one. - void InsertEntry(NavigationEntry* entry); - // Returns the identifier used by session restore. const SessionID& session_id() const { return session_id_; } @@ -274,58 +324,97 @@ class NavigationController { SSLManager* ssl_manager() { return &ssl_manager_; } - // Broadcasts the NOTIFY_NAV_ENTRY_CHANGED notification for the - // navigation corresponding to the given page. This will keep things in sync - // like saved session. - void NotifyEntryChangedByPageID(TabContentsType type, - SiteInstance* instance, - int32 page_id); - - void SetActive(bool is_active); - - // If this NavigationController was restored from history and the selected - // page has not loaded, it is loaded. - void LoadIfNecessary(); - - // Clone the receiving navigation controller. Only the active tab contents is - // duplicated. It is created as a child of the provided HWND. - NavigationController* Clone(HWND hwnd); - // Returns true if a reload happens when activated (SetActive(true) is // invoked). This is true for session/tab restore and cloned tabs. bool needs_reload() const { return needs_reload_; } - // Disables checking for a repost and prompting the user. This is used during - // testing. - static void DisablePromptOnRepost(); - // Returns the largest restored page ID seen in this navigation controller, // if it was restored from a previous session. (-1 otherwise) int max_restored_page_id() const { return max_restored_page_id_; } - // Returns the index of the specified entry, or -1 if entry is not contained - // in this NavigationControllerBase. - int GetIndexOfEntry(const NavigationEntry* entry) const; - - // Removes the last committed entry. - void RemoveLastEntry(); + // Disables checking for a repost and prompting the user. This is used during + // testing. + static void DisablePromptOnRepost(); private: FRIEND_TEST(NavigationControllerTest, EnforceMaxNavigationCount); - class RestoreHelper; friend class RestoreHelper; + friend class TabContents; // For invoking OnReservedPageIDRange. + + // Indicates different types of navigations that can occur that we will handle + // separately. This is computed by ClassifyNavigation. + enum NavClass { + // A new page was navigated in the main frame. + NAV_NEW_PAGE, + + // Renavigating to an existing navigation entry. The entry is guaranteed to + // exist in the list, or else it would be a new page or IGNORE navigation. + NAV_EXISTING_PAGE, + + // The same page has been reloaded as a result of the user requesting + // navigation to that same page (like pressing Enter in the URL bar). This + // is not the same as an in-page navigation because we'll actually have a + // pending entry for the load, which is then meaningless. + NAV_SAME_PAGE, + + // In page navigations are when the reference fragment changes. This will + // be in the main frame only (we won't even get notified of in-page + // subframe navigations). It may be for any page, not necessarily the last + // committed one (for example, whey going back to a page with a ref). + NAV_IN_PAGE, + + // A new subframe was manually navigated by the user. We will create a new + // NavigationEntry so they can go back to the previous subframe content + // using the back button. + NAV_NEW_SUBFRAME, + + // A subframe in the page was automatically loaded or navigated to such that + // a new navigation entry should not be created. There are two cases: + // 1. Stuff like iframes containing ads that the page loads automatically. + // The user doesn't want to see these, so we just update the existing + // navigation entry. + // 2. Going back/forward to previous subframe navigations. We don't create + // a new entry here either, just update the last committed entry. + // These two cases are actually pretty different, they just happen to + // require almost the same code to handle. + NAV_AUTO_SUBFRAME, + + // Nothing happened. This happens when we get information about a page we + // don't know anything about. It can also happen when an iframe in a popup + // navigated to about:blank is navigated. Nothing needs to be done. + NAV_IGNORE, + }; - // For invoking OnReservedPageIDRange. - friend class TabContents; - // For invoking GetMaxPageID. - friend class WebContents; - // For invoking GetMaxPageID. - friend class printing::PrintViewManager; + // Classifies the given renderer navigation (see the NavigationType enum). + NavClass ClassifyNavigation( + const ViewHostMsg_FrameNavigate_Params& params) const; - // Returns the largest page ID seen. When PageIDs come in larger than - // this (via DidNavigateToEntry), we know that we've navigated to a new page. - int GetMaxPageID() const; + // Causes the controller to load the specified entry. The function assumes + // ownership of the pointer since it is put in the navigation list. + // NOTE: Do not pass an entry that the controller already owns! + void LoadEntry(NavigationEntry* entry); + + // Handlers for the different types of navigation types. They will actually + // handle the navigations corresponding to the different NavClasses above. + // They will NOT broadcast the commit notification, that should be handled by + // the caller. + // + // RendererDidNavigateAutoSubframe is special, it may not actually change + // anything if some random subframe is loaded. It will return true if anything + // changed, or false if not. + void RendererDidNavigateToNewPage( + const ViewHostMsg_FrameNavigate_Params& params); + void RendererDidNavigateToExistingPage( + const ViewHostMsg_FrameNavigate_Params& params); + void RendererDidNavigateToSamePage( + const ViewHostMsg_FrameNavigate_Params& params); + void RendererDidNavigateInPage( + const ViewHostMsg_FrameNavigate_Params& params); + void RendererDidNavigateNewSubframe( + const ViewHostMsg_FrameNavigate_Params& params); + bool RendererDidNavigateAutoSubframe( + const ViewHostMsg_FrameNavigate_Params& params); // Actually issues the navigation held in pending_entry. void NavigateToPendingEntry(bool reload); @@ -334,11 +423,6 @@ class NavigationController { // 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, - // and the user types in cnet, then digg and yahoo are pruned. - void NotifyPrunedEntries(); - // Invoked when the index of the active entry may have changed. // The prev_commited_index parameter specifies the previous value // of the last commited index before this navigation event happened @@ -354,9 +438,6 @@ class NavigationController { // and deleted by this navigation controller void RegisterTabContents(TabContents* some_contents); - // Broadcasts a notification that the given entry changed. - void NotifyEntryChanged(const NavigationEntry* entry, int index); - // Removes the entry at the specified index. Note that you should not remove // the pending entry or the last committed entry. void RemoveEntryAtIndex(int index); @@ -385,15 +466,14 @@ class NavigationController { // contents. void FinishRestore(HWND parent_hwnd, 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); + // Discards the pending entry without updating active_contents_ void DiscardPendingEntryInternal(); - // Return the index of the entry with the corresponding type, instance, and - // page_id, or -1 if not found. Use a NULL instance if the type is not - // TAB_CONTENTS_WEB. - int GetEntryIndexWithPageID(TabContentsType type, - SiteInstance* instance, - int32 page_id) const; + // --------------------------------------------------------------------------- // The user profile associated with this controller Profile* profile_; @@ -420,7 +500,7 @@ class NavigationController { // Tab contents. One entry per type used. The tab controller owns // every tab contents used. - typedef base::hash_map<TabContentsType, TabContents*> TabContentsMap; + typedef std::map<TabContentsType, TabContents*> TabContentsMap; TabContentsMap tab_contents_map_; // A map of TabContentsType -> TabContentsCollector containing all the diff --git a/chrome/browser/navigation_controller_unittest.cc b/chrome/browser/navigation_controller_unittest.cc index 035c2ad..cfd372b 100644 --- a/chrome/browser/navigation_controller_unittest.cc +++ b/chrome/browser/navigation_controller_unittest.cc @@ -10,6 +10,7 @@ #include "chrome/browser/navigation_entry.h" #include "chrome/browser/profile_manager.h" #include "chrome/browser/history/history.h" +#include "chrome/browser/session_service.h" #include "chrome/browser/session_service_test_helper.h" #include "chrome/browser/tab_contents.h" #include "chrome/browser/tab_contents_delegate.h" @@ -33,6 +34,10 @@ const TabContentsType kTestContentsType1 = const TabContentsType kTestContentsType2 = static_cast<TabContentsType>(TAB_CONTENTS_NUM_TYPES + 2); +// Tests can set this to set the site instance for all the test contents. This +// refcounted pointer will be automatically derefed on cleanup. +static SiteInstance* site_instance; + // TestContents ---------------------------------------------------------------- class TestContents : public TabContents { @@ -43,24 +48,41 @@ class TestContents : public TabContents { TestContents(TabContentsType type) : TabContents(type) { } - // Just record the navigation so it can be checked by the test case - bool Navigate(const NavigationEntry& entry, bool reload) { - pending_entry_.reset(new NavigationEntry(entry)); + // Overridden from TabContents so we can provide a non-NULL site instance in + // some cases. To use, the test will have to set the site_instance_ member + // variable to some site instance it creates. + virtual SiteInstance* GetSiteInstance() const { + return site_instance; + } + + // Just record the navigation so it can be checked by the test case. We don't + // want the normal behavior of TabContents just saying it committed since we + // want to behave more like the renderer and call RendererDidNavigate. + virtual bool NavigateToPendingEntry(bool reload) { + pending_entry_.reset(new NavigationEntry(*controller()->GetPendingEntry())); return true; } - void CompleteNavigation(int page_id) { - DCHECK(pending_entry_.get()); - pending_entry_->set_page_id(page_id); + // Sets up a call to RendererDidNavigate pretending to be a main frame + // navigation to the given URL. + void CompleteNavigationAsRenderer(int page_id, const GURL& url) { + ViewHostMsg_FrameNavigate_Params params; + params.page_id = page_id; + params.url = url; + params.transition = PageTransition::LINK; + params.should_update_history = false; + params.gesture = NavigationGestureUser; + params.is_post = false; + NavigationController::LoadCommittedDetails details; - DidNavigateToEntry(pending_entry_.get(), &details); - controller()->NotifyEntryChangedByPageID(type(), NULL, page_id); - pending_entry_.release(); + controller()->RendererDidNavigate(params, false, &details); } NavigationEntry* pending_entry() const { return pending_entry_.get(); } void set_pending_entry(NavigationEntry* e) { pending_entry_.reset(e); } + protected: + private: scoped_ptr<NavigationEntry> pending_entry_; }; @@ -116,6 +138,11 @@ class NavigationControllerTest : public testing::Test, } virtual void TearDown() { + if (site_instance) { + site_instance->Release(); + site_instance = NULL; + } + // Make sure contents is valid. NavigationControllerHistoryTest ends up // resetting this before TearDown is invoked. if (contents) @@ -300,7 +327,6 @@ TEST_F(NavigationControllerTest, Defaults) { EXPECT_EQ(contents->controller()->GetEntryCount(), 0); EXPECT_FALSE(contents->controller()->CanGoBack()); EXPECT_FALSE(contents->controller()->CanGoForward()); - EXPECT_FALSE(contents->controller()->CanStop()); } TEST_F(NavigationControllerTest, LoadURL) { @@ -329,9 +355,8 @@ TEST_F(NavigationControllerTest, LoadURL) { // We should have gotten no notifications from the preceeding checks. EXPECT_EQ(0, notifications.size()); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // The load should now be committed. EXPECT_EQ(contents->controller()->GetEntryCount(), 1); @@ -358,9 +383,8 @@ TEST_F(NavigationControllerTest, LoadURL) { EXPECT_FALSE(contents->controller()->CanGoForward()); EXPECT_EQ(contents->GetMaxPageID(), 0); - contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(1, url2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // The load should now be committed. EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -385,18 +409,15 @@ TEST_F(NavigationControllerTest, LoadURL_SamePage) { contents->controller()->LoadURL(url1, PageTransition::TYPED); EXPECT_EQ(0, notifications.size()); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); - + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->LoadURL(url1, PageTransition::TYPED); EXPECT_EQ(0, notifications.size()); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); - // should not have produced a new session history entry + // We should not have produced a new session history entry. EXPECT_EQ(contents->controller()->GetEntryCount(), 1); EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 0); EXPECT_EQ(contents->controller()->GetPendingEntryIndex(), -1); @@ -416,9 +437,8 @@ TEST_F(NavigationControllerTest, LoadURL_Discarded) { contents->controller()->LoadURL(url1, PageTransition::TYPED); EXPECT_EQ(0, notifications.size()); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->LoadURL(url2, PageTransition::TYPED); contents->controller()->DiscardPendingEntry(); @@ -443,18 +463,12 @@ TEST_F(NavigationControllerTest, LoadURL_NoPending) { // First make an existing committed entry. const GURL kExistingURL1("test1:eh"); contents->controller()->LoadURL(kExistingURL1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); - + contents->CompleteNavigationAsRenderer(0, kExistingURL1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + // Do a new navigation without making a pending one. const GURL kNewURL("test1:see"); - NavigationEntry* entry = new NavigationEntry(kTestContentsType1); - entry->set_page_id(2); - entry->set_url(kNewURL); - entry->set_title(L"Hello, world"); - NavigationController::LoadCommittedDetails details; - contents->controller()->DidNavigateToEntry(entry, &details); + contents->CompleteNavigationAsRenderer(99, kNewURL); // There should no longer be any pending entry, and the third navigation we // just made should be committed. @@ -475,9 +489,8 @@ TEST_F(NavigationControllerTest, LoadURL_NewPending) { // First make an existing committed entry. const GURL kExistingURL1("test1:eh"); contents->controller()->LoadURL(kExistingURL1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, kExistingURL1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // Make a pending entry to somewhere new. const GURL kExistingURL2("test1:bee"); @@ -486,18 +499,10 @@ TEST_F(NavigationControllerTest, LoadURL_NewPending) { // Before that commits, do a new navigation. const GURL kNewURL("test1:see"); - NavigationEntry* entry = new NavigationEntry(kTestContentsType1); - entry->set_page_id(3); - entry->set_url(kNewURL); - entry->set_title(L"Hello, world"); - NavigationController::LoadCommittedDetails details; - contents->controller()->DidNavigateToEntry(entry, &details); + contents->CompleteNavigationAsRenderer(3, kNewURL); // There should no longer be any pending entry, and the third navigation we // just made should be committed. - // Note that we don't expect a CHANGED notification. It turns out that this - // is sent by the TestContents and not any code we're interested in testing, - // and this function doesn't get called when we manually call the controller. EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); EXPECT_EQ(-1, contents->controller()->GetPendingEntryIndex()); EXPECT_EQ(1, contents->controller()->GetLastCommittedEntryIndex()); @@ -514,15 +519,13 @@ TEST_F(NavigationControllerTest, LoadURL_ExistingPending) { // First make some history. const GURL kExistingURL1("test1:eh"); contents->controller()->LoadURL(kExistingURL1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, kExistingURL1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); const GURL kExistingURL2("test1:bee"); contents->controller()->LoadURL(kExistingURL2, PageTransition::TYPED); - contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(1, kExistingURL2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // Now make a pending back/forward navigation. The zeroth entry should be // pending. @@ -533,12 +536,8 @@ TEST_F(NavigationControllerTest, LoadURL_ExistingPending) { // Before that commits, do a new navigation. const GURL kNewURL("test1:see"); - NavigationEntry* entry = new NavigationEntry(kTestContentsType1); - entry->set_page_id(3); - entry->set_url(kNewURL); - entry->set_title(L"Hello, world"); NavigationController::LoadCommittedDetails details; - contents->controller()->DidNavigateToEntry(entry, &details); + contents->CompleteNavigationAsRenderer(3, kNewURL); // There should no longer be any pending entry, and the third navigation we // just made should be committed. @@ -556,9 +555,8 @@ TEST_F(NavigationControllerTest, Reload) { contents->controller()->LoadURL(url1, PageTransition::TYPED); EXPECT_EQ(0, notifications.size()); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->Reload(); EXPECT_EQ(0, notifications.size()); @@ -572,9 +570,8 @@ TEST_F(NavigationControllerTest, Reload) { EXPECT_FALSE(contents->controller()->CanGoBack()); EXPECT_FALSE(contents->controller()->CanGoForward()); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // Now the reload is committed. EXPECT_EQ(contents->controller()->GetEntryCount(), 1); @@ -595,18 +592,16 @@ TEST_F(NavigationControllerTest, Reload_GeneratesNewPage) { const GURL url2("test1:foo2"); contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->Reload(); EXPECT_EQ(0, notifications.size()); contents->pending_entry()->set_url(url2); contents->pending_entry()->set_transition_type(PageTransition::LINK); - contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(1, url2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // Now the reload is committed. EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -624,17 +619,12 @@ TEST_F(NavigationControllerTest, Back) { RegisterForAllNavNotifications(¬ifications, contents->controller()); const GURL url1("test1:foo1"); - const GURL url2("test1:foo2"); - - contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); - contents->controller()->LoadURL(url2, PageTransition::TYPED); - contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + const GURL url2("test1:foo2"); + contents->CompleteNavigationAsRenderer(1, url2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->GoBack(); EXPECT_EQ(0, notifications.size()); @@ -648,9 +638,8 @@ TEST_F(NavigationControllerTest, Back) { EXPECT_FALSE(contents->controller()->CanGoBack()); EXPECT_TRUE(contents->controller()->CanGoForward()); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // The back navigation completed successfully. EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -672,14 +661,12 @@ TEST_F(NavigationControllerTest, Back_GeneratesNewPage) { const GURL url3("test1:foo3"); contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->LoadURL(url2, PageTransition::TYPED); - contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(1, url2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->GoBack(); EXPECT_EQ(0, notifications.size()); @@ -693,11 +680,8 @@ TEST_F(NavigationControllerTest, Back_GeneratesNewPage) { EXPECT_FALSE(contents->controller()->CanGoBack()); EXPECT_TRUE(contents->controller()->CanGoForward()); - contents->pending_entry()->set_url(url3); - contents->pending_entry()->set_transition_type(PageTransition::LINK); - contents->CompleteNavigation(2); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(2, url3); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // The back navigation resulted in a completely new navigation. // TODO(darin): perhaps this behavior will be confusing to users? @@ -720,15 +704,12 @@ TEST_F(NavigationControllerTest, Back_NewPending) { const GURL kUrl3("test1:foo3"); // First navigate two places so we have some back history. - contents->controller()->LoadURL(kUrl1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, kUrl1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); - contents->controller()->LoadURL(kUrl2, PageTransition::TYPED); - contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + //contents->controller()->LoadURL(kUrl2, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(1, kUrl2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // Now start a new pending navigation and go back before it commits. contents->controller()->LoadURL(kUrl3, PageTransition::TYPED); @@ -750,32 +731,23 @@ TEST_F(NavigationControllerTest, Back_OtherBackPending) { const GURL kUrl3("test1:foo3"); // First navigate three places so we have some back history. - contents->controller()->LoadURL(kUrl1, PageTransition::TYPED); - contents->CompleteNavigation(0); - contents->controller()->LoadURL(kUrl2, PageTransition::TYPED); - contents->CompleteNavigation(1); - contents->controller()->LoadURL(kUrl3, PageTransition::TYPED); - contents->CompleteNavigation(2); + contents->CompleteNavigationAsRenderer(0, kUrl1); + contents->CompleteNavigationAsRenderer(1, kUrl2); + contents->CompleteNavigationAsRenderer(2, kUrl3); // With nothing pending, say we get a navigation to the second entry. - const std::wstring kNewTitle1(L"Hello, world"); - NavigationEntry* entry = new NavigationEntry(kTestContentsType1); - entry->set_page_id(1); - entry->set_url(kUrl2); - entry->set_title(kNewTitle1); - NavigationController::LoadCommittedDetails details; - contents->controller()->DidNavigateToEntry(entry, &details); + contents->CompleteNavigationAsRenderer(1, kUrl2); // That second URL should be the last committed and it should have gotten the // new title. - EXPECT_EQ(kNewTitle1, contents->controller()->GetEntryWithPageID( - kTestContentsType1, NULL, 1)->title()); + EXPECT_EQ(kUrl2, contents->controller()->GetEntryWithPageID( + kTestContentsType1, NULL, 1)->url()); EXPECT_EQ(1, contents->controller()->GetLastCommittedEntryIndex()); EXPECT_EQ(-1, contents->controller()->GetPendingEntryIndex()); // Now go forward to the last item again and say it was committed. contents->controller()->GoForward(); - contents->CompleteNavigation(2); + contents->CompleteNavigationAsRenderer(2, kUrl3); // Now start going back one to the second page. It will be pending. contents->controller()->GoBack(); @@ -784,21 +756,14 @@ TEST_F(NavigationControllerTest, Back_OtherBackPending) { // Not synthesize a totally new back event to the first page. This will not // match the pending one. - const std::wstring kNewTitle2(L"Hello, world"); - entry = new NavigationEntry(kTestContentsType1); - entry->set_page_id(0); - entry->set_url(kUrl1); - entry->set_title(kNewTitle2); - contents->controller()->DidNavigateToEntry(entry, &details); + contents->CompleteNavigationAsRenderer(0, kUrl1); // The navigation should not have affected the pending entry. EXPECT_EQ(1, contents->controller()->GetPendingEntryIndex()); - // But the navigated entry should be updated to the new title, and should be - // the last committed. - EXPECT_EQ(kNewTitle2, contents->controller()->GetEntryWithPageID( - kTestContentsType1, NULL, 0)->title()); + // But the navigated entry should be the last committed. EXPECT_EQ(0, contents->controller()->GetLastCommittedEntryIndex()); + EXPECT_EQ(kUrl1, contents->controller()->GetLastCommittedEntry()->url()); } // Tests what happens when we navigate forward successfully. @@ -809,20 +774,15 @@ TEST_F(NavigationControllerTest, Forward) { const GURL url1("test1:foo1"); const GURL url2("test1:foo2"); - contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); - contents->controller()->LoadURL(url2, PageTransition::TYPED); - contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(1, url2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->GoBack(); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->GoForward(); @@ -835,9 +795,8 @@ TEST_F(NavigationControllerTest, Forward) { EXPECT_TRUE(contents->controller()->CanGoBack()); EXPECT_FALSE(contents->controller()->CanGoForward()); - contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(1, url2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // The forward navigation completed successfully. EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -858,20 +817,14 @@ TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) { const GURL url2("test1:foo2"); const GURL url3("test1:foo3"); - contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); - - contents->controller()->LoadURL(url2, PageTransition::TYPED); - contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + contents->CompleteNavigationAsRenderer(1, url2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->GoBack(); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->GoForward(); EXPECT_EQ(0, notifications.size()); @@ -885,12 +838,9 @@ TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) { EXPECT_TRUE(contents->controller()->CanGoBack()); EXPECT_FALSE(contents->controller()->CanGoForward()); - contents->pending_entry()->set_url(url3); - contents->pending_entry()->set_transition_type(PageTransition::LINK); - contents->CompleteNavigation(2); - EXPECT_TRUE(notifications.Check3AndReset(NOTIFY_NAV_LIST_PRUNED, - NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(2, url3); + EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_LIST_PRUNED, + NOTIFY_NAV_ENTRY_COMMITTED)); EXPECT_EQ(contents->controller()->GetEntryCount(), 2); EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 1); @@ -901,6 +851,130 @@ TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) { EXPECT_FALSE(contents->controller()->CanGoForward()); } +// Tests navigation via link click within a subframe. A new navigation entry +// should be created. +TEST_F(NavigationControllerTest, NewSubframe) { + TestNotificationTracker notifications; + RegisterForAllNavNotifications(¬ifications, contents->controller()); + + const GURL url1("test1:foo1"); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + + const GURL url2("test1:foo2"); + ViewHostMsg_FrameNavigate_Params params; + params.page_id = 1; + params.url = url2; + params.transition = PageTransition::MANUAL_SUBFRAME; + params.should_update_history = false; + params.gesture = NavigationGestureUser; + params.is_post = false; + + NavigationController::LoadCommittedDetails details; + EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false, + &details)); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + EXPECT_EQ(url1, details.previous_url); + EXPECT_FALSE(details.is_auto); + EXPECT_FALSE(details.is_in_page); + EXPECT_FALSE(details.is_main_frame); + + // The new entry should be appended. + EXPECT_EQ(2, contents->controller()->GetEntryCount()); + + // New entry should refer to the new page, but the old URL (entries only + // reflect the toplevel URL). + EXPECT_EQ(url1, details.entry->url()); + EXPECT_EQ(params.page_id, details.entry->page_id()); +} + +// Auto subframes are ones the page loads automatically like ads. They should +// not create new navigation entries. +TEST_F(NavigationControllerTest, AutoSubframe) { + TestNotificationTracker notifications; + RegisterForAllNavNotifications(¬ifications, contents->controller()); + + const GURL url1("test1:foo1"); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + + const GURL url2("test1:foo2"); + ViewHostMsg_FrameNavigate_Params params; + params.page_id = 0; + params.url = url2; + params.transition = PageTransition::AUTO_SUBFRAME; + params.should_update_history = false; + params.gesture = NavigationGestureUser; + params.is_post = false; + + // Navigating should do nothing. + NavigationController::LoadCommittedDetails details; + EXPECT_FALSE(contents->controller()->RendererDidNavigate(params, false, + &details)); + EXPECT_EQ(0, notifications.size()); + + // There should still be only one entry. + EXPECT_EQ(1, contents->controller()->GetEntryCount()); +} + +// Tests navigation and then going back to a subframe navigation. +TEST_F(NavigationControllerTest, BackSubframe) { + TestNotificationTracker notifications; + RegisterForAllNavNotifications(¬ifications, contents->controller()); + + // Main page. + const GURL url1("test1:foo1"); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + + // First manual subframe navigation. + const GURL url2("test1:foo2"); + ViewHostMsg_FrameNavigate_Params params; + params.page_id = 1; + params.url = url2; + params.transition = PageTransition::MANUAL_SUBFRAME; + params.should_update_history = false; + params.gesture = NavigationGestureUser; + params.is_post = false; + + // This should generate a new entry. + NavigationController::LoadCommittedDetails details; + EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false, + &details)); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + EXPECT_EQ(2, contents->controller()->GetEntryCount()); + + // Second manual subframe navigation should also make a new entry. + const GURL url3("test1:foo3"); + params.page_id = 2; + params.url = url3; + EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false, + &details)); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + EXPECT_EQ(3, contents->controller()->GetEntryCount()); + EXPECT_EQ(2, contents->controller()->GetCurrentEntryIndex()); + + // Go back one. + contents->controller()->GoBack(); + params.url = url2; + params.page_id = 1; + EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false, + &details)); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + EXPECT_EQ(3, contents->controller()->GetEntryCount()); + EXPECT_EQ(1, contents->controller()->GetCurrentEntryIndex()); + + // Go back one more. + contents->controller()->GoBack(); + params.url = url1; + params.page_id = 0; + EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false, + &details)); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + EXPECT_EQ(3, contents->controller()->GetEntryCount()); + EXPECT_EQ(0, contents->controller()->GetCurrentEntryIndex()); +} + TEST_F(NavigationControllerTest, LinkClick) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); @@ -908,18 +982,15 @@ TEST_F(NavigationControllerTest, LinkClick) { const GURL url1("test1:foo1"); const GURL url2("test1:foo2"); - contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->set_pending_entry(new NavigationEntry(kTestContentsType1, NULL, 0, url2, std::wstring(), PageTransition::LINK)); - contents->CompleteNavigation(1); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(1, url2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // Should not have produced a new session history entry. EXPECT_EQ(contents->controller()->GetEntryCount(), 2); @@ -931,6 +1002,72 @@ TEST_F(NavigationControllerTest, LinkClick) { EXPECT_FALSE(contents->controller()->CanGoForward()); } +TEST_F(NavigationControllerTest, InPage) { + TestNotificationTracker notifications; + RegisterForAllNavNotifications(¬ifications, contents->controller()); + + // Main page. Note that we need "://" so this URL is treated as "standard" + // which are the only ones that can have a ref. + const GURL url1("test1://foo"); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + + // First navigation. + const GURL url2("test1://foo#a"); + ViewHostMsg_FrameNavigate_Params params; + params.page_id = 1; + params.url = url2; + params.transition = PageTransition::LINK; + params.should_update_history = false; + params.gesture = NavigationGestureUser; + params.is_post = false; + + // This should generate a new entry. + NavigationController::LoadCommittedDetails details; + EXPECT_TRUE(contents->controller()->RendererDidNavigate(params, false, + &details)); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + EXPECT_EQ(2, contents->controller()->GetEntryCount()); + + // Go back one. + ViewHostMsg_FrameNavigate_Params back_params(params); + contents->controller()->GoBack(); + back_params.url = url1; + back_params.page_id = 0; + EXPECT_TRUE(contents->controller()->RendererDidNavigate(back_params, false, + &details)); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + EXPECT_EQ(2, contents->controller()->GetEntryCount()); + EXPECT_EQ(0, contents->controller()->GetCurrentEntryIndex()); + EXPECT_EQ(back_params.url, contents->controller()->GetActiveEntry()->url()); + + // Go forward + ViewHostMsg_FrameNavigate_Params forward_params(params); + contents->controller()->GoForward(); + forward_params.url = url2; + forward_params.page_id = 1; + EXPECT_TRUE(contents->controller()->RendererDidNavigate(forward_params, false, + &details)); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); + EXPECT_EQ(2, contents->controller()->GetEntryCount()); + EXPECT_EQ(1, contents->controller()->GetCurrentEntryIndex()); + EXPECT_EQ(forward_params.url, + contents->controller()->GetActiveEntry()->url()); + + // Now go back and forward again. This is to work around a bug where we would + // compare the incoming URL with the last committed entry rather than the + // one identified by an existing page ID. This would result in the second URL + // losing the reference fragment when you navigate away from it and then back. + contents->controller()->GoBack(); + EXPECT_TRUE(contents->controller()->RendererDidNavigate(back_params, false, + &details)); + contents->controller()->GoForward(); + EXPECT_TRUE(contents->controller()->RendererDidNavigate(forward_params, false, + &details)); + EXPECT_EQ(forward_params.url, + contents->controller()->GetActiveEntry()->url()); +} + TEST_F(NavigationControllerTest, SwitchTypes) { TestNotificationTracker notifications; RegisterForAllNavNotifications(¬ifications, contents->controller()); @@ -938,21 +1075,17 @@ TEST_F(NavigationControllerTest, SwitchTypes) { const GURL url1("test1:foo"); const GURL url2("test2:foo"); - contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); TestContents* initial_contents = contents; - contents->controller()->LoadURL(url2, PageTransition::TYPED); // The tab contents should have been replaced ASSERT_TRUE(initial_contents != contents); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(1, url2); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); // A second navigation entry should have been committed even though the // PageIDs are the same. PageIDs are scoped to the tab contents type. @@ -967,9 +1100,8 @@ TEST_F(NavigationControllerTest, SwitchTypes) { // Navigate back... contents->controller()->GoBack(); ASSERT_TRUE(initial_contents == contents); // switched again! - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); EXPECT_EQ(contents->controller()->GetEntryCount(), 2); EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 0); @@ -992,10 +1124,8 @@ TEST_F(NavigationControllerTest, SwitchTypes_Discard) { const GURL url1("test1:foo"); const GURL url2("test2:foo"); - contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(0); - EXPECT_TRUE(notifications.Check2AndReset(NOTIFY_NAV_ENTRY_COMMITTED, - NOTIFY_NAV_ENTRY_CHANGED)); + contents->CompleteNavigationAsRenderer(0, url1); + EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); TestContents* initial_contents = contents; @@ -1030,22 +1160,23 @@ TEST_F(NavigationControllerTest, SwitchTypesCleanup) { const GURL url2("test2:foo"); const GURL url3("test2:bar"); + // Note that we need the LoadURL calls so that pending entries and the + // different tab contents types are created. "Renderer" navigations won't + // actually cross tab contents boundaries without these. contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(0); - + contents->CompleteNavigationAsRenderer(0, url1); contents->controller()->LoadURL(url2, PageTransition::TYPED); - contents->CompleteNavigation(0); - + contents->CompleteNavigationAsRenderer(1, url2); contents->controller()->LoadURL(url3, PageTransition::TYPED); - contents->CompleteNavigation(1); + contents->CompleteNavigationAsRenderer(2, url3); - // Navigate back to the start + // Navigate back to the start. contents->controller()->GoToIndex(0); - contents->CompleteNavigation(0); + contents->CompleteNavigationAsRenderer(0, url1); - // Now jump to the end + // Now jump to the end. contents->controller()->GoToIndex(2); - contents->CompleteNavigation(1); + contents->CompleteNavigationAsRenderer(2, url3); // There may be TabContentsCollector tasks pending, so flush them from queue. MessageLoop::current()->RunAllPending(); @@ -1068,16 +1199,18 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) { // Load up to the max count, all entries should be there. for (url_index = 0; url_index < kMaxEntryCount; url_index++) { SNPrintF(buffer, 128, "test1://www.a.com/%d", url_index); - contents->controller()->LoadURL(GURL(buffer), PageTransition::TYPED); - contents->CompleteNavigation(url_index); + GURL url(buffer); + contents->controller()->LoadURL(url, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(url_index, url); } EXPECT_EQ(contents->controller()->GetEntryCount(), kMaxEntryCount); // Navigate some more. SNPrintF(buffer, 128, "test1://www.a.com/%d", url_index); - contents->controller()->LoadURL(GURL(buffer), PageTransition::TYPED); - contents->CompleteNavigation(url_index); + GURL url(buffer); + contents->controller()->LoadURL(url, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(url_index, url); url_index++; // We expect http://www.a.com/0 to be gone. @@ -1088,8 +1221,9 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) { // More navigations. for (int i = 0; i < 3; i++) { SNPrintF(buffer, 128, "test1://www.a.com/%d", url_index); - contents->controller()->LoadURL(GURL(buffer), PageTransition::TYPED); - contents->CompleteNavigation(url_index); + url = GURL(buffer); + contents->controller()->LoadURL(url, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(url_index, url); url_index++; } EXPECT_EQ(contents->controller()->GetEntryCount(), kMaxEntryCount); @@ -1097,35 +1231,120 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) { GURL("test1://www.a.com/4")); } +// Tests that we can do a restore and navigate to the restored entries and +// everything is updated properly. This can be tricky since there is no +// SiteInstance for the entries created initially. +TEST_F(NavigationControllerTest, RestoreNavigate) { + site_instance = SiteInstance::CreateSiteInstance(profile); + + // Create a NavigationController with a restored set of tabs. + GURL url("test1:foo"); + std::vector<TabNavigation> navigations; + navigations.push_back(TabNavigation(0, url, L"Title", "state", + PageTransition::LINK)); + NavigationController* controller = + new NavigationController(profile, navigations, 0, NULL); + controller->GoToIndex(0); + + // We should now have one entry, and it should be "pending". + EXPECT_EQ(1, controller->GetEntryCount()); + EXPECT_EQ(controller->GetEntryAtIndex(0), controller->GetPendingEntry()); + EXPECT_EQ(0, controller->GetEntryAtIndex(0)->page_id()); + + // Say we navigated to that entry. + ViewHostMsg_FrameNavigate_Params params; + params.page_id = 0; + params.url = url; + params.transition = PageTransition::LINK; + params.should_update_history = false; + params.gesture = NavigationGestureUser; + params.is_post = false; + NavigationController::LoadCommittedDetails details; + controller->RendererDidNavigate(params, false, &details); + + // There should be no longer any pending entry and one committed one. This + // means that we were able to locate the entry, assign its site instance, and + // commit it properly. + EXPECT_EQ(1, controller->GetEntryCount()); + EXPECT_EQ(0, controller->GetLastCommittedEntryIndex()); + EXPECT_FALSE(controller->GetPendingEntry()); + EXPECT_EQ(site_instance, + controller->GetLastCommittedEntry()->site_instance()); +} + +// Make sure that the page type and stuff is correct after an interstitial. +TEST_F(NavigationControllerTest, Interstitial) { + // First navigate somewhere normal. + const GURL url1("test1:foo"); + contents->controller()->LoadURL(url1, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(0, url1); + + // Now navigate somewhere with an interstitial. + const GURL url2("test1:bar"); + contents->controller()->LoadURL(url1, PageTransition::TYPED); + contents->controller()->GetPendingEntry()->set_page_type( + NavigationEntry::INTERSTITIAL_PAGE); + + // At this point the interstitial will be displayed and the load will still + // be pending. If the user continues, the load will commit. + contents->CompleteNavigationAsRenderer(1, url2); + + // The page should be a normal page again. + EXPECT_EQ(url2, contents->controller()->GetLastCommittedEntry()->url()); + EXPECT_EQ(NavigationEntry::NORMAL_PAGE, + contents->controller()->GetLastCommittedEntry()->page_type()); +} + +// Tests that IsInPageNavigation returns appropriate results. Prevents +// regression for bug 1126349. +TEST_F(NavigationControllerTest, IsInPageNavigation) { + // Navigate to URL with no refs. + const GURL url("http://www.google.com/home.html"); + contents->CompleteNavigationAsRenderer(0, url); + + // Reloading the page is not an in-page navigation. + EXPECT_FALSE(contents->controller()->IsURLInPageNavigation(url)); + const GURL other_url("http://www.google.com/add.html"); + EXPECT_FALSE(contents->controller()->IsURLInPageNavigation(other_url)); + const GURL url_with_ref("http://www.google.com/home.html#my_ref"); + EXPECT_TRUE(contents->controller()->IsURLInPageNavigation(url_with_ref)); + + // Navigate to URL with refs. + contents->CompleteNavigationAsRenderer(1, url_with_ref); + + // Reloading the page is not an in-page navigation. + EXPECT_FALSE(contents->controller()->IsURLInPageNavigation(url_with_ref)); + EXPECT_FALSE(contents->controller()->IsURLInPageNavigation(url)); + EXPECT_FALSE(contents->controller()->IsURLInPageNavigation(other_url)); + const GURL other_url_with_ref("http://www.google.com/home.html#my_other_ref"); + EXPECT_TRUE(contents->controller()->IsURLInPageNavigation( + other_url_with_ref)); +} + // A basic test case. Navigates to a single url, and make sure the history // db matches. TEST_F(NavigationControllerHistoryTest, Basic) { - contents->controller()->LoadURL(url0, PageTransition::TYPED); - contents->CompleteNavigation(0); + contents->controller()->LoadURL(url0, PageTransition::LINK); + contents->CompleteNavigationAsRenderer(0, url0); GetLastSession(); helper_.AssertSingleWindowWithSingleTab(windows_, 1); helper_.AssertTabEquals(0, 0, 1, *(windows_[0]->tabs[0])); TabNavigation nav1(0, url0, std::wstring(), std::string(), - PageTransition::TYPED); + PageTransition::LINK); helper_.AssertNavigationEquals(nav1, windows_[0]->tabs[0]->navigations[0]); } // Navigates to three urls, then goes back and make sure the history database // is in sync. TEST_F(NavigationControllerHistoryTest, NavigationThenBack) { - contents->controller()->LoadURL(url0, PageTransition::TYPED); - contents->CompleteNavigation(0); - - contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(1); - - contents->controller()->LoadURL(url2, PageTransition::TYPED); - contents->CompleteNavigation(2); + contents->CompleteNavigationAsRenderer(0, url0); + contents->CompleteNavigationAsRenderer(1, url1); + contents->CompleteNavigationAsRenderer(2, url2); contents->controller()->GoBack(); - contents->CompleteNavigation(1); + contents->CompleteNavigationAsRenderer(1, url1); GetLastSession(); @@ -1133,7 +1352,7 @@ TEST_F(NavigationControllerHistoryTest, NavigationThenBack) { helper_.AssertTabEquals(0, 1, 3, *(windows_[0]->tabs[0])); TabNavigation nav(0, url0, std::wstring(), std::string(), - PageTransition::TYPED); + PageTransition::LINK); helper_.AssertNavigationEquals(nav, windows_[0]->tabs[0]->navigations[0]); nav.url = url1; helper_.AssertNavigationEquals(nav, windows_[0]->tabs[0]->navigations[1]); @@ -1143,23 +1362,17 @@ TEST_F(NavigationControllerHistoryTest, NavigationThenBack) { // Navigates to three urls, then goes back twice, then loads a new url. TEST_F(NavigationControllerHistoryTest, NavigationPruning) { - contents->controller()->LoadURL(url0, PageTransition::TYPED); - contents->CompleteNavigation(0); - - contents->controller()->LoadURL(url1, PageTransition::TYPED); - contents->CompleteNavigation(1); - - contents->controller()->LoadURL(url2, PageTransition::TYPED); - contents->CompleteNavigation(2); + contents->CompleteNavigationAsRenderer(0, url0); + contents->CompleteNavigationAsRenderer(1, url1); + contents->CompleteNavigationAsRenderer(2, url2); contents->controller()->GoBack(); - contents->CompleteNavigation(1); + contents->CompleteNavigationAsRenderer(1, url1); contents->controller()->GoBack(); - contents->CompleteNavigation(0); + contents->CompleteNavigationAsRenderer(0, url0); - contents->controller()->LoadURL(url2, PageTransition::TYPED); - contents->CompleteNavigation(3); + contents->CompleteNavigationAsRenderer(3, url2); // Now have url0, and url2. @@ -1169,9 +1382,8 @@ TEST_F(NavigationControllerHistoryTest, NavigationPruning) { helper_.AssertTabEquals(0, 1, 2, *(windows_[0]->tabs[0])); TabNavigation nav(0, url0, std::wstring(), std::string(), - PageTransition::TYPED); + PageTransition::LINK); helper_.AssertNavigationEquals(nav, windows_[0]->tabs[0]->navigations[0]); nav.url = url2; helper_.AssertNavigationEquals(nav, windows_[0]->tabs[0]->navigations[1]); } - diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc index c5d0fe1..0a9d6cf 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc @@ -169,20 +169,10 @@ void SafeBrowsingBlockingPage::DisplayBlockingPage() { // (typically the navigation was initiated by the page), we create a fake // navigation entry (so the location bar shows the page's URL). if (is_main_frame_ && tab_->controller()->GetPendingEntryIndex() == -1) { - // New navigation. - NavigationEntry* nav_entry = new NavigationEntry(TAB_CONTENTS_WEB); - - // We set the page ID to max page id so to ensure the controller considers - // this dummy entry a new one. Because we'll remove the entry when the - // interstitial is going away, it will not conflict with any future - // navigations. - nav_entry->set_page_id(tab_->GetMaxPageID() + 1); - nav_entry->set_page_type(NavigationEntry::INTERSTITIAL_PAGE); - nav_entry->set_url(url_); - - // The default details is "new navigation", and that's OK with us. - NavigationController::LoadCommittedDetails details; - tab_->controller()->DidNavigateToEntry(nav_entry, &details); + NavigationEntry new_entry(TAB_CONTENTS_WEB); + new_entry.set_url(url_); + new_entry.set_page_type(NavigationEntry::INTERSTITIAL_PAGE); + tab_->controller()->AddDummyEntryForInterstitial(new_entry); created_temporary_entry_ = true; } @@ -244,7 +234,7 @@ bool SafeBrowsingBlockingPage::GoBack() { // Remove the navigation entry for the malware page. Note that we always // remove the entry even if we did not create it as it has been flagged as // malware and we don't want the user navigating back to it. - web_contents->controller()->RemoveLastEntry(); + web_contents->controller()->RemoveLastEntryForInterstitial(); return true; } @@ -296,7 +286,7 @@ void SafeBrowsingBlockingPage::Continue(const std::string& user_action) { // We are continuing, if we have created a temporary navigation entry, // delete it as a new will be created on navigation. if (created_temporary_entry_) - web->controller()->RemoveLastEntry(); + web->controller()->RemoveLastEntryForInterstitial(); if (is_main_frame_) web->HideInterstitialPage(true, true); else diff --git a/chrome/browser/session_service.cc b/chrome/browser/session_service.cc index 9784f8c..2694357 100644 --- a/chrome/browser/session_service.cc +++ b/chrome/browser/session_service.cc @@ -424,11 +424,16 @@ void SessionService::Observe(NotificationType type, changed->index, *changed->changed_entry); break; } - case NOTIFY_NAV_ENTRY_COMMITTED: + case NOTIFY_NAV_ENTRY_COMMITTED: { + int current_entry_index = controller->GetCurrentEntryIndex(); SetSelectedNavigationIndex(controller->window_id(), controller->session_id(), - controller->GetCurrentEntryIndex()); + current_entry_index); + UpdateTabNavigation(controller->window_id(), controller->session_id(), + current_entry_index, + *controller->GetEntryAtIndex(current_entry_index)); break; + } default: NOTREACHED(); } diff --git a/chrome/browser/session_service_test_helper.cc b/chrome/browser/session_service_test_helper.cc index a793ed0..49cedd8 100644 --- a/chrome/browser/session_service_test_helper.cc +++ b/chrome/browser/session_service_test_helper.cc @@ -68,7 +68,7 @@ void SessionServiceTestHelper::AssertNavigationEquals( void SessionServiceTestHelper::AssertSingleWindowWithSingleTab( const std::vector<SessionWindow*>& windows, int nav_count) { - EXPECT_EQ(1, windows.size()); + ASSERT_EQ(1, windows.size()); EXPECT_EQ(1, windows[0]->tabs.size()); EXPECT_EQ(nav_count, windows[0]->tabs[0]->navigations.size()); } diff --git a/chrome/browser/ssl_blocking_page.cc b/chrome/browser/ssl_blocking_page.cc index 699c9e2..ba90efe 100644 --- a/chrome/browser/ssl_blocking_page.cc +++ b/chrome/browser/ssl_blocking_page.cc @@ -51,7 +51,7 @@ SSLBlockingPage::SSLBlockingPage(SSLManager::CertError* error, // Since WebContents::InterstitialPageGone won't be called, we need // to clear the last NavigationEntry manually. - tab_->controller()->RemoveLastEntry(); + tab_->controller()->RemoveLastEntryForInterstitial(); } (*tab_to_blocking_page_)[tab_] = this; @@ -132,31 +132,31 @@ void SSLBlockingPage::Show() { int cert_id = CertStore::GetSharedInstance()->StoreCert( ssl_info.cert, tab->render_view_host()->process()->host_id()); - NavigationEntry* nav_entry = new NavigationEntry(TAB_CONTENTS_WEB); if (tab_->controller()->GetPendingEntryIndex() == -1) { - // New navigation. - // We set the page ID to max page id so to ensure the controller considers - // this dummy entry a new one. Because we'll remove the entry when the - // interstitial is going away, it will not conflict with any future - // navigations. + // For new navigations, we just create a new navigation entry. + NavigationEntry new_entry(TAB_CONTENTS_WEB); + new_entry.set_url(error_->request_url()); + tab_->controller()->AddDummyEntryForInterstitial(new_entry); created_nav_entry_ = true; - nav_entry->set_page_id(tab_->GetMaxPageID() + 1); - nav_entry->set_url(error_->request_url()); } else { - // Make sure to update the current entry ssl state to reflect the error. - *nav_entry = *(tab_->controller()->GetPendingEntry()); + // When there is a pending entry index, that means we're doing a + // back/forward navigation. Clone that entry instead. + tab_->controller()->AddDummyEntryForInterstitial( + *tab_->controller()->GetPendingEntry()); } - nav_entry->set_page_type(NavigationEntry::INTERSTITIAL_PAGE); - nav_entry->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN); - nav_entry->ssl().set_cert_id(cert_id); - 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. - - // The default details is "new navigation", and that's OK with us. - NavigationController::LoadCommittedDetails details; - tab_->controller()->DidNavigateToEntry(nav_entry, &details); + NavigationEntry* entry = tab_->controller()->GetActiveEntry(); + entry->set_page_type(NavigationEntry::INTERSTITIAL_PAGE); + + entry->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN); + entry->ssl().set_cert_id(cert_id); + entry->ssl().set_cert_status(ssl_info.cert_status); + entry->ssl().set_security_bits(ssl_info.security_bits); + NotificationService::current()->Notify( + NOTIFY_SSL_STATE_CHANGED, + Source<NavigationController>(tab_->controller()), + NotificationService::NoDetails()); + tab->ShowInterstitialPage(html_text, NULL); } @@ -176,7 +176,7 @@ void SSLBlockingPage::Observe(NotificationType type, // as their navigation history is not saved. if (remove_last_entry_ && browser && !browser->tabstrip_model()->closing_all()) { - tab_->controller()->RemoveLastEntry(); + tab_->controller()->RemoveLastEntryForInterstitial(); } delete this; break; @@ -220,7 +220,7 @@ void SSLBlockingPage::DontProceed() { // We are navigating, remove the current entry before we mess with it. remove_last_entry_ = false; - tab_->controller()->RemoveLastEntry(); + tab_->controller()->RemoveLastEntryForInterstitial(); NavigationEntry* entry = tab_->controller()->GetActiveEntry(); if (!entry) { diff --git a/chrome/browser/ssl_manager.cc b/chrome/browser/ssl_manager.cc index bc4e957..ffb0645 100644 --- a/chrome/browser/ssl_manager.cc +++ b/chrome/browser/ssl_manager.cc @@ -658,7 +658,7 @@ void SSLManager::DidCommitProvisionalLoad(ProvisionalLoadDetails* details) { NotificationService::current()->Notify( NOTIFY_SSL_STATE_CHANGED, Source<NavigationController>(controller_), - Details<NavigationEntry>(controller_->GetActiveEntry())); + NotificationService::NoDetails()); } } diff --git a/chrome/browser/ssl_policy.cc b/chrome/browser/ssl_policy.cc index 4ae02a2..beb2978 100644 --- a/chrome/browser/ssl_policy.cc +++ b/chrome/browser/ssl_policy.cc @@ -448,7 +448,7 @@ void SSLPolicy::OnRequestStarted(SSLManager* manager, const GURL& url, NotificationService::current()->Notify( NOTIFY_SSL_STATE_CHANGED, Source<NavigationController>(manager->controller()), - Details<NavigationEntry>(entry)); + NotificationService::NoDetails()); } } diff --git a/chrome/browser/ssl_uitest.cc b/chrome/browser/ssl_uitest.cc index f712fc5..f2bcdef 100644 --- a/chrome/browser/ssl_uitest.cc +++ b/chrome/browser/ssl_uitest.cc @@ -224,6 +224,9 @@ TEST_F(SSLUITest, TestMixedContents) { EXPECT_EQ(NavigationEntry::SSLStatus::MIXED_CONTENT, mixed_content_state); } + +/* TODO(jcampan) bug 2004: fix this test. + // Visits a page with unsafe content and make sure that: // - frames content is replaced with warning // - images and scripts are filtered out entirely @@ -279,6 +282,7 @@ TEST_F(SSLUITest, TestUnsafeContents) { &js_result)); EXPECT_FALSE(js_result); } +*/ // Visits a page with mixed content loaded by JS (after the initial page load). TEST_F(SSLUITest, TestMixedContentsLoadedFromJS) { diff --git a/chrome/browser/tab_contents.cc b/chrome/browser/tab_contents.cc index d0606d6..e80431c 100644 --- a/chrome/browser/tab_contents.cc +++ b/chrome/browser/tab_contents.cc @@ -60,21 +60,23 @@ void TabContents::HideContents() { } int32 TabContents::GetMaxPageID() { - if (AsWebContents()) - return AsWebContents()->site_instance()->max_page_id(); + if (GetSiteInstance()) + return GetSiteInstance()->max_page_id(); else return max_page_id_; } void TabContents::UpdateMaxPageID(int32 page_id) { - if (AsWebContents()) { - // Ensure both the SiteInstance and RenderProcessHost update their max page - // IDs in sync. - AsWebContents()->site_instance()->UpdateMaxPageID(page_id); + // Ensure both the SiteInstance and RenderProcessHost update their max page + // IDs in sync. Only WebContents will also have site instances, except during + // testing. + if (GetSiteInstance()) + GetSiteInstance()->UpdateMaxPageID(page_id); + + if (AsWebContents()) AsWebContents()->process()->UpdateMaxPageID(page_id); - } else { + else max_page_id_ = std::max(max_page_id_, page_id); - } } const std::wstring TabContents::GetDefaultTitle() const { @@ -256,35 +258,10 @@ void TabContents::SetIsLoading(bool is_loading, NotificationService::NoDetails()); } -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, details); - - // update after informing the navigation controller so it can check the - // previous value of the max page id. - UpdateMaxPageID(new_page_id); -} - -bool TabContents::Navigate(const NavigationEntry& entry, bool reload) { - NavigationEntry* new_entry = new NavigationEntry(entry); - if (new_entry->page_id() == -1) { - // This is a new navigation. Our behavior is to always navigate to the - // same page (page 0) in response to a navigation. - new_entry->set_page_id(0); - new_entry->set_title(GetDefaultTitle()); - } - - // 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); +bool TabContents::NavigateToPendingEntry(bool reload) { + // Our benavior is just to report that the entry was committed. + controller()->GetPendingEntry()->set_title(GetDefaultTitle()); + controller()->CommitPendingEntry(); return true; } diff --git a/chrome/browser/tab_contents.h b/chrome/browser/tab_contents.h index a8eaec3..e8990d6 100644 --- a/chrome/browser/tab_contents.h +++ b/chrome/browser/tab_contents.h @@ -134,6 +134,11 @@ class TabContents : public PageNavigator, // Updates the max PageID to be at least the given PageID. void UpdateMaxPageID(int32 page_id); + // Returns the site instance associated with the current page. By default, + // there is no site instance. WebContents overrides this to provide proper + // access to its site instance. + virtual SiteInstance* GetSiteInstance() const { return NULL; } + // Initial title assigned to NavigationEntries from Navigate. virtual const std::wstring GetDefaultTitle() const; @@ -274,17 +279,17 @@ class TabContents : public PageNavigator, bool is_active() const { return is_active_; } // Called by the NavigationController to cause the TabContents to navigate to - // the specified entry. Either TabContents::DidNavigateToEntry or the - // navigation controller's DiscardPendingEntry method should be called in - // response (possibly sometime later). + // the current pending entry. The NavigationController should be called back + // with CommitPendingEntry/RendererDidNavigate on success or + // DiscardPendingEntry. The callbacks can be inside of this function, or at + // some future time. // // The entry has a PageID of -1 if newly created (corresponding to navigation // to a new URL). // // If this method returns false, then the navigation is discarded (equivalent // to calling DiscardPendingEntry on the NavigationController). - // - virtual bool Navigate(const NavigationEntry& entry, bool reload); + virtual bool NavigateToPendingEntry(bool reload); // Stop any pending navigation. virtual void Stop() {} @@ -481,13 +486,6 @@ class TabContents : public PageNavigator, // (but can be null if not applicable) void SetIsLoading(bool is_loading, LoadNotificationDetails* details); - // Called by subclasses when a navigation occurs. Ownership of the 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 // if necessary. diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc index 492d9c8..98552cf 100644 --- a/chrome/browser/tabs/tab_strip_model_unittest.cc +++ b/chrome/browser/tabs/tab_strip_model_unittest.cc @@ -31,25 +31,8 @@ class TabStripModelTestTabContents : public TabContents { TabStripModelTestTabContents(const TabContentsType type) : TabContents(type) { } - - bool Navigate(const NavigationEntry& entry, bool reload) { - NavigationEntry* pending_entry = new NavigationEntry(entry); - if (pending_entry->page_id() == -1) { - pending_entry->set_page_id(g_page_id_++); - } - NavigationController::LoadCommittedDetails details; - DidNavigateToEntry(pending_entry, &details); - - return true; - } - - private: - // We need to use valid, incrementing page ids otherwise the TabContents - // and NavController will not play nice when we try to go back and forward. - static int g_page_id_; }; -int TabStripModelTestTabContents::g_page_id_ = 0; // This constructs our fake TabContents. class TabStripModelTestTabContentsFactory : public TabContentsFactory { diff --git a/chrome/browser/web_contents.cc b/chrome/browser/web_contents.cc index a5226b6..7084bd8 100644 --- a/chrome/browser/web_contents.cc +++ b/chrome/browser/web_contents.cc @@ -104,12 +104,6 @@ void InitWebContentsClass() { } } -GURL GURLWithoutRef(const GURL& url) { - url_canon::Replacements<char> replacements; - replacements.ClearRef(); - return url.ReplaceComponents(replacements); -} - } // namespace /////////////////////////////////////////////////////////////////////////////// @@ -570,22 +564,23 @@ void WebContents::SavePage(const std::wstring& main_file, // 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) { - RenderViewHost* dest_render_view_host = render_manager_.Navigate(entry); +bool WebContents::NavigateToPendingEntry(bool reload) { + NavigationEntry* entry = controller()->GetPendingEntry(); + RenderViewHost* dest_render_view_host = render_manager_.Navigate(*entry); // Used for page load time metrics. current_load_start_ = TimeTicks::Now(); - // Navigate in the desired RenderViewHost - dest_render_view_host->NavigateToEntry(entry, reload); + // Navigate in the desired RenderViewHost. + dest_render_view_host->NavigateToEntry(*entry, reload); - if (entry.page_id() == -1) { + if (entry->page_id() == -1) { // HACK!! This code suppresses javascript: URLs from being added to // session history, which is what we want to do for javascript: URLs that // do not generate content. What we really need is a message from the // renderer telling us that a new page was not created. The same message // could be used for mailto: URLs and the like. - if (entry.url().SchemeIs("javascript")) + if (entry->url().SchemeIs("javascript")) return false; } @@ -593,7 +588,7 @@ bool WebContents::Navigate(const NavigationEntry& entry, bool reload) { HistoryService* history = profile()->GetHistoryService(Profile::IMPLICIT_ACCESS); if (history) - history->SetFavIconOutOfDateForPage(entry.url()); + history->SetFavIconOutOfDateForPage(entry->url()); } return true; @@ -1020,7 +1015,7 @@ PluginInstaller* WebContents::GetPluginInstaller() { bool WebContents::IsActiveEntry(int32 page_id) { NavigationEntry* active_entry = controller()->GetActiveEntry(); return (active_entry != NULL && - active_entry->site_instance() == site_instance() && + active_entry->site_instance() == GetSiteInstance() && active_entry->page_id() == page_id); } @@ -1039,7 +1034,7 @@ Profile* WebContents::GetProfile() const { void WebContents::CreateView(int route_id, HANDLE modal_dialog_event) { WebContents* new_view = new WebContents(profile(), - site_instance(), + GetSiteInstance(), render_view_factory_, route_id, modal_dialog_event); @@ -1179,83 +1174,16 @@ void WebContents::DidNavigate(RenderViewHost* rvh, render_manager_.DidNavigateMainFrame(rvh); // In the case of interstitial, we don't mess with the navigation entries. + // TODO(brettw) this seems like a bug. What happens if the page goes and + // does something on its own (or something that just got delayed), then + // we won't have a navigation entry for that stuff when the interstitial + // is hidden. if (render_manager_.showing_interstitial_page()) return; - // Check for navigations we don't expect. - if (!controller() || !is_active_ || params.page_id == -1) { - if (params.page_id == -1) { - DCHECK(controller()->GetActiveEntry() == NULL) - << "The renderer is permitted to send a FrameNavigate event for an " - "invalid |page_id| if, and only if, this is the initial blank " - "page for a main frame."; - } - BroadcastProvisionalLoadCommit(rvh, params); - 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 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 - // 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)); - - // Commit the entry to the navigation controller. - 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 (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()); - - // Now that we've assigned a SiteInstance to this entry, we need to - // assign it to the NavigationController's pending entry as well. This - // allows us to find it via GetEntryWithPageID, etc. - if (controller()->GetPendingEntry()) - controller()->GetPendingEntry()->set_site_instance(entry->site_instance()); + // We can't do anything about navigations when we're inactive. + if (!controller() || !is_active_) + return; // Update the site of the SiteInstance if it doesn't have one yet, unless we // are showing an interstitial page. If we are, we should wait until the @@ -1264,67 +1192,26 @@ NavigationEntry* WebContents::CreateNavigationEntryForCommit( // TODO(brettw) the old code only checked for INTERSTIAL, this new code also // checks for LEAVING_INTERSTITIAL mode in the manager. Is this difference // important? - if (!site_instance()->has_site() && + if (!GetSiteInstance()->has_site() && !render_manager_.showing_interstitial_page()) - site_instance()->SetSite(params.url); - - // When the navigation is just a change in ref or a sub-frame navigation, the - // new page should inherit the existing entry's title and favicon, since it - // will be the same. A change in ref also inherits the security style and SSL - // associated info. - bool in_page_nav; - if ((in_page_nav = IsInPageNavigation(params.url)) || - !PageTransition::IsMainFrame(params.transition)) { - // 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! - NavigationEntry* old_entry = controller()->GetLastCommittedEntry(); - if (old_entry) { - entry->set_title(old_entry->title()); - entry->favicon() = old_entry->favicon(); - if (in_page_nav) - entry->ssl() = old_entry->ssl(); - } - } + GetSiteInstance()->SetSite(params.url); - 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(entry->ssl().content_status() == 0) << "We should never " - "be setting the status bits from 1 to 0 on navigate."; - entry->ssl() = last_committed->ssl(); - } - } + NavigationController::LoadCommittedDetails details; + if (!controller()->RendererDidNavigate( + params, + render_manager_.IsRenderViewInterstitial(rvh), + &details)) + return; // No navigation happened. - return entry; + // DO NOT ADD MORE STUFF TO THIS FUNCTION! Your component should either listen + // for the appropriate notification (best) or you can add it to + // DidNavigateMainFramePostCommit / DidNavigateAnyFramePostCommit (only if + // necessary, please). + + // Run post-commit tasks. + if (details.is_main_frame) + DidNavigateMainFramePostCommit(details, params); + DidNavigateAnyFramePostCommit(rvh, details, params); } void WebContents::DidNavigateMainFramePostCommit( @@ -1413,17 +1300,11 @@ void WebContents::DidNavigateAnyFramePostCommit( UpdateHistoryForNavigation(GetURL(), params); } - // Have the controller save the current session. - controller()->NotifyEntryChangedByPageID(type(), site_instance(), - 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); } bool WebContents::IsWebApplicationActive() const { @@ -1445,20 +1326,6 @@ void WebContents::WebAppImagesChanged(WebApp* web_app) { delegate()->NavigationStateChanged(this, TabContents::INVALIDATE_FAVICON); } -void WebContents::BroadcastProvisionalLoadCommit( - RenderViewHost* render_view_host, - const ViewHostMsg_FrameNavigate_Params& params) { - ProvisionalLoadDetails details( - PageTransition::IsMainFrame(params.transition), - render_manager_.IsRenderViewInterstitial(render_view_host), - IsInPageNavigation(params.url), - params.url, params.security_info); - NotificationService::current()-> - Notify(NOTIFY_FRAME_PROVISIONAL_LOAD_COMMITTED, - Source<NavigationController>(controller()), - Details<ProvisionalLoadDetails>(&details)); -} - void WebContents::UpdateStarredStateForCurrentURL() { BookmarkModel* model = profile()->GetBookmarkModel(); const bool old_state = is_starred_; @@ -1511,11 +1378,11 @@ void WebContents::UpdateState(RenderViewHost* rvh, // the next page. The navigation controller will look up the appropriate // NavigationEntry and update it when it is notified via the delegate. - NavigationEntry* entry = controller()->GetEntryWithPageID( - type(), site_instance(), page_id); - if (!entry) + int entry_index = controller()->GetEntryIndexWithPageID( + type(), GetSiteInstance(), page_id); + if (entry_index < 0) return; - + NavigationEntry* entry = controller()->GetEntryAtIndex(entry_index); unsigned changed_flags = 0; // Update the URL. @@ -1558,7 +1425,7 @@ void WebContents::UpdateState(RenderViewHost* rvh, // Notify everybody of the changes (only when the current page changed). if (changed_flags && entry == controller()->GetActiveEntry()) NotifyNavigationStateChanged(changed_flags); - controller()->NotifyEntryChangedByPageID(type(), site_instance(), page_id); + controller()->NotifyEntryChanged(entry, entry_index); } void WebContents::UpdateTitle(RenderViewHost* rvh, @@ -1579,7 +1446,8 @@ void WebContents::UpdateTitle(RenderViewHost* rvh, // so just use that. entry = controller()->GetLastCommittedEntry(); } else { - entry = controller()->GetEntryWithPageID(type(), site_instance(), page_id); + entry = controller()->GetEntryWithPageID(type(), GetSiteInstance(), + page_id); } if (!entry) @@ -1685,7 +1553,7 @@ void WebContents::DidStartProvisionalLoadForFrame( ProvisionalLoadDetails details( is_main_frame, render_manager_.IsRenderViewInterstitial(render_view_host), - IsInPageNavigation(url), + controller()->IsURLInPageNavigation(url), url, std::string()); NotificationService::current()-> Notify(NOTIFY_FRAME_PROVISIONAL_LOAD_START, @@ -1697,12 +1565,14 @@ void WebContents::DidRedirectProvisionalLoad(int32 page_id, const GURL& source_url, const GURL& target_url) { NavigationEntry* entry; - if (page_id == -1) + if (page_id == -1) { entry = controller()->GetPendingEntry(); - else - entry = controller()->GetEntryWithPageID(type(), site_instance(), page_id); + } else { + entry = controller()->GetEntryWithPageID(type(), GetSiteInstance(), + page_id); + } if (!entry || entry->tab_type() != type() || entry->url() != source_url) - return; + return; entry->set_url(target_url); } @@ -1750,7 +1620,7 @@ void WebContents::DidFailProvisionalLoadWithError( ProvisionalLoadDetails details( is_main_frame, render_manager_.IsRenderViewInterstitial(render_view_host), - IsInPageNavigation(url), + controller()->IsURLInPageNavigation(url), url, std::string()); details.set_error_code(error_code); @@ -2465,18 +2335,6 @@ void WebContents::FileSelectionCanceled(void* params) { /////////////////////////////////////////////////////////////////////////////// -bool WebContents::IsInPageNavigation(const GURL& url) const { - // We compare to the last committed entry and not the active entry as the - // active entry is the current pending entry (if any). - // When this method is called when a navigation initiated from the browser - // (ex: when typing the URL in the location bar) is committed, the pending - // entry URL is the same as |url|. - NavigationEntry* entry = controller()->GetLastCommittedEntry(); - return (entry && url.has_ref() && - (url != entry->url()) && // Test for reload of a URL with a ref. - GURLWithoutRef(entry->url()) == GURLWithoutRef(url)); -} - SkBitmap WebContents::GetFavIcon() { if (web_app_.get() && IsWebApplicationActive()) { SkBitmap app_icon = web_app_->GetFavIcon(); diff --git a/chrome/browser/web_contents.h b/chrome/browser/web_contents.h index cc6c11c..866862a 100644 --- a/chrome/browser/web_contents.h +++ b/chrome/browser/web_contents.h @@ -65,10 +65,12 @@ class WebContents : public TabContents, // cancel the close of the page. virtual void FirePageUnload(); - // TabContents virtual WebContents* AsWebContents() { return this; } - virtual bool Navigate(const NavigationEntry& entry, bool reload); + virtual SiteInstance* GetSiteInstance() const { + return render_manager_.current_host()->site_instance(); + } + virtual bool NavigateToPendingEntry(bool reload); virtual void Stop(); virtual void DidBecomeSelected(); virtual void WasHidden(); @@ -242,9 +244,6 @@ class WebContents : public TabContents, RenderViewHost* render_view_host() const { return render_manager_.current_host(); } - SiteInstance* site_instance() const { - return render_manager_.current_host()->site_instance(); - } RenderWidgetHostView* view() const { return render_manager_.current_view(); } @@ -324,7 +323,7 @@ class WebContents : public TabContents, bool notify_disconnection() const { return notify_disconnection_; } protected: - FRIEND_TEST(WebContentsTest, OnMessageReceived); + FRIEND_TEST(WebContentsTest, UpdateTitle); // Should be deleted via CloseContents. virtual ~WebContents(); @@ -545,11 +544,6 @@ class WebContents : public TabContents, // // These functions are helpers for Navigate() and DidNavigate(). - // Creates a new navigation entry (malloced, the caller will have to free) - // for the given committed load. Used by DidNavigate. Will not return NULL. - NavigationEntry* CreateNavigationEntryForCommit( - const ViewHostMsg_FrameNavigate_Params& params); - // Handles post-navigation tasks in DidNavigate AFTER the entry has been // committed to the navigation controller. Note that the navigation entry is // not provided since it may be invalid/changed after being committed. The @@ -566,17 +560,6 @@ class WebContents : public TabContents, // domain is changing. void MaybeCloseChildWindows(const ViewHostMsg_FrameNavigate_Params& params); - // Broadcasts a notification for the provisional load committing, used by - // DidNavigate. - void BroadcastProvisionalLoadCommit( - RenderViewHost* render_view_host, - const ViewHostMsg_FrameNavigate_Params& params); - - // Convenience method that returns true if navigating to the specified URL - // from the current one is an in-page navigation (jumping to a ref in the - // page). - bool IsInPageNavigation(const GURL& url) const; - // Updates the starred state from the bookmark bar model. If the state has // changed, the delegate is notified. void UpdateStarredStateForCurrentURL(); diff --git a/chrome/browser/web_contents_unittest.cc b/chrome/browser/web_contents_unittest.cc index 515a963..5d386c5 100644 --- a/chrome/browser/web_contents_unittest.cc +++ b/chrome/browser/web_contents_unittest.cc @@ -229,11 +229,6 @@ class TestWebContents : public WebContents { render_view_host->is_loading = false; } - // Promote IsInPageNavigation to public. - bool TestIsInPageNavigation(const GURL& url) { - return IsInPageNavigation(url); - } - // Promote GetWebkitPrefs to public. WebPreferences TestGetWebkitPrefs() { return GetWebkitPrefs(); @@ -301,12 +296,13 @@ class WebContentsTest : public testing::Test { MessageLoopForUI message_loop_; }; -// Test to make sure that title updates get stripped of whitespace -TEST_F(WebContentsTest, OnMessageReceived) { +// Test to make sure that title updates get stripped of whitespace. +TEST_F(WebContentsTest, UpdateTitle) { + ViewHostMsg_FrameNavigate_Params params; + InitNavigateParams(¶ms, 0, GURL("about:blank")); + NavigationController::LoadCommittedDetails details; - contents->controller()->DidNavigateToEntry(new NavigationEntry( - contents->type(), contents->site_instance(), 0, GURL("about:blank"), - std::wstring(), PageTransition::TYPED), &details); + contents->controller()->RendererDidNavigate(params, false, &details); contents->UpdateTitle(NULL, 0, L" Lots O' Whitespace\n"); EXPECT_EQ(std::wstring(L"Lots O' Whitespace"), contents->GetTitle()); @@ -315,7 +311,7 @@ TEST_F(WebContentsTest, OnMessageReceived) { // Test simple same-SiteInstance navigation. TEST_F(WebContentsTest, SimpleNavigation) { TestRenderViewHost* orig_rvh = contents->rvh(); - SiteInstance* instance1 = contents->site_instance(); + SiteInstance* instance1 = contents->GetSiteInstance(); EXPECT_TRUE(contents->pending_rvh() == NULL); EXPECT_TRUE(contents->original_rvh() == NULL); EXPECT_TRUE(contents->interstitial_rvh() == NULL); @@ -585,7 +581,7 @@ TEST_F(WebContentsTest, CrossSiteBoundaries) { TestRenderViewHost* orig_rvh = contents->rvh(); int orig_rvh_delete_count = 0; orig_rvh->set_delete_counter(&orig_rvh_delete_count); - SiteInstance* instance1 = contents->site_instance(); + SiteInstance* instance1 = contents->GetSiteInstance(); // Navigate to URL. First URL should use first RenderViewHost. const GURL url("http://www.google.com"); @@ -611,7 +607,7 @@ TEST_F(WebContentsTest, CrossSiteBoundaries) { ViewHostMsg_FrameNavigate_Params params2; InitNavigateParams(¶ms2, 1, url2); contents->TestDidNavigate(pending_rvh, params2); - SiteInstance* instance2 = contents->site_instance(); + SiteInstance* instance2 = contents->GetSiteInstance(); EXPECT_TRUE(contents->state_is_normal()); EXPECT_EQ(pending_rvh, contents->render_view_host()); @@ -632,7 +628,7 @@ TEST_F(WebContentsTest, CrossSiteBoundaries) { EXPECT_TRUE(contents->state_is_normal()); EXPECT_EQ(goback_rvh, contents->render_view_host()); EXPECT_EQ(pending_rvh_delete_count, 1); - EXPECT_EQ(instance1, contents->site_instance()); + EXPECT_EQ(instance1, contents->GetSiteInstance()); } // Test that navigating across a site boundary after a crash creates a new @@ -642,7 +638,7 @@ TEST_F(WebContentsTest, CrossSiteBoundariesAfterCrash) { TestRenderViewHost* orig_rvh = contents->rvh(); int orig_rvh_delete_count = 0; orig_rvh->set_delete_counter(&orig_rvh_delete_count); - SiteInstance* instance1 = contents->site_instance(); + SiteInstance* instance1 = contents->GetSiteInstance(); // Navigate to URL. First URL should use first RenderViewHost. const GURL url("http://www.google.com"); @@ -674,7 +670,7 @@ TEST_F(WebContentsTest, CrossSiteBoundariesAfterCrash) { ViewHostMsg_FrameNavigate_Params params2; InitNavigateParams(¶ms2, 1, url2); contents->TestDidNavigate(new_rvh, params2); - SiteInstance* instance2 = contents->site_instance(); + SiteInstance* instance2 = contents->GetSiteInstance(); EXPECT_TRUE(contents->state_is_normal()); EXPECT_EQ(new_rvh, contents->render_view_host()); @@ -689,7 +685,7 @@ TEST_F(WebContentsTest, CrossSiteBoundariesAfterCrash) { TEST_F(WebContentsTest, CrossSiteInterstitialDontProceed) { contents->transition_cross_site = true; TestRenderViewHost* orig_rvh = contents->rvh(); - SiteInstance* instance1 = contents->site_instance(); + SiteInstance* instance1 = contents->GetSiteInstance(); // Navigate to URL. First URL should use first RenderViewHost. const GURL url("http://www.google.com"); @@ -740,7 +736,7 @@ TEST_F(WebContentsTest, CrossSiteInterstitialProceed) { int orig_rvh_delete_count = 0; TestRenderViewHost* orig_rvh = contents->rvh(); orig_rvh->set_delete_counter(&orig_rvh_delete_count); - SiteInstance* instance1 = contents->site_instance(); + SiteInstance* instance1 = contents->GetSiteInstance(); // Navigate to URL. First URL should use first RenderViewHost. const GURL url("http://www.google.com"); @@ -782,7 +778,7 @@ TEST_F(WebContentsTest, CrossSiteInterstitialProceed) { ViewHostMsg_FrameNavigate_Params params3; InitNavigateParams(¶ms3, 2, url2); contents->TestDidNavigate(pending_rvh, params3); - SiteInstance* instance2 = contents->site_instance(); + SiteInstance* instance2 = contents->GetSiteInstance(); EXPECT_TRUE(contents->state_is_normal()); EXPECT_EQ(pending_rvh, contents->render_view_host()); EXPECT_TRUE(contents->original_rvh() == NULL); @@ -804,7 +800,7 @@ TEST_F(WebContentsTest, CrossSiteInterstitialProceed) { contents->TestDidNavigate(goback_rvh, params1); EXPECT_TRUE(contents->state_is_normal()); EXPECT_EQ(goback_rvh, contents->render_view_host()); - EXPECT_EQ(instance1, contents->site_instance()); + EXPECT_EQ(instance1, contents->GetSiteInstance()); EXPECT_EQ(pending_rvh_delete_count, 1); // The second page's rvh should die. } @@ -987,7 +983,7 @@ TEST_F(WebContentsTest, CrossSiteInterstitialCrashesThenNavigate) { TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) { contents->transition_cross_site = true; TestRenderViewHost* orig_rvh = contents->rvh(); - SiteInstance* instance1 = contents->site_instance(); + SiteInstance* instance1 = contents->GetSiteInstance(); // Navigate to URL. First URL should use first RenderViewHost. const GURL url("http://www.google.com"); @@ -1010,7 +1006,7 @@ TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) { ViewHostMsg_FrameNavigate_Params params2a; InitNavigateParams(¶ms2a, 1, url2a); contents->TestDidNavigate(pending_rvh_a, params2a); - SiteInstance* instance2a = contents->site_instance(); + SiteInstance* instance2a = contents->GetSiteInstance(); EXPECT_NE(instance1, instance2a); // Navigate second tab to the same site as the first tab @@ -1027,7 +1023,7 @@ TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) { ViewHostMsg_FrameNavigate_Params params2b; InitNavigateParams(¶ms2b, 2, url2b); contents2->TestDidNavigate(pending_rvh_b, params2b); - SiteInstance* instance2b = contents2->site_instance(); + SiteInstance* instance2b = contents2->GetSiteInstance(); EXPECT_NE(instance1, instance2b); // Both tabs should now be in the same SiteInstance. @@ -1041,7 +1037,7 @@ TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) { TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) { contents->transition_cross_site = true; TestRenderViewHost* orig_rvh = contents->rvh(); - SiteInstance* instance1 = contents->site_instance(); + SiteInstance* instance1 = contents->GetSiteInstance(); // Navigate to URL. const GURL url("http://www.google.com"); @@ -1063,7 +1059,7 @@ TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) { ViewHostMsg_FrameNavigate_Params params2; InitNavigateParams(¶ms2, 2, url2); contents2->TestDidNavigate(rvh2, params2); - SiteInstance* instance2 = contents2->site_instance(); + SiteInstance* instance2 = contents2->GetSiteInstance(); EXPECT_NE(instance1, instance2); EXPECT_TRUE(contents2->state_is_normal()); @@ -1072,7 +1068,7 @@ TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) { ViewHostMsg_FrameNavigate_Params params3; InitNavigateParams(¶ms3, 2, url2); contents->TestDidNavigate(orig_rvh, params3); - SiteInstance* instance3 = contents->site_instance(); + SiteInstance* instance3 = contents->GetSiteInstance(); EXPECT_EQ(instance1, instance3); EXPECT_TRUE(contents->state_is_normal()); @@ -1084,7 +1080,7 @@ TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) { ViewHostMsg_FrameNavigate_Params params4; InitNavigateParams(¶ms4, 3, url3); contents->TestDidNavigate(orig_rvh, params4); - SiteInstance* instance4 = contents->site_instance(); + SiteInstance* instance4 = contents->GetSiteInstance(); EXPECT_EQ(instance1, instance4); contents2->CloseContents(); @@ -1095,7 +1091,7 @@ TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) { TEST_F(WebContentsTest, CrossSiteUnloadHandlers) { contents->transition_cross_site = true; TestRenderViewHost* orig_rvh = contents->rvh(); - SiteInstance* instance1 = contents->site_instance(); + SiteInstance* instance1 = contents->GetSiteInstance(); // Navigate to URL. First URL should use first RenderViewHost. const GURL url("http://www.google.com"); @@ -1128,7 +1124,7 @@ TEST_F(WebContentsTest, CrossSiteUnloadHandlers) { ViewHostMsg_FrameNavigate_Params params2; InitNavigateParams(¶ms2, 1, url2); contents->TestDidNavigate(pending_rvh, params2); - SiteInstance* instance2 = contents->site_instance(); + SiteInstance* instance2 = contents->GetSiteInstance(); EXPECT_TRUE(contents->state_is_normal()); EXPECT_EQ(pending_rvh, contents->render_view_host()); EXPECT_NE(instance1, instance2); @@ -1193,38 +1189,6 @@ TEST_F(WebContentsTest, NavigationEntryContentStateNewWindow) { EXPECT_FALSE(entry->content_state().empty()); } -// Tests that IsInPageNavigation returns appropriate results. Prevents -// regression for bug 1126349. -TEST_F(WebContentsTest, IsInPageNavigation) { - TestRenderViewHost* rvh = contents->rvh(); - - // Navigate to URL with no refs. - const GURL url("http://www.google.com/home.html"); - contents->controller()->LoadURL(url, PageTransition::TYPED); - ViewHostMsg_FrameNavigate_Params params; - InitNavigateParams(¶ms, 1, url); - contents->TestDidNavigate(rvh, params); - - // Reloading the page is not an in-page navigation. - EXPECT_FALSE(contents->TestIsInPageNavigation(url)); - const GURL other_url("http://www.google.com/add.html"); - EXPECT_FALSE(contents->TestIsInPageNavigation(other_url)); - const GURL url_with_ref("http://www.google.com/home.html#my_ref"); - EXPECT_TRUE(contents->TestIsInPageNavigation(url_with_ref)); - - // Navigate to URL with refs. - contents->controller()->LoadURL(url_with_ref, PageTransition::TYPED); - InitNavigateParams(¶ms, 2, url_with_ref); - contents->TestDidNavigate(rvh, params); - - // Reloading the page is not an in-page navigation. - EXPECT_FALSE(contents->TestIsInPageNavigation(url_with_ref)); - EXPECT_FALSE(contents->TestIsInPageNavigation(url)); - EXPECT_FALSE(contents->TestIsInPageNavigation(other_url)); - const GURL other_url_with_ref("http://www.google.com/home.html#my_other_ref"); - EXPECT_TRUE(contents->TestIsInPageNavigation(other_url_with_ref)); -} - // Tests to see that webkit preferences are properly loaded and copied over // to a WebPreferences object. TEST_F(WebContentsTest, WebKitPrefs) { diff --git a/chrome/common/notification_types.h b/chrome/common/notification_types.h index 9bf7170..d9e3b44 100644 --- a/chrome/common/notification_types.h +++ b/chrome/common/notification_types.h @@ -67,6 +67,10 @@ enum NotificationType { // Indicates that a NavigationEntry has changed. The source will be the // NavigationController that owns the NavigationEntry. The details will be // a NavigationController::EntryChangedDetails struct. + // + // This will NOT be sent on navigation, interested parties should also listen + // for NOTIFY_NAV_ENTRY_COMMITTED to handle that case. This will be sent when + // the entry is updated outside of navigation (like when a new title comes). NOTIFY_NAV_ENTRY_CHANGED, // Other load-related (not from NavigationController) ------------------------ diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h index e39b392e..6f82feb 100644 --- a/chrome/common/render_messages.h +++ b/chrome/common/render_messages.h @@ -61,11 +61,11 @@ struct ViewHostMsg_FrameNavigate_Params { // iframes are loaded automatically. int32 page_id; - // URL of the page being loaded. NON-CANONICAL. + // URL of the page being loaded. GURL url; // URL of the referrer of this load. WebKit generates this based on the - // source of the event that caused the load. NON-CANONICAL. + // source of the event that caused the load. GURL referrer; // The type of transition. |