diff options
74 files changed, 1005 insertions, 1344 deletions
diff --git a/chrome/browser/app_modal_dialog.cc b/chrome/browser/app_modal_dialog.cc index 91cffdd..dac60e4 100644 --- a/chrome/browser/app_modal_dialog.cc +++ b/chrome/browser/app_modal_dialog.cc @@ -36,7 +36,8 @@ void AppModalDialog::Observe(NotificationType type, return; if (type == NotificationType::NAV_ENTRY_COMMITTED && - Source<NavigationController>(source).ptr() == web_contents_->controller()) + Source<NavigationController>(source).ptr() == + &web_contents_->controller()) web_contents_ = NULL; if (type == NotificationType::TAB_CONTENTS_DESTROYED && diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc index ba4dfe4..1845fd7 100644 --- a/chrome/browser/automation/automation_provider.cc +++ b/chrome/browser/automation/automation_provider.cc @@ -1069,7 +1069,7 @@ void AutomationProvider::AppendTab(int handle, const GURL& url, true, -1, NULL); if (tab_contents) { append_tab_response = - GetIndexForNavigationController(tab_contents->controller(), browser); + GetIndexForNavigationController(&tab_contents->controller(), browser); } } @@ -1730,7 +1730,7 @@ void AutomationProvider::GetTab(int win_handle, int tab_index, if (tab_index < browser->tab_count()) { TabContents* tab_contents = browser->GetTabContentsAt(tab_index); - *tab_handle = tab_tracker_->Add(tab_contents->controller()); + *tab_handle = tab_tracker_->Add(&tab_contents->controller()); } } } @@ -2292,7 +2292,7 @@ void AutomationProvider::CreateExternalTab(HWND parent, external_tab_container->Init(profile_, parent, dimensions, style); TabContents* tab_contents = external_tab_container->tab_contents(); if (tab_contents) { - *tab_handle = tab_tracker_->Add(tab_contents->controller()); + *tab_handle = tab_tracker_->Add(&tab_contents->controller()); *tab_container_window = *external_tab_container; } else { delete external_tab_container; diff --git a/chrome/browser/back_forward_menu_model.cc b/chrome/browser/back_forward_menu_model.cc index 380f70b..83b8e53 100644 --- a/chrome/browser/back_forward_menu_model.cc +++ b/chrome/browser/back_forward_menu_model.cc @@ -21,16 +21,14 @@ const int BackForwardMenuModel::kMaxChapterStops = 5; int BackForwardMenuModel::GetHistoryItemCount() const { TabContents* contents = GetTabContents(); - NavigationController* controller = contents->controller(); - int items = 0; if (model_type_ == FORWARD_MENU_DELEGATE) { // Only count items from n+1 to end (if n is current entry) - items = controller->entry_count() - - controller->GetCurrentEntryIndex() - 1; + items = contents->controller().entry_count() - + contents->controller().GetCurrentEntryIndex() - 1; } else { - items = controller->GetCurrentEntryIndex(); + items = contents->controller().GetCurrentEntryIndex(); } if (items > kMaxHistoryItems) @@ -43,10 +41,9 @@ int BackForwardMenuModel::GetHistoryItemCount() const { int BackForwardMenuModel::GetChapterStopCount(int history_items) const { TabContents* contents = GetTabContents(); - NavigationController* controller = contents->controller(); int chapter_stops = 0; - int current_entry = controller->GetCurrentEntryIndex(); + int current_entry = contents->controller().GetCurrentEntryIndex(); if (history_items == kMaxHistoryItems) { int chapter_id = current_entry; @@ -91,9 +88,9 @@ int BackForwardMenuModel::GetTotalItemCount() const { int BackForwardMenuModel::GetIndexOfNextChapterStop(int start_from, bool forward) const { TabContents* contents = GetTabContents(); - NavigationController* controller = contents->controller(); + NavigationController& controller = contents->controller(); - int max_count = controller->entry_count(); + int max_count = controller.entry_count(); if (start_from < 0 || start_from >= max_count) return -1; // Out of bounds. @@ -107,7 +104,7 @@ int BackForwardMenuModel::GetIndexOfNextChapterStop(int start_from, } } - NavigationEntry* start_entry = controller->GetEntryAtIndex(start_from); + NavigationEntry* start_entry = controller.GetEntryAtIndex(start_from); const GURL& url = start_entry->url(); if (!forward) { @@ -115,7 +112,7 @@ int BackForwardMenuModel::GetIndexOfNextChapterStop(int start_from, // different domain. for (int i = start_from - 1; i >= 0; --i) { if (!net::RegistryControlledDomainService::SameDomainOrHost(url, - controller->GetEntryAtIndex(i)->url())) + controller.GetEntryAtIndex(i)->url())) return i; } // We have reached the beginning without finding a chapter stop. @@ -125,7 +122,7 @@ int BackForwardMenuModel::GetIndexOfNextChapterStop(int start_from, // different domain. for (int i = start_from + 1; i < max_count; ++i) { if (!net::RegistryControlledDomainService::SameDomainOrHost(url, - controller->GetEntryAtIndex(i)->url())) + controller.GetEntryAtIndex(i)->url())) return i - 1; } // Last entry is always considered a chapter stop. @@ -143,10 +140,7 @@ int BackForwardMenuModel::FindChapterStop(int offset, offset *= -1; TabContents* contents = GetTabContents(); - NavigationController* controller = contents->controller(); - - int entry = controller->GetCurrentEntryIndex() + offset; - + int entry = contents->controller().GetCurrentEntryIndex() + offset; for (int i = 0; i < skip + 1; i++) entry = GetIndexOfNextChapterStop(entry, forward); @@ -155,14 +149,14 @@ int BackForwardMenuModel::FindChapterStop(int offset, void BackForwardMenuModel::ExecuteCommandById(int menu_id) { TabContents* contents = GetTabContents(); - NavigationController* controller = contents->controller(); + NavigationController& controller = contents->controller(); DCHECK(!IsSeparator(menu_id)); // Execute the command for the last item: "Show Full History". if (menu_id == GetTotalItemCount()) { UserMetrics::RecordComputedAction(BuildActionName(L"ShowFullHistory", -1), - controller->profile()); + controller.profile()); #if defined(OS_WIN) browser_->ShowSingleDOMUITab(GURL(chrome::kChromeUIHistoryURL)); #else @@ -174,16 +168,16 @@ void BackForwardMenuModel::ExecuteCommandById(int menu_id) { // Log whether it was a history or chapter click. if (menu_id <= GetHistoryItemCount()) { UserMetrics::RecordComputedAction( - BuildActionName(L"HistoryClick", menu_id), controller->profile()); + BuildActionName(L"HistoryClick", menu_id), controller.profile()); } else { UserMetrics::RecordComputedAction( BuildActionName(L"ChapterClick", menu_id - GetHistoryItemCount() - 1), - controller->profile()); + controller.profile()); } int index = MenuIdToNavEntryIndex(menu_id); - if (index >= 0 && index < controller->entry_count()) - controller->GoToIndex(index); + if (index >= 0 && index < controller.entry_count()) + controller.GoToIndex(index); } bool BackForwardMenuModel::IsSeparator(int menu_id) const { @@ -217,7 +211,7 @@ std::wstring BackForwardMenuModel::GetItemLabel(int menu_id) const { NavigationEntry* entry = GetNavigationEntry(menu_id); return UTF16ToWideHack(entry->GetTitleForDisplay( - GetTabContents()->controller())); + &GetTabContents()->controller())); } const SkBitmap& BackForwardMenuModel::GetItemIcon(int menu_id) const { @@ -249,8 +243,6 @@ TabContents* BackForwardMenuModel::GetTabContents() const { int BackForwardMenuModel::MenuIdToNavEntryIndex(int menu_id) const { TabContents* contents = GetTabContents(); - NavigationController* controller = contents->controller(); - int history_items = GetHistoryItemCount(); DCHECK(menu_id > 0); @@ -259,10 +251,10 @@ int BackForwardMenuModel::MenuIdToNavEntryIndex(int menu_id) const { if (menu_id <= history_items) { if (model_type_ == FORWARD_MENU_DELEGATE) { // The |menu_id| is relative to our current position, so we need to add. - menu_id += controller->GetCurrentEntryIndex(); + menu_id += contents->controller().GetCurrentEntryIndex(); } else { // Back menu is reverse. - menu_id = controller->GetCurrentEntryIndex() - menu_id; + menu_id = contents->controller().GetCurrentEntryIndex() - menu_id; } return menu_id; } @@ -281,11 +273,8 @@ int BackForwardMenuModel::MenuIdToNavEntryIndex(int menu_id) const { } NavigationEntry* BackForwardMenuModel::GetNavigationEntry(int menu_id) const { - TabContents* contents = GetTabContents(); - NavigationController* controller = contents->controller(); - int index = MenuIdToNavEntryIndex(menu_id); - return controller->GetEntryAtIndex(index); + return GetTabContents()->controller().GetEntryAtIndex(index); } std::wstring BackForwardMenuModel::BuildActionName( diff --git a/chrome/browser/back_forward_menu_model_unittest.cc b/chrome/browser/back_forward_menu_model_unittest.cc index a171302..708847d 100644 --- a/chrome/browser/back_forward_menu_model_unittest.cc +++ b/chrome/browser/back_forward_menu_model_unittest.cc @@ -33,33 +33,33 @@ class BackFwdMenuModelTest : public RenderViewHostTestHarness { void LoadURLAndUpdateState(const char* url, const char* title) { NavigateAndCommit(GURL(url)); - controller()->GetLastCommittedEntry()->set_title(UTF8ToUTF16(title)); + controller().GetLastCommittedEntry()->set_title(UTF8ToUTF16(title)); } // Navigate back or forward the given amount and commits the entry (which // will be pending after we ask to navigate there). void NavigateToOffset(int offset) { - controller()->GoToOffset(offset); - const NavigationEntry* entry = controller()->pending_entry(); + controller().GoToOffset(offset); + const NavigationEntry* entry = controller().pending_entry(); rvh()->SendNavigate(entry->page_id(), entry->url()); } // Same as NavigateToOffset but goes to an absolute index. void NavigateToIndex(int index) { - controller()->GoToIndex(index); - const NavigationEntry* entry = controller()->pending_entry(); + controller().GoToIndex(index); + const NavigationEntry* entry = controller().pending_entry(); rvh()->SendNavigate(entry->page_id(), entry->url()); } // Goes back/forward and commits the load. void GoBack() { - controller()->GoBack(); - const NavigationEntry* entry = controller()->pending_entry(); + controller().GoBack(); + const NavigationEntry* entry = controller().pending_entry(); rvh()->SendNavigate(entry->page_id(), entry->url()); } void GoForward() { - controller()->GoForward(); - const NavigationEntry* entry = controller()->pending_entry(); + controller().GoForward(); + const NavigationEntry* entry = controller().pending_entry(); rvh()->SendNavigate(entry->page_id(), entry->url()); } }; diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index 0a2ebad..a8501d9 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -536,6 +536,7 @@ TabContents* Browser::AddTabWithURL( return contents; } +// TODO(brettw) this should be just AddTab and it should take a TabContents. TabContents* Browser::AddTabWithNavigationController( NavigationController* ctrl, PageTransition::Type type) { TabContents* tc = ctrl->tab_contents(); @@ -543,35 +544,34 @@ TabContents* Browser::AddTabWithNavigationController( return tc; } -NavigationController* Browser::AddRestoredTab( +TabContents* Browser::AddRestoredTab( const std::vector<TabNavigation>& navigations, int tab_index, int selected_navigation, bool select) { - NavigationController* restored_controller = - BuildRestoredNavigationController(navigations, selected_navigation); + TabContents* new_tab = new WebContents(profile(), NULL, + MSG_ROUTING_NONE, NULL); + new_tab->controller().RestoreFromState(navigations, selected_navigation); - tabstrip_model_.InsertTabContentsAt( - tab_index, - restored_controller->tab_contents(), - select, false); + tabstrip_model_.InsertTabContentsAt(tab_index, new_tab, select, false); if (profile_->HasSessionService()) { SessionService* session_service = profile_->GetSessionService(); if (session_service) - session_service->TabRestored(restored_controller); + session_service->TabRestored(&new_tab->controller()); } - return restored_controller; + return new_tab; } void Browser::ReplaceRestoredTab( const std::vector<TabNavigation>& navigations, int selected_navigation) { - NavigationController* restored_controller = - BuildRestoredNavigationController(navigations, selected_navigation); + TabContents* replacement = new WebContents(profile(), NULL, + MSG_ROUTING_NONE, NULL); + replacement->controller().RestoreFromState(navigations, selected_navigation); tabstrip_model_.ReplaceNavigationControllerAt( tabstrip_model_.selected_index(), - restored_controller); + &replacement->controller()); } bool Browser::CanRestoreTab() { @@ -611,33 +611,34 @@ void Browser::GoBack(WindowOpenDisposition disposition) { return; } - if (current_tab->controller()->CanGoBack()) { - NavigationController* controller = 0; - if (disposition == NEW_FOREGROUND_TAB || disposition == NEW_BACKGROUND_TAB){ - controller = GetSelectedTabContents()->controller()->Clone(); + if (current_tab->controller().CanGoBack()) { + NavigationController* controller = NULL; + if (disposition == NEW_FOREGROUND_TAB || + disposition == NEW_BACKGROUND_TAB) { tabstrip_model_.AddTabContents( - controller->tab_contents(), -1, + GetSelectedTabContents()->Clone(), -1, PageTransition::LINK, disposition == NEW_FOREGROUND_TAB); } else { // Default disposition is CURRENT_TAB. - controller = current_tab->controller(); + controller = ¤t_tab->controller(); } controller->GoBack(); } } void Browser::GoForward(WindowOpenDisposition disp) { + // TODO(brettw) this is mostly duplicated from GoBack, these should have a + // common backend or something. UserMetrics::RecordAction(L"Forward", profile_); - if (GetSelectedTabContents()->controller()->CanGoForward()) { + if (GetSelectedTabContents()->controller().CanGoForward()) { NavigationController* controller = 0; if (disp == NEW_FOREGROUND_TAB || disp == NEW_BACKGROUND_TAB) { - controller = GetSelectedTabContents()->controller()->Clone(); tabstrip_model_.AddTabContents( - controller->tab_contents(), -1, + GetSelectedTabContents()->Clone(), -1, PageTransition::LINK, disp == NEW_FOREGROUND_TAB); } else { // Default disposition is CURRENT_TAB. - controller = GetSelectedTabContents()->controller(); + controller = &GetSelectedTabContents()->controller(); } controller->GoForward(); } @@ -651,7 +652,7 @@ void Browser::Reload() { if (current_tab) { WebContents* web_contents = current_tab->AsWebContents(); if (web_contents && web_contents->showing_interstitial_page()) { - NavigationEntry* entry = current_tab->controller()->GetActiveEntry(); + NavigationEntry* entry = current_tab->controller().GetActiveEntry(); DCHECK(entry); // Should exist if interstitial is showing. OpenURL(entry->url(), GURL(), CURRENT_TAB, PageTransition::RELOAD); return; @@ -661,7 +662,7 @@ void Browser::Reload() { if (current_tab) { // As this is caused by a user action, give the focus to the page. current_tab->Focus(); - current_tab->controller()->Reload(true); + current_tab->controller().Reload(true); } } @@ -797,7 +798,7 @@ void Browser::BookmarkCurrentPage() { if (!model || !model->IsLoaded()) return; // Ignore requests until bookmarks are loaded. - NavigationEntry* entry = contents->controller()->GetActiveEntry(); + NavigationEntry* entry = contents->controller().GetActiveEntry(); if (!entry) return; // Can't star if there is no URL. const GURL& url = entry->display_url(); @@ -822,7 +823,7 @@ void Browser::ViewSource() { UserMetrics::RecordAction(L"ViewSource", profile_); TabContents* current_tab = GetSelectedTabContents(); - NavigationEntry* entry = current_tab->controller()->GetLastCommittedEntry(); + NavigationEntry* entry = current_tab->controller().GetLastCommittedEntry(); if (entry) { GURL url("view-source:" + entry->url().spec()); OpenURL(url, GURL(), NEW_FOREGROUND_TAB, PageTransition::LINK); @@ -1390,23 +1391,19 @@ TabContents* Browser::CreateTabContentsForURL( SiteInstance* instance) const { WebContents* contents = new WebContents(profile, instance, MSG_ROUTING_NONE, NULL); - contents->SetupController(profile); if (!defer_load) { // Load the initial URL before adding the new tab contents to the tab strip // so that the tab contents has navigation state. - contents->controller()->LoadURL(url, referrer, transition); + contents->controller().LoadURL(url, referrer, transition); } return contents; } bool Browser::CanDuplicateContentsAt(int index) { - TabContents* contents = GetTabContentsAt(index); - DCHECK(contents); - - NavigationController* nc = contents->controller(); - return nc ? (nc->tab_contents() && nc->GetLastCommittedEntry()) : false; + NavigationController& nc = GetTabContentsAt(index)->controller(); + return nc.tab_contents() && nc.GetLastCommittedEntry(); } void Browser::DuplicateContentsAt(int index) { @@ -1417,7 +1414,7 @@ void Browser::DuplicateContentsAt(int index) { if (type_ == TYPE_NORMAL) { // If this is a tabbed browser, just create a duplicate tab inside the same // window next to the tab being duplicated. - new_contents = contents->controller()->Clone()->tab_contents(); + new_contents = contents->Clone(); // If you duplicate a tab that is not selected, we need to make sure to // select the tab being duplicated so that DetermineInsertionIndex returns // the right index (if tab 5 is selected and we right-click tab 1 we want @@ -1446,14 +1443,14 @@ void Browser::DuplicateContentsAt(int index) { // The page transition below is only for the purpose of inserting the tab. new_contents = browser->AddTabWithNavigationController( - contents->controller()->Clone(), + &contents->Clone()->controller(), PageTransition::LINK); } if (profile_->HasSessionService()) { SessionService* session_service = profile_->GetSessionService(); if (session_service) - session_service->TabRestored(new_contents->controller()); + session_service->TabRestored(&new_contents->controller()); } } @@ -1479,7 +1476,7 @@ void Browser::CreateHistoricalTab(TabContents* contents) { // We only create historical tab entries for normal tabbed browser windows. if (type() == TYPE_NORMAL) { profile()->GetTabRestoreService()->CreateHistoricalTab( - contents->controller()); + &contents->controller()); } } @@ -1510,7 +1507,7 @@ void Browser::TabInsertedAt(TabContents* contents, int index, bool foreground) { contents->set_delegate(this); - contents->controller()->SetWindowID(session_id()); + contents->controller().SetWindowID(session_id()); SyncHistoryWithTabs(tabstrip_model_.GetIndexOfTabContents(contents)); @@ -1527,11 +1524,9 @@ void Browser::TabInsertedAt(TabContents* contents, } void Browser::TabClosingAt(TabContents* contents, int index) { - NavigationController* controller = contents->controller(); - DCHECK(controller); NotificationService::current()->Notify( NotificationType::TAB_CLOSING, - Source<NavigationController>(controller), + Source<NavigationController>(&contents->controller()), NotificationService::NoDetails()); // Sever the TabContents' connection back to us. @@ -1704,15 +1699,8 @@ void Browser::OpenURLFromTab(TabContents* source, } else if ((disposition == CURRENT_TAB) && current_tab) { tabstrip_model_.TabNavigating(current_tab, transition); - // TODO(beng): remove all this once there are no TabContents types. - // It seems like under some circumstances current_tab can be dust after the - // call to LoadURL (perhaps related to TabContents type switching), so we - // save the NavigationController here. - NavigationController* controller = current_tab->controller(); - controller->LoadURL(url, referrer, transition); - // If the TabContents type has been swapped, we need to point to the current - // active type otherwise there will be weirdness. - new_contents = controller->tab_contents(); + current_tab->controller().LoadURL(url, referrer, transition); + new_contents = current_tab; if (GetStatusBubble()) GetStatusBubble()->Hide(); @@ -1886,7 +1874,7 @@ void Browser::ConvertContentsToApplication(TabContents* contents) { if (index < 0) return; - const GURL& url = contents->controller()->GetActiveEntry()->url(); + const GURL& url = contents->controller().GetActiveEntry()->url(); std::wstring app_name = ComputeApplicationNameFromURL(url); RegisterAppPrefs(app_name); @@ -1994,7 +1982,7 @@ void Browser::Observe(NotificationType type, // actually be for a different window while we're doing asynchronous // closing of this one. if (GetSelectedTabContents() && - GetSelectedTabContents()->controller() == + &GetSelectedTabContents()->controller() == Source<NavigationController>(source).ptr()) UpdateToolbar(false); break; @@ -2137,9 +2125,9 @@ void Browser::UpdateCommandsForTabState() { return; // Navigation commands - NavigationController* nc = current_tab->controller(); - command_updater_.UpdateCommandEnabled(IDC_BACK, nc->CanGoBack()); - command_updater_.UpdateCommandEnabled(IDC_FORWARD, nc->CanGoForward()); + NavigationController& nc = current_tab->controller(); + command_updater_.UpdateCommandEnabled(IDC_BACK, nc.CanGoBack()); + command_updater_.UpdateCommandEnabled(IDC_FORWARD, nc.CanGoForward()); // Window management commands command_updater_.UpdateCommandEnabled(IDC_DUPLICATE_TAB, @@ -2151,7 +2139,7 @@ void Browser::UpdateCommandsForTabState() { bool is_web_contents = web_contents != NULL; // Current navigation entry, may be NULL. - NavigationEntry* active_entry = current_tab->controller()->GetActiveEntry(); + NavigationEntry* active_entry = current_tab->controller().GetActiveEntry(); // Page-related commands // Only allow bookmarking for web content in normal windows. @@ -2245,7 +2233,7 @@ void Browser::ScheduleUIUpdate(const TabContents* source, // this for any tab so they start & stop quickly, but the source can be // NULL, so we have to check for that. tabstrip_model_.UpdateTabContentsStateAt( - tabstrip_model_.GetIndexOfController(source->controller()), true); + tabstrip_model_.GetIndexOfController(&source->controller()), true); } // If the only updates were synchronously handled above, we're done. @@ -2273,8 +2261,8 @@ void Browser::ProcessPendingUIUpdates() { for (size_t i = 0; i < scheduled_updates_.size(); i++) { bool found = false; for (int tab = 0; tab < tab_count(); tab++) { - if (GetTabContentsAt(tab)->controller() == - scheduled_updates_[i].source->controller()) { + if (&GetTabContentsAt(tab)->controller() == + &scheduled_updates_[i].source->controller()) { found = true; break; } @@ -2331,7 +2319,7 @@ void Browser::ProcessPendingUIUpdates() { if (invalidate_tab) { // INVALIDATE_TITLE or INVALIDATE_FAVICON. tabstrip_model_.UpdateTabContentsStateAt( - tabstrip_model_.GetIndexOfController(contents->controller()), false); + tabstrip_model_.GetIndexOfController(&contents->controller()), false); window_->UpdateTitleBar(); if (contents == GetSelectedTabContents()) { @@ -2382,13 +2370,13 @@ void Browser::SyncHistoryWithTabs(int index) { TabContents* contents = GetTabContentsAt(i); if (contents) { session_service->SetTabIndexInWindow( - session_id(), contents->controller()->session_id(), i); + session_id(), contents->controller().session_id(), i); } } } } -NavigationController* Browser::BuildRestoredNavigationController( +TabContents* Browser::BuildRestoredTab( const std::vector<TabNavigation>& navigations, int selected_navigation) { if (!navigations.empty()) { @@ -2396,13 +2384,14 @@ NavigationController* Browser::BuildRestoredNavigationController( selected_navigation < static_cast<int>(navigations.size())); // Create a NavigationController. This constructor creates the appropriate // set of TabContents. - return new NavigationController(profile_, navigations, selected_navigation); + TabContents* new_tab = new WebContents(profile_, NULL, + MSG_ROUTING_NONE, NULL); + new_tab->controller().RestoreFromState(navigations, selected_navigation); + return new_tab; } else { // No navigations. Create a tab with about:blank. - TabContents* contents = - CreateTabContentsForURL(GURL("about:blank"), GURL(), profile_, - PageTransition::START_PAGE, false, NULL); - return new NavigationController(contents, profile_); + return CreateTabContentsForURL(GURL("about:blank"), GURL(), profile_, + PageTransition::START_PAGE, false, NULL); } } diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h index af0fb52..d0f2d6f 100644 --- a/chrome/browser/browser.h +++ b/chrome/browser/browser.h @@ -227,15 +227,13 @@ class Browser : public TabStripModelDelegate, PageTransition::Type type); // Add a tab with its session history restored from the SessionRestore - // system. If select is true, the tab is selected. Returns the created - // NavigationController. |tab_index| gives the index to insert the tab at. - // |selected_navigation| is the index of the TabNavigation in |navigations| - // to select. - NavigationController* AddRestoredTab( - const std::vector<TabNavigation>& navigations, - int tab_index, - int selected_navigation, - bool select); + // system. If select is true, the tab is selected. |tab_index| gives the index + // to insert the tab at. |selected_navigation| is the index of the + // TabNavigation in |navigations| to select. + TabContents* AddRestoredTab(const std::vector<TabNavigation>& navigations, + int tab_index, + int selected_navigation, + bool select); // Replaces the state of the currently selected tab with the session // history restored from the SessionRestore system. @@ -525,11 +523,10 @@ class Browser : public TabStripModelDelegate, void SyncHistoryWithTabs(int index); // Called from AddRestoredTab and ReplaceRestoredTab to build a - // NavigationController from an incoming vector of TabNavigations. - // Caller takes ownership of the returned NavigationController. - NavigationController* BuildRestoredNavigationController( - const std::vector<TabNavigation>& navigations, - int selected_navigation); + // TabContents from an incoming vector of TabNavigations. + // Caller takes ownership of the returned TabContents. + TabContents* BuildRestoredTab(const std::vector<TabNavigation>& navigations, + int selected_navigation); // OnBeforeUnload handling ////////////////////////////////////////////////// @@ -639,7 +636,7 @@ class Browser : public TabStripModelDelegate, // toolbar during window creation (i.e. before any tabs have been added // to the window). TabContents* current_tab = browser_->GetSelectedTabContents(); - return current_tab ? current_tab->controller() : NULL; + return current_tab ? ¤t_tab->controller() : NULL; } private: diff --git a/chrome/browser/browser_commands_unittest.cc b/chrome/browser/browser_commands_unittest.cc index f55cb8e..5cecf0a 100644 --- a/chrome/browser/browser_commands_unittest.cc +++ b/chrome/browser/browser_commands_unittest.cc @@ -68,13 +68,13 @@ TEST_F(BrowserCommandsTest, DuplicateTab) { ASSERT_EQ(2, browser()->tab_count()); // Verify the stack of urls. - NavigationController* controller = + NavigationController& controller = browser()->GetTabContentsAt(1)->controller(); - ASSERT_EQ(3, controller->entry_count()); - ASSERT_EQ(2, controller->GetCurrentEntryIndex()); - ASSERT_TRUE(url1 == controller->GetEntryAtIndex(0)->url()); - ASSERT_TRUE(url2 == controller->GetEntryAtIndex(1)->url()); - ASSERT_TRUE(url3 == controller->GetEntryAtIndex(2)->url()); + ASSERT_EQ(3, controller.entry_count()); + ASSERT_EQ(2, controller.GetCurrentEntryIndex()); + ASSERT_TRUE(url1 == controller.GetEntryAtIndex(0)->url()); + ASSERT_TRUE(url2 == controller.GetEntryAtIndex(1)->url()); + ASSERT_TRUE(url3 == controller.GetEntryAtIndex(2)->url()); } TEST_F(BrowserCommandsTest, BookmarkCurrentPage) { diff --git a/chrome/browser/debugger/debugger_host_impl.cpp b/chrome/browser/debugger/debugger_host_impl.cpp index e22a391..1f98f4b8 100644 --- a/chrome/browser/debugger/debugger_host_impl.cpp +++ b/chrome/browser/debugger/debugger_host_impl.cpp @@ -16,7 +16,7 @@ class TabContentsReference : public NotificationObserver { public: TabContentsReference(TabContents *c) : navigation_controller_(NULL) { - navigation_controller_ = c->controller(); + navigation_controller_ = &c->controller(); NotificationService* service = NotificationService::current(); DCHECK(service); diff --git a/chrome/browser/debugger/debugger_node.cc b/chrome/browser/debugger/debugger_node.cc index c22bd7f..0beedce 100644 --- a/chrome/browser/debugger/debugger_node.cc +++ b/chrome/browser/debugger/debugger_node.cc @@ -291,12 +291,12 @@ v8::Handle<v8::Value> TabListNode::IndexGetter(uint32_t index, ///////////////////////////////////////////// TabNode::TabNode(TabContents *c) { - data_ = c->controller(); + data_ = &c->controller(); NotificationService* service = NotificationService::current(); DCHECK(service); service->AddObserver(this, NotificationType::TAB_CLOSING, - Source<NavigationController>(c->controller())); + Source<NavigationController>(&c->controller())); observing_ = true; } diff --git a/chrome/browser/debugger/debugger_view.cc b/chrome/browser/debugger/debugger_view.cc index 3a72171..0c9f6f8 100644 --- a/chrome/browser/debugger/debugger_view.cc +++ b/chrome/browser/debugger/debugger_view.cc @@ -99,14 +99,13 @@ void DebuggerView::OnInit() { Profile* profile = BrowserList::GetLastActive()->profile(); web_contents_ = new WebContents(profile, NULL, MSG_ROUTING_NONE, NULL); - web_contents_->SetupController(profile); web_contents_->set_delegate(this); web_container_->SetTabContents(web_contents_); web_contents_->render_view_host()->AllowDOMUIBindings(); GURL contents("chrome-ui://inspector/debugger.html"); - web_contents_->controller()->LoadURL(contents, GURL(), - PageTransition::START_PAGE); + web_contents_->controller().LoadURL(contents, GURL(), + PageTransition::START_PAGE); } void DebuggerView::OnShow() { @@ -115,7 +114,7 @@ void DebuggerView::OnShow() { void DebuggerView::OnClose() { web_container_->SetTabContents(NULL); - web_contents_->CloseContents(); + delete web_contents_; } void DebuggerView::OpenURLFromTab(TabContents* source, diff --git a/chrome/browser/debugger/devtools_manager.cc b/chrome/browser/debugger/devtools_manager.cc index f048689..6afa7a3 100644 --- a/chrome/browser/debugger/devtools_manager.cc +++ b/chrome/browser/debugger/devtools_manager.cc @@ -35,22 +35,18 @@ void DevToolsManager::Observe(NotificationType type, return; } - NavigationController* controller = src->controller(); - bool active = (controller->tab_contents() == src.ptr()); - if (active) { - // Active tab contents disconnecting from its renderer means that the tab - // is closing. - client_host->InspectedTabClosing(); - UnregisterDevToolsClientHost(client_host, controller); - } + // Active tab contents disconnecting from its renderer means that the tab + // is closing. + client_host->InspectedTabClosing(); + UnregisterDevToolsClientHost(client_host, &src->controller()); } } DevToolsClientHost* DevToolsManager::GetDevToolsClientHostFor( const WebContents& web_contents) { - NavigationController* navigation_controller = web_contents.controller(); + const NavigationController& navigation_controller = web_contents.controller(); ClientHostMap::const_iterator it = - navcontroller_to_client_host_.find(navigation_controller); + navcontroller_to_client_host_.find(&navigation_controller); if (it != navcontroller_to_client_host_.end()) { return it->second; } @@ -58,11 +54,11 @@ DevToolsClientHost* DevToolsManager::GetDevToolsClientHostFor( } void DevToolsManager::RegisterDevToolsClientHostFor( - const WebContents& web_contents, + WebContents& web_contents, DevToolsClientHost* client_host) { DCHECK(!GetDevToolsClientHostFor(web_contents)); - NavigationController* navigation_controller = web_contents.controller(); + NavigationController* navigation_controller = &web_contents.controller(); navcontroller_to_client_host_[navigation_controller] = client_host; client_host_to_navcontroller_[client_host] = navigation_controller; client_host->set_close_listener(this); diff --git a/chrome/browser/debugger/devtools_manager.h b/chrome/browser/debugger/devtools_manager.h index fe6fc4f..77ffaf5 100644 --- a/chrome/browser/debugger/devtools_manager.h +++ b/chrome/browser/debugger/devtools_manager.h @@ -36,7 +36,7 @@ class DevToolsManager : public NotificationObserver, // Registers new DevToolsClientHost for |web_contents|. There must be no // other DevToolsClientHosts registered for the WebContents at the moment. - void RegisterDevToolsClientHostFor(const WebContents& web_contents, + void RegisterDevToolsClientHostFor(WebContents& web_contents, DevToolsClientHost* client_host); void ForwardToDevToolsAgent(const RenderViewHost& client_rvh, diff --git a/chrome/browser/debugger/devtools_view.cc b/chrome/browser/debugger/devtools_view.cc index 483f87f..179eb7e 100644 --- a/chrome/browser/debugger/devtools_view.cc +++ b/chrome/browser/debugger/devtools_view.cc @@ -50,7 +50,6 @@ void DevToolsView::Init() { Profile* profile = BrowserList::GetLastActive()->profile(); web_contents_ = new WebContents(profile, NULL, MSG_ROUTING_NONE, NULL); - web_contents_->SetupController(profile); web_contents_->set_delegate(this); web_container_->SetTabContents(web_contents_); web_contents_->render_view_host()->AllowDOMUIBindings(); @@ -59,8 +58,8 @@ void DevToolsView::Init() { GURL contents(std::string(chrome::kChromeUIDevToolsURL) + "devtools.html"); // this will call CreateRenderView to create renderer process - web_contents_->controller()->LoadURL(contents, GURL(), - PageTransition::START_PAGE); + web_contents_->controller().LoadURL(contents, GURL(), + PageTransition::START_PAGE); } void DevToolsView::OnWindowClosing() { @@ -70,7 +69,7 @@ void DevToolsView::OnWindowClosing() { web_container_->SetTabContents(NULL); // Destroy the tab and navigation controller. - web_contents_->CloseContents(); + delete web_contents_; web_contents_ = NULL; } } diff --git a/chrome/browser/debugger/inspectable_tab_proxy.cc b/chrome/browser/debugger/inspectable_tab_proxy.cc index e141bca..a329d6a 100644 --- a/chrome/browser/debugger/inspectable_tab_proxy.cc +++ b/chrome/browser/debugger/inspectable_tab_proxy.cc @@ -86,9 +86,9 @@ const InspectableTabProxy::ControllersMap& end = BrowserList::end(); it != end; ++it) { TabStripModel* model = (*it)->tabstrip_model(); for (int i = 0, size = model->count(); i < size; ++i) { - NavigationController* controller = + NavigationController& controller = model->GetTabContentsAt(i)->controller(); - controllers_map_[controller->session_id().id()] = controller; + controllers_map_[controller.session_id().id()] = &controller; } } } diff --git a/chrome/browser/dom_ui/dom_ui_unittest.cc b/chrome/browser/dom_ui/dom_ui_unittest.cc index 51b045a..e108c23 100644 --- a/chrome/browser/dom_ui/dom_ui_unittest.cc +++ b/chrome/browser/dom_ui/dom_ui_unittest.cc @@ -16,7 +16,7 @@ class DOMUITest : public RenderViewHostTestHarness { // ID that we should use is passed as a parameter. We'll use the next two // values. This must be increasing for the life of the tests. static void DoNavigationTest(WebContents* contents, int page_id) { - NavigationController* controller = contents->controller(); + NavigationController* controller = &contents->controller(); // Start a pending load. GURL new_tab_url(chrome::kChromeUINewTabURL); @@ -88,23 +88,19 @@ TEST_F(DOMUITest, DOMUIToStandard) { // slightly different than the very-first-navigation case since the // SiteInstance will be the same (the original WebContents must still be // alive), which will trigger different behavior in RenderViewHostManager. - WebContents* contents2 = new TestWebContents(profile_.get(), NULL); - NavigationController* controller2 = - new NavigationController(contents2, profile_.get()); - contents2->set_controller(controller2); + TestWebContents contents2(profile_.get(), NULL); - DoNavigationTest(contents2, 101); - contents2->CloseContents(); + DoNavigationTest(&contents2, 101); } TEST_F(DOMUITest, DOMUIToDOMUI) { // Do a load (this state is tested above). GURL new_tab_url(chrome::kChromeUINewTabURL); - controller()->LoadURL(new_tab_url, GURL(), PageTransition::LINK); + controller().LoadURL(new_tab_url, GURL(), PageTransition::LINK); rvh()->SendNavigate(1, new_tab_url); // Start another pending load of the new tab page. - controller()->LoadURL(new_tab_url, GURL(), PageTransition::LINK); + controller().LoadURL(new_tab_url, GURL(), PageTransition::LINK); rvh()->SendNavigate(2, new_tab_url); // The flags should be the same as the non-pending state. @@ -117,7 +113,7 @@ TEST_F(DOMUITest, DOMUIToDOMUI) { TEST_F(DOMUITest, StandardToDOMUI) { // Start a pending navigation to a regular page. GURL std_url("http://google.com/"); - controller()->LoadURL(std_url, GURL(), PageTransition::LINK); + controller().LoadURL(std_url, GURL(), PageTransition::LINK); // The state should now reflect the default. EXPECT_TRUE(contents()->ShouldDisplayURL()); @@ -134,7 +130,7 @@ TEST_F(DOMUITest, StandardToDOMUI) { // Start a pending load for a DOMUI. GURL new_tab_url(chrome::kChromeUINewTabURL); - controller()->LoadURL(new_tab_url, GURL(), PageTransition::LINK); + controller().LoadURL(new_tab_url, GURL(), PageTransition::LINK); EXPECT_FALSE(contents()->ShouldDisplayURL()); EXPECT_TRUE(contents()->ShouldDisplayFavIcon()); EXPECT_FALSE(contents()->IsBookmarkBarAlwaysVisible()); diff --git a/chrome/browser/dom_ui/new_tab_ui.cc b/chrome/browser/dom_ui/new_tab_ui.cc index cd9b5d9..90bf3dc 100644 --- a/chrome/browser/dom_ui/new_tab_ui.cc +++ b/chrome/browser/dom_ui/new_tab_ui.cc @@ -890,9 +890,8 @@ RecentlyClosedTabsHandler::~RecentlyClosedTabsHandler() { } void RecentlyClosedTabsHandler::HandleReopenTab(const Value* content) { - NavigationController* controller = dom_ui_->web_contents()->controller(); Browser* browser = Browser::GetBrowserForController( - controller, NULL); + &dom_ui_->web_contents()->controller(), NULL); if (!browser) return; @@ -1053,9 +1052,7 @@ void HistoryHandler::HandleSearchHistoryPage(const Value* content) { #if defined(OS_WIN) // TODO(port): include this once history is converted to HTML - NavigationController* controller = - dom_ui_->web_contents()->controller(); - controller->LoadURL( + dom_ui_->web_contents()->controller().LoadURL( HistoryUI::GetHistoryURLWithSearchText(wstring_value), GURL(), PageTransition::LINK); diff --git a/chrome/browser/download/download_request_manager.cc b/chrome/browser/download/download_request_manager.cc index 98efb19..6078fce 100644 --- a/chrome/browser/download/download_request_manager.cc +++ b/chrome/browser/download/download_request_manager.cc @@ -177,7 +177,7 @@ DownloadRequestManager::~DownloadRequestManager() { DownloadRequestManager::DownloadStatus DownloadRequestManager::GetDownloadStatus(TabContents* tab) { - TabDownloadState* state = GetDownloadState(tab->controller(), NULL, false); + TabDownloadState* state = GetDownloadState(&tab->controller(), NULL, false); return state ? state->download_status() : ALLOW_ONE_DOWNLOAD; } @@ -193,13 +193,7 @@ void DownloadRequestManager::CanDownloadOnIOThread(int render_process_host_id, } void DownloadRequestManager::OnUserGesture(TabContents* tab) { - NavigationController* controller = tab->controller(); - if (!controller) { - NOTREACHED(); - return; - } - - TabDownloadState* state = GetDownloadState(controller, NULL, false); + TabDownloadState* state = GetDownloadState(&tab->controller(), NULL, false); if (!state) return; @@ -256,10 +250,8 @@ void DownloadRequestManager::CanDownloadImpl( effective_tab->delegate()->GetConstrainingContents(effective_tab); } - NavigationController* controller = effective_tab->controller(); - DCHECK(controller); TabDownloadState* state = GetDownloadState( - controller, originating_tab->controller(), true); + &effective_tab->controller(), &originating_tab->controller(), true); switch (state->download_status()) { case ALLOW_ALL_DOWNLOADS: ScheduleNotification(callback, true); diff --git a/chrome/browser/download/download_request_manager_unittest.cc b/chrome/browser/download/download_request_manager_unittest.cc index 943b439..a8ece7a 100644 --- a/chrome/browser/download/download_request_manager_unittest.cc +++ b/chrome/browser/download/download_request_manager_unittest.cc @@ -38,7 +38,7 @@ class DownloadRequestManagerTest void CanDownload() { download_request_manager_->CanDownloadImpl( - controller()->tab_contents(), this); + controller().tab_contents(), this); } bool ShouldAllowDownload() { @@ -81,13 +81,13 @@ TEST_F(DownloadRequestManagerTest, Allow) { // All tabs should initially start at ALLOW_ONE_DOWNLOAD. ASSERT_EQ(DownloadRequestManager::ALLOW_ONE_DOWNLOAD, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // Ask if the tab can do a download. This moves to PROMPT_BEFORE_DOWNLOAD. CanDownload(); ASSERT_EQ(DownloadRequestManager::PROMPT_BEFORE_DOWNLOAD, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // We should have been told we can download. ASSERT_EQ(1, continue_count_); ASSERT_EQ(0, cancel_count_); @@ -102,7 +102,7 @@ TEST_F(DownloadRequestManagerTest, Allow) { ask_allow_count_ = 0; ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // We should have been told we can download. ASSERT_EQ(1, continue_count_); ASSERT_EQ(0, cancel_count_); @@ -114,7 +114,7 @@ TEST_F(DownloadRequestManagerTest, Allow) { ASSERT_EQ(0, ask_allow_count_); ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // We should have been told we can download. ASSERT_EQ(1, continue_count_); ASSERT_EQ(0, cancel_count_); @@ -131,7 +131,7 @@ TEST_F(DownloadRequestManagerTest, ResetOnNavigation) { ask_allow_count_ = continue_count_ = cancel_count_ = 0; ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // Navigate to a new URL with the same host, which shouldn't reset the allow // all state. @@ -143,20 +143,20 @@ TEST_F(DownloadRequestManagerTest, ResetOnNavigation) { ask_allow_count_ = continue_count_ = cancel_count_ = 0; ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // Do a user gesture, because we're at allow all, this shouldn't change the // state. - download_request_manager_->OnUserGesture(controller()->tab_contents()); + download_request_manager_->OnUserGesture(controller().tab_contents()); ASSERT_EQ(DownloadRequestManager::ALLOW_ALL_DOWNLOADS, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // Navigate to a completely different host, which should reset the state. NavigateAndCommit(GURL("http://fooey.com")); ASSERT_EQ(DownloadRequestManager::ALLOW_ONE_DOWNLOAD, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); } TEST_F(DownloadRequestManagerTest, ResetOnUserGesture) { @@ -167,13 +167,13 @@ TEST_F(DownloadRequestManagerTest, ResetOnUserGesture) { ask_allow_count_ = continue_count_ = cancel_count_ = 0; ASSERT_EQ(DownloadRequestManager::PROMPT_BEFORE_DOWNLOAD, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // Do a user gesture, which should reset back to allow one. - download_request_manager_->OnUserGesture(controller()->tab_contents()); + download_request_manager_->OnUserGesture(controller().tab_contents()); ASSERT_EQ(DownloadRequestManager::ALLOW_ONE_DOWNLOAD, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // Ask twice, which triggers calling the delegate. Don't allow the download // so that we end up with not allowed. @@ -182,13 +182,13 @@ TEST_F(DownloadRequestManagerTest, ResetOnUserGesture) { CanDownload(); ASSERT_EQ(DownloadRequestManager::DOWNLOADS_NOT_ALLOWED, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // A user gesture now should NOT change the state. - download_request_manager_->OnUserGesture(controller()->tab_contents()); + download_request_manager_->OnUserGesture(controller().tab_contents()); ASSERT_EQ(DownloadRequestManager::DOWNLOADS_NOT_ALLOWED, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); // And make sure we really can't download. ask_allow_count_ = continue_count_ = cancel_count_ = 0; CanDownload(); @@ -198,5 +198,5 @@ TEST_F(DownloadRequestManagerTest, ResetOnUserGesture) { // And the state shouldn't have changed. ASSERT_EQ(DownloadRequestManager::DOWNLOADS_NOT_ALLOWED, download_request_manager_->GetDownloadStatus( - controller()->tab_contents())); + controller().tab_contents())); } diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc index 27cf982..06bbcb5 100644 --- a/chrome/browser/extensions/extension_tabs_module.cc +++ b/chrome/browser/extensions/extension_tabs_module.cc @@ -24,11 +24,11 @@ int ExtensionTabUtil::GetWindowId(const Browser* browser) { } int ExtensionTabUtil::GetTabId(const TabContents* tab_contents) { - return tab_contents->controller()->session_id().id(); + return tab_contents->controller().session_id().id(); } int ExtensionTabUtil::GetWindowIdOfTab(const TabContents* tab_contents) { - return tab_contents->controller()->window_id().id(); + return tab_contents->controller().window_id().id(); } bool GetWindowsFunction::RunImpl() { @@ -150,9 +150,6 @@ bool GetTabFunction::RunImpl() { if (!GetIndexOfTabId(tab_strip, tab_id, &tab_index)) return false; - TabContents* tab_contents = tab_strip->GetTabContentsAt(tab_index); - NavigationController* controller = tab_contents->controller(); - DCHECK(controller); result_.reset(CreateTabValue(tab_strip, tab_index)); return true; } @@ -178,8 +175,7 @@ bool UpdateTabFunction::RunImpl() { return false; TabContents* tab_contents = tab_strip->GetTabContentsAt(tab_index); - NavigationController* controller = tab_contents->controller(); - DCHECK(controller); + NavigationController& controller = tab_contents->controller(); // TODO(rafaelw): handle setting remaining tab properties: // -title @@ -190,7 +186,7 @@ bool UpdateTabFunction::RunImpl() { if (args->GetString(L"url", &url)) { GURL new_gurl(url); if (new_gurl.is_valid()) { - controller->LoadURL(new_gurl, GURL(), PageTransition::TYPED); + controller.LoadURL(new_gurl, GURL(), PageTransition::TYPED); } else { // TODO(rafaelw): return some reasonable error? } @@ -227,10 +223,6 @@ bool MoveTabFunction::RunImpl() { if (!GetIndexOfTabId(tab_strip, tab_id, &tab_index)) return false; - TabContents* tab_contents = tab_strip->GetTabContentsAt(tab_index); - NavigationController* controller = tab_contents->controller(); - DCHECK(controller); - // TODO(rafaelw): support moving tabs between windows // -windowId @@ -273,10 +265,7 @@ bool RemoveTabFunction::RunImpl() { int tab_index; TabStripModel* tab_strip = browser->tabstrip_model(); if (GetIndexOfTabId(tab_strip, tab_id, &tab_index)) { - TabContents* tab_contents = tab_strip->GetTabContentsAt(tab_index); - NavigationController* controller = tab_contents->controller(); - DCHECK(controller); - browser->CloseContents(tab_contents); + browser->CloseContents(tab_strip->GetTabContentsAt(tab_index)); return true; } @@ -314,8 +303,6 @@ static ListValue* CreateTabList(Browser* browser) { static DictionaryValue* CreateTabValue(TabStripModel* tab_strip, int tab_index) { TabContents* contents = tab_strip->GetTabContentsAt(tab_index); - NavigationController* controller = contents->controller(); - DCHECK(controller); // TODO(aa): Is this a valid assumption? DictionaryValue* result = new DictionaryValue(); result->SetInteger(L"id", ExtensionTabUtil::GetTabId(contents)); @@ -325,7 +312,7 @@ static DictionaryValue* CreateTabValue(TabStripModel* tab_strip, result->SetString(L"title", UTF16ToWide(contents->GetTitle())); result->SetBoolean(L"selected", tab_index == tab_strip->selected_index()); - NavigationEntry* entry = controller->GetActiveEntry(); + NavigationEntry* entry = contents->controller().GetActiveEntry(); if (entry) { if (entry->favicon().is_valid()) result->SetString(L"favIconUrl", entry->favicon().url().spec()); @@ -338,10 +325,7 @@ static bool GetIndexOfTabId(const TabStripModel* tab_strip, int tab_id, int* tab_index) { for (int i = 0; i < tab_strip->count(); ++i) { TabContents* tab_contents = tab_strip->GetTabContentsAt(i); - NavigationController* controller = tab_contents->controller(); - DCHECK(controller); // TODO(aa): Is this a valid assumption? - - if (controller->session_id().id() == tab_id) { + if (tab_contents->controller().session_id().id() == tab_id) { *tab_index = i; return true; } diff --git a/chrome/browser/external_tab_container.cc b/chrome/browser/external_tab_container.cc index d9d375c..79a84fc 100644 --- a/chrome/browser/external_tab_container.cc +++ b/chrome/browser/external_tab_container.cc @@ -68,7 +68,6 @@ bool ExternalTabContainer::Init(Profile* profile, HWND parent, focus_manager->AddKeystrokeListener(this); tab_contents_ = new WebContents(profile, NULL, MSG_ROUTING_NONE, NULL); - tab_contents_->SetupController(profile); tab_contents_->set_delegate(this); tab_contents_->render_view_host()->AllowExternalHostBindings(); @@ -91,8 +90,7 @@ bool ExternalTabContainer::Init(Profile* profile, HWND parent, DCHECK(dummy->IsFocusable()); root_view_.AddChildView(dummy); - NavigationController* controller = tab_contents_->controller(); - DCHECK(controller); + NavigationController* controller = &tab_contents_->controller(); registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED, Source<NavigationController>(controller)); registrar_.Add(this, NotificationType::FAIL_PROVISIONAL_LOAD_WITH_ERROR, @@ -131,17 +129,12 @@ bool ExternalTabContainer::Uninitialize(HWND window) { root_view_.RemoveAllChildViews(true); if (tab_contents_) { - NavigationController* controller = tab_contents_->controller(); - DCHECK(controller); - NotificationService::current()->Notify( NotificationType::EXTERNAL_TAB_CLOSED, - Source<NavigationController>(controller), + Source<NavigationController>(&tab_contents_->controller()), Details<ExternalTabContainer>(this)); - tab_contents_->set_delegate(NULL); - tab_contents_->CloseContents(); - // WARNING: tab_contents_ has likely been deleted. + delete tab_contents_; tab_contents_ = NULL; } @@ -309,7 +302,7 @@ void ExternalTabContainer::Observe(NotificationType type, automation_->Send(new AutomationMsg_DidNavigate( 0, commit->type, commit->previous_entry_index - - tab_contents_->controller()->last_committed_entry_index(), + tab_contents_->controller().last_committed_entry_index(), commit->entry->url())); } break; diff --git a/chrome/browser/fav_icon_helper.cc b/chrome/browser/fav_icon_helper.cc index 3b23e3b..a3c3fcc 100644 --- a/chrome/browser/fav_icon_helper.cc +++ b/chrome/browser/fav_icon_helper.cc @@ -129,7 +129,7 @@ void FavIconHelper::UpdateFavIcon(NavigationEntry* entry, } NavigationEntry* FavIconHelper::GetEntry() { - NavigationEntry* entry = web_contents_->controller()->GetActiveEntry(); + NavigationEntry* entry = web_contents_->controller().GetActiveEntry(); if (entry && entry->url() == url_ && web_contents_->IsActiveEntry(entry->page_id())) { return entry; diff --git a/chrome/browser/find_backend_unittest.cc b/chrome/browser/find_backend_unittest.cc index 2a2dba4..260e897 100644 --- a/chrome/browser/find_backend_unittest.cc +++ b/chrome/browser/find_backend_unittest.cc @@ -15,16 +15,13 @@ TEST_F(FindBackendTest, InternalState) { EXPECT_EQ(string16(), contents()->find_text()); // Get another WebContents object ready. - WebContents* contents2 = new TestWebContents(profile_.get(), NULL); - NavigationController* controller2 = - new NavigationController(contents2, profile_.get()); - contents2->set_controller(controller2); + TestWebContents contents2(profile_.get(), NULL); // No search has still been issued, strings should be blank. EXPECT_EQ(string16(), contents()->find_prepopulate_text()); EXPECT_EQ(string16(), contents()->find_text()); - EXPECT_EQ(string16(), contents2->find_prepopulate_text()); - EXPECT_EQ(string16(), contents2->find_text()); + EXPECT_EQ(string16(), contents2.find_prepopulate_text()); + EXPECT_EQ(string16(), contents2.find_text()); string16 search_term1 = L" I had a 401K "; string16 search_term2 = L" but the economy "; @@ -37,18 +34,18 @@ TEST_F(FindBackendTest, InternalState) { // should not. EXPECT_EQ(search_term1, contents()->find_prepopulate_text()); EXPECT_EQ(search_term1, contents()->find_text()); - EXPECT_EQ(search_term1, contents2->find_prepopulate_text()); - EXPECT_EQ(string16(), contents2->find_text()); + EXPECT_EQ(search_term1, contents2.find_prepopulate_text()); + EXPECT_EQ(string16(), contents2.find_text()); // Now search in the other WebContents. - contents2->StartFinding(search_term2, true); // true=forward. + contents2.StartFinding(search_term2, true); // true=forward. // Again, pre-populate string should always match between the two, but // find_text should not. EXPECT_EQ(search_term2, contents()->find_prepopulate_text()); EXPECT_EQ(search_term1, contents()->find_text()); - EXPECT_EQ(search_term2, contents2->find_prepopulate_text()); - EXPECT_EQ(search_term2, contents2->find_text()); + EXPECT_EQ(search_term2, contents2.find_prepopulate_text()); + EXPECT_EQ(search_term2, contents2.find_text()); // Search again in the first WebContents. contents()->StartFinding(search_term3, true); // true=forward. @@ -57,8 +54,6 @@ TEST_F(FindBackendTest, InternalState) { // find_text should not. EXPECT_EQ(search_term3, contents()->find_prepopulate_text()); EXPECT_EQ(search_term3, contents()->find_text()); - EXPECT_EQ(search_term3, contents2->find_prepopulate_text()); - EXPECT_EQ(search_term2, contents2->find_text()); - - contents2->CloseContents(); + EXPECT_EQ(search_term3, contents2.find_prepopulate_text()); + EXPECT_EQ(search_term2, contents2.find_text()); } diff --git a/chrome/browser/find_bar_controller.cc b/chrome/browser/find_bar_controller.cc index bdcb94b..fb5cf3b 100644 --- a/chrome/browser/find_bar_controller.cc +++ b/chrome/browser/find_bar_controller.cc @@ -55,7 +55,7 @@ void FindBarController::ChangeWebContents(WebContents* contents) { Source<TabContents>(web_contents_)); NotificationService::current()->RemoveObserver( this, NotificationType::NAV_ENTRY_COMMITTED, - Source<NavigationController>(web_contents_->controller())); + Source<NavigationController>(&web_contents_->controller())); find_bar_->StopAnimation(); } @@ -74,7 +74,7 @@ void FindBarController::ChangeWebContents(WebContents* contents) { Source<TabContents>(web_contents_)); NotificationService::current()->AddObserver( this, NotificationType::NAV_ENTRY_COMMITTED, - Source<NavigationController>(web_contents_->controller())); + Source<NavigationController>(&web_contents_->controller())); // Find out what we should show in the find text box. Usually, this will be // the last search in this tab, but if no search has been issued in this tab @@ -125,7 +125,7 @@ void FindBarController::Observe(NotificationType type, } else if (type == NotificationType::NAV_ENTRY_COMMITTED) { NavigationController* source_controller = Source<NavigationController>(source).ptr(); - if (source_controller == web_contents_->controller()) { + if (source_controller == &web_contents_->controller()) { NavigationController::LoadCommittedDetails* commit_details = Details<NavigationController::LoadCommittedDetails>(details).ptr(); PageTransition::Type transition_type = diff --git a/chrome/browser/gtk/tab_contents_container_gtk.cc b/chrome/browser/gtk/tab_contents_container_gtk.cc index 9e73389..086d5c0 100644 --- a/chrome/browser/gtk/tab_contents_container_gtk.cc +++ b/chrome/browser/gtk/tab_contents_container_gtk.cc @@ -79,7 +79,7 @@ void TabContentsContainerGtk::AddObservers() { // we crash after that NOTIMPLEMENTED(), we'll need it. NotificationService::current()->AddObserver( this, NotificationType::RENDER_VIEW_HOST_CHANGED, - Source<NavigationController>(tab_contents_->controller())); + Source<NavigationController>(&tab_contents_->controller())); } NotificationService::current()->AddObserver( this, @@ -93,7 +93,7 @@ void TabContentsContainerGtk::RemoveObservers() { NotificationService::current()->RemoveObserver( this, NotificationType::RENDER_VIEW_HOST_CHANGED, - Source<NavigationController>(tab_contents_->controller())); + Source<NavigationController>(&tab_contents_->controller())); } NotificationService::current()->RemoveObserver( this, diff --git a/chrome/browser/login_prompt.cc b/chrome/browser/login_prompt.cc index ae2cc4d..c7b30c9 100644 --- a/chrome/browser/login_prompt.cc +++ b/chrome/browser/login_prompt.cc @@ -266,7 +266,7 @@ class LoginHandlerImpl : public LoginHandler, if (!requesting_contents) return; - NavigationController* controller = requesting_contents->controller(); + NavigationController* controller = &requesting_contents->controller(); if (!WasAuthHandled(false)) { LoginNotificationDetails details(this); diff --git a/chrome/browser/memory_details.cc b/chrome/browser/memory_details.cc index 1e7b29a..a6dc4dc 100644 --- a/chrome/browser/memory_details.cc +++ b/chrome/browser/memory_details.cc @@ -231,9 +231,9 @@ void MemoryDetails::CollectChildInfoOnUIThread() { // // Either the pending or last committed entries can be NULL. const NavigationEntry* pending_entry = - contents->controller()->pending_entry(); + contents->controller().pending_entry(); const NavigationEntry* last_committed_entry = - contents->controller()->GetLastCommittedEntry(); + contents->controller().GetLastCommittedEntry(); if ((last_committed_entry && LowerCaseEqualsASCII(last_committed_entry->display_url().spec(), chrome::kAboutMemoryURL)) || diff --git a/chrome/browser/navigation_controller_unittest.cc b/chrome/browser/navigation_controller_unittest.cc index 06ecf00..419ad2f 100644 --- a/chrome/browser/navigation_controller_unittest.cc +++ b/chrome/browser/navigation_controller_unittest.cc @@ -63,8 +63,8 @@ class NavigationControllerHistoryTest : public NavigationControllerTest { service->SetWindowType(window_id, Browser::TYPE_NORMAL); service->SetWindowBounds(window_id, gfx::Rect(0, 1, 2, 3), false); service->SetTabIndexInWindow(window_id, - controller()->session_id(), 0); - controller()->SetWindowID(window_id); + controller().session_id(), 0); + controller().SetWindowID(window_id); session_helper_.set_service(service); } @@ -103,8 +103,8 @@ class NavigationControllerHistoryTest : public NavigationControllerTest { } void GetLastSession() { - profile()->GetSessionService()->TabClosed(controller()->window_id(), - controller()->session_id()); + profile()->GetSessionService()->TabClosed(controller().window_id(), + controller().session_id()); ReopenDatabase(); Time close_time; @@ -146,36 +146,35 @@ void RegisterForAllNavNotifications(TestNotificationTracker* tracker, // ----------------------------------------------------------------------------- TEST_F(NavigationControllerTest, Defaults) { - EXPECT_TRUE(controller()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_FALSE(controller()->GetLastCommittedEntry()); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_EQ(controller()->last_committed_entry_index(), -1); - EXPECT_EQ(controller()->entry_count(), 0); - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_FALSE(controller().GetLastCommittedEntry()); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_EQ(controller().last_committed_entry_index(), -1); + EXPECT_EQ(controller().entry_count(), 0); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); } TEST_F(NavigationControllerTest, LoadURL) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); const GURL url2("http://foo2"); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); + controller().LoadURL(url1, GURL(), PageTransition::TYPED); // Creating a pending notification should not have issued any of the // notifications we're listening for. EXPECT_EQ(0U, notifications.size()); // The load should now be pending. - EXPECT_EQ(controller()->entry_count(), 0); - EXPECT_EQ(controller()->last_committed_entry_index(), -1); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_FALSE(controller()->GetLastCommittedEntry()); - EXPECT_TRUE(controller()->pending_entry()); - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 0); + EXPECT_EQ(controller().last_committed_entry_index(), -1); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_FALSE(controller().GetLastCommittedEntry()); + EXPECT_TRUE(controller().pending_entry()); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); EXPECT_EQ(contents()->GetMaxPageID(), -1); // We should have gotten no notifications from the preceeding checks. @@ -186,27 +185,27 @@ TEST_F(NavigationControllerTest, LoadURL) { NotificationType::NAV_ENTRY_COMMITTED)); // The load should now be committed. - EXPECT_EQ(controller()->entry_count(), 1); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 1); + EXPECT_EQ(controller().last_committed_entry_index(), 0); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); EXPECT_EQ(contents()->GetMaxPageID(), 0); // Load another... - controller()->LoadURL(url2, GURL(), PageTransition::TYPED); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); // The load should now be pending. - EXPECT_EQ(controller()->entry_count(), 1); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_TRUE(controller()->pending_entry()); + EXPECT_EQ(controller().entry_count(), 1); + EXPECT_EQ(controller().last_committed_entry_index(), 0); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_TRUE(controller().pending_entry()); // TODO(darin): maybe this should really be true? - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); EXPECT_EQ(contents()->GetMaxPageID(), 0); rvh()->SendNavigate(1, url2); @@ -214,13 +213,13 @@ TEST_F(NavigationControllerTest, LoadURL) { NotificationType::NAV_ENTRY_COMMITTED)); // The load should now be committed. - EXPECT_EQ(controller()->entry_count(), 2); - EXPECT_EQ(controller()->last_committed_entry_index(), 1); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_TRUE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 2); + EXPECT_EQ(controller().last_committed_entry_index(), 1); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_TRUE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); EXPECT_EQ(contents()->GetMaxPageID(), 1); } @@ -230,69 +229,69 @@ TEST_F(NavigationControllerTest, LoadURL) { // the load commits (because WebCore didn't actually make a new entry). TEST_F(NavigationControllerTest, LoadURL_SamePage) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); + controller().LoadURL(url1, GURL(), PageTransition::TYPED); EXPECT_EQ(0U, notifications.size()); rvh()->SendNavigate(0, url1); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); + controller().LoadURL(url1, GURL(), PageTransition::TYPED); EXPECT_EQ(0U, notifications.size()); rvh()->SendNavigate(0, url1); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); // We should not have produced a new session history entry. - EXPECT_EQ(controller()->entry_count(), 1); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 1); + EXPECT_EQ(controller().last_committed_entry_index(), 0); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); } // Tests loading a URL but discarding it before the load commits. TEST_F(NavigationControllerTest, LoadURL_Discarded) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); const GURL url2("http://foo2"); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); + controller().LoadURL(url1, GURL(), PageTransition::TYPED); EXPECT_EQ(0U, notifications.size()); rvh()->SendNavigate(0, url1); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->LoadURL(url2, GURL(), PageTransition::TYPED); - controller()->DiscardNonCommittedEntries(); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); + controller().DiscardNonCommittedEntries(); EXPECT_EQ(0U, notifications.size()); // Should not have produced a new session history entry. - EXPECT_EQ(controller()->entry_count(), 1); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 1); + EXPECT_EQ(controller().last_committed_entry_index(), 0); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); } // Tests navigations that come in unrequested. This happens when the user // navigates from the web page, and here we test that there is no pending entry. TEST_F(NavigationControllerTest, LoadURL_NoPending) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); // First make an existing committed entry. const GURL kExistingURL1("http://eh"); - controller()->LoadURL(kExistingURL1, GURL(), + controller().LoadURL(kExistingURL1, GURL(), PageTransition::TYPED); rvh()->SendNavigate(0, kExistingURL1); EXPECT_TRUE(notifications.Check1AndReset( @@ -306,9 +305,9 @@ TEST_F(NavigationControllerTest, LoadURL_NoPending) { // just made should be committed. EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(-1, controller()->pending_entry_index()); - EXPECT_EQ(1, controller()->last_committed_entry_index()); - EXPECT_EQ(kNewURL, controller()->GetActiveEntry()->url()); + EXPECT_EQ(-1, controller().pending_entry_index()); + EXPECT_EQ(1, controller().last_committed_entry_index()); + EXPECT_EQ(kNewURL, controller().GetActiveEntry()->url()); } // Tests navigating to a new URL when there is a new pending navigation that is @@ -317,11 +316,11 @@ TEST_F(NavigationControllerTest, LoadURL_NoPending) { // commits. TEST_F(NavigationControllerTest, LoadURL_NewPending) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); // First make an existing committed entry. const GURL kExistingURL1("http://eh"); - controller()->LoadURL(kExistingURL1, GURL(), + controller().LoadURL(kExistingURL1, GURL(), PageTransition::TYPED); rvh()->SendNavigate(0, kExistingURL1); EXPECT_TRUE(notifications.Check1AndReset( @@ -329,7 +328,7 @@ TEST_F(NavigationControllerTest, LoadURL_NewPending) { // Make a pending entry to somewhere new. const GURL kExistingURL2("http://bee"); - controller()->LoadURL(kExistingURL2, GURL(), + controller().LoadURL(kExistingURL2, GURL(), PageTransition::TYPED); EXPECT_EQ(0U, notifications.size()); @@ -341,9 +340,9 @@ TEST_F(NavigationControllerTest, LoadURL_NewPending) { // just made should be committed. EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(-1, controller()->pending_entry_index()); - EXPECT_EQ(1, controller()->last_committed_entry_index()); - EXPECT_EQ(kNewURL, controller()->GetActiveEntry()->url()); + EXPECT_EQ(-1, controller().pending_entry_index()); + EXPECT_EQ(1, controller().last_committed_entry_index()); + EXPECT_EQ(kNewURL, controller().GetActiveEntry()->url()); } // Tests navigating to a new URL when there is a pending back/forward @@ -351,18 +350,18 @@ TEST_F(NavigationControllerTest, LoadURL_NewPending) { // they navigate somewhere new. TEST_F(NavigationControllerTest, LoadURL_ExistingPending) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); // First make some history. const GURL kExistingURL1("http://eh"); - controller()->LoadURL(kExistingURL1, GURL(), + controller().LoadURL(kExistingURL1, GURL(), PageTransition::TYPED); rvh()->SendNavigate(0, kExistingURL1); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); const GURL kExistingURL2("http://bee"); - controller()->LoadURL(kExistingURL2, GURL(), + controller().LoadURL(kExistingURL2, GURL(), PageTransition::TYPED); rvh()->SendNavigate(1, kExistingURL2); EXPECT_TRUE(notifications.Check1AndReset( @@ -370,10 +369,10 @@ TEST_F(NavigationControllerTest, LoadURL_ExistingPending) { // Now make a pending back/forward navigation. The zeroth entry should be // pending. - controller()->GoBack(); + controller().GoBack(); EXPECT_EQ(0U, notifications.size()); - EXPECT_EQ(0, controller()->pending_entry_index()); - EXPECT_EQ(1, controller()->last_committed_entry_index()); + EXPECT_EQ(0, controller().pending_entry_index()); + EXPECT_EQ(1, controller().last_committed_entry_index()); // Before that commits, do a new navigation. const GURL kNewURL("http://see"); @@ -384,63 +383,63 @@ TEST_F(NavigationControllerTest, LoadURL_ExistingPending) { // just made should be committed. EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(-1, controller()->pending_entry_index()); - EXPECT_EQ(2, controller()->last_committed_entry_index()); - EXPECT_EQ(kNewURL, controller()->GetActiveEntry()->url()); + EXPECT_EQ(-1, controller().pending_entry_index()); + EXPECT_EQ(2, controller().last_committed_entry_index()); + EXPECT_EQ(kNewURL, controller().GetActiveEntry()->url()); } TEST_F(NavigationControllerTest, Reload) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); + controller().LoadURL(url1, GURL(), PageTransition::TYPED); EXPECT_EQ(0U, notifications.size()); rvh()->SendNavigate(0, url1); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->Reload(true); + controller().Reload(true); EXPECT_EQ(0U, notifications.size()); // The reload is pending. - EXPECT_EQ(controller()->entry_count(), 1); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); - EXPECT_EQ(controller()->pending_entry_index(), 0); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_TRUE(controller()->pending_entry()); - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 1); + EXPECT_EQ(controller().last_committed_entry_index(), 0); + EXPECT_EQ(controller().pending_entry_index(), 0); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_TRUE(controller().pending_entry()); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); rvh()->SendNavigate(0, url1); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); // Now the reload is committed. - EXPECT_EQ(controller()->entry_count(), 1); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 1); + EXPECT_EQ(controller().last_committed_entry_index(), 0); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); } // Tests what happens when a reload navigation produces a new page. TEST_F(NavigationControllerTest, Reload_GeneratesNewPage) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); const GURL url2("http://foo2"); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); + controller().LoadURL(url1, GURL(), PageTransition::TYPED); rvh()->SendNavigate(0, url1); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->Reload(true); + controller().Reload(true); EXPECT_EQ(0U, notifications.size()); rvh()->SendNavigate(1, url2); @@ -448,19 +447,19 @@ TEST_F(NavigationControllerTest, Reload_GeneratesNewPage) { NotificationType::NAV_ENTRY_COMMITTED)); // Now the reload is committed. - EXPECT_EQ(controller()->entry_count(), 2); - EXPECT_EQ(controller()->last_committed_entry_index(), 1); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_TRUE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 2); + EXPECT_EQ(controller().last_committed_entry_index(), 1); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_TRUE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); } // Tests what happens when we navigate back successfully TEST_F(NavigationControllerTest, Back) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); rvh()->SendNavigate(0, url1); @@ -472,62 +471,62 @@ TEST_F(NavigationControllerTest, Back) { EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->GoBack(); + controller().GoBack(); EXPECT_EQ(0U, notifications.size()); // We should now have a pending navigation to go back. - EXPECT_EQ(controller()->entry_count(), 2); - EXPECT_EQ(controller()->last_committed_entry_index(), 1); - EXPECT_EQ(controller()->pending_entry_index(), 0); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_TRUE(controller()->pending_entry()); - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_TRUE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 2); + EXPECT_EQ(controller().last_committed_entry_index(), 1); + EXPECT_EQ(controller().pending_entry_index(), 0); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_TRUE(controller().pending_entry()); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_TRUE(controller().CanGoForward()); rvh()->SendNavigate(0, url2); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); // The back navigation completed successfully. - EXPECT_EQ(controller()->entry_count(), 2); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_TRUE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 2); + EXPECT_EQ(controller().last_committed_entry_index(), 0); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_TRUE(controller().CanGoForward()); } // Tests what happens when a back navigation produces a new page. TEST_F(NavigationControllerTest, Back_GeneratesNewPage) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); const GURL url2("http://foo2"); const GURL url3("http://foo3"); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); + controller().LoadURL(url1, GURL(), PageTransition::TYPED); rvh()->SendNavigate(0, url1); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->LoadURL(url2, GURL(), PageTransition::TYPED); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); rvh()->SendNavigate(1, url2); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->GoBack(); + controller().GoBack(); EXPECT_EQ(0U, notifications.size()); // We should now have a pending navigation to go back. - EXPECT_EQ(controller()->entry_count(), 2); - EXPECT_EQ(controller()->last_committed_entry_index(), 1); - EXPECT_EQ(controller()->pending_entry_index(), 0); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_TRUE(controller()->pending_entry()); - EXPECT_FALSE(controller()->CanGoBack()); - EXPECT_TRUE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 2); + EXPECT_EQ(controller().last_committed_entry_index(), 1); + EXPECT_EQ(controller().pending_entry_index(), 0); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_TRUE(controller().pending_entry()); + EXPECT_FALSE(controller().CanGoBack()); + EXPECT_TRUE(controller().CanGoForward()); rvh()->SendNavigate(2, url3); EXPECT_TRUE(notifications.Check1AndReset( @@ -535,19 +534,19 @@ TEST_F(NavigationControllerTest, Back_GeneratesNewPage) { // The back navigation resulted in a completely new navigation. // TODO(darin): perhaps this behavior will be confusing to users? - EXPECT_EQ(controller()->entry_count(), 3); - EXPECT_EQ(controller()->last_committed_entry_index(), 2); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_TRUE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 3); + EXPECT_EQ(controller().last_committed_entry_index(), 2); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_TRUE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); } // Receives a back message when there is a new pending navigation entry. TEST_F(NavigationControllerTest, Back_NewPending) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL kUrl1("http://foo1"); const GURL kUrl2("http://foo2"); @@ -558,21 +557,21 @@ TEST_F(NavigationControllerTest, Back_NewPending) { EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - //controller()->LoadURL(kUrl2, PageTransition::TYPED); + //controller().LoadURL(kUrl2, PageTransition::TYPED); rvh()->SendNavigate(1, kUrl2); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); // Now start a new pending navigation and go back before it commits. - controller()->LoadURL(kUrl3, GURL(), PageTransition::TYPED); - EXPECT_EQ(-1, controller()->pending_entry_index()); - EXPECT_EQ(kUrl3, controller()->pending_entry()->url()); - controller()->GoBack(); + controller().LoadURL(kUrl3, GURL(), PageTransition::TYPED); + EXPECT_EQ(-1, controller().pending_entry_index()); + EXPECT_EQ(kUrl3, controller().pending_entry()->url()); + controller().GoBack(); // The pending navigation should now be the "back" item and the new one // should be gone. - EXPECT_EQ(0, controller()->pending_entry_index()); - EXPECT_EQ(kUrl1, controller()->pending_entry()->url()); + EXPECT_EQ(0, controller().pending_entry_index()); + EXPECT_EQ(kUrl1, controller().pending_entry()->url()); } // Receives a back message when there is a different renavigation already @@ -593,39 +592,39 @@ TEST_F(NavigationControllerTest, Back_OtherBackPending) { // We know all the entries have the same site instance, so we can just grab // a random one for looking up other entries. SiteInstance* site_instance = - controller()->GetLastCommittedEntry()->site_instance(); + controller().GetLastCommittedEntry()->site_instance(); // That second URL should be the last committed and it should have gotten the // new title. - EXPECT_EQ(kUrl2, controller()->GetEntryWithPageID(site_instance, 1)->url()); - EXPECT_EQ(1, controller()->last_committed_entry_index()); - EXPECT_EQ(-1, controller()->pending_entry_index()); + EXPECT_EQ(kUrl2, controller().GetEntryWithPageID(site_instance, 1)->url()); + EXPECT_EQ(1, controller().last_committed_entry_index()); + EXPECT_EQ(-1, controller().pending_entry_index()); // Now go forward to the last item again and say it was committed. - controller()->GoForward(); + controller().GoForward(); rvh()->SendNavigate(2, kUrl3); // Now start going back one to the second page. It will be pending. - controller()->GoBack(); - EXPECT_EQ(1, controller()->pending_entry_index()); - EXPECT_EQ(2, controller()->last_committed_entry_index()); + controller().GoBack(); + EXPECT_EQ(1, controller().pending_entry_index()); + EXPECT_EQ(2, controller().last_committed_entry_index()); // Not synthesize a totally new back event to the first page. This will not // match the pending one. rvh()->SendNavigate(0, kUrl1); // The navigation should not have affected the pending entry. - EXPECT_EQ(1, controller()->pending_entry_index()); + EXPECT_EQ(1, controller().pending_entry_index()); // But the navigated entry should be the last committed. - EXPECT_EQ(0, controller()->last_committed_entry_index()); - EXPECT_EQ(kUrl1, controller()->GetLastCommittedEntry()->url()); + EXPECT_EQ(0, controller().last_committed_entry_index()); + EXPECT_EQ(kUrl1, controller().GetLastCommittedEntry()->url()); } // Tests what happens when we navigate forward successfully. TEST_F(NavigationControllerTest, Forward) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); const GURL url2("http://foo2"); @@ -638,40 +637,40 @@ TEST_F(NavigationControllerTest, Forward) { EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->GoBack(); + controller().GoBack(); rvh()->SendNavigate(0, url1); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->GoForward(); + controller().GoForward(); // We should now have a pending navigation to go forward. - EXPECT_EQ(controller()->entry_count(), 2); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); - EXPECT_EQ(controller()->pending_entry_index(), 1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_TRUE(controller()->pending_entry()); - EXPECT_TRUE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 2); + EXPECT_EQ(controller().last_committed_entry_index(), 0); + EXPECT_EQ(controller().pending_entry_index(), 1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_TRUE(controller().pending_entry()); + EXPECT_TRUE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); rvh()->SendNavigate(1, url2); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); // The forward navigation completed successfully. - EXPECT_EQ(controller()->entry_count(), 2); - EXPECT_EQ(controller()->last_committed_entry_index(), 1); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_TRUE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 2); + EXPECT_EQ(controller().last_committed_entry_index(), 1); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_TRUE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); } // Tests what happens when a forward navigation produces a new page. TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); const GURL url2("http://foo2"); @@ -684,42 +683,42 @@ TEST_F(NavigationControllerTest, Forward_GeneratesNewPage) { EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->GoBack(); + controller().GoBack(); rvh()->SendNavigate(0, url1); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - controller()->GoForward(); + controller().GoForward(); EXPECT_EQ(0U, notifications.size()); // Should now have a pending navigation to go forward. - EXPECT_EQ(controller()->entry_count(), 2); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); - EXPECT_EQ(controller()->pending_entry_index(), 1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_TRUE(controller()->pending_entry()); - EXPECT_TRUE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 2); + EXPECT_EQ(controller().last_committed_entry_index(), 0); + EXPECT_EQ(controller().pending_entry_index(), 1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_TRUE(controller().pending_entry()); + EXPECT_TRUE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); rvh()->SendNavigate(2, url3); EXPECT_TRUE(notifications.Check2AndReset( NotificationType::NAV_LIST_PRUNED, NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(controller()->entry_count(), 2); - EXPECT_EQ(controller()->last_committed_entry_index(), 1); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_TRUE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 2); + EXPECT_EQ(controller().last_committed_entry_index(), 1); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_TRUE(controller().CanGoBack()); + EXPECT_FALSE(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, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); rvh()->SendNavigate(0, url1); @@ -736,7 +735,7 @@ TEST_F(NavigationControllerTest, NewSubframe) { params.is_post = false; NavigationController::LoadCommittedDetails details; - EXPECT_TRUE(controller()->RendererDidNavigate(params, &details)); + EXPECT_TRUE(controller().RendererDidNavigate(params, &details)); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); EXPECT_EQ(url1, details.previous_url); @@ -745,7 +744,7 @@ TEST_F(NavigationControllerTest, NewSubframe) { EXPECT_FALSE(details.is_main_frame); // The new entry should be appended. - EXPECT_EQ(2, controller()->entry_count()); + EXPECT_EQ(2, controller().entry_count()); // New entry should refer to the new page, but the old URL (entries only // reflect the toplevel URL). @@ -758,7 +757,7 @@ TEST_F(NavigationControllerTest, NewSubframe) { // just get thrown on the ground, but we shouldn't crash. TEST_F(NavigationControllerTest, SubframeOnEmptyPage) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); // Navigation controller currently has no entries. const GURL url("http://foo2"); @@ -771,7 +770,7 @@ TEST_F(NavigationControllerTest, SubframeOnEmptyPage) { params.is_post = false; NavigationController::LoadCommittedDetails details; - EXPECT_FALSE(controller()->RendererDidNavigate(params, &details)); + EXPECT_FALSE(controller().RendererDidNavigate(params, &details)); EXPECT_EQ(0U, notifications.size()); } @@ -779,7 +778,7 @@ TEST_F(NavigationControllerTest, SubframeOnEmptyPage) { // not create new navigation entries. TEST_F(NavigationControllerTest, AutoSubframe) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); rvh()->SendNavigate(0, url1); @@ -797,17 +796,17 @@ TEST_F(NavigationControllerTest, AutoSubframe) { // Navigating should do nothing. NavigationController::LoadCommittedDetails details; - EXPECT_FALSE(controller()->RendererDidNavigate(params, &details)); + EXPECT_FALSE(controller().RendererDidNavigate(params, &details)); EXPECT_EQ(0U, notifications.size()); // There should still be only one entry. - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); } // Tests navigation and then going back to a subframe navigation. TEST_F(NavigationControllerTest, BackSubframe) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); // Main page. const GURL url1("http://foo1"); @@ -827,45 +826,45 @@ TEST_F(NavigationControllerTest, BackSubframe) { // This should generate a new entry. NavigationController::LoadCommittedDetails details; - EXPECT_TRUE(controller()->RendererDidNavigate(params, &details)); + EXPECT_TRUE(controller().RendererDidNavigate(params, &details)); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(2, controller()->entry_count()); + EXPECT_EQ(2, controller().entry_count()); // Second manual subframe navigation should also make a new entry. const GURL url3("http://foo3"); params.page_id = 2; params.url = url3; - EXPECT_TRUE(controller()->RendererDidNavigate(params, &details)); + EXPECT_TRUE(controller().RendererDidNavigate(params, &details)); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(3, controller()->entry_count()); - EXPECT_EQ(2, controller()->GetCurrentEntryIndex()); + EXPECT_EQ(3, controller().entry_count()); + EXPECT_EQ(2, controller().GetCurrentEntryIndex()); // Go back one. - controller()->GoBack(); + controller().GoBack(); params.url = url2; params.page_id = 1; - EXPECT_TRUE(controller()->RendererDidNavigate(params, &details)); + EXPECT_TRUE(controller().RendererDidNavigate(params, &details)); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(3, controller()->entry_count()); - EXPECT_EQ(1, controller()->GetCurrentEntryIndex()); + EXPECT_EQ(3, controller().entry_count()); + EXPECT_EQ(1, controller().GetCurrentEntryIndex()); // Go back one more. - controller()->GoBack(); + controller().GoBack(); params.url = url1; params.page_id = 0; - EXPECT_TRUE(controller()->RendererDidNavigate(params, &details)); + EXPECT_TRUE(controller().RendererDidNavigate(params, &details)); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(3, controller()->entry_count()); - EXPECT_EQ(0, controller()->GetCurrentEntryIndex()); + EXPECT_EQ(3, controller().entry_count()); + EXPECT_EQ(0, controller().GetCurrentEntryIndex()); } TEST_F(NavigationControllerTest, LinkClick) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url1("http://foo1"); const GURL url2("http://foo2"); @@ -879,18 +878,18 @@ TEST_F(NavigationControllerTest, LinkClick) { NotificationType::NAV_ENTRY_COMMITTED)); // Should not have produced a new session history entry. - EXPECT_EQ(controller()->entry_count(), 2); - EXPECT_EQ(controller()->last_committed_entry_index(), 1); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_TRUE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(controller().entry_count(), 2); + EXPECT_EQ(controller().last_committed_entry_index(), 1); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_TRUE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); } TEST_F(NavigationControllerTest, InPage) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); // Main page. Note that we need "://" so this URL is treated as "standard" // which are the only ones that can have a ref. @@ -911,50 +910,50 @@ TEST_F(NavigationControllerTest, InPage) { // This should generate a new entry. NavigationController::LoadCommittedDetails details; - EXPECT_TRUE(controller()->RendererDidNavigate(params, &details)); + EXPECT_TRUE(controller().RendererDidNavigate(params, &details)); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(2, controller()->entry_count()); + EXPECT_EQ(2, controller().entry_count()); // Go back one. ViewHostMsg_FrameNavigate_Params back_params(params); - controller()->GoBack(); + controller().GoBack(); back_params.url = url1; back_params.page_id = 0; - EXPECT_TRUE(controller()->RendererDidNavigate(back_params, + EXPECT_TRUE(controller().RendererDidNavigate(back_params, &details)); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(2, controller()->entry_count()); - EXPECT_EQ(0, controller()->GetCurrentEntryIndex()); - EXPECT_EQ(back_params.url, controller()->GetActiveEntry()->url()); + EXPECT_EQ(2, controller().entry_count()); + EXPECT_EQ(0, controller().GetCurrentEntryIndex()); + EXPECT_EQ(back_params.url, controller().GetActiveEntry()->url()); // Go forward ViewHostMsg_FrameNavigate_Params forward_params(params); - controller()->GoForward(); + controller().GoForward(); forward_params.url = url2; forward_params.page_id = 1; - EXPECT_TRUE(controller()->RendererDidNavigate(forward_params, + EXPECT_TRUE(controller().RendererDidNavigate(forward_params, &details)); EXPECT_TRUE(notifications.Check1AndReset( NotificationType::NAV_ENTRY_COMMITTED)); - EXPECT_EQ(2, controller()->entry_count()); - EXPECT_EQ(1, controller()->GetCurrentEntryIndex()); + EXPECT_EQ(2, controller().entry_count()); + EXPECT_EQ(1, controller().GetCurrentEntryIndex()); EXPECT_EQ(forward_params.url, - controller()->GetActiveEntry()->url()); + 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. - controller()->GoBack(); - EXPECT_TRUE(controller()->RendererDidNavigate(back_params, + controller().GoBack(); + EXPECT_TRUE(controller().RendererDidNavigate(back_params, &details)); - controller()->GoForward(); - EXPECT_TRUE(controller()->RendererDidNavigate(forward_params, + controller().GoForward(); + EXPECT_TRUE(controller().RendererDidNavigate(forward_params, &details)); EXPECT_EQ(forward_params.url, - controller()->GetActiveEntry()->url()); + controller().GetActiveEntry()->url()); } namespace { @@ -1003,18 +1002,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++) { GURL url(StringPrintf("http://www.a.com/%d", url_index)); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); + controller().LoadURL(url, GURL(), PageTransition::TYPED); rvh()->SendNavigate(url_index, url); } - EXPECT_EQ(controller()->entry_count(), kMaxEntryCount); + EXPECT_EQ(controller().entry_count(), kMaxEntryCount); // Created a PrunedListener to observe prune notifications. - PrunedListener listener(controller()); + PrunedListener listener(&controller()); // Navigate some more. GURL url(StringPrintf("http://www.a.com/%d", url_index)); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); + controller().LoadURL(url, GURL(), PageTransition::TYPED); rvh()->SendNavigate(url_index, url); url_index++; @@ -1024,19 +1023,19 @@ TEST_F(NavigationControllerTest, EnforceMaxNavigationCount) { EXPECT_EQ(1, listener.details_.count); // We expect http://www.a.com/0 to be gone. - EXPECT_EQ(controller()->entry_count(), kMaxEntryCount); - EXPECT_EQ(controller()->GetEntryAtIndex(0)->url(), + EXPECT_EQ(controller().entry_count(), kMaxEntryCount); + EXPECT_EQ(controller().GetEntryAtIndex(0)->url(), GURL("http:////www.a.com/1")); // More navigations. for (int i = 0; i < 3; i++) { url = GURL(StringPrintf("http:////www.a.com/%d", url_index)); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); + controller().LoadURL(url, GURL(), PageTransition::TYPED); rvh()->SendNavigate(url_index, url); url_index++; } - EXPECT_EQ(controller()->entry_count(), kMaxEntryCount); - EXPECT_EQ(controller()->GetEntryAtIndex(0)->url(), + EXPECT_EQ(controller().entry_count(), kMaxEntryCount); + EXPECT_EQ(controller().GetEntryAtIndex(0)->url(), GURL("http:////www.a.com/4")); NavigationController::set_max_entry_count(original_count); @@ -1052,15 +1051,16 @@ TEST_F(NavigationControllerTest, RestoreNavigate) { navigations.push_back(TabNavigation(0, url, GURL(), ASCIIToUTF16("Title"), "state", PageTransition::LINK)); - NavigationController* our_controller = - new NavigationController(profile(), navigations, 0); - our_controller->GoToIndex(0); + WebContents our_contents(profile(), NULL, MSG_ROUTING_NONE, NULL); + NavigationController& our_controller = our_contents.controller(); + our_controller.RestoreFromState(navigations, 0); + our_controller.GoToIndex(0); // We should now have one entry, and it should be "pending". - EXPECT_EQ(1, our_controller->entry_count()); - EXPECT_EQ(our_controller->GetEntryAtIndex(0), - our_controller->pending_entry()); - EXPECT_EQ(0, our_controller->GetEntryAtIndex(0)->page_id()); + EXPECT_EQ(1, our_controller.entry_count()); + EXPECT_EQ(our_controller.GetEntryAtIndex(0), + our_controller.pending_entry()); + EXPECT_EQ(0, our_controller.GetEntryAtIndex(0)->page_id()); // Say we navigated to that entry. ViewHostMsg_FrameNavigate_Params params; @@ -1071,32 +1071,29 @@ TEST_F(NavigationControllerTest, RestoreNavigate) { params.gesture = NavigationGestureUser; params.is_post = false; NavigationController::LoadCommittedDetails details; - our_controller->RendererDidNavigate(params, &details); + our_controller.RendererDidNavigate(params, &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, our_controller->entry_count()); - EXPECT_EQ(0, our_controller->last_committed_entry_index()); - EXPECT_FALSE(our_controller->pending_entry()); + EXPECT_EQ(1, our_controller.entry_count()); + EXPECT_EQ(0, our_controller.last_committed_entry_index()); + EXPECT_FALSE(our_controller.pending_entry()); EXPECT_EQ(url, - our_controller->GetLastCommittedEntry()->site_instance()->site()); - - // Clean up the navigation controller. - our_controller->Destroy(); + our_controller.GetLastCommittedEntry()->site_instance()->site()); } // Make sure that the page type and stuff is correct after an interstitial. TEST_F(NavigationControllerTest, Interstitial) { // First navigate somewhere normal. const GURL url1("http://foo"); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); + controller().LoadURL(url1, GURL(), PageTransition::TYPED); rvh()->SendNavigate(0, url1); // Now navigate somewhere with an interstitial. const GURL url2("http://bar"); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); - controller()->pending_entry()->set_page_type( + controller().LoadURL(url1, GURL(), PageTransition::TYPED); + controller().pending_entry()->set_page_type( NavigationEntry::INTERSTITIAL_PAGE); // At this point the interstitial will be displayed and the load will still @@ -1104,9 +1101,9 @@ TEST_F(NavigationControllerTest, Interstitial) { rvh()->SendNavigate(1, url2); // The page should be a normal page again. - EXPECT_EQ(url2, controller()->GetLastCommittedEntry()->url()); + EXPECT_EQ(url2, controller().GetLastCommittedEntry()->url()); EXPECT_EQ(NavigationEntry::NORMAL_PAGE, - controller()->GetLastCommittedEntry()->page_type()); + controller().GetLastCommittedEntry()->page_type()); } TEST_F(NavigationControllerTest, RemoveEntry) { @@ -1118,60 +1115,60 @@ TEST_F(NavigationControllerTest, RemoveEntry) { const GURL pending_url("http://pending"); const GURL default_url("http://default"); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); + controller().LoadURL(url1, GURL(), PageTransition::TYPED); rvh()->SendNavigate(0, url1); - controller()->LoadURL(url2, GURL(), PageTransition::TYPED); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); rvh()->SendNavigate(1, url2); - controller()->LoadURL(url3, GURL(), PageTransition::TYPED); + controller().LoadURL(url3, GURL(), PageTransition::TYPED); rvh()->SendNavigate(2, url3); - controller()->LoadURL(url4, GURL(), PageTransition::TYPED); + controller().LoadURL(url4, GURL(), PageTransition::TYPED); rvh()->SendNavigate(3, url4); - controller()->LoadURL(url5, GURL(), PageTransition::TYPED); + controller().LoadURL(url5, GURL(), PageTransition::TYPED); rvh()->SendNavigate(4, url5); // Remove the last entry. - controller()->RemoveEntryAtIndex( - controller()->entry_count() - 1, default_url); - EXPECT_EQ(4, controller()->entry_count()); - EXPECT_EQ(3, controller()->last_committed_entry_index()); - NavigationEntry* pending_entry = controller()->pending_entry(); + controller().RemoveEntryAtIndex( + controller().entry_count() - 1, default_url); + EXPECT_EQ(4, controller().entry_count()); + EXPECT_EQ(3, controller().last_committed_entry_index()); + NavigationEntry* pending_entry = controller().pending_entry(); EXPECT_TRUE(pending_entry && pending_entry->url() == url4); // Add a pending entry. - controller()->LoadURL(pending_url, GURL(), PageTransition::TYPED); + controller().LoadURL(pending_url, GURL(), PageTransition::TYPED); // Now remove the last entry. - controller()->RemoveEntryAtIndex( - controller()->entry_count() - 1, default_url); + controller().RemoveEntryAtIndex( + controller().entry_count() - 1, default_url); // The pending entry should have been discarded and the last committed entry // removed. - EXPECT_EQ(3, controller()->entry_count()); - EXPECT_EQ(2, controller()->last_committed_entry_index()); - pending_entry = controller()->pending_entry(); + EXPECT_EQ(3, controller().entry_count()); + EXPECT_EQ(2, controller().last_committed_entry_index()); + pending_entry = controller().pending_entry(); EXPECT_TRUE(pending_entry && pending_entry->url() == url3); // Remove an entry which is not the last committed one. - controller()->RemoveEntryAtIndex(0, default_url); - EXPECT_EQ(2, controller()->entry_count()); - EXPECT_EQ(1, controller()->last_committed_entry_index()); + controller().RemoveEntryAtIndex(0, default_url); + EXPECT_EQ(2, controller().entry_count()); + EXPECT_EQ(1, controller().last_committed_entry_index()); // No navigation should have been initiated since we did not remove the // current entry. - EXPECT_FALSE(controller()->pending_entry()); + EXPECT_FALSE(controller().pending_entry()); // Remove the 2 remaining entries. - controller()->RemoveEntryAtIndex(1, default_url); - controller()->RemoveEntryAtIndex(0, default_url); + controller().RemoveEntryAtIndex(1, default_url); + controller().RemoveEntryAtIndex(0, default_url); // This should have created a pending default entry. - EXPECT_EQ(0, controller()->entry_count()); - EXPECT_EQ(-1, controller()->last_committed_entry_index()); - pending_entry = controller()->pending_entry(); + EXPECT_EQ(0, controller().entry_count()); + EXPECT_EQ(-1, controller().last_committed_entry_index()); + pending_entry = controller().pending_entry(); EXPECT_TRUE(pending_entry && pending_entry->url() == default_url); } // Tests the transient entry, making sure it goes away with all navigations. TEST_F(NavigationControllerTest, TransientEntry) { TestNotificationTracker notifications; - RegisterForAllNavNotifications(¬ifications, controller()); + RegisterForAllNavNotifications(¬ifications, &controller()); const GURL url0("http://foo0"); const GURL url1("http://foo1"); @@ -1180,9 +1177,9 @@ TEST_F(NavigationControllerTest, TransientEntry) { const GURL url4("http://foo4"); const GURL transient_url("http://transient"); - controller()->LoadURL(url0, GURL(), PageTransition::TYPED); + controller().LoadURL(url0, GURL(), PageTransition::TYPED); rvh()->SendNavigate(0, url0); - controller()->LoadURL(url1, GURL(), PageTransition::TYPED); + controller().LoadURL(url1, GURL(), PageTransition::TYPED); rvh()->SendNavigate(1, url1); notifications.Reset(); @@ -1190,103 +1187,103 @@ TEST_F(NavigationControllerTest, TransientEntry) { // Adding a transient with no pending entry. NavigationEntry* transient_entry = new NavigationEntry; transient_entry->set_url(transient_url); - controller()->AddTransientEntry(transient_entry); + controller().AddTransientEntry(transient_entry); // We should not have received any notifications. EXPECT_EQ(0U, notifications.size()); // Check our state. - EXPECT_EQ(transient_url, controller()->GetActiveEntry()->url()); - EXPECT_EQ(controller()->entry_count(), 3); - EXPECT_EQ(controller()->last_committed_entry_index(), 1); - EXPECT_EQ(controller()->pending_entry_index(), -1); - EXPECT_TRUE(controller()->GetLastCommittedEntry()); - EXPECT_FALSE(controller()->pending_entry()); - EXPECT_TRUE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); + EXPECT_EQ(transient_url, controller().GetActiveEntry()->url()); + EXPECT_EQ(controller().entry_count(), 3); + EXPECT_EQ(controller().last_committed_entry_index(), 1); + EXPECT_EQ(controller().pending_entry_index(), -1); + EXPECT_TRUE(controller().GetLastCommittedEntry()); + EXPECT_FALSE(controller().pending_entry()); + EXPECT_TRUE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); EXPECT_EQ(contents()->GetMaxPageID(), 1); // Navigate. - controller()->LoadURL(url2, GURL(), PageTransition::TYPED); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); rvh()->SendNavigate(2, url2); // We should have navigated, transient entry should be gone. - EXPECT_EQ(url2, controller()->GetActiveEntry()->url()); - EXPECT_EQ(controller()->entry_count(), 3); + EXPECT_EQ(url2, controller().GetActiveEntry()->url()); + EXPECT_EQ(controller().entry_count(), 3); // Add a transient again, then navigate with no pending entry this time. transient_entry = new NavigationEntry; transient_entry->set_url(transient_url); - controller()->AddTransientEntry(transient_entry); - EXPECT_EQ(transient_url, controller()->GetActiveEntry()->url()); + controller().AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, controller().GetActiveEntry()->url()); rvh()->SendNavigate(3, url3); // Transient entry should be gone. - EXPECT_EQ(url3, controller()->GetActiveEntry()->url()); - EXPECT_EQ(controller()->entry_count(), 4); + EXPECT_EQ(url3, controller().GetActiveEntry()->url()); + EXPECT_EQ(controller().entry_count(), 4); // Initiate a navigation, add a transient then commit navigation. - controller()->LoadURL(url4, GURL(), PageTransition::TYPED); + controller().LoadURL(url4, GURL(), PageTransition::TYPED); transient_entry = new NavigationEntry; transient_entry->set_url(transient_url); - controller()->AddTransientEntry(transient_entry); - EXPECT_EQ(transient_url, controller()->GetActiveEntry()->url()); + controller().AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, controller().GetActiveEntry()->url()); rvh()->SendNavigate(4, url4); - EXPECT_EQ(url4, controller()->GetActiveEntry()->url()); - EXPECT_EQ(controller()->entry_count(), 5); + EXPECT_EQ(url4, controller().GetActiveEntry()->url()); + EXPECT_EQ(controller().entry_count(), 5); // Add a transient and go back. This should simply remove the transient. transient_entry = new NavigationEntry; transient_entry->set_url(transient_url); - controller()->AddTransientEntry(transient_entry); - EXPECT_EQ(transient_url, controller()->GetActiveEntry()->url()); - EXPECT_TRUE(controller()->CanGoBack()); - EXPECT_FALSE(controller()->CanGoForward()); - controller()->GoBack(); + controller().AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, controller().GetActiveEntry()->url()); + EXPECT_TRUE(controller().CanGoBack()); + EXPECT_FALSE(controller().CanGoForward()); + controller().GoBack(); // Transient entry should be gone. - EXPECT_EQ(url4, controller()->GetActiveEntry()->url()); - EXPECT_EQ(controller()->entry_count(), 5); + EXPECT_EQ(url4, controller().GetActiveEntry()->url()); + EXPECT_EQ(controller().entry_count(), 5); rvh()->SendNavigate(3, url3); // Add a transient and go to an entry before the current one. transient_entry = new NavigationEntry; transient_entry->set_url(transient_url); - controller()->AddTransientEntry(transient_entry); - EXPECT_EQ(transient_url, controller()->GetActiveEntry()->url()); - controller()->GoToIndex(1); + controller().AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, controller().GetActiveEntry()->url()); + controller().GoToIndex(1); // The navigation should have been initiated, transient entry should be gone. - EXPECT_EQ(url1, controller()->GetActiveEntry()->url()); + EXPECT_EQ(url1, controller().GetActiveEntry()->url()); rvh()->SendNavigate(1, url1); // Add a transient and go to an entry after the current one. transient_entry = new NavigationEntry; transient_entry->set_url(transient_url); - controller()->AddTransientEntry(transient_entry); - EXPECT_EQ(transient_url, controller()->GetActiveEntry()->url()); - controller()->GoToIndex(3); + controller().AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, controller().GetActiveEntry()->url()); + controller().GoToIndex(3); // The navigation should have been initiated, transient entry should be gone. // Because of the transient entry that is removed, going to index 3 makes us // land on url2. - EXPECT_EQ(url2, controller()->GetActiveEntry()->url()); + EXPECT_EQ(url2, controller().GetActiveEntry()->url()); rvh()->SendNavigate(2, url2); // Add a transient and go forward. transient_entry = new NavigationEntry; transient_entry->set_url(transient_url); - controller()->AddTransientEntry(transient_entry); - EXPECT_EQ(transient_url, controller()->GetActiveEntry()->url()); - EXPECT_TRUE(controller()->CanGoForward()); - controller()->GoForward(); + controller().AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, controller().GetActiveEntry()->url()); + EXPECT_TRUE(controller().CanGoForward()); + controller().GoForward(); // We should have navigated, transient entry should be gone. - EXPECT_EQ(url3, controller()->GetActiveEntry()->url()); + EXPECT_EQ(url3, controller().GetActiveEntry()->url()); rvh()->SendNavigate(3, url3); // Ensure the URLS are correct. - EXPECT_EQ(controller()->entry_count(), 5); - EXPECT_EQ(controller()->GetEntryAtIndex(0)->url(), url0); - EXPECT_EQ(controller()->GetEntryAtIndex(1)->url(), url1); - EXPECT_EQ(controller()->GetEntryAtIndex(2)->url(), url2); - EXPECT_EQ(controller()->GetEntryAtIndex(3)->url(), url3); - EXPECT_EQ(controller()->GetEntryAtIndex(4)->url(), url4); + EXPECT_EQ(controller().entry_count(), 5); + EXPECT_EQ(controller().GetEntryAtIndex(0)->url(), url0); + EXPECT_EQ(controller().GetEntryAtIndex(1)->url(), url1); + EXPECT_EQ(controller().GetEntryAtIndex(2)->url(), url2); + EXPECT_EQ(controller().GetEntryAtIndex(3)->url(), url3); + EXPECT_EQ(controller().GetEntryAtIndex(4)->url(), url4); } // Tests that IsInPageNavigation returns appropriate results. Prevents @@ -1297,21 +1294,21 @@ TEST_F(NavigationControllerTest, IsInPageNavigation) { rvh()->SendNavigate(0, url); // Reloading the page is not an in-page navigation. - EXPECT_FALSE(controller()->IsURLInPageNavigation(url)); + EXPECT_FALSE(controller().IsURLInPageNavigation(url)); const GURL other_url("http://www.google.com/add.html"); - EXPECT_FALSE(controller()->IsURLInPageNavigation(other_url)); + EXPECT_FALSE(controller().IsURLInPageNavigation(other_url)); const GURL url_with_ref("http://www.google.com/home.html#my_ref"); - EXPECT_TRUE(controller()->IsURLInPageNavigation(url_with_ref)); + EXPECT_TRUE(controller().IsURLInPageNavigation(url_with_ref)); // Navigate to URL with refs. rvh()->SendNavigate(1, url_with_ref); // Reloading the page is not an in-page navigation. - EXPECT_FALSE(controller()->IsURLInPageNavigation(url_with_ref)); - EXPECT_FALSE(controller()->IsURLInPageNavigation(url)); - EXPECT_FALSE(controller()->IsURLInPageNavigation(other_url)); + EXPECT_FALSE(controller().IsURLInPageNavigation(url_with_ref)); + EXPECT_FALSE(controller().IsURLInPageNavigation(url)); + EXPECT_FALSE(controller().IsURLInPageNavigation(other_url)); const GURL other_url_with_ref("http://www.google.com/home.html#my_other_ref"); - EXPECT_TRUE(controller()->IsURLInPageNavigation( + EXPECT_TRUE(controller().IsURLInPageNavigation( other_url_with_ref)); } @@ -1325,8 +1322,8 @@ TEST_F(NavigationControllerTest, SameSubframe) { rvh()->SendNavigate(0, url); // We should be at the first navigation entry. - EXPECT_EQ(controller()->entry_count(), 1); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); + EXPECT_EQ(controller().entry_count(), 1); + EXPECT_EQ(controller().last_committed_entry_index(), 0); // Navigate a subframe that would normally count as in-page. const GURL subframe("http://www.google.com/#"); @@ -1338,11 +1335,11 @@ TEST_F(NavigationControllerTest, SameSubframe) { params.gesture = NavigationGestureAuto; params.is_post = false; NavigationController::LoadCommittedDetails details; - EXPECT_FALSE(controller()->RendererDidNavigate(params, &details)); + EXPECT_FALSE(controller().RendererDidNavigate(params, &details)); // Nothing should have changed. - EXPECT_EQ(controller()->entry_count(), 1); - EXPECT_EQ(controller()->last_committed_entry_index(), 0); + EXPECT_EQ(controller().entry_count(), 1); + EXPECT_EQ(controller().last_committed_entry_index(), 0); } /* TODO(brettw) These test pass on my local machine but fail on the XP buildbot @@ -1352,7 +1349,7 @@ TEST_F(NavigationControllerTest, SameSubframe) { // A basic test case. Navigates to a single url, and make sure the history // db matches. TEST_F(NavigationControllerHistoryTest, Basic) { - controller()->LoadURL(url0, GURL(), PageTransition::LINK); + controller().LoadURL(url0, GURL(), PageTransition::LINK); rvh()->SendNavigate(0, url0); GetLastSession(); @@ -1372,7 +1369,7 @@ TEST_F(NavigationControllerHistoryTest, NavigationThenBack) { rvh()->SendNavigate(1, url1); rvh()->SendNavigate(2, url2); - controller()->GoBack(); + controller().GoBack(); rvh()->SendNavigate(1, url1); GetLastSession(); @@ -1399,10 +1396,10 @@ TEST_F(NavigationControllerHistoryTest, NavigationPruning) { rvh()->SendNavigate(1, url1); rvh()->SendNavigate(2, url2); - controller()->GoBack(); + controller().GoBack(); rvh()->SendNavigate(1, url1); - controller()->GoBack(); + controller().GoBack(); rvh()->SendNavigate(0, url0); rvh()->SendNavigate(3, url2); diff --git a/chrome/browser/password_manager/password_manager.cc b/chrome/browser/password_manager/password_manager.cc index 769c08e..2144806 100644 --- a/chrome/browser/password_manager/password_manager.cc +++ b/chrome/browser/password_manager/password_manager.cc @@ -98,8 +98,9 @@ PasswordManager::~PasswordManager() { } void PasswordManager::ProvisionallySavePassword(PasswordForm form) { - if (!web_contents_->controller() || !web_contents_->profile() || - web_contents_->profile()->IsOffTheRecord() || !*password_manager_enabled_) + if (!web_contents_->profile() || + web_contents_->profile()->IsOffTheRecord() || + !*password_manager_enabled_) return; // No password to save? Then don't. @@ -134,7 +135,7 @@ void PasswordManager::ProvisionallySavePassword(PasswordForm form) { return; form.ssl_valid = form.origin.SchemeIsSecure() && - !web_contents_->controller()->ssl_manager()-> + !web_contents_->controller().ssl_manager()-> ProcessedSSLErrorFromRequest(); form.preferred = true; manager->ProvisionallySave(form); @@ -166,9 +167,6 @@ void PasswordManager::DidStopLoading() { if (!web_contents_->profile() || !web_contents_->profile()->GetWebDataService(Profile::IMPLICIT_ACCESS)) return; - if (!web_contents_->controller()) - return; - if (provisional_save_manager_->IsNewLogin()) { web_contents_->AddInfoBar( new SavePasswordInfoBarDelegate(web_contents_, @@ -186,13 +184,11 @@ void PasswordManager::PasswordFormsSeen( if (!web_contents_->profile() || !web_contents_->profile()->GetWebDataService(Profile::EXPLICIT_ACCESS)) return; - if (!web_contents_->controller()) - return; if (!*password_manager_enabled_) return; // Ask the SSLManager for current security. - bool had_ssl_error = web_contents_->controller()->ssl_manager()-> + bool had_ssl_error = web_contents_->controller().ssl_manager()-> ProcessedSSLErrorFromRequest(); std::vector<PasswordForm>::const_iterator iter; diff --git a/chrome/browser/printing/print_view_manager.cc b/chrome/browser/printing/print_view_manager.cc index b8d1095..4bbc4ce 100644 --- a/chrome/browser/printing/print_view_manager.cc +++ b/chrome/browser/printing/print_view_manager.cc @@ -29,9 +29,6 @@ PrintViewManager::PrintViewManager(WebContents& owner) } PrintViewManager::~PrintViewManager() { -} - -void PrintViewManager::Destroy() { DisconnectFromCurrentPrintJob(); } @@ -125,7 +122,7 @@ std::wstring PrintViewManager::RenderSourceName() { } GURL PrintViewManager::RenderSourceUrl() { - NavigationEntry* entry = owner_.controller()->GetActiveEntry(); + NavigationEntry* entry = owner_.controller().GetActiveEntry(); if (entry) return entry->display_url(); else diff --git a/chrome/browser/renderer_host/render_view_host_unittest.cc b/chrome/browser/renderer_host/render_view_host_unittest.cc index 9e91318..b70293a 100644 --- a/chrome/browser/renderer_host/render_view_host_unittest.cc +++ b/chrome/browser/renderer_host/render_view_host_unittest.cc @@ -12,6 +12,6 @@ class RenderViewHostTest : public RenderViewHostTestHarness { // See RenderViewHost::OnMsgNavigate for a discussion. TEST_F(RenderViewHostTest, FilterAbout) { rvh()->SendNavigate(1, GURL("about:cache")); - ASSERT_TRUE(controller()->GetActiveEntry()); - EXPECT_EQ(GURL("about:blank"), controller()->GetActiveEntry()->url()); + ASSERT_TRUE(controller().GetActiveEntry()); + EXPECT_EQ(GURL("about:blank"), controller().GetActiveEntry()->url()); } diff --git a/chrome/browser/renderer_host/resource_dispatcher_host.cc b/chrome/browser/renderer_host/resource_dispatcher_host.cc index a4cd3ba..631ea6c 100644 --- a/chrome/browser/renderer_host/resource_dispatcher_host.cc +++ b/chrome/browser/renderer_host/resource_dispatcher_host.cc @@ -1295,7 +1295,7 @@ class NotificationTask : public Task { // Issue the notification. NotificationService::current()->Notify( type_, - Source<NavigationController>(tab_contents->controller()), + Source<NavigationController>(&tab_contents->controller()), Details<ResourceRequestDetails>(details_.get())); } } diff --git a/chrome/browser/renderer_host/test_render_view_host.cc b/chrome/browser/renderer_host/test_render_view_host.cc index abc5699..4255393 100644 --- a/chrome/browser/renderer_host/test_render_view_host.cc +++ b/chrome/browser/renderer_host/test_render_view_host.cc @@ -70,7 +70,7 @@ BackingStore* TestRenderWidgetHostView::AllocBackingStore( } void RenderViewHostTestHarness::NavigateAndCommit(const GURL& url) { - controller()->LoadURL(url, GURL(), 0); + controller().LoadURL(url, GURL(), 0); rvh()->SendNavigate(process()->max_page_id() + 1, url); } @@ -87,11 +87,8 @@ void RenderViewHostTestHarness::SetUp() { } void RenderViewHostTestHarness::TearDown() { - if (contents_) { - contents_->CloseContents(); - contents_ = NULL; - } - controller_ = NULL; + if (contents_) + delete contents_; // Make sure that we flush any messages related to WebContents destruction // before we destroy the profile. diff --git a/chrome/browser/renderer_host/test_render_view_host.h b/chrome/browser/renderer_host/test_render_view_host.h index 63e6da6..2d07975 100644 --- a/chrome/browser/renderer_host/test_render_view_host.h +++ b/chrome/browser/renderer_host/test_render_view_host.h @@ -184,7 +184,7 @@ class RenderViewHostTestHarness : public testing::Test { controller_(NULL) {} virtual ~RenderViewHostTestHarness() {} - NavigationController* controller() { + NavigationController& controller() { return contents_->controller(); } @@ -200,7 +200,7 @@ class RenderViewHostTestHarness : public testing::Test { return profile_.get(); } - MockRenderProcessHost* process() { + MockRenderProcessHost* process() { return static_cast<MockRenderProcessHost*>(rvh()->process()); } diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc index 0c1e95e..fd07b9d6 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc @@ -85,7 +85,7 @@ SafeBrowsingBlockingPage::SafeBrowsingBlockingPage( unsafe_resources_(unsafe_resources) { if (!is_main_frame_) { navigation_entry_index_to_remove_ = - tab()->controller()->last_committed_entry_index(); + tab()->controller().last_committed_entry_index(); } else { navigation_entry_index_to_remove_ = -1; } @@ -402,8 +402,8 @@ void SafeBrowsingBlockingPage::DontProceed() { // would trigger a navigation that would cause trouble as the render view host // for the tab has by then already been destroyed. if (navigation_entry_index_to_remove_ != -1 && !tab()->is_being_destroyed()) { - tab()->controller()->RemoveEntryAtIndex(navigation_entry_index_to_remove_, - GURL(chrome::kChromeUINewTabURL)); + tab()->controller().RemoveEntryAtIndex(navigation_entry_index_to_remove_, + GURL(chrome::kChromeUINewTabURL)); navigation_entry_index_to_remove_ = -1; } InterstitialPage::DontProceed(); diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc index e8311db..32f3ab6 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc @@ -154,7 +154,7 @@ class SafeBrowsingBlockingPageTest : public RenderViewHostTestHarness, // Tests showing a blocking page for a malware page and not proceeding. TEST_F(SafeBrowsingBlockingPageTest, MalwarePageDontProceed) { // Start a load. - controller()->LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED); + controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED); // Simulate the load causing a safe browsing interstitial to be shown. ShowInterstitial(ResourceType::MAIN_FRAME, kBadURL); @@ -169,13 +169,13 @@ TEST_F(SafeBrowsingBlockingPageTest, MalwarePageDontProceed) { EXPECT_FALSE(GetSafeBrowsingBlockingPage()); // We did not proceed, the pending entry should be gone. - EXPECT_FALSE(controller()->pending_entry()); + EXPECT_FALSE(controller().pending_entry()); } // Tests showing a blocking page for a malware page and then proceeding. TEST_F(SafeBrowsingBlockingPageTest, MalwarePageProceed) { // Start a load. - controller()->LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED); + controller().LoadURL(GURL(kBadURL), GURL(), PageTransition::TYPED); // Simulate the load causing a safe browsing interstitial to be shown. ShowInterstitial(ResourceType::MAIN_FRAME, kBadURL); @@ -215,8 +215,8 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMalwareResourceDontProceed) { // We did not proceed, we should be back to the first page, the 2nd one should // have been removed from the navigation controller. - ASSERT_EQ(1, controller()->entry_count()); - EXPECT_EQ(kGoogleURL, controller()->GetActiveEntry()->url().spec()); + ASSERT_EQ(1, controller().entry_count()); + EXPECT_EQ(kGoogleURL, controller().GetActiveEntry()->url().spec()); } // Tests showing a blocking page for a page that contains malware subresources @@ -237,8 +237,8 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMalwareResourceProceed) { EXPECT_FALSE(GetSafeBrowsingBlockingPage()); // We did proceed, we should be back to showing the page. - ASSERT_EQ(1, controller()->entry_count()); - EXPECT_EQ(kGoodURL, controller()->GetActiveEntry()->url().spec()); + ASSERT_EQ(1, controller().entry_count()); + EXPECT_EQ(kGoodURL, controller().GetActiveEntry()->url().spec()); } // Tests showing a blocking page for a page that contains multiple malware @@ -270,8 +270,8 @@ TEST_F(SafeBrowsingBlockingPageTest, // We did not proceed, we should be back to the first page, the 2nd one should // have been removed from the navigation controller. - ASSERT_EQ(1, controller()->entry_count()); - EXPECT_EQ(kGoogleURL, controller()->GetActiveEntry()->url().spec()); + ASSERT_EQ(1, controller().entry_count()); + EXPECT_EQ(kGoogleURL, controller().GetActiveEntry()->url().spec()); } // Tests showing a blocking page for a page that contains multiple malware @@ -313,8 +313,8 @@ TEST_F(SafeBrowsingBlockingPageTest, // We did not proceed, we should be back to the first page, the 2nd one should // have been removed from the navigation controller. - ASSERT_EQ(1, controller()->entry_count()); - EXPECT_EQ(kGoogleURL, controller()->GetActiveEntry()->url().spec()); + ASSERT_EQ(1, controller().entry_count()); + EXPECT_EQ(kGoogleURL, controller().GetActiveEntry()->url().spec()); } // Tests showing a blocking page for a page that contains multiple malware @@ -350,6 +350,6 @@ TEST_F(SafeBrowsingBlockingPageTest, PageWithMultipleMalwareResourceProceed) { EXPECT_EQ(OK, user_response()); // We did proceed, we should be back to the initial page. - ASSERT_EQ(1, controller()->entry_count()); - EXPECT_EQ(kGoodURL, controller()->GetActiveEntry()->url().spec()); + ASSERT_EQ(1, controller().entry_count()); + EXPECT_EQ(kGoodURL, controller().GetActiveEntry()->url().spec()); } diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc index 1a1c88a3..6369b3d 100644 --- a/chrome/browser/safe_browsing/safe_browsing_service.cc +++ b/chrome/browser/safe_browsing/safe_browsing_service.cc @@ -285,11 +285,9 @@ void SafeBrowsingService::DoDisplayBlockingPage( resource.threat_type == SafeBrowsingService::URL_MALWARE) { GURL page_url = wc->GetURL(); GURL referrer_url; - if (wc->controller()) { - NavigationEntry* entry = wc->controller()->GetActiveEntry(); - if (entry) - referrer_url = entry->referrer(); - } + NavigationEntry* entry = wc->controller().GetActiveEntry(); + if (entry) + referrer_url = entry->referrer(); io_loop_->PostTask(FROM_HERE, NewRunnableMethod(this, &SafeBrowsingService::ReportMalware, diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc index 47fbad9..f05a444 100644 --- a/chrome/browser/sessions/session_restore.cc +++ b/chrome/browser/sessions/session_restore.cc @@ -342,10 +342,10 @@ class SessionRestoreImpl : public NotificationObserver { std::min(selected_index, static_cast<int>(tab.navigations.size() - 1))); tab_loader_->AddTab( - browser->AddRestoredTab(tab.navigations, - static_cast<int>(i - window.tabs.begin()), - selected_index, - false)); + &browser->AddRestoredTab(tab.navigations, + static_cast<int>(i - window.tabs.begin()), + selected_index, + false)->controller()); } } @@ -383,7 +383,7 @@ class SessionRestoreImpl : public NotificationObserver { void NotifySessionServiceOfRestoredTabs(Browser* browser, int initial_count) { SessionService* session_service = profile_->GetSessionService(); for (int i = initial_count; i < browser->tab_count(); ++i) - session_service->TabRestored(browser->GetTabContentsAt(i)->controller()); + session_service->TabRestored(&browser->GetTabContentsAt(i)->controller()); } // The profile to create the sessions for. diff --git a/chrome/browser/sessions/session_service.cc b/chrome/browser/sessions/session_service.cc index ba473c9..c0b8158 100644 --- a/chrome/browser/sessions/session_service.cc +++ b/chrome/browser/sessions/session_service.cc @@ -938,7 +938,7 @@ void SessionService::BuildCommandsForBrowser( TabContents* tab = browser->GetTabContentsAt(i); DCHECK(tab); if (tab->profile() == profile()) { - BuildCommandsForTab(browser->session_id(), tab->controller(), i, + BuildCommandsForTab(browser->session_id(), &tab->controller(), i, commands, tab_to_available_range); if (windows_to_track && !added_to_windows_to_track) { windows_to_track->insert(browser->session_id().id()); diff --git a/chrome/browser/sessions/tab_restore_service.cc b/chrome/browser/sessions/tab_restore_service.cc index 340bf34..5892013 100644 --- a/chrome/browser/sessions/tab_restore_service.cc +++ b/chrome/browser/sessions/tab_restore_service.cc @@ -146,7 +146,7 @@ void TabRestoreService::BrowserClosing(Browser* browser) { size_t entry_index = 0; for (int tab_index = 0; tab_index < browser->tab_count(); ++tab_index) { PopulateTabFromController( - browser->GetTabContentsAt(tab_index)->controller(), + &browser->GetTabContentsAt(tab_index)->controller(), &(window->tabs[entry_index])); if (window->tabs[entry_index].navigations.empty()) window->tabs.erase(window->tabs.begin() + entry_index); @@ -229,13 +229,13 @@ void TabRestoreService::RestoreEntryById(Browser* browser, Browser* browser = Browser::Create(profile()); for (size_t tab_i = 0; tab_i < window->tabs.size(); ++tab_i) { const Tab& tab = window->tabs[tab_i]; - NavigationController* restored_controller = + TabContents* restored_tab = browser->AddRestoredTab(tab.navigations, browser->tab_count(), tab.current_navigation_index, (static_cast<int>(tab_i) == window->selected_tab_index)); - if (restored_controller) - restored_controller->LoadIfNecessary(); + if (restored_tab) + restored_tab->controller().LoadIfNecessary(); } browser->window()->Show(); } else { diff --git a/chrome/browser/sessions/tab_restore_service_unittest.cc b/chrome/browser/sessions/tab_restore_service_unittest.cc index 9be6429..7a24a91 100644 --- a/chrome/browser/sessions/tab_restore_service_unittest.cc +++ b/chrome/browser/sessions/tab_restore_service_unittest.cc @@ -41,9 +41,9 @@ class TabRestoreServiceTest : public RenderViewHostTestHarness { void NavigateToIndex(int index) { // Navigate back. We have to do this song and dance as NavigationController // isn't happy if you navigate immediately while going back. - controller()->GoToIndex(index); - rvh()->SendNavigate(controller()->pending_entry()->page_id(), - controller()->pending_entry()->url()); + controller().GoToIndex(index); + rvh()->SendNavigate(controller().pending_entry()->page_id(), + controller().pending_entry()->url()); } void RecreateService() { @@ -92,7 +92,7 @@ TEST_F(TabRestoreServiceTest, Basic) { AddThreeNavigations(); // Have the service record the tab. - service_->CreateHistoricalTab(controller()); + service_->CreateHistoricalTab(&controller()); // Make sure an entry was created. ASSERT_EQ(1U, service_->entries().size()); @@ -110,7 +110,7 @@ TEST_F(TabRestoreServiceTest, Basic) { NavigateToIndex(1); // And check again. - service_->CreateHistoricalTab(controller()); + service_->CreateHistoricalTab(&controller()); // There should be two entries now. ASSERT_EQ(2U, service_->entries().size()); @@ -129,7 +129,7 @@ TEST_F(TabRestoreServiceTest, Basic) { // Make sure TabRestoreService doesn't create an entry for a tab with no // navigations. TEST_F(TabRestoreServiceTest, DontCreateEmptyTab) { - service_->CreateHistoricalTab(controller()); + service_->CreateHistoricalTab(&controller()); EXPECT_TRUE(service_->entries().empty()); } @@ -138,7 +138,7 @@ TEST_F(TabRestoreServiceTest, Restore) { AddThreeNavigations(); // Have the service record the tab. - service_->CreateHistoricalTab(controller()); + service_->CreateHistoricalTab(&controller()); // Recreate the service and have it load the tabs. RecreateService(); @@ -162,7 +162,7 @@ TEST_F(TabRestoreServiceTest, DontLoadRestoredTab) { AddThreeNavigations(); // Have the service record the tab. - service_->CreateHistoricalTab(controller()); + service_->CreateHistoricalTab(&controller()); ASSERT_EQ(1U, service_->entries().size()); // Restore the tab. @@ -179,12 +179,12 @@ TEST_F(TabRestoreServiceTest, DontLoadRestoredTab) { // Make sure we persist entries to disk that have post data. TEST_F(TabRestoreServiceTest, DontPersistPostData) { AddThreeNavigations(); - controller()->GetEntryAtIndex(0)->set_has_post_data(true); - controller()->GetEntryAtIndex(1)->set_has_post_data(true); - controller()->GetEntryAtIndex(2)->set_has_post_data(true); + controller().GetEntryAtIndex(0)->set_has_post_data(true); + controller().GetEntryAtIndex(1)->set_has_post_data(true); + controller().GetEntryAtIndex(2)->set_has_post_data(true); // Have the service record the tab. - service_->CreateHistoricalTab(controller()); + service_->CreateHistoricalTab(&controller()); ASSERT_EQ(1U, service_->entries().size()); // Recreate the service and have it load the tabs. @@ -209,7 +209,7 @@ TEST_F(TabRestoreServiceTest, DontLoadTwice) { AddThreeNavigations(); // Have the service record the tab. - service_->CreateHistoricalTab(controller()); + service_->CreateHistoricalTab(&controller()); ASSERT_EQ(1U, service_->entries().size()); // Recreate the service and have it load the tabs. @@ -276,7 +276,7 @@ TEST_F(TabRestoreServiceTest, LoadPreviousSessionAndTabs) { AddThreeNavigations(); - service_->CreateHistoricalTab(controller()); + service_->CreateHistoricalTab(&controller()); RecreateService(); @@ -317,7 +317,7 @@ TEST_F(TabRestoreServiceTest, ManyWindowsInSessionService) { AddThreeNavigations(); - service_->CreateHistoricalTab(controller()); + service_->CreateHistoricalTab(&controller()); RecreateService(); diff --git a/chrome/browser/site_instance_unittest.cc b/chrome/browser/site_instance_unittest.cc index 83e3baf..883de56 100644 --- a/chrome/browser/site_instance_unittest.cc +++ b/chrome/browser/site_instance_unittest.cc @@ -5,6 +5,7 @@ #include "base/string16.h" #include "chrome/browser/renderer_host/browser_render_process_host.h" #include "chrome/browser/renderer_host/render_view_host.h" +#include "chrome/browser/renderer_host/test_render_view_host.h" #include "chrome/browser/tab_contents/navigation_entry.h" #include "chrome/browser/tab_contents/web_contents.h" #include "chrome/common/render_messages.h" @@ -68,6 +69,10 @@ class TestSiteInstance : public SiteInstance { // Test to ensure no memory leaks for SiteInstance objects. TEST_F(SiteInstanceTest, SiteInstanceDestructor) { + // The existance of these factories will cause WebContents to create our test + // one instead of the real one. + MockRenderProcessHostFactory rph_factory; + TestRenderViewHostFactory rvh_factory(&rph_factory); int siteDeleteCounter = 0; int browsingDeleteCounter = 0; const GURL url("test:foo"); @@ -107,15 +112,11 @@ TEST_F(SiteInstanceTest, SiteInstanceDestructor) { TestSiteInstance::CreateTestSiteInstance(profile.get(), &siteDeleteCounter, &browsingDeleteCounter); - WebContents* contents = new WebContents( - profile.get(), instance, MSG_ROUTING_NONE, NULL); - contents->SetupController(profile.get()); - EXPECT_EQ(1, siteDeleteCounter); - EXPECT_EQ(1, browsingDeleteCounter); - - contents->CloseContents(); - // Make sure that we flush any messages related to WebContents destruction. - MessageLoop::current()->RunAllPending(); + { + WebContents contents(profile.get(), instance, MSG_ROUTING_NONE, NULL); + EXPECT_EQ(1, siteDeleteCounter); + EXPECT_EQ(1, browsingDeleteCounter); + } EXPECT_EQ(2, siteDeleteCounter); EXPECT_EQ(2, browsingDeleteCounter); diff --git a/chrome/browser/ssl/ssl_blocking_page.cc b/chrome/browser/ssl/ssl_blocking_page.cc index 5abf71a..ce9d9da 100644 --- a/chrome/browser/ssl/ssl_blocking_page.cc +++ b/chrome/browser/ssl/ssl_blocking_page.cc @@ -81,7 +81,7 @@ void SSLBlockingPage::UpdateEntry(NavigationEntry* entry) { entry->ssl().set_security_bits(ssl_info.security_bits); NotificationService::current()->Notify( NotificationType::SSL_VISIBLE_STATE_CHANGED, - Source<NavigationController>(web->controller()), + Source<NavigationController>(&web->controller()), NotificationService::NoDetails()); } diff --git a/chrome/browser/ssl/ssl_manager.cc b/chrome/browser/ssl/ssl_manager.cc index bbdf0e2..afc48ce 100644 --- a/chrome/browser/ssl/ssl_manager.cc +++ b/chrome/browser/ssl/ssl_manager.cc @@ -292,7 +292,7 @@ void SSLManager::ErrorHandler::Dispatch() { } // Hand ourselves off to the SSLManager. - manager_ = web_contents->controller()->ssl_manager(); + manager_ = web_contents->controller().ssl_manager(); OnDispatched(); } diff --git a/chrome/browser/ssl/ssl_policy.cc b/chrome/browser/ssl/ssl_policy.cc index 3cb5975..ebe5e60 100644 --- a/chrome/browser/ssl/ssl_policy.cc +++ b/chrome/browser/ssl/ssl_policy.cc @@ -136,7 +136,7 @@ static void ShowErrorPage(SSLPolicy* policy, SSLManager::CertError* error) { true, error->request_url(), security_info); - tab->controller()->GetActiveEntry()->set_page_type( + tab->controller().GetActiveEntry()->set_page_type( NavigationEntry::ERROR_PAGE); } diff --git a/chrome/browser/tab_contents/infobar_delegate.cc b/chrome/browser/tab_contents/infobar_delegate.cc index 6925dda..9a466fb 100644 --- a/chrome/browser/tab_contents/infobar_delegate.cc +++ b/chrome/browser/tab_contents/infobar_delegate.cc @@ -29,7 +29,7 @@ InfoBarDelegate::InfoBarDelegate(TabContents* contents) } void InfoBarDelegate::StoreActiveEntryUniqueID(TabContents* contents) { - NavigationEntry* active_entry = contents->controller()->GetActiveEntry(); + NavigationEntry* active_entry = contents->controller().GetActiveEntry(); contents_unique_id_ = active_entry ? active_entry->unique_id() : 0; } diff --git a/chrome/browser/tab_contents/interstitial_page.cc b/chrome/browser/tab_contents/interstitial_page.cc index 206bc34..1ffff17 100644 --- a/chrome/browser/tab_contents/interstitial_page.cc +++ b/chrome/browser/tab_contents/interstitial_page.cc @@ -121,7 +121,7 @@ InterstitialPage::InterstitialPage(WebContents* tab, // (which is the case when the interstitial was triggered by a sub-resource on // a page) when we have a pending entry (in the process of loading a new top // frame). - DCHECK(new_navigation || !tab->controller()->pending_entry()); + DCHECK(new_navigation || !tab->controller().pending_entry()); } InterstitialPage::~InterstitialPage() { @@ -169,7 +169,7 @@ void InterstitialPage::Show() { // Give sub-classes a chance to set some states on the navigation entry. UpdateEntry(entry); - tab_->controller()->AddTransientEntry(entry); + tab_->controller().AddTransientEntry(entry); } DCHECK(!render_view_host_); @@ -183,9 +183,9 @@ void InterstitialPage::Show() { notification_registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED, Source<TabContents>(tab_)); notification_registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED, - Source<NavigationController>(tab_->controller())); + Source<NavigationController>(&tab_->controller())); notification_registrar_.Add(this, NotificationType::NAV_ENTRY_PENDING, - Source<NavigationController>(tab_->controller())); + Source<NavigationController>(&tab_->controller())); } void InterstitialPage::Hide() { @@ -194,7 +194,7 @@ void InterstitialPage::Hide() { if (tab_->interstitial_page()) tab_->remove_interstitial_page(); // Let's revert to the original title if necessary. - NavigationEntry* entry = tab_->controller()->GetActiveEntry(); + NavigationEntry* entry = tab_->controller().GetActiveEntry(); if (!new_navigation_ && should_revert_tab_title_) { entry->set_title(WideToUTF16Hack(original_tab_title_)); tab_->NotifyNavigationStateChanged(TabContents::INVALIDATE_TITLE); @@ -321,7 +321,7 @@ void InterstitialPage::DontProceed() { // explicitely. Note that by calling DiscardNonCommittedEntries() we also // discard the pending entry, which is what we want, since the navigation is // cancelled. - tab_->controller()->DiscardNonCommittedEntries(); + tab_->controller().DiscardNonCommittedEntries(); } Hide(); @@ -383,7 +383,7 @@ void InterstitialPage::UpdateTitle(RenderViewHost* render_view_host, int32 page_id, const std::wstring& title) { DCHECK(render_view_host == render_view_host_); - NavigationEntry* entry = tab_->controller()->GetActiveEntry(); + NavigationEntry* entry = tab_->controller().GetActiveEntry(); // If this interstitial is shown on an existing navigation entry, we'll need // to remember its title so we can revert to it when hidden. if (!new_navigation_ && !should_revert_tab_title_) { diff --git a/chrome/browser/tab_contents/navigation_controller.cc b/chrome/browser/tab_contents/navigation_controller.cc index 77cf714..268eadb 100644 --- a/chrome/browser/tab_contents/navigation_controller.cc +++ b/chrome/browser/tab_contents/navigation_controller.cc @@ -15,7 +15,6 @@ #include "chrome/browser/tab_contents/navigation_entry.h" #include "chrome/browser/tab_contents/repost_form_warning.h" #include "chrome/browser/tab_contents/site_instance.h" -#include "chrome/browser/tab_contents/web_contents.h" #include "chrome/common/navigation_types.h" #include "chrome/common/notification_service.h" #include "chrome/common/render_messages.h" @@ -121,45 +120,34 @@ NavigationController::NavigationController(TabContents* contents, ALLOW_THIS_IN_INITIALIZER_LIST(ssl_manager_(this, NULL)), needs_reload_(false), load_pending_entry_when_active_(false) { - if (contents) - contents->set_controller(this); DCHECK(profile_); } -NavigationController::NavigationController( - Profile* profile, +NavigationController::~NavigationController() { + DiscardNonCommittedEntriesInternal(); + + NotificationService::current()->Notify( + NotificationType::TAB_CLOSED, + Source<NavigationController>(this), + NotificationService::NoDetails()); +} + +void NavigationController::RestoreFromState( const std::vector<TabNavigation>& navigations, - int selected_navigation) - : profile_(profile), - pending_entry_(NULL), - last_committed_entry_index_(-1), - pending_entry_index_(-1), - transient_entry_index_(-1), - tab_contents_(NULL), - max_restored_page_id_(-1), - ALLOW_THIS_IN_INITIALIZER_LIST(ssl_manager_(this, NULL)), - needs_reload_(true), - load_pending_entry_when_active_(false) { - DCHECK(profile_); + int selected_navigation) { + // Verify that this controller is unused and that the input is valid. + DCHECK(entry_count() == 0 && !pending_entry()); DCHECK(selected_navigation >= 0 && selected_navigation < static_cast<int>(navigations.size())); // Populate entries_ from the supplied TabNavigations. + needs_reload_ = true; CreateNavigationEntriesFromTabNavigations(navigations, &entries_); // And finish the restore. FinishRestore(selected_navigation); } -NavigationController::~NavigationController() { - DiscardNonCommittedEntriesInternal(); - - NotificationService::current()->Notify( - NotificationType::TAB_CLOSED, - Source<NavigationController>(this), - NotificationService::NoDetails()); -} - void NavigationController::Reload(bool check_for_repost) { // Reloading a transient entry does nothing. if (transient_entry_index_ != -1) @@ -351,19 +339,6 @@ void NavigationController::RemoveEntryAtIndex(int index, } } -void NavigationController::Destroy() { - // TODO(brettw) the destruction of TabContents/NavigationController makes no - // sense (see TabContentsWasDestroyed) - tab_contents_->Destroy(); - // We are now deleted. -} - -void NavigationController::TabContentsWasDestroyed() { - // TODO(brettw) the destruction of TabContents/NavigationController makes no - // sense (see Destroy). - delete this; -} - NavigationEntry* NavigationController::CreateNavigationEntry( const GURL& url, const GURL& referrer, PageTransition::Type transition) { // Allow the browser URL handler to rewrite the URL. This will, for example, @@ -421,7 +396,7 @@ void NavigationController::LoadURLLazily(const GURL& url, load_pending_entry_when_active_ = true; } -bool NavigationController::LoadingURLLazily() { +bool NavigationController::LoadingURLLazily() const { return load_pending_entry_when_active_; } @@ -741,6 +716,7 @@ bool NavigationController::RendererDidNavigateAutoSubframe( return false; } +// TODO(brettw) I think this function is unnecessary. void NavigationController::CommitPendingEntry() { DiscardTransientEntry(); @@ -803,6 +779,22 @@ bool NavigationController::IsURLInPageNavigation(const GURL& url) const { return AreURLsInPageNavigation(last_committed->url(), url); } +void NavigationController::CopyStateFrom(const NavigationController& source) { + // Verify that we look new. + DCHECK(entry_count() == 0 && !pending_entry()); + + if (source.entry_count() == 0) + return; // Nothing new to do. + + needs_reload_ = true; + for (int i = 0; i < source.entry_count(); i++) { + entries_.push_back(linked_ptr<NavigationEntry>( + new NavigationEntry(*source.entries_[i]))); + } + + FinishRestore(source.last_committed_entry_index_); +} + void NavigationController::DiscardNonCommittedEntries() { bool transient = transient_entry_index_ != -1; DiscardNonCommittedEntriesInternal(); @@ -867,9 +859,6 @@ void NavigationController::NavigateToPendingEntry(bool reload) { pending_entry_ = entries_[pending_entry_index_].get(); } - tab_contents_ = GetTabContentsCreateIfNecessary(*pending_entry_); - - NavigationEntry temp_entry(*pending_entry_); if (!tab_contents_->NavigateToPendingEntry(reload)) DiscardNonCommittedEntries(); } @@ -889,17 +878,6 @@ void NavigationController::NotifyNavigationEntryCommitted( Details<LoadCommittedDetails>(details)); } -TabContents* NavigationController::GetTabContentsCreateIfNecessary( - const NavigationEntry& entry) { - if (tab_contents_) - return tab_contents_; - - tab_contents_ = new WebContents(profile_, entry.site_instance(), - MSG_ROUTING_NONE, NULL); - tab_contents_->set_controller(this); - return tab_contents_; // TODO(brettw) it's stupid to both set and return it. -} - // static void NavigationController::DisablePromptOnRepost() { check_for_repost_ = false; @@ -938,25 +916,6 @@ void NavigationController::NotifyEntryChanged(const NavigationEntry* entry, Details<EntryChangedDetails>(&det)); } -NavigationController* NavigationController::Clone() { - NavigationController* nc = new NavigationController(NULL, profile_); - - if (entry_count() == 0) - return nc; - - nc->needs_reload_ = true; - - nc->entries_.reserve(entries_.size()); - for (int i = 0, c = entry_count(); i < c; ++i) { - nc->entries_.push_back(linked_ptr<NavigationEntry>( - new NavigationEntry(*GetEntryAtIndex(i)))); - } - - nc->FinishRestore(last_committed_entry_index_); - - return nc; -} - void NavigationController::FinishRestore(int selected_index) { DCHECK(selected_index >= 0 && selected_index < entry_count()); ConfigureEntriesForRestore(&entries_); @@ -964,9 +923,6 @@ void NavigationController::FinishRestore(int selected_index) { set_max_restored_page_id(entry_count()); last_committed_entry_index_ = selected_index; - - // Callers assume we have an active_contents after restoring, so set it now. - tab_contents_ = GetTabContentsCreateIfNecessary(*entries_[selected_index]); } void NavigationController::DiscardNonCommittedEntriesInternal() { diff --git a/chrome/browser/tab_contents/navigation_controller.h b/chrome/browser/tab_contents/navigation_controller.h index af7cf60..e42f4ad 100644 --- a/chrome/browser/tab_contents/navigation_controller.h +++ b/chrome/browser/tab_contents/navigation_controller.h @@ -123,34 +123,21 @@ 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). - NavigationController( - Profile* profile, - const std::vector<TabNavigation>& navigations, - int selected_navigation); + NavigationController(TabContents* tab_contents, Profile* profile); ~NavigationController(); - // 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. - NavigationController* Clone(); - // Returns the profile for this controller. It can never be NULL. Profile* profile() const { return profile_; } + // Initializes this NavigationController with the given saved navigations, + // using selected_navigation as the currently loaded entry. Before this call + // the controller should be unused (there should be no current entry). This is + // used for session restore. + void RestoreFromState(const std::vector<TabNavigation>& navigations, + int selected_navigation); + // Active entry -------------------------------------------------------------- // Returns the active entry, which is the transient entry if any, the pending @@ -295,10 +282,6 @@ class NavigationController { // TabContents --------------------------------------------------------------- - // Notifies the controller that a TabContents that it owns has been destroyed. - // This is part of the NavigationController's Destroy sequence. - void TabContentsWasDestroyed(); - // Returns the tab contents associated with this controller. Non-NULL except // during set-up of the tab. TabContents* tab_contents() const { @@ -346,13 +329,17 @@ class NavigationController { // refs without reload, only change to "#" which we don't count as empty). bool IsURLInPageNavigation(const GURL& url) const; + // Copies the navigation state from the given controller to this one. This + // one should be empty (just created). + void CopyStateFrom(const NavigationController& source); + // Random data --------------------------------------------------------------- // Returns true if this NavigationController is is configured to load a URL // lazily. If true, use GetLazyTitle() and GetLazyFavIcon() to discover the // titles and favicons. Since no request was made, this is the only info // we have about this page. This feature is used by web application clusters. - bool LoadingURLLazily(); + bool LoadingURLLazily() const; const string16& GetLazyTitle() const; const SkBitmap& GetLazyFavIcon() const; @@ -425,11 +412,6 @@ class NavigationController { // committed. This will fill in the active entry to the details structure. void NotifyNavigationEntryCommitted(LoadCommittedDetails* details); - // Returns the TabContents for the |entry|'s type. If the TabContents - // doesn't yet exist, it is created. If a new TabContents is created, its - // parent is |parent|. Becomes part of |entry|'s SiteInstance. - TabContents* GetTabContentsCreateIfNecessary(const NavigationEntry& entry); - // Sets the max restored page ID this NavigationController has seen, if it // was restored from a previous session. void set_max_restored_page_id(int max_id) { max_restored_page_id_ = max_id; } diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc index fd94e13..698a65a 100644 --- a/chrome/browser/tab_contents/render_view_context_menu.cc +++ b/chrome/browser/tab_contents/render_view_context_menu.cc @@ -208,10 +208,10 @@ bool RenderViewContextMenu::IsItemCommandEnabled(int id) const { switch (id) { case IDS_CONTENT_CONTEXT_BACK: - return source_web_contents_->controller()->CanGoBack(); + return source_web_contents_->controller().CanGoBack(); case IDS_CONTENT_CONTEXT_FORWARD: - return source_web_contents_->controller()->CanGoForward(); + return source_web_contents_->controller().CanGoForward(); case IDS_CONTENT_CONTEXT_VIEWPAGESOURCE: case IDS_CONTENT_CONTEXT_VIEWFRAMESOURCE: @@ -283,7 +283,7 @@ bool RenderViewContextMenu::IsItemCommandEnabled(int id) const { return !params_.misspelled_word.empty(); case IDS_CONTENT_CONTEXT_VIEWPAGEINFO: - return (source_web_contents_->controller()->GetActiveEntry() != NULL); + return (source_web_contents_->controller().GetActiveEntry() != NULL); case IDS_CONTENT_CONTEXT_RELOAD: case IDS_CONTENT_CONTEXT_COPYIMAGE: @@ -393,11 +393,11 @@ void RenderViewContextMenu::ExecuteItemCommand(int id) { break; case IDS_CONTENT_CONTEXT_BACK: - source_web_contents_->controller()->GoBack(); + source_web_contents_->controller().GoBack(); break; case IDS_CONTENT_CONTEXT_FORWARD: - source_web_contents_->controller()->GoForward(); + source_web_contents_->controller().GoForward(); break; case IDS_CONTENT_CONTEXT_SAVEPAGEAS: @@ -405,7 +405,7 @@ void RenderViewContextMenu::ExecuteItemCommand(int id) { break; case IDS_CONTENT_CONTEXT_RELOAD: - source_web_contents_->controller()->Reload(true); + source_web_contents_->controller().Reload(true); break; case IDS_CONTENT_CONTEXT_PRINT: @@ -424,7 +424,7 @@ void RenderViewContextMenu::ExecuteItemCommand(int id) { case IDS_CONTENT_CONTEXT_VIEWPAGEINFO: { #if defined(OS_WIN) NavigationEntry* nav_entry = - source_web_contents_->controller()->GetActiveEntry(); + source_web_contents_->controller().GetActiveEntry(); PageInfoWindow::CreatePageInfo( source_web_contents_->profile(), nav_entry, @@ -581,7 +581,7 @@ bool RenderViewContextMenu::IsDevCommandEnabled(int id) const { return true; NavigationEntry *active_entry = - source_web_contents_->controller()->GetActiveEntry(); + source_web_contents_->controller().GetActiveEntry(); if (!active_entry) return false; diff --git a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc index 9846a77..9268850 100644 --- a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc +++ b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc @@ -29,7 +29,6 @@ void RenderViewHostDelegateViewHelper::CreateNewWindow(int route_id, site, route_id, modal_dialog_event); - new_contents->SetupController(profile); WebContentsView* new_view = new_contents->view(); // TODO(brettw) it seems bogus that we have to call this function on the diff --git a/chrome/browser/tab_contents/render_view_host_manager.cc b/chrome/browser/tab_contents/render_view_host_manager.cc index a178356..8b2c276 100644 --- a/chrome/browser/tab_contents/render_view_host_manager.cc +++ b/chrome/browser/tab_contents/render_view_host_manager.cc @@ -37,9 +37,13 @@ RenderViewHostManager::RenderViewHostManager( } RenderViewHostManager::~RenderViewHostManager() { - // Shutdown should have been called which should have cleaned these up. - DCHECK(!render_view_host_); - DCHECK(!pending_render_view_host_); + if (pending_render_view_host_) + CancelPending(); + + // We should always have a main RenderViewHost. + RenderViewHost* render_view_host = render_view_host_; + render_view_host_ = NULL; + render_view_host->Shutdown(); } void RenderViewHostManager::Init(Profile* profile, @@ -55,16 +59,6 @@ void RenderViewHostManager::Init(Profile* profile, site_instance, render_view_delegate_, routing_id, modal_dialog_event); } -void RenderViewHostManager::Shutdown() { - if (pending_render_view_host_) - CancelPending(); - - // We should always have a main RenderViewHost. - RenderViewHost* render_view_host = render_view_host_; - render_view_host_ = NULL; - render_view_host->Shutdown(); -} - RenderViewHost* RenderViewHostManager::Navigate(const NavigationEntry& entry) { // This will possibly create (set to NULL) a DOM UI object for the pending // page. We'll use this later to give the page special access. This must @@ -105,7 +99,7 @@ RenderViewHost* RenderViewHostManager::Navigate(const NavigationEntry& entry) { NotificationService::current()->Notify( NotificationType::RENDER_VIEW_HOST_CHANGED, Source<NavigationController>( - delegate_->GetControllerForRenderManager()), + &delegate_->GetControllerForRenderManager()), Details<RenderViewHostSwitchedDetails>(&details)); } } @@ -356,12 +350,12 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry( // For now, though, we're in a hybrid model where you only switch // SiteInstances if you type in a cross-site URL. This means we have to // compare the entry's URL to the last committed entry's URL. - NavigationController* controller = delegate_->GetControllerForRenderManager(); - NavigationEntry* curr_entry = controller->GetLastCommittedEntry(); + NavigationController& controller = delegate_->GetControllerForRenderManager(); + NavigationEntry* curr_entry = controller.GetLastCommittedEntry(); if (interstitial_page_) { // The interstitial is currently the last committed entry, but we want to // compare against the last non-interstitial entry. - curr_entry = controller->GetEntryAtOffset(-1); + curr_entry = controller.GetEntryAtOffset(-1); } // If there is no last non-interstitial entry (and curr_instance already // has a site), then we must have been opened from another tab. We want @@ -386,7 +380,7 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry( // that page, we want to explicity ignore that BrowsingInstance and make a // new process. return SiteInstance::CreateSiteInstance( - delegate_->GetControllerForRenderManager()->profile()); + delegate_->GetControllerForRenderManager().profile()); } else { // Start the new renderer in a new SiteInstance, but in the current // BrowsingInstance. It is important to immediately give this new @@ -399,7 +393,7 @@ SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry( bool RenderViewHostManager::CreatePendingRenderView(SiteInstance* instance) { NavigationEntry* curr_entry = - delegate_->GetControllerForRenderManager()->GetLastCommittedEntry(); + delegate_->GetControllerForRenderManager().GetLastCommittedEntry(); if (curr_entry) { DCHECK(!curr_entry->content_state().empty()); // TODO(creis): Should send a message to the RenderView to let it know @@ -466,7 +460,7 @@ void RenderViewHostManager::CommitPending() { details.old_host = old_render_view_host; NotificationService::current()->Notify( NotificationType::RENDER_VIEW_HOST_CHANGED, - Source<NavigationController>(delegate_->GetControllerForRenderManager()), + Source<NavigationController>(&delegate_->GetControllerForRenderManager()), Details<RenderViewHostSwitchedDetails>(&details)); old_render_view_host->Shutdown(); diff --git a/chrome/browser/tab_contents/render_view_host_manager.h b/chrome/browser/tab_contents/render_view_host_manager.h index b3a5b25..15ee081 100644 --- a/chrome/browser/tab_contents/render_view_host_manager.h +++ b/chrome/browser/tab_contents/render_view_host_manager.h @@ -47,7 +47,7 @@ class RenderViewHostManager : public NotificationObserver { RenderViewHost* render_view_host) = 0; virtual void UpdateRenderViewSizeForRenderManager() = 0; virtual void NotifySwappedFromRenderManager() = 0; - virtual NavigationController* GetControllerForRenderManager() = 0; + virtual NavigationController& GetControllerForRenderManager() = 0; // Creates a DOMUI object for the given URL if one applies. Ownership of the // returned pointer will be passed to the caller. If no DOMUI applies, @@ -64,8 +64,7 @@ class RenderViewHostManager : public NotificationObserver { // They must outlive this class. The RenderViewHostDelegate is what will be // installed into all RenderViewHosts that are created. // - // You must call Init() before using this class and Shutdown() before - // deleting it. + // You must call Init() before using this class. RenderViewHostManager(RenderViewHostDelegate* render_view_delegate, Delegate* delegate); ~RenderViewHostManager(); @@ -76,9 +75,6 @@ class RenderViewHostManager : public NotificationObserver { int routing_id, base::WaitableEvent* modal_dialog_event); - // Schedules all RenderViewHosts for destruction. - void Shutdown(); - // Returns the currently actuive RenderViewHost. // // This will be non-NULL between Init() and Shutdown(). You may want to NULL diff --git a/chrome/browser/tab_contents/render_view_host_manager_unittest.cc b/chrome/browser/tab_contents/render_view_host_manager_unittest.cc index 02f5473..f0caccd 100644 --- a/chrome/browser/tab_contents/render_view_host_manager_unittest.cc +++ b/chrome/browser/tab_contents/render_view_host_manager_unittest.cc @@ -23,27 +23,22 @@ TEST_F(RenderViewHostManagerTest, NewTabPageProcesses) { NavigateAndCommit(dest); // Make a second tab. - WebContents* contents2 = new TestWebContents(profile_.get(), NULL); - NavigationController* controller2 = - new NavigationController(contents2, profile_.get()); - contents2->set_controller(controller2); + TestWebContents contents2(profile_.get(), NULL); // Load the two URLs in the second tab. Note that the first navigation creates // a RVH that's not pending (since there is no cross-site transition), so // we use the committed one, but the second one is the opposite. - contents2->controller()->LoadURL(ntp, GURL(), PageTransition::LINK); - static_cast<TestRenderViewHost*>(contents2->render_manager()-> + contents2.controller().LoadURL(ntp, GURL(), PageTransition::LINK); + static_cast<TestRenderViewHost*>(contents2.render_manager()-> current_host())->SendNavigate(100, ntp); - contents2->controller()->LoadURL(dest, GURL(), PageTransition::LINK); - static_cast<TestRenderViewHost*>(contents2->render_manager()-> + contents2.controller().LoadURL(dest, GURL(), PageTransition::LINK); + static_cast<TestRenderViewHost*>(contents2.render_manager()-> pending_render_view_host())->SendNavigate(101, dest); // The two RVH's should be different in every way. - EXPECT_NE(rvh()->process(), contents2->render_view_host()->process()); + EXPECT_NE(rvh()->process(), contents2.render_view_host()->process()); EXPECT_NE(rvh()->site_instance(), - contents2->render_view_host()->site_instance()); + contents2.render_view_host()->site_instance()); EXPECT_NE(rvh()->site_instance()->browsing_instance(), - contents2->render_view_host()->site_instance()->browsing_instance()); - - contents2->CloseContents(); + contents2.render_view_host()->site_instance()->browsing_instance()); } diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index ea5a9b5..226ce5b 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -24,9 +24,9 @@ #include "chrome/views/controls/scrollbar/native_scroll_bar.h" #endif -TabContents::TabContents() +TabContents::TabContents(Profile* profile) : delegate_(NULL), - controller_(NULL), + controller_(this, profile), is_loading_(false), is_crashed_(false), waiting_for_response_(false), @@ -45,71 +45,13 @@ void TabContents::RegisterUserPrefs(PrefService* prefs) { prefs->RegisterBooleanPref(prefs::kBlockPopups, false); } -void TabContents::CloseContents() { - // Destroy our NavigationController, which will Destroy all tabs it owns. - controller_->Destroy(); - // Note that the controller may have deleted us at this point, - // so don't touch any member variables here. -} - -void TabContents::Destroy() { - DCHECK(!is_being_destroyed_); - is_being_destroyed_ = true; - - // First cleanly close all child windows. - // TODO(mpcomplete): handle case if MaybeCloseChildWindows() already asked - // some of these to close. CloseWindows is async, so it might get called - // twice before it runs. - int size = static_cast<int>(child_windows_.size()); - for (int i = size - 1; i >= 0; --i) { - ConstrainedWindow* window = child_windows_[i]; - if (window) - window->CloseConstrainedWindow(); - } - - // Notify any lasting InfobarDelegates that have not yet been removed that - // whatever infobar they were handling in this TabContents has closed, - // because the TabContents is going away entirely. - for (int i = 0; i < infobar_delegate_count(); ++i) { - InfoBarDelegate* delegate = GetInfoBarDelegateAt(i); - delegate->InfoBarClosed(); - } - infobar_delegates_.clear(); - - // Notify any observer that have a reference on this tab contents. - NotificationService::current()->Notify( - NotificationType::TAB_CONTENTS_DESTROYED, - Source<TabContents>(this), - NotificationService::NoDetails()); - -#if defined(OS_WIN) - // If we still have a window handle, destroy it. GetNativeView can return - // NULL if this contents was part of a window that closed. - if (GetNativeView()) - ::DestroyWindow(GetNativeView()); -#endif - - // Notify our NavigationController. Make sure we are deleted first, so - // that the controller is the last to die. - NavigationController* controller = controller_; - - delete this; - - controller->TabContentsWasDestroyed(); -} - -void TabContents::SetupController(Profile* profile) { - DCHECK(!controller_); - controller_ = new NavigationController(this, profile); -} - bool TabContents::SupportsURL(GURL* url) { return true; } const GURL& TabContents::GetURL() const { // We may not have a navigation entry yet - NavigationEntry* entry = controller_->GetActiveEntry(); + NavigationEntry* entry = controller_.GetActiveEntry(); return entry ? entry->display_url() : GURL::EmptyGURL(); } @@ -140,22 +82,22 @@ const std::wstring TabContents::GetDefaultTitle() const { SkBitmap TabContents::GetFavIcon() const { // Like GetTitle(), we also want to use the favicon for the last committed // entry rather than a pending navigation entry. - NavigationEntry* entry = controller_->GetTransientEntry(); + NavigationEntry* entry = controller_.GetTransientEntry(); if (entry) return entry->favicon().bitmap(); - entry = controller_->GetLastCommittedEntry(); + entry = controller_.GetLastCommittedEntry(); if (entry) return entry->favicon().bitmap(); - else if (controller_->LoadingURLLazily()) - return controller_->GetLazyFavIcon(); + else if (controller_.LoadingURLLazily()) + return controller_.GetLazyFavIcon(); return SkBitmap(); } #if defined(OS_WIN) SecurityStyle TabContents::GetSecurityStyle() const { // We may not have a navigation entry yet. - NavigationEntry* entry = controller_->GetActiveEntry(); + NavigationEntry* entry = controller_.GetActiveEntry(); return entry ? entry->ssl().security_style() : SECURITY_STYLE_UNKNOWN; } @@ -165,7 +107,7 @@ bool TabContents::GetSSLEVText(std::wstring* ev_text, ev_text->clear(); ev_tooltip_text->clear(); - NavigationEntry* entry = controller_->GetActiveEntry(); + NavigationEntry* entry = controller_.GetActiveEntry(); if (!entry || net::IsCertStatusError(entry->ssl().cert_status()) || ((entry->ssl().cert_status() & net::CERT_STATUS_IS_EV) == 0)) @@ -211,8 +153,8 @@ void TabContents::OpenURL(const GURL& url, const GURL& referrer, bool TabContents::NavigateToPendingEntry(bool reload) { // Our benavior is just to report that the entry was committed. string16 default_title = WideToUTF16Hack(GetDefaultTitle()); - controller()->pending_entry()->set_title(default_title); - controller()->CommitPendingEntry(); + controller_.pending_entry()->set_title(default_title); + controller_.CommitPendingEntry(); return true; } @@ -302,9 +244,8 @@ void TabContents::AddInfoBar(InfoBarDelegate* delegate) { // added. We use this notification to expire InfoBars that need to expire on // page transitions. if (infobar_delegates_.size() == 1) { - DCHECK(controller()); registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED, - Source<NavigationController>(controller())); + Source<NavigationController>(&controller_)); } } @@ -322,7 +263,7 @@ void TabContents::RemoveInfoBar(InfoBarDelegate* delegate) { // Remove ourselves as an observer if we are tracking no more InfoBars. if (infobar_delegates_.empty()) { registrar_.Remove(this, NotificationType::NAV_ENTRY_COMMITTED, - Source<NavigationController>(controller())); + Source<NavigationController>(&controller_)); } } } diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h index 26dda95..aa636f3 100644 --- a/chrome/browser/tab_contents/tab_contents.h +++ b/chrome/browser/tab_contents/tab_contents.h @@ -44,23 +44,8 @@ class SkBitmap; class SiteInstance; class WebContents; -// Describes what goes in the main content area of a tab. For example, -// the WebContents is one such thing. -// -// When instantiating a new TabContents explicitly, the TabContents will not -// have an associated NavigationController. To setup a NavigationController -// for the TabContents, its SetupController method should be called. -// -// Once they reside within a NavigationController, TabContents objects are -// owned by that NavigationController. When the active TabContents within that -// NavigationController is closed, that TabContents destroys the -// NavigationController, which then destroys all of the TabContentses in it. -// -// NOTE: When the NavigationController is navigated to an URL corresponding to -// a different type of TabContents (see the TabContents::TypeForURL method), -// the NavigationController makes the active TabContents inactive, notifies the -// TabContentsDelegate that the TabContents is being replaced, and then -// activates the new TabContents. +// Describes what goes in the main content area of a tab. WebContents is +// the only type of TabContents, and these should be merged together. class TabContents : public PageNavigator, public NotificationObserver { public: @@ -77,22 +62,9 @@ class TabContents : public PageNavigator, INVALIDATE_EVERYTHING = 0xFFFFFFFF }; - static void RegisterUserPrefs(PrefService* prefs); - - // Creation & destruction ---------------------------------------------------- - - // Request this tab to shut down. This kills the tab's NavigationController, - // which then Destroy()s all tabs it controls. - void CloseContents(); + virtual ~TabContents(); - // Unregister/shut down any pending tasks involving this tab. - // This is called as the tab is shutting down, before the - // NavigationController (and consequently profile) are gone. - // - // If you override this, be sure to call this implementation at the end - // of yours. - // See also Close(). - virtual void Destroy() = 0; + static void RegisterUserPrefs(PrefService* prefs); // Intrinsic tab state ------------------------------------------------------- @@ -116,27 +88,13 @@ class TabContents : public PageNavigator, TabContentsDelegate* delegate() const { return delegate_; } void set_delegate(TabContentsDelegate* d) { delegate_ = d; } - // This can only be null if the TabContents has been created but - // SetupController has not been called. The controller should always outlive - // its TabContents. - NavigationController* controller() const { return controller_; } - void set_controller(NavigationController* c) { controller_ = c; } - - // Sets up a new NavigationController for this TabContents. - // |profile| is the user profile that should be associated with - // the new controller. - // - // TODO(brettw) this seems bogus and I couldn't find any legitimate need for - // it. I think it should be passed in the constructor. - void SetupController(Profile* profile); + // Gets the controller for this tab contents. + NavigationController& controller() { return controller_; } + const NavigationController& controller() const { return controller_; } // Returns the user profile associated with this TabContents (via the - // NavigationController). This will return NULL if there isn't yet a - // NavigationController on this TabContents. - // TODO(darin): make it so that controller_ can never be null - Profile* profile() const { - return controller_ ? controller_->profile() : NULL; - } + // NavigationController). + Profile* profile() const { return controller_.profile(); } // Returns whether this tab contents supports the provided URL.This method // matches the tab contents type with the result of TypeForURL(). |url| points @@ -273,6 +231,10 @@ class TabContents : public PageNavigator, // Called on a TabContents when it isn't a popup, but a new window. virtual void DisassociateFromPopupCount() = 0; + // Creates a new TabContents with the same state as this one. The returned + // heap-allocated pointer is owned by the caller. + virtual TabContents* Clone() = 0; + // Window management --------------------------------------------------------- #if defined(OS_WIN) @@ -386,15 +348,7 @@ class TabContents : public PageNavigator, // automation purposes. friend class AutomationProvider; - TabContents(); - - // NOTE: the TabContents destructor can run after the NavigationController - // has gone away, so any complicated unregistering that expects the profile - // or other shared objects to still be around does not belong in a - // destructor. - // For those purposes, instead see Destroy(). - // Protected so that others don't try to delete this directly. - virtual ~TabContents(); + TabContents(Profile* profile); // Changes the IsLoading state and notifies delegate as needed // |details| is used to provide details on the load that just finished @@ -432,7 +386,7 @@ class TabContents : public PageNavigator, // Data ---------------------------------------------------------------------- TabContentsDelegate* delegate_; - NavigationController* controller_; + NavigationController controller_; PropertyBag property_bag_; diff --git a/chrome/browser/tab_contents/web_contents.cc b/chrome/browser/tab_contents/web_contents.cc index bf70542..8592522 100644 --- a/chrome/browser/tab_contents/web_contents.cc +++ b/chrome/browser/tab_contents/web_contents.cc @@ -203,7 +203,8 @@ WebContents::WebContents(Profile* profile, SiteInstance* site_instance, int routing_id, base::WaitableEvent* modal_dialog_event) - : view_(WebContentsView::Create(this)), + : TabContents(profile), + view_(WebContentsView::Create(this)), ALLOW_THIS_IN_INITIALIZER_LIST(render_manager_(this, this)), printing_(*this), notify_disconnection_(false), @@ -234,14 +235,12 @@ WebContents::WebContents(Profile* profile, } // Register for notifications about URL starredness changing on any profile. - NotificationService::current()->AddObserver( - this, NotificationType::URLS_STARRED, NotificationService::AllSources()); - NotificationService::current()->AddObserver( - this, NotificationType::BOOKMARK_MODEL_LOADED, - NotificationService::AllSources()); - NotificationService::current()->AddObserver( - this, NotificationType::RENDER_WIDGET_HOST_DESTROYED, - NotificationService::AllSources()); + registrar_.Add(this, NotificationType::URLS_STARRED, + NotificationService::AllSources()); + registrar_.Add(this, NotificationType::BOOKMARK_MODEL_LOADED, + NotificationService::AllSources()); + registrar_.Add(this, NotificationType::RENDER_WIDGET_HOST_DESTROYED, + NotificationService::AllSources()); // Keep a global copy of the previous search string (if any). static string16 global_last_search = string16(); @@ -249,11 +248,60 @@ WebContents::WebContents(Profile* profile, } WebContents::~WebContents() { + is_being_destroyed_ = true; + + // We don't want any notifications while we're runnign our destructor. + registrar_.RemoveAll(); + + // Unregister the notifications of all observed prefs change. + PrefService* prefs = profile()->GetPrefs(); + if (prefs) { + for (int i = 0; i < kPrefsToObserveLength; ++i) + prefs->RemovePrefObserver(kPrefsToObserve[i], this); + } + + // Clean up subwindows like plugins and the find in page bar. + view_->OnContentsDestroy(); + + NotifyDisconnected(); + HungRendererWarning::HideForWebContents(this); + if (pending_install_.callback_functor) pending_install_.callback_functor->Cancel(); - NotificationService::current()->RemoveObserver( - this, NotificationType::RENDER_WIDGET_HOST_DESTROYED, - NotificationService::AllSources()); + + // First cleanly close all child windows. + // TODO(mpcomplete): handle case if MaybeCloseChildWindows() already asked + // some of these to close. CloseWindows is async, so it might get called + // twice before it runs. + int size = static_cast<int>(child_windows_.size()); + for (int i = size - 1; i >= 0; --i) { + ConstrainedWindow* window = child_windows_[i]; + if (window) + window->CloseConstrainedWindow(); + } + + // Notify any lasting InfobarDelegates that have not yet been removed that + // whatever infobar they were handling in this TabContents has closed, + // because the TabContents is going away entirely. + for (int i = 0; i < infobar_delegate_count(); ++i) { + InfoBarDelegate* delegate = GetInfoBarDelegateAt(i); + delegate->InfoBarClosed(); + } + infobar_delegates_.clear(); + + // Notify any observer that have a reference on this tab contents. + NotificationService::current()->Notify( + NotificationType::TAB_CONTENTS_DESTROYED, + Source<TabContents>(this), + NotificationService::NoDetails()); + + // TODO(brettw) this should be moved to the view. +#if defined(OS_WIN) + // If we still have a window handle, destroy it. GetNativeView can return + // NULL if this contents was part of a window that closed. + if (GetNativeView()) + ::DestroyWindow(GetNativeView()); +#endif } // static @@ -330,78 +378,6 @@ PluginInstaller* WebContents::GetPluginInstaller() { return plugin_installer_.get(); } -void WebContents::Destroy() { - DCHECK(!is_being_destroyed_); - is_being_destroyed_ = true; - - // Tell the notification service we no longer want notifications. - NotificationService::current()->RemoveObserver( - this, NotificationType::URLS_STARRED, NotificationService::AllSources()); - NotificationService::current()->RemoveObserver( - this, NotificationType::BOOKMARK_MODEL_LOADED, - NotificationService::AllSources()); - - // Destroy the print manager right now since a Print command may be pending. - printing_.Destroy(); - - // Unregister the notifications of all observed prefs change. - PrefService* prefs = profile()->GetPrefs(); - if (prefs) { - for (int i = 0; i < kPrefsToObserveLength; ++i) - prefs->RemovePrefObserver(kPrefsToObserve[i], this); - } - - cancelable_consumer_.CancelAllRequests(); - - // Clean up subwindows like plugins and the find in page bar. - view_->OnContentsDestroy(); - - NotifyDisconnected(); - HungRendererWarning::HideForWebContents(this); - render_manager_.Shutdown(); - - // First cleanly close all child windows. - // TODO(mpcomplete): handle case if MaybeCloseChildWindows() already asked - // some of these to close. CloseWindows is async, so it might get called - // twice before it runs. - int size = static_cast<int>(child_windows_.size()); - for (int i = size - 1; i >= 0; --i) { - ConstrainedWindow* window = child_windows_[i]; - if (window) - window->CloseConstrainedWindow(); - } - - // Notify any lasting InfobarDelegates that have not yet been removed that - // whatever infobar they were handling in this TabContents has closed, - // because the TabContents is going away entirely. - for (int i = 0; i < infobar_delegate_count(); ++i) { - InfoBarDelegate* delegate = GetInfoBarDelegateAt(i); - delegate->InfoBarClosed(); - } - infobar_delegates_.clear(); - - // Notify any observer that have a reference on this tab contents. - NotificationService::current()->Notify( - NotificationType::TAB_CONTENTS_DESTROYED, - Source<TabContents>(this), - NotificationService::NoDetails()); - -#if defined(OS_WIN) - // If we still have a window handle, destroy it. GetNativeView can return - // NULL if this contents was part of a window that closed. - if (GetNativeView()) - ::DestroyWindow(GetNativeView()); -#endif - - // Notify our NavigationController. Make sure we are deleted first, so - // that the controller is the last to die. - NavigationController* controller = controller_; - - delete this; - - controller->TabContentsWasDestroyed(); -} - const string16& WebContents::GetTitle() const { DOMUI* our_dom_ui = render_manager_.pending_dom_ui() ? render_manager_.pending_dom_ui() : render_manager_.dom_ui(); @@ -418,15 +394,15 @@ const string16& WebContents::GetTitle() const { // title. // The exception is with transient pages, for which we really want to use // their title, as they are not committed. - NavigationEntry* entry = controller_->GetTransientEntry(); + NavigationEntry* entry = controller_.GetTransientEntry(); if (entry) - return entry->GetTitleForDisplay(controller_); + return entry->GetTitleForDisplay(&controller_); - entry = controller_->GetLastCommittedEntry(); + entry = controller_.GetLastCommittedEntry(); if (entry) - return entry->GetTitleForDisplay(controller_); - else if (controller_->LoadingURLLazily()) - return controller_->GetLazyTitle(); + return entry->GetTitleForDisplay(&controller_); + else if (controller_.LoadingURLLazily()) + return controller_.GetLazyTitle(); return EmptyString16(); } @@ -443,7 +419,7 @@ bool WebContents::ShouldDisplayURL() { bool WebContents::ShouldDisplayFavIcon() { // Always display a throbber during pending loads. - if (controller()->GetLastCommittedEntry() && controller()->pending_entry()) + if (controller_.GetLastCommittedEntry() && controller_.pending_entry()) return true; DOMUI* dom_ui = GetDOMUIForCurrentState(); @@ -480,7 +456,7 @@ std::wstring WebContents::GetStatusText() const { } bool WebContents::NavigateToPendingEntry(bool reload) { - const NavigationEntry& entry = *controller()->pending_entry(); + const NavigationEntry& entry = *controller_.pending_entry(); RenderViewHost* dest_render_view_host = render_manager_.Navigate(entry); if (!dest_render_view_host) @@ -543,15 +519,19 @@ void WebContents::DisassociateFromPopupCount() { render_view_host()->DisassociateFromPopupCount(); } -void WebContents::DidBecomeSelected() { - if (controller_) - controller_->SetActive(true); +TabContents* WebContents::Clone() { + // We create a new SiteInstance so that the new tab won't share processes + // with the old one. This can be changed in the future if we need it to share + // processes for some reason. + TabContents* tc = new WebContents(profile(), + SiteInstance::CreateSiteInstance(profile()), + MSG_ROUTING_NONE, NULL); + tc->controller().CopyStateFrom(controller_); + return tc; +} -#if defined(OS_WIN) - // Invalidate all descendants. (take care to exclude invalidating ourselves!) - // TODO(brettw) this should be moved to the view! -// EnumChildWindows(GetNativeView(), InvalidateWindow, 0); -#endif +void WebContents::DidBecomeSelected() { + controller_.SetActive(true); if (render_widget_host_view()) render_widget_host_view()->DidBecomeSelected(); @@ -612,7 +592,7 @@ bool WebContents::IsBookmarkBarAlwaysVisible() { // See GetDOMUIForCurrentState() comment for more info. This case is very // similar, but for non-first loads, we want to use the committed entry. This // is so the bookmarks bar disappears at the same time the page does. - if (controller()->GetLastCommittedEntry()) { + if (controller_.GetLastCommittedEntry()) { // Not the first load, always use the committed DOM UI. if (render_manager_.dom_ui()) return render_manager_.dom_ui()->force_bookmark_bar_visible(); @@ -677,7 +657,7 @@ void WebContents::GetContainerBounds(gfx::Rect *out) const { } void WebContents::CreateShortcut() { - NavigationEntry* entry = controller()->GetLastCommittedEntry(); + NavigationEntry* entry = controller_.GetLastCommittedEntry(); if (!entry) return; @@ -795,7 +775,7 @@ bool WebContents::PrintNow() { } bool WebContents::IsActiveEntry(int32 page_id) { - NavigationEntry* active_entry = controller()->GetActiveEntry(); + NavigationEntry* active_entry = controller_.GetActiveEntry(); return (active_entry != NULL && active_entry->site_instance() == GetSiteInstance() && active_entry->page_id() == page_id); @@ -835,7 +815,7 @@ void WebContents::SetIsLoading(bool is_loading, if (details) det = Details<LoadNotificationDetails>(details); NotificationService::current()->Notify(type, - Source<NavigationController>(this->controller()), + Source<NavigationController>(&controller_), det); } @@ -852,9 +832,7 @@ Profile* WebContents::GetProfile() const { } void WebContents::RenderViewCreated(RenderViewHost* render_view_host) { - if (!controller()) - return; - NavigationEntry* entry = controller()->GetActiveEntry(); + NavigationEntry* entry = controller_.GetActiveEntry(); if (!entry) return; @@ -906,10 +884,6 @@ void WebContents::DidNavigate(RenderViewHost* rvh, if (PageTransition::IsMainFrame(params.transition)) render_manager_.DidNavigateMainFrame(rvh); - // We can't do anything about navigations when we're inactive. - if (!controller()) - return; - // Update the site of the SiteInstance if it doesn't have one yet. if (!GetSiteInstance()->has_site()) GetSiteInstance()->SetSite(params.url); @@ -926,7 +900,7 @@ void WebContents::DidNavigate(RenderViewHost* rvh, contents_mime_type_ = params.contents_mime_type; NavigationController::LoadCommittedDetails details; - if (!controller()->RendererDidNavigate(params, &details)) + if (!controller_.RendererDidNavigate(params, &details)) return; // No navigation happened. // DO NOT ADD MORE STUFF TO THIS FUNCTION! Your component should either listen @@ -944,8 +918,6 @@ void WebContents::UpdateState(RenderViewHost* rvh, int32 page_id, const std::string& state) { DCHECK(rvh == render_view_host()); - if (!controller()) - return; // We must be prepared to handle state updates for any page, these occur // when the user is scrolling and entering form data, as well as when we're @@ -953,49 +925,43 @@ 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. - int entry_index = controller()->GetEntryIndexWithPageID( + int entry_index = controller_.GetEntryIndexWithPageID( GetSiteInstance(), page_id); if (entry_index < 0) return; - NavigationEntry* entry = controller()->GetEntryAtIndex(entry_index); + NavigationEntry* entry = controller_.GetEntryAtIndex(entry_index); if (state == entry->content_state()) return; // Nothing to update. entry->set_content_state(state); - controller()->NotifyEntryChanged(entry, entry_index); + controller_.NotifyEntryChanged(entry, entry_index); } void WebContents::UpdateTitle(RenderViewHost* rvh, int32 page_id, const std::wstring& title) { - if (!controller()) - return; - // If we have a title, that's a pretty good indication that we've started // getting useful data. SetNotWaitingForResponse(); DCHECK(rvh == render_view_host()); - NavigationEntry* entry = controller()->GetEntryWithPageID(GetSiteInstance(), + NavigationEntry* entry = controller_.GetEntryWithPageID(GetSiteInstance(), page_id); if (!entry || !UpdateTitleForEntry(entry, title)) return; // Broadcast notifications when the UI should be updated. - if (entry == controller()->GetEntryAtOffset(0)) + if (entry == controller_.GetEntryAtOffset(0)) NotifyNavigationStateChanged(INVALIDATE_TITLE); } void WebContents::UpdateFeedList( RenderViewHost* rvh, const ViewHostMsg_UpdateFeedList_Params& params) { - if (!controller()) - return; - // We might have an old RenderViewHost sending messages, and we should ignore // those messages. if (rvh != render_view_host()) return; - NavigationEntry* entry = controller()->GetEntryWithPageID(GetSiteInstance(), + NavigationEntry* entry = controller_.GetEntryWithPageID(GetSiteInstance(), params.page_id); if (!entry) return; @@ -1003,7 +969,7 @@ void WebContents::UpdateFeedList( entry->set_feedlist(params.feedlist); // Broadcast notifications when the UI should be updated. - if (entry == controller()->GetEntryAtOffset(0)) + if (entry == controller_.GetEntryAtOffset(0)) NotifyNavigationStateChanged(INVALIDATE_FEEDLIST); } @@ -1045,24 +1011,23 @@ void WebContents::DidStartLoading(RenderViewHost* rvh, int32 page_id) { void WebContents::DidStopLoading(RenderViewHost* rvh, int32 page_id) { scoped_ptr<LoadNotificationDetails> details; - if (controller()) { - NavigationEntry* entry = controller()->GetActiveEntry(); - // An entry may not exist for a stop when loading an initial blank page or - // if an iframe injected by script into a blank page finishes loading. - if (entry) { - scoped_ptr<base::ProcessMetrics> metrics( - base::ProcessMetrics::CreateProcessMetrics( - process()->process().handle())); - - TimeDelta elapsed = TimeTicks::Now() - current_load_start_; - - details.reset(new LoadNotificationDetails( - entry->display_url(), - entry->transition_type(), - elapsed, - controller(), - controller()->GetCurrentEntryIndex())); - } + + NavigationEntry* entry = controller_.GetActiveEntry(); + // An entry may not exist for a stop when loading an initial blank page or + // if an iframe injected by script into a blank page finishes loading. + if (entry) { + scoped_ptr<base::ProcessMetrics> metrics( + base::ProcessMetrics::CreateProcessMetrics( + process()->process().handle())); + + TimeDelta elapsed = TimeTicks::Now() - current_load_start_; + + details.reset(new LoadNotificationDetails( + entry->display_url(), + entry->transition_type(), + elapsed, + &controller_, + controller_.GetCurrentEntryIndex())); } // Tell PasswordManager we've finished a page load, which serves as a @@ -1077,11 +1042,11 @@ void WebContents::DidStartProvisionalLoadForFrame( bool is_main_frame, const GURL& url) { ProvisionalLoadDetails details(is_main_frame, - controller()->IsURLInPageNavigation(url), + controller_.IsURLInPageNavigation(url), url, std::string(), false); NotificationService::current()->Notify( NotificationType::FRAME_PROVISIONAL_LOAD_START, - Source<NavigationController>(controller()), + Source<NavigationController>(&controller_), Details<ProvisionalLoadDetails>(&details)); } @@ -1090,9 +1055,9 @@ void WebContents::DidRedirectProvisionalLoad(int32 page_id, const GURL& target_url) { NavigationEntry* entry; if (page_id == -1) - entry = controller()->pending_entry(); + entry = controller_.pending_entry(); else - entry = controller()->GetEntryWithPageID(GetSiteInstance(), page_id); + entry = controller_.GetEntryWithPageID(GetSiteInstance(), page_id); if (!entry || entry->url() != source_url) return; entry->set_url(target_url); @@ -1103,9 +1068,6 @@ void WebContents::DidLoadResourceFromMemoryCache( const std::string& frame_origin, const std::string& main_frame_origin, const std::string& security_info) { - if (!controller()) - return; - // Send out a notification that we loaded a resource from our memory cache. int cert_id = 0, cert_status = 0, security_bits = 0; SSLManager::DeserializeSecurityInfo(security_info, @@ -1116,7 +1078,7 @@ void WebContents::DidLoadResourceFromMemoryCache( NotificationService::current()->Notify( NotificationType::LOAD_FROM_MEMORY_CACHE, - Source<NavigationController>(controller()), + Source<NavigationController>(&controller_), Details<LoadFromMemoryCacheDetails>(&details)); } @@ -1126,9 +1088,6 @@ void WebContents::DidFailProvisionalLoadWithError( int error_code, const GURL& url, bool showing_repost_interstitial) { - if (!controller()) - return; - if (net::ERR_ABORTED == error_code) { // EVIL HACK ALERT! Ignore failed loads when we're showing interstitials. // This means that the interstitial won't be torn down properly, which is @@ -1157,22 +1116,22 @@ void WebContents::DidFailProvisionalLoadWithError( // decided to download the file instead of load it). Only discard the // pending entry if the URLs match, otherwise the user initiated a navigate // before the page loaded so that the discard would discard the wrong entry. - NavigationEntry* pending_entry = controller()->pending_entry(); + NavigationEntry* pending_entry = controller_.pending_entry(); if (pending_entry && pending_entry->url() == url) - controller()->DiscardNonCommittedEntries(); + controller_.DiscardNonCommittedEntries(); render_manager_.RendererAbortedProvisionalLoad(render_view_host); } // Send out a notification that we failed a provisional load with an error. ProvisionalLoadDetails details(is_main_frame, - controller()->IsURLInPageNavigation(url), + controller_.IsURLInPageNavigation(url), url, std::string(), false); details.set_error_code(error_code); NotificationService::current()->Notify( NotificationType::FAIL_PROVISIONAL_LOAD_WITH_ERROR, - Source<NavigationController>(controller()), + Source<NavigationController>(&controller_), Details<ProvisionalLoadDetails>(&details)); } @@ -1244,21 +1203,14 @@ void WebContents::ProcessExternalHostMessage(const std::string& message, } void WebContents::GoToEntryAtOffset(int offset) { - if (!controller()) - return; - controller()->GoToOffset(offset); + controller_.GoToOffset(offset); } void WebContents::GetHistoryListCount(int* back_list_count, int* forward_list_count) { - *back_list_count = 0; - *forward_list_count = 0; - - if (controller()) { - int current_index = controller()->last_committed_entry_index(); - *back_list_count = current_index; - *forward_list_count = controller()->entry_count() - current_index - 1; - } + int current_index = controller_.last_committed_entry_index(); + *back_list_count = current_index; + *forward_list_count = controller_.entry_count() - current_index - 1; } void WebContents::RunFileChooser(bool multiple_files, @@ -1354,7 +1306,7 @@ void WebContents::PageHasOSDD(RenderViewHost* render_view_host, bool autodetected) { // Make sure page_id is the current page, and the TemplateURLModel is loaded. DCHECK(url.is_valid()); - if (!controller() || !IsActiveEntry(page_id)) + if (!IsActiveEntry(page_id)) return; TemplateURLModel* url_model = profile()->GetTemplateURLModel(); if (!url_model) @@ -1369,18 +1321,18 @@ void WebContents::PageHasOSDD(RenderViewHost* render_view_host, if (profile()->IsOffTheRecord()) return; - const NavigationEntry* entry = controller()->GetLastCommittedEntry(); + const NavigationEntry* entry = controller_.GetLastCommittedEntry(); DCHECK(entry); const NavigationEntry* base_entry = entry; if (IsFormSubmit(base_entry)) { // If the current page is a form submit, find the last page that was not // a form submit and use its url to generate the keyword from. - int index = controller()->last_committed_entry_index() - 1; - while (index >= 0 && IsFormSubmit(controller()->GetEntryAtIndex(index))) + int index = controller_.last_committed_entry_index() - 1; + while (index >= 0 && IsFormSubmit(controller_.GetEntryAtIndex(index))) index--; if (index >= 0) - base_entry = controller()->GetEntryAtIndex(index); + base_entry = controller_.GetEntryAtIndex(index); else base_entry = NULL; } @@ -1633,9 +1585,7 @@ DOMUI* WebContents::CreateDOMUIForRenderManager(const GURL& url) { NavigationEntry* WebContents::GetLastCommittedNavigationEntryForRenderManager() { - if (!controller()) - return NULL; - return controller()->GetLastCommittedEntry(); + return controller_.GetLastCommittedEntry(); } bool WebContents::CreateRenderViewForRenderManager( @@ -1694,7 +1644,7 @@ void WebContents::Observe(NotificationType type, break; case NotificationType::NAV_ENTRY_COMMITTED: { - DCHECK(controller() == Source<NavigationController>(source).ptr()); + DCHECK(&controller_ == Source<NavigationController>(source).ptr()); NavigationController::LoadCommittedDetails& committed_details = *(Details<NavigationController::LoadCommittedDetails>(details).ptr()); @@ -1828,7 +1778,7 @@ void WebContents::UpdateWebPreferences() { void WebContents::OnGearsCreateShortcutDone( const GearsShortcutData2& shortcut_data, bool success) { - NavigationEntry* current_entry = controller()->GetLastCommittedEntry(); + NavigationEntry* current_entry = controller_.GetLastCommittedEntry(); bool same_page = current_entry && pending_install_.page_id == current_entry->page_id(); @@ -1852,7 +1802,7 @@ void WebContents::UpdateMaxPageIDIfNecessary(SiteInstance* site_instance, // Note that it is ok for conflicting page IDs to exist in another tab // (i.e., NavigationController), but if any page ID is larger than the max, // the back/forward list will get confused. - int max_restored_page_id = controller()->max_restored_page_id(); + int max_restored_page_id = controller_.max_restored_page_id(); if (max_restored_page_id > 0) { int curr_max_page_id = site_instance->max_page_id(); if (max_restored_page_id > curr_max_page_id) { @@ -1967,14 +1917,13 @@ void WebContents::NotifyDisconnected() { void WebContents::GenerateKeywordIfNecessary( const ViewHostMsg_FrameNavigate_Params& params) { - DCHECK(controller()); if (!params.searchable_form_url.is_valid()) return; if (profile()->IsOffTheRecord()) return; - int last_index = controller()->last_committed_entry_index(); + int last_index = controller_.last_committed_entry_index(); // When there was no previous page, the last index will be 0. This is // normally due to a form submit that opened in a new tab. // TODO(brettw) bug 916126: we should support keywords when form submits @@ -1982,7 +1931,7 @@ void WebContents::GenerateKeywordIfNecessary( if (last_index <= 0) return; const NavigationEntry* previous_entry = - controller()->GetEntryAtIndex(last_index - 1); + controller_.GetEntryAtIndex(last_index - 1); if (IsFormSubmit(previous_entry)) { // Only generate a keyword if the previous page wasn't itself a form // submit. @@ -2023,9 +1972,9 @@ void WebContents::GenerateKeywordIfNecessary( new_url->set_short_name(keyword); new_url->SetURL(url, 0, 0); new_url->add_input_encoding(params.searchable_form_encoding); - DCHECK(controller()->GetLastCommittedEntry()); + DCHECK(controller_.GetLastCommittedEntry()); const GURL& favicon_url = - controller()->GetLastCommittedEntry()->favicon().url(); + controller_.GetLastCommittedEntry()->favicon().url(); if (favicon_url.is_valid()) { new_url->SetFavIconURL(favicon_url); } else { @@ -2073,8 +2022,8 @@ DOMUI* WebContents::GetDOMUIForCurrentState() { // // - Normal state with no load: committed nav entry + no pending nav entry: // -> Use committed DOM UI. - if (controller()->pending_entry() && - (controller()->GetLastCommittedEntry() || + if (controller_.pending_entry() && + (controller_.GetLastCommittedEntry() || render_manager_.pending_dom_ui())) return render_manager_.pending_dom_ui(); return render_manager_.dom_ui(); diff --git a/chrome/browser/tab_contents/web_contents.h b/chrome/browser/tab_contents/web_contents.h index b89efd3..9128868 100644 --- a/chrome/browser/tab_contents/web_contents.h +++ b/chrome/browser/tab_contents/web_contents.h @@ -73,6 +73,8 @@ class WebContents : public TabContents, int routing_id, base::WaitableEvent* modal_dialog_event); + virtual ~WebContents(); + static void RegisterUserPrefs(PrefService* prefs); // Getters ------------------------------------------------------------------- @@ -126,7 +128,6 @@ class WebContents : public TabContents, // TabContents (public overrides) -------------------------------------------- - virtual void Destroy(); virtual WebContents* AsWebContents() { return this; } const string16& GetTitle() const; virtual SiteInstance* GetSiteInstance() const; @@ -139,6 +140,7 @@ class WebContents : public TabContents, virtual void Copy(); virtual void Paste(); virtual void DisassociateFromPopupCount(); + virtual TabContents* Clone(); virtual void DidBecomeSelected(); virtual void WasHidden(); virtual void ShowContents(); @@ -292,9 +294,6 @@ class WebContents : public TabContents, } protected: - // Should be deleted via CloseContents. - virtual ~WebContents(); - RenderWidgetHostView* render_widget_host_view() const { return render_manager_.current_view(); } @@ -453,7 +452,7 @@ class WebContents : public TabContents, virtual void NotifySwappedFromRenderManager() { NotifySwapped(); } - virtual NavigationController* GetControllerForRenderManager() { + virtual NavigationController& GetControllerForRenderManager() { return controller(); } virtual DOMUI* CreateDOMUIForRenderManager(const GURL& url); diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc index c5e58e1..207a724 100644 --- a/chrome/browser/tab_contents/web_contents_unittest.cc +++ b/chrome/browser/tab_contents/web_contents_unittest.cc @@ -206,7 +206,7 @@ TEST_F(WebContentsTest, UpdateTitle) { InitNavigateParams(¶ms, 0, GURL("about:blank")); NavigationController::LoadCommittedDetails details; - controller()->RendererDidNavigate(params, &details); + controller().RendererDidNavigate(params, &details); contents()->UpdateTitle(rvh(), 0, L" Lots O' Whitespace\n"); EXPECT_EQ(std::wstring(L"Lots O' Whitespace"), @@ -221,12 +221,12 @@ TEST_F(WebContentsTest, SimpleNavigation) { // Navigate to URL const GURL url("http://www.google.com"); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); + controller().LoadURL(url, GURL(), PageTransition::TYPED); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(instance1, orig_rvh->site_instance()); // Controller's pending entry will have a NULL site instance until we assign // it in DidNavigate. - EXPECT_TRUE(controller()->GetActiveEntry()->site_instance() == NULL); + EXPECT_TRUE(controller().GetActiveEntry()->site_instance() == NULL); // DidNavigate from the page ViewHostMsg_FrameNavigate_Params params; @@ -237,7 +237,7 @@ TEST_F(WebContentsTest, SimpleNavigation) { EXPECT_EQ(instance1, orig_rvh->site_instance()); // Controller's entry should now have the SiteInstance, or else we won't be // able to find it later. - EXPECT_EQ(instance1, controller()->GetActiveEntry()->site_instance()); + EXPECT_EQ(instance1, controller().GetActiveEntry()->site_instance()); } // Test that navigating across a site boundary creates a new RenderViewHost @@ -251,7 +251,7 @@ TEST_F(WebContentsTest, CrossSiteBoundaries) { // Navigate to URL. First URL should use first RenderViewHost. const GURL url("http://www.google.com"); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); + controller().LoadURL(url, GURL(), PageTransition::TYPED); ViewHostMsg_FrameNavigate_Params params1; InitNavigateParams(¶ms1, 1, url); contents()->TestDidNavigate(orig_rvh, params1); @@ -261,7 +261,7 @@ TEST_F(WebContentsTest, CrossSiteBoundaries) { // Navigate to new site const GURL url2("http://www.yahoo.com"); - controller()->LoadURL(url2, GURL(), PageTransition::TYPED); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); EXPECT_TRUE(contents()->cross_navigation_pending()); TestRenderViewHost* pending_rvh = contents()->pending_rvh(); int pending_rvh_delete_count = 0; @@ -281,7 +281,7 @@ TEST_F(WebContentsTest, CrossSiteBoundaries) { // Going back should switch SiteInstances again. The first SiteInstance is // stored in the NavigationEntry, so it should be the same as at the start. - controller()->GoBack(); + controller().GoBack(); TestRenderViewHost* goback_rvh = contents()->pending_rvh(); EXPECT_TRUE(contents()->cross_navigation_pending()); @@ -304,7 +304,7 @@ TEST_F(WebContentsTest, CrossSiteBoundariesAfterCrash) { // Navigate to URL. First URL should use first RenderViewHost. const GURL url("http://www.google.com"); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); + controller().LoadURL(url, GURL(), PageTransition::TYPED); ViewHostMsg_FrameNavigate_Params params1; InitNavigateParams(¶ms1, 1, url); contents()->TestDidNavigate(orig_rvh, params1); @@ -317,7 +317,7 @@ TEST_F(WebContentsTest, CrossSiteBoundariesAfterCrash) { // Navigate to new site. We should not go into PENDING. const GURL url2("http://www.yahoo.com"); - controller()->LoadURL(url2, GURL(), PageTransition::TYPED); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); TestRenderViewHost* new_rvh = rvh(); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_TRUE(contents()->pending_rvh() == NULL); @@ -345,24 +345,23 @@ TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) { // Navigate to URL. First URL should use first RenderViewHost. const GURL url("http://www.google.com"); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); + controller().LoadURL(url, GURL(), PageTransition::TYPED); ViewHostMsg_FrameNavigate_Params params1; InitNavigateParams(¶ms1, 1, url); contents()->TestDidNavigate(orig_rvh, params1); // Open a new tab with the same SiteInstance, navigated to the same site. - TestWebContents* contents2 = new TestWebContents(profile(), instance1); + TestWebContents contents2(profile(), instance1); params1.page_id = 2; // Need this since the site instance is the same (which // is the scope of page IDs) and we want to consider // this a new page. - contents2->transition_cross_site = true; - contents2->SetupController(profile()); - contents2->controller()->LoadURL(url, GURL(), PageTransition::TYPED); - contents2->TestDidNavigate(contents2->render_view_host(), params1); + contents2.transition_cross_site = true; + contents2.controller().LoadURL(url, GURL(), PageTransition::TYPED); + contents2.TestDidNavigate(contents2.render_view_host(), params1); // Navigate first tab to a new site const GURL url2a("http://www.yahoo.com"); - controller()->LoadURL(url2a, GURL(), PageTransition::TYPED); + controller().LoadURL(url2a, GURL(), PageTransition::TYPED); TestRenderViewHost* pending_rvh_a = contents()->pending_rvh(); ViewHostMsg_FrameNavigate_Params params2a; InitNavigateParams(¶ms2a, 1, url2a); @@ -372,10 +371,10 @@ TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) { // Navigate second tab to the same site as the first tab const GURL url2b("http://mail.yahoo.com"); - contents2->controller()->LoadURL(url2b, GURL(), PageTransition::TYPED); - TestRenderViewHost* pending_rvh_b = contents2->pending_rvh(); + contents2.controller().LoadURL(url2b, GURL(), PageTransition::TYPED); + TestRenderViewHost* pending_rvh_b = contents2.pending_rvh(); EXPECT_TRUE(pending_rvh_b != NULL); - EXPECT_TRUE(contents2->cross_navigation_pending()); + EXPECT_TRUE(contents2.cross_navigation_pending()); // NOTE(creis): We used to be in danger of showing a sad tab page here if the // second tab hadn't navigated somewhere first (bug 1145430). That case is @@ -383,14 +382,12 @@ TEST_F(WebContentsTest, NavigateTwoTabsCrossSite) { ViewHostMsg_FrameNavigate_Params params2b; InitNavigateParams(¶ms2b, 2, url2b); - contents2->TestDidNavigate(pending_rvh_b, params2b); - SiteInstance* instance2b = contents2->GetSiteInstance(); + contents2.TestDidNavigate(pending_rvh_b, params2b); + SiteInstance* instance2b = contents2.GetSiteInstance(); EXPECT_NE(instance1, instance2b); // Both tabs should now be in the same SiteInstance. EXPECT_EQ(instance2a, instance2b); - - contents2->CloseContents(); } // Tests that WebContents uses the current URL, not the SiteInstance's site, to @@ -402,28 +399,27 @@ TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) { // Navigate to URL. const GURL url("http://www.google.com"); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); + controller().LoadURL(url, GURL(), PageTransition::TYPED); ViewHostMsg_FrameNavigate_Params params1; InitNavigateParams(¶ms1, 1, url); contents()->TestDidNavigate(orig_rvh, params1); // Open a related tab to a second site. - TestWebContents* contents2 = new TestWebContents(profile(), instance1); - contents2->transition_cross_site = true; - contents2->SetupController(profile()); + TestWebContents contents2(profile(), instance1); + contents2.transition_cross_site = true; const GURL url2("http://www.yahoo.com"); - contents2->controller()->LoadURL(url2, GURL(), PageTransition::TYPED); + contents2.controller().LoadURL(url2, GURL(), PageTransition::TYPED); // The first RVH in contents2 isn't live yet, so we shortcut the cross site // pending. TestRenderViewHost* rvh2 = static_cast<TestRenderViewHost*>( - contents2->render_view_host()); - EXPECT_FALSE(contents2->cross_navigation_pending()); + contents2.render_view_host()); + EXPECT_FALSE(contents2.cross_navigation_pending()); ViewHostMsg_FrameNavigate_Params params2; InitNavigateParams(¶ms2, 2, url2); - contents2->TestDidNavigate(rvh2, params2); - SiteInstance* instance2 = contents2->GetSiteInstance(); + contents2.TestDidNavigate(rvh2, params2); + SiteInstance* instance2 = contents2.GetSiteInstance(); EXPECT_NE(instance1, instance2); - EXPECT_FALSE(contents2->cross_navigation_pending()); + EXPECT_FALSE(contents2.cross_navigation_pending()); // Simulate a link click in first tab to second site. Doesn't switch // SiteInstances, because we don't intercept WebKit navigations. @@ -437,15 +433,13 @@ TEST_F(WebContentsTest, CrossSiteComparesAgainstCurrentPage) { // Navigate to the new site. Doesn't switch SiteInstancees, because we // compare against the current URL, not the SiteInstance's site. const GURL url3("http://mail.yahoo.com"); - controller()->LoadURL(url3, GURL(), PageTransition::TYPED); + controller().LoadURL(url3, GURL(), PageTransition::TYPED); EXPECT_FALSE(contents()->cross_navigation_pending()); ViewHostMsg_FrameNavigate_Params params4; InitNavigateParams(¶ms4, 3, url3); contents()->TestDidNavigate(orig_rvh, params4); SiteInstance* instance4 = contents()->GetSiteInstance(); EXPECT_EQ(instance1, instance4); - - contents2->CloseContents(); } // Test that the onbeforeunload and onunload handlers run when navigating @@ -457,7 +451,7 @@ TEST_F(WebContentsTest, CrossSiteUnloadHandlers) { // Navigate to URL. First URL should use first RenderViewHost. const GURL url("http://www.google.com"); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); + controller().LoadURL(url, GURL(), PageTransition::TYPED); ViewHostMsg_FrameNavigate_Params params1; InitNavigateParams(¶ms1, 1, url); contents()->TestDidNavigate(orig_rvh, params1); @@ -466,13 +460,13 @@ TEST_F(WebContentsTest, CrossSiteUnloadHandlers) { // Navigate to new site, but simulate an onbeforeunload denial. const GURL url2("http://www.yahoo.com"); - controller()->LoadURL(url2, GURL(), PageTransition::TYPED); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); orig_rvh->TestOnMessageReceived(ViewHostMsg_ShouldClose_ACK(0, false)); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(orig_rvh, contents()->render_view_host()); // Navigate again, but simulate an onbeforeunload approval. - controller()->LoadURL(url2, GURL(), PageTransition::TYPED); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); orig_rvh->TestOnMessageReceived(ViewHostMsg_ShouldClose_ACK(0, true)); EXPECT_TRUE(contents()->cross_navigation_pending()); TestRenderViewHost* pending_rvh = static_cast<TestRenderViewHost*>( @@ -500,34 +494,34 @@ TEST_F(WebContentsTest, NavigationEntryContentState) { // Navigate to URL. There should be no committed entry yet. const GURL url("http://www.google.com"); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); - NavigationEntry* entry = controller()->GetLastCommittedEntry(); + controller().LoadURL(url, GURL(), PageTransition::TYPED); + NavigationEntry* entry = controller().GetLastCommittedEntry(); EXPECT_TRUE(entry == NULL); // Committed entry should have content state after DidNavigate. ViewHostMsg_FrameNavigate_Params params1; InitNavigateParams(¶ms1, 1, url); contents()->TestDidNavigate(orig_rvh, params1); - entry = controller()->GetLastCommittedEntry(); + entry = controller().GetLastCommittedEntry(); EXPECT_FALSE(entry->content_state().empty()); // Navigate to same site. const GURL url2("http://images.google.com"); - controller()->LoadURL(url2, GURL(), PageTransition::TYPED); - entry = controller()->GetLastCommittedEntry(); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); + entry = controller().GetLastCommittedEntry(); EXPECT_FALSE(entry->content_state().empty()); // Committed entry should have content state after DidNavigate. ViewHostMsg_FrameNavigate_Params params2; InitNavigateParams(¶ms2, 2, url2); contents()->TestDidNavigate(orig_rvh, params2); - entry = controller()->GetLastCommittedEntry(); + entry = controller().GetLastCommittedEntry(); EXPECT_FALSE(entry->content_state().empty()); // Now go back. Committed entry should still have content state. - controller()->GoBack(); + controller().GoBack(); contents()->TestDidNavigate(orig_rvh, params1); - entry = controller()->GetLastCommittedEntry(); + entry = controller().GetLastCommittedEntry(); EXPECT_FALSE(entry->content_state().empty()); } @@ -545,7 +539,7 @@ TEST_F(WebContentsTest, NavigationEntryContentStateNewWindow) { contents()->TestDidNavigate(orig_rvh, params1); // Should have a content state here. - NavigationEntry* entry = controller()->GetLastCommittedEntry(); + NavigationEntry* entry = controller().GetLastCommittedEntry(); EXPECT_FALSE(entry->content_state().empty()); } @@ -577,10 +571,10 @@ TEST_F(WebContentsTest, // Navigate to a page. GURL url1("http://www.google.com"); rvh()->SendNavigate(1, url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); // Initiate a browser navigation that will trigger the interstitial - controller()->LoadURL(GURL("http://www.evil.com"), GURL(), + controller().LoadURL(GURL("http://www.evil.com"), GURL(), PageTransition::TYPED); // Show an interstitial. @@ -601,7 +595,7 @@ TEST_F(WebContentsTest, EXPECT_TRUE(interstitial->is_showing()); EXPECT_TRUE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == interstitial); - NavigationEntry* entry = controller()->GetActiveEntry(); + NavigationEntry* entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == url2); @@ -611,10 +605,10 @@ TEST_F(WebContentsTest, EXPECT_EQ(TestInterstitialPage::CANCELED, state); EXPECT_FALSE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == NULL); - entry = controller()->GetActiveEntry(); + entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); } // Test navigating to a page (with the navigation initiated from the renderer, @@ -625,7 +619,7 @@ TEST_F(WebContentsTest, // Navigate to a page. GURL url1("http://www.google.com"); rvh()->SendNavigate(1, url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); // Show an interstitial (no pending entry, the interstitial would have been // triggered by clicking on a link). @@ -646,7 +640,7 @@ TEST_F(WebContentsTest, EXPECT_TRUE(interstitial->is_showing()); EXPECT_TRUE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == interstitial); - NavigationEntry* entry = controller()->GetActiveEntry(); + NavigationEntry* entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == url2); @@ -656,10 +650,10 @@ TEST_F(WebContentsTest, EXPECT_EQ(TestInterstitialPage::CANCELED, state); EXPECT_FALSE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == NULL); - entry = controller()->GetActiveEntry(); + entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); } // Test navigating to a page that shows an interstitial without creating a new @@ -669,7 +663,7 @@ TEST_F(WebContentsTest, ShowInterstitialNoNewNavigationDontProceed) { // Navigate to a page. GURL url1("http://www.google.com"); rvh()->SendNavigate(1, url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); // Show an interstitial. TestInterstitialPage::InterstitialState state = @@ -689,7 +683,7 @@ TEST_F(WebContentsTest, ShowInterstitialNoNewNavigationDontProceed) { EXPECT_TRUE(interstitial->is_showing()); EXPECT_TRUE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == interstitial); - NavigationEntry* entry = controller()->GetActiveEntry(); + NavigationEntry* entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); // The URL specified to the interstitial should have been ignored. EXPECT_TRUE(entry->url() == url1); @@ -700,10 +694,10 @@ TEST_F(WebContentsTest, ShowInterstitialNoNewNavigationDontProceed) { EXPECT_EQ(TestInterstitialPage::CANCELED, state); EXPECT_FALSE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == NULL); - entry = controller()->GetActiveEntry(); + entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); } // Test navigating to a page (with the navigation initiated from the browser, @@ -714,10 +708,10 @@ TEST_F(WebContentsTest, // Navigate to a page. GURL url1("http://www.google.com"); rvh()->SendNavigate(1, url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); // Initiate a browser navigation that will trigger the interstitial - controller()->LoadURL(GURL("http://www.evil.com"), GURL(), + controller().LoadURL(GURL("http://www.evil.com"), GURL(), PageTransition::TYPED); // Show an interstitial. @@ -738,7 +732,7 @@ TEST_F(WebContentsTest, EXPECT_TRUE(interstitial->is_showing()); EXPECT_TRUE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == interstitial); - NavigationEntry* entry = controller()->GetActiveEntry(); + NavigationEntry* entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == url2); @@ -758,11 +752,11 @@ TEST_F(WebContentsTest, EXPECT_TRUE(deleted); EXPECT_FALSE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == NULL); - entry = controller()->GetActiveEntry(); + entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == url3); - EXPECT_EQ(2, controller()->entry_count()); + EXPECT_EQ(2, controller().entry_count()); } // Test navigating to a page (with the navigation initiated from the renderer, @@ -773,7 +767,7 @@ TEST_F(WebContentsTest, // Navigate to a page. GURL url1("http://www.google.com"); rvh()->SendNavigate(1, url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); // Show an interstitial. TestInterstitialPage::InterstitialState state = @@ -793,7 +787,7 @@ TEST_F(WebContentsTest, EXPECT_TRUE(interstitial->is_showing()); EXPECT_TRUE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == interstitial); - NavigationEntry* entry = controller()->GetActiveEntry(); + NavigationEntry* entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == url2); @@ -813,11 +807,11 @@ TEST_F(WebContentsTest, EXPECT_TRUE(deleted); EXPECT_FALSE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == NULL); - entry = controller()->GetActiveEntry(); + entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == url3); - EXPECT_EQ(2, controller()->entry_count()); + EXPECT_EQ(2, controller().entry_count()); } // Test navigating to a page that shows an interstitial without creating a new @@ -827,7 +821,7 @@ TEST_F(WebContentsTest, ShowInterstitialNoNewNavigationProceed) { // Navigate to a page so we have a navigation entry in the controller. GURL url1("http://www.google.com"); rvh()->SendNavigate(1, url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); // Show an interstitial. TestInterstitialPage::InterstitialState state = @@ -847,7 +841,7 @@ TEST_F(WebContentsTest, ShowInterstitialNoNewNavigationProceed) { EXPECT_TRUE(interstitial->is_showing()); EXPECT_TRUE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == interstitial); - NavigationEntry* entry = controller()->GetActiveEntry(); + NavigationEntry* entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); // The URL specified to the interstitial should have been ignored. EXPECT_TRUE(entry->url() == url1); @@ -860,11 +854,11 @@ TEST_F(WebContentsTest, ShowInterstitialNoNewNavigationProceed) { EXPECT_EQ(TestInterstitialPage::OKED, state); EXPECT_FALSE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == NULL); - entry = controller()->GetActiveEntry(); + entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); } // Test navigating to a page that shows an interstitial, then navigating away. @@ -902,7 +896,7 @@ TEST_F(WebContentsTest, ShowInterstitialThenCloseTab) { interstitial->TestDidNavigate(1, url); // Now close the tab. - contents()->CloseContents(); + delete contents(); ContentsCleanedUp(); EXPECT_TRUE(deleted); EXPECT_EQ(TestInterstitialPage::CANCELED, state); @@ -914,7 +908,7 @@ TEST_F(WebContentsTest, ShowInterstitialProceedMultipleCommands) { // Navigate to a page so we have a navigation entry in the controller. GURL url1("http://www.google.com"); rvh()->SendNavigate(1, url1); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); // Show an interstitial. TestInterstitialPage::InterstitialState state = @@ -948,7 +942,7 @@ TEST_F(WebContentsTest, ShowInterstitialOnInterstitial) { // Navigate to a page so we have a navigation entry in the controller. GURL start_url("http://www.google.com"); rvh()->SendNavigate(1, start_url); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); // Show an interstitial. TestInterstitialPage::InterstitialState state1 = @@ -986,10 +980,10 @@ TEST_F(WebContentsTest, ShowInterstitialOnInterstitial) { EXPECT_TRUE(deleted2); EXPECT_FALSE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == NULL); - NavigationEntry* entry = controller()->GetActiveEntry(); + NavigationEntry* entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == landing_url); - EXPECT_EQ(2, controller()->entry_count()); + EXPECT_EQ(2, controller().entry_count()); } // Test showing an interstitial, proceeding and then navigating to another @@ -998,7 +992,7 @@ TEST_F(WebContentsTest, ShowInterstitialProceedShowInterstitial) { // Navigate to a page so we have a navigation entry in the controller. GURL start_url("http://www.google.com"); rvh()->SendNavigate(1, start_url); - EXPECT_EQ(1, controller()->entry_count()); + EXPECT_EQ(1, controller().entry_count()); // Show an interstitial. TestInterstitialPage::InterstitialState state1 = @@ -1041,10 +1035,10 @@ TEST_F(WebContentsTest, ShowInterstitialProceedShowInterstitial) { EXPECT_TRUE(deleted2); EXPECT_FALSE(contents()->showing_interstitial_page()); EXPECT_TRUE(contents()->interstitial_page() == NULL); - NavigationEntry* entry = controller()->GetActiveEntry(); + NavigationEntry* entry = controller().GetActiveEntry(); ASSERT_TRUE(entry != NULL); EXPECT_TRUE(entry->url() == landing_url); - EXPECT_EQ(2, controller()->entry_count()); + EXPECT_EQ(2, controller().entry_count()); } // Test that navigating away from an interstitial while it's loading cause it @@ -1064,7 +1058,7 @@ TEST_F(WebContentsTest, NavigateBeforeInterstitialShows) { // Let's simulate a navigation initiated from the browser before the // interstitial finishes loading. const GURL url("http://www.google.com"); - controller()->LoadURL(url, GURL(), PageTransition::TYPED); + controller().LoadURL(url, GURL(), PageTransition::TYPED); ASSERT_FALSE(deleted); EXPECT_FALSE(interstitial->is_showing()); diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc index 2fc7e51..0470efa 100644 --- a/chrome/browser/tabs/tab_strip_model.cc +++ b/chrome/browser/tabs/tab_strip_model.cc @@ -94,7 +94,7 @@ void TabStripModel::InsertTabContentsAt(int index, ForgetAllOpeners(); } // Anything opened by a link we deem to have an opener. - data->SetGroup(selected_contents->controller()); + data->SetGroup(&selected_contents->controller()); } contents_data_.insert(contents_data_.begin() + index, data); @@ -198,7 +198,7 @@ int TabStripModel::GetIndexOfController( int index = 0; TabContentsDataVector::const_iterator iter = contents_data_.begin(); for (; iter != contents_data_.end(); ++iter, ++index) { - if ((*iter)->contents->controller() == controller) + if (&(*iter)->contents->controller() == controller) return index; } return kNoTab; @@ -403,10 +403,8 @@ bool TabStripModel::IsContextMenuCommandEnabled( case CommandCloseTabsToRight: return context_index < (count() - 1); case CommandCloseTabsOpenedBy: { - NavigationController* opener = - GetTabContentsAt(context_index)->controller(); - int next_index = - GetIndexOfNextTabContentsOpenedBy(opener, context_index, true); + int next_index = GetIndexOfNextTabContentsOpenedBy( + &GetTabContentsAt(context_index)->controller(), context_index, true); return next_index != kNoTab; } case CommandDuplicate: @@ -429,7 +427,7 @@ void TabStripModel::ExecuteContextMenuCommand( break; case CommandReload: UserMetrics::RecordAction(L"TabContextMenu_Reload", profile_); - GetContentsAt(context_index)->controller()->Reload(true); + GetContentsAt(context_index)->controller().Reload(true); break; case CommandDuplicate: UserMetrics::RecordAction(L"TabContextMenu_Duplicate", profile_); @@ -457,7 +455,7 @@ void TabStripModel::ExecuteContextMenuCommand( case CommandCloseTabsOpenedBy: { UserMetrics::RecordAction(L"TabContextMenu_CloseTabsOpenedBy", profile_); NavigationController* opener = - GetTabContentsAt(context_index)->controller(); + &GetTabContentsAt(context_index)->controller(); for (int i = count() - 1; i >= 0; --i) { if (OpenerMatches(contents_data_.at(i), opener, true)) @@ -478,7 +476,7 @@ void TabStripModel::ExecuteContextMenuCommand( std::vector<int> TabStripModel::GetIndexesOpenedBy(int index) const { std::vector<int> indices; - NavigationController* opener = GetTabContentsAt(index)->controller(); + NavigationController* opener = &GetTabContentsAt(index)->controller(); for (int i = count() - 1; i >= 0; --i) { if (OpenerMatches(contents_data_.at(i), opener, true)) indices.push_back(i); @@ -511,7 +509,7 @@ bool TabStripModel::IsNewTabAtEndOfTabStrip(TabContents* contents) const { return LowerCaseEqualsASCII(contents->GetURL().spec(), chrome::kChromeUINewTabURL) && contents == GetContentsAt(count() - 1) && - contents->controller()->entry_count() == 1; + contents->controller().entry_count() == 1; } bool TabStripModel::InternalCloseTabContentsAt(int index, @@ -534,9 +532,9 @@ bool TabStripModel::InternalCloseTabContentsAt(int index, if (create_historical_tab) delegate_->CreateHistoricalTab(detached_contents); - detached_contents->CloseContents(); - // Closing the TabContents will later call back to us via - // NotificationObserver and detach it. + // Deleting the TabContents will call back to us via NotificationObserver + // and detach it. + delete detached_contents; } return true; } @@ -564,7 +562,7 @@ void TabStripModel::ChangeSelectedContentsFrom( void TabStripModel::SetOpenerForContents(TabContents* contents, TabContents* opener) { int index = GetIndexOfTabContents(contents); - contents_data_.at(index)->opener = opener->controller(); + contents_data_.at(index)->opener = &opener->controller(); } // static diff --git a/chrome/browser/tabs/tab_strip_model_order_controller.cc b/chrome/browser/tabs/tab_strip_model_order_controller.cc index 0c6326a..1db0525 100644 --- a/chrome/browser/tabs/tab_strip_model_order_controller.cc +++ b/chrome/browser/tabs/tab_strip_model_order_controller.cc @@ -36,7 +36,7 @@ int TabStripModelOrderController::DetermineInsertionIndex( return tabstrip_->selected_index() + 1; } NavigationController* opener = - tabstrip_->GetSelectedTabContents()->controller(); + &tabstrip_->GetSelectedTabContents()->controller(); // Get the index of the next item opened by this tab, and insert before // it... int index = tabstrip_->GetIndexOfLastTabContentsOpenedBy( @@ -60,7 +60,7 @@ int TabStripModelOrderController::DetermineNewSelectedIndex( // want to select the first in that child group, not the next tab in the same // group of the removed tab. NavigationController* removed_controller = - tabstrip_->GetTabContentsAt(removing_index)->controller(); + &tabstrip_->GetTabContentsAt(removing_index)->controller(); int index = tabstrip_->GetIndexOfNextTabContentsOpenedBy(removed_controller, removing_index, false); @@ -109,8 +109,8 @@ void TabStripModelOrderController::TabSelectedAt(TabContents* old_contents, NavigationController* new_opener = tabstrip_->GetOpenerOfTabContentsAt(index); if (user_gesture && new_opener != old_opener && - new_opener != old_contents->controller() && - old_opener != new_contents->controller()) { + new_opener != &old_contents->controller() && + old_opener != &new_contents->controller()) { tabstrip_->ForgetAllOpeners(); } } diff --git a/chrome/browser/tabs/tab_strip_model_unittest.cc b/chrome/browser/tabs/tab_strip_model_unittest.cc index 6b1f3d0..8e052544 100644 --- a/chrome/browser/tabs/tab_strip_model_unittest.cc +++ b/chrome/browser/tabs/tab_strip_model_unittest.cc @@ -68,23 +68,21 @@ class TabStripDummyDelegate : public TabStripModelDelegate { class TabStripModelTest : public RenderViewHostTestHarness { public: TabContents* CreateTabContents() { - WebContents* con = new WebContents(profile(), NULL, 0, NULL); - con->SetupController(profile()); - return con; + return new WebContents(profile(), NULL, 0, NULL); } // Forwards a URL "load" request through to our dummy TabContents // implementation. void LoadURL(TabContents* con, const std::wstring& url) { - controller()->LoadURL(GURL(url), GURL(), PageTransition::LINK); + controller().LoadURL(GURL(url), GURL(), PageTransition::LINK); } void GoBack(TabContents* contents) { - controller()->GoBack(); + controller().GoBack(); } void GoForward(TabContents* contents) { - controller()->GoForward(); + controller().GoForward(); } void SwitchTabTo(TabContents* contents) { @@ -360,8 +358,8 @@ TEST_F(TabStripModelTest, TestBasicAPI) { EXPECT_EQ(contents1, tabstrip.GetTabContentsAt(1)); EXPECT_EQ(0, tabstrip.GetIndexOfTabContents(contents2)); EXPECT_EQ(1, tabstrip.GetIndexOfTabContents(contents1)); - EXPECT_EQ(0, tabstrip.GetIndexOfController(contents2->controller())); - EXPECT_EQ(1, tabstrip.GetIndexOfController(contents1->controller())); + EXPECT_EQ(0, tabstrip.GetIndexOfController(&contents2->controller())); + EXPECT_EQ(1, tabstrip.GetIndexOfController(&contents1->controller())); } // Test UpdateTabContentsStateAt @@ -414,7 +412,7 @@ TEST_F(TabStripModelTest, TestBasicOpenerAPI) { // background with opener_contents set as their opener. TabContents* opener_contents = CreateTabContents(); - NavigationController* opener = opener_contents->controller(); + NavigationController* opener = &opener_contents->controller(); tabstrip.AppendTabContents(opener_contents, true); TabContents* contents1 = CreateTabContents(); TabContents* contents2 = CreateTabContents(); @@ -447,7 +445,7 @@ TEST_F(TabStripModelTest, TestBasicOpenerAPI) { // For a tab that has opened no other tabs, the return value should always be // -1... - NavigationController* o1 = contents1->controller(); + NavigationController* o1 = &contents1->controller(); EXPECT_EQ(-1, tabstrip.GetIndexOfNextTabContentsOpenedBy(o1, 3, false)); EXPECT_EQ(-1, tabstrip.GetIndexOfLastTabContentsOpenedBy(o1, 3)); @@ -513,7 +511,7 @@ TEST_F(TabStripModelTest, TestInsertionIndexDetermination) { EXPECT_TRUE(tabstrip.empty()); TabContents* opener_contents = CreateTabContents(); - NavigationController* opener = opener_contents->controller(); + NavigationController* opener = &opener_contents->controller(); tabstrip.AppendTabContents(opener_contents, true); // Open some other random unrelated tab in the background to monkey with our @@ -592,7 +590,7 @@ TEST_F(TabStripModelTest, TestSelectOnClose) { EXPECT_TRUE(tabstrip.empty()); TabContents* opener_contents = CreateTabContents(); - NavigationController* opener = opener_contents->controller(); + NavigationController* opener = &opener_contents->controller(); tabstrip.AppendTabContents(opener_contents, true); TabContents* contents1 = CreateTabContents(); @@ -670,7 +668,7 @@ TEST_F(TabStripModelTest, TestContextMenuCloseCommands) { EXPECT_TRUE(tabstrip.empty()); TabContents* opener_contents = CreateTabContents(); - NavigationController* opener = opener_contents->controller(); + NavigationController* opener = &opener_contents->controller(); tabstrip.AppendTabContents(opener_contents, true); TabContents* contents1 = CreateTabContents(); @@ -945,8 +943,8 @@ TEST_F(TabStripModelTest, AddTabContents_ForgetOpeners) { // Added for http://b/issue?id=958960 TEST_F(TabStripModelTest, AppendContentsReselectionTest) { - TabContents* fake_destinations_tab = CreateTabContents(); - TabStripDummyDelegate delegate(fake_destinations_tab); + WebContents fake_destinations_tab(profile(), NULL, 0, NULL); + TabStripDummyDelegate delegate(&fake_destinations_tab); TabStripModel tabstrip(&delegate, profile()); EXPECT_TRUE(tabstrip.empty()); diff --git a/chrome/browser/task_manager_resource_providers.cc b/chrome/browser/task_manager_resource_providers.cc index 0a992dd..5364b4d 100644 --- a/chrome/browser/task_manager_resource_providers.cc +++ b/chrome/browser/task_manager_resource_providers.cc @@ -40,11 +40,6 @@ TaskManagerWebContentsResource::~TaskManagerWebContentsResource() { } std::wstring TaskManagerWebContentsResource::GetTitle() const { - // GetTitle() and GetURL() can only be invoked when the WebContents has a - // controller. - if (!web_contents_->controller()) - return std::wstring(); - // Fall back on the URL if there's no title. std::wstring tab_title(UTF16ToWideHack(web_contents_->GetTitle())); if (tab_title.empty()) { diff --git a/chrome/browser/views/blocked_popup_container.cc b/chrome/browser/views/blocked_popup_container.cc index ca5b996..3906d5b 100644 --- a/chrome/browser/views/blocked_popup_container.cc +++ b/chrome/browser/views/blocked_popup_container.cc @@ -254,12 +254,12 @@ void BlockedPopupContainer::AddTabContents(TabContents* blocked_contents, const gfx::Rect& bounds) { if (has_been_dismissed_) { // We simply bounce this popup without notice. - blocked_contents->CloseContents(); + delete blocked_contents; return; } if (blocked_popups_.size() > kImpossibleNumberOfPopups) { - blocked_contents->CloseContents(); + delete blocked_contents; LOG(INFO) << "Warning: Renderer is sending more popups to us then should be" " possible. Renderer compromised?"; return; @@ -477,7 +477,7 @@ void BlockedPopupContainer::SetPosition() { void BlockedPopupContainer::CloseEachTabContents() { while (!blocked_popups_.empty()) { blocked_popups_.back().first->set_delegate(NULL); - blocked_popups_.back().first->CloseContents(); + delete blocked_popups_.back().first; blocked_popups_.pop_back(); } diff --git a/chrome/browser/views/blocked_popup_container.h b/chrome/browser/views/blocked_popup_container.h index e469da6..c634308 100644 --- a/chrome/browser/views/blocked_popup_container.h +++ b/chrome/browser/views/blocked_popup_container.h @@ -220,7 +220,7 @@ class BlockedPopupContainer : public ConstrainedWindow, // change. void SetPosition(); - // Sends a CloseContents() to each message in |blocked_popups_|. + // Deletes each contents in |blocked_popups_|. void CloseEachTabContents(); // The TabContents that owns and constrains this BlockedPopupContainer. diff --git a/chrome/browser/views/bug_report_view.cc b/chrome/browser/views/bug_report_view.cc index 1144065..647dfc1 100644 --- a/chrome/browser/views/bug_report_view.cc +++ b/chrome/browser/views/bug_report_view.cc @@ -482,7 +482,7 @@ void BugReportView::SendReport() { } void BugReportView::ReportPhishing() { - tab_->controller()->LoadURL( + tab_->controller().LoadURL( safe_browsing_util::GeneratePhishingReportUrl( kReportPhishingUrl, WideToUTF8(page_url_text_->GetText())), GURL(), diff --git a/chrome/browser/views/dom_view.cc b/chrome/browser/views/dom_view.cc index 3e1c777..ffa9c1d 100644 --- a/chrome/browser/views/dom_view.cc +++ b/chrome/browser/views/dom_view.cc @@ -11,11 +11,8 @@ DOMView::DOMView() : initialized_(false), web_contents_(NULL) { } DOMView::~DOMView() { - if (web_contents_) { + if (web_contents_.get()) Detach(); - web_contents_->Destroy(); - web_contents_ = NULL; - } } bool DOMView::Init(Profile* profile, SiteInstance* instance) { @@ -23,13 +20,13 @@ bool DOMView::Init(Profile* profile, SiteInstance* instance) { return true; initialized_ = true; - web_contents_ = new WebContents(profile, instance, MSG_ROUTING_NONE, NULL); + web_contents_.reset(new WebContents(profile, instance, + MSG_ROUTING_NONE, NULL)); views::HWNDView::Attach(web_contents_->GetNativeView()); - web_contents_->SetupController(profile); return true; } void DOMView::LoadURL(const GURL& url) { DCHECK(initialized_); - web_contents_->controller()->LoadURL(url, GURL(), PageTransition::START_PAGE); + web_contents_->controller().LoadURL(url, GURL(), PageTransition::START_PAGE); } diff --git a/chrome/browser/views/dom_view.h b/chrome/browser/views/dom_view.h index bfa6d13..132a444 100644 --- a/chrome/browser/views/dom_view.h +++ b/chrome/browser/views/dom_view.h @@ -8,6 +8,7 @@ #ifndef CHROME_BROWSER_VIEWS_DOM_VIEW_H_ #define CHROME_BROWSER_VIEWS_DOM_VIEW_H_ +#include "base/scoped_ptr.h" #include "chrome/views/controls/hwnd_view.h" #include "googleurl/src/gurl.h" @@ -34,7 +35,7 @@ class DOMView : public views::HWNDView { protected: virtual bool CanProcessTabKeyEvents() { return true; } - WebContents* web_contents_; + scoped_ptr<WebContents> web_contents_; private: bool initialized_; diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc index 2505ff8..2cd0823 100644 --- a/chrome/browser/views/frame/browser_view.cc +++ b/chrome/browser/views/frame/browser_view.cc @@ -769,10 +769,10 @@ void BrowserView::ShowReportBugDialog() { BugReportView* bug_report_view = new BugReportView(browser_->profile(), current_tab); - if (current_tab->controller()->GetLastCommittedEntry()) { + if (current_tab->controller().GetLastCommittedEntry()) { // URL for the current page bug_report_view->SetUrl( - current_tab->controller()->GetActiveEntry()->url()); + current_tab->controller().GetActiveEntry()->url()); } // retrieve the application version info diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc index c1b96b9..59bce3b 100644 --- a/chrome/browser/views/location_bar_view.cc +++ b/chrome/browser/views/location_bar_view.cc @@ -989,7 +989,7 @@ bool LocationBarView::SecurityImageView::OnMousePressed( const views::MouseEvent& event) { NavigationEntry* nav_entry = BrowserList::GetLastActive()->GetSelectedTabContents()-> - controller()->GetActiveEntry(); + controller().GetActiveEntry(); if (!nav_entry) { NOTREACHED(); return true; @@ -1031,7 +1031,7 @@ bool LocationBarView::RssImageView::OnMousePressed( const views::MouseEvent& event) { NavigationEntry* entry = BrowserList::GetLastActive()->GetSelectedTabContents()-> - controller()->GetActiveEntry(); + controller().GetActiveEntry(); if (!entry) { NOTREACHED(); return true; diff --git a/chrome/browser/views/tab_contents_container_view.cc b/chrome/browser/views/tab_contents_container_view.cc index 4b721da..04c9354 100644 --- a/chrome/browser/views/tab_contents_container_view.cc +++ b/chrome/browser/views/tab_contents_container_view.cc @@ -181,7 +181,7 @@ void TabContentsContainerView::AddObservers() { // the focus subclass on the shown HWND so we intercept focus change events. NotificationService::current()->AddObserver( this, NotificationType::RENDER_VIEW_HOST_CHANGED, - Source<NavigationController>(tab_contents_->controller())); + Source<NavigationController>(&tab_contents_->controller())); } NotificationService::current()->AddObserver( this, @@ -195,7 +195,7 @@ void TabContentsContainerView::RemoveObservers() { NotificationService::current()->RemoveObserver( this, NotificationType::RENDER_VIEW_HOST_CHANGED, - Source<NavigationController>(tab_contents_->controller())); + Source<NavigationController>(&tab_contents_->controller())); } NotificationService::current()->RemoveObserver( this, diff --git a/chrome/browser/views/tabs/tab_strip.cc b/chrome/browser/views/tabs/tab_strip.cc index 51baf95..5328927 100644 --- a/chrome/browser/views/tabs/tab_strip.cc +++ b/chrome/browser/views/tabs/tab_strip.cc @@ -703,8 +703,8 @@ int TabStrip::OnPerformDrop(const DropTargetEvent& event) { } else { UserMetrics::RecordAction(L"Tab_DropURLOnTab", model_->profile()); - model_->GetTabContentsAt(drop_index)->controller()-> - LoadURL(url, GURL(), PageTransition::GENERATED); + model_->GetTabContentsAt(drop_index)->controller().LoadURL( + url, GURL(), PageTransition::GENERATED); model_->SelectTabContentsAt(drop_index, true); } diff --git a/chrome/test/browser_with_test_window_test.cc b/chrome/test/browser_with_test_window_test.cc index 1881a54..b0d6a84 100644 --- a/chrome/test/browser_with_test_window_test.cc +++ b/chrome/test/browser_with_test_window_test.cc @@ -51,7 +51,7 @@ void BrowserWithTestWindowTest::AddTab(Browser* browser, const GURL& url) { TabContents* new_tab = browser->AddTabWithURL(url, GURL(), PageTransition::TYPED, true, 0, NULL); - CommitPendingLoadAsNewNavigation(new_tab->controller(), url); + CommitPendingLoadAsNewNavigation(&new_tab->controller(), url); } void BrowserWithTestWindowTest::CommitPendingLoadAsNewNavigation( @@ -73,5 +73,5 @@ void BrowserWithTestWindowTest::NavigateAndCommit( } void BrowserWithTestWindowTest::NavigateAndCommitActiveTab(const GURL& url) { - NavigateAndCommit(browser()->GetSelectedTabContents()->controller(), url); + NavigateAndCommit(&browser()->GetSelectedTabContents()->controller(), url); } diff --git a/chrome/test/in_process_browser_test.cc b/chrome/test/in_process_browser_test.cc index 47e0e81..25cc6ee 100644 --- a/chrome/test/in_process_browser_test.cc +++ b/chrome/test/in_process_browser_test.cc @@ -175,7 +175,7 @@ Browser* InProcessBrowserTest::CreateBrowser(Profile* profile) { // Wait for the page to finish loading. ui_test_utils::WaitForNavigation( - browser->GetSelectedTabContents()->controller()); + &browser->GetSelectedTabContents()->controller()); browser->window()->Show(); diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc index be86d17..2d91bcc 100644 --- a/chrome/test/ui_test_utils.cc +++ b/chrome/test/ui_test_utils.cc @@ -93,7 +93,7 @@ void NavigateToURLBlockUntilNavigationsComplete(Browser* browser, const GURL& url, int number_of_navigations) { NavigationController* controller = - browser->GetSelectedTabContents()->controller(); + &browser->GetSelectedTabContents()->controller(); browser->OpenURLFromTab(browser->GetSelectedTabContents(), url, GURL(), CURRENT_TAB, PageTransition::TYPED); WaitForNavigations(controller, number_of_navigations); |