Fix the ownership model of TabContents and NavigationController. Previously the

NavigationController owned the TabContents, and there were extra steps required
at creation and destruction to clean everything up properly.

NavigationController is now a member of TabContents, and there is no setup or
tear down necessary other than the constructor and destructor. I could remove
the tab contents creation in the NavigationController, as well as all the
weird destruction code in WebContents which got moved to the destructor.

I made the controller getter return a reference since the ownership is clear
and there is no possibility of NULL. This required changing a lot of tiles, but
many of them were simplified since they no longer have to NULL check.
Review URL: http://codereview.chromium.org/69043

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@14005 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 20 insertions, 31 deletions

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(