Destroy TaskManager's view when its window is closed.

TEST=Open the task manager. Close it. Open again and close. Browser should not crash.

http://crbug.com/13361

Review URL: http://codereview.chromium.org/119299

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

4 files changed, 59 insertions, 47 deletions

diff --git a/chrome/browser/task_manager.cc b/chrome/browser/task_manager.cc
index f11a91a..8f174cc 100644
--- a/chrome/browser/task_manager.cc
+++ b/chrome/browser/task_manager.cc
@@ -78,7 +78,6 @@ int TaskManagerModel::ResourceCount() const {
 }
 
 void TaskManagerModel::SetObserver(TaskManagerModelObserver* observer) {
-  DCHECK(!observer_) << "can set observer only once";
   observer_ = observer;
 }
 
@@ -444,8 +443,8 @@ void TaskManagerModel::AddResource(TaskManager::Resource* resource) {
   }
 
   // Notify the table that the contents have changed for it to redraw.
-  DCHECK(observer_);
-  observer_->OnItemsAdded(new_entry_index, 1);
+  if (observer_)
+    observer_->OnItemsAdded(new_entry_index, 1);
 }
 
 void TaskManagerModel::RemoveResource(TaskManager::Resource* resource) {
@@ -497,7 +496,8 @@ void TaskManagerModel::RemoveResource(TaskManager::Resource* resource) {
     displayed_network_usage_map_.erase(net_iter);
 
   // Notify the table that the contents have changed.
-  observer_->OnItemsRemoved(index, 1);
+  if (observer_)
+    observer_->OnItemsRemoved(index, 1);
 }
 
 void TaskManagerModel::Clear() {
@@ -524,7 +524,8 @@ void TaskManagerModel::Clear() {
     current_byte_count_map_.clear();
     displayed_network_usage_map_.clear();
 
-    observer_->OnItemsRemoved(0, size);
+    if (observer_)
+      observer_->OnItemsRemoved(0, size);
   }
 }
 
@@ -570,7 +571,7 @@ void TaskManagerModel::Refresh() {
     // Then we reset the current byte count.
     iter->second = 0;
   }
-  if (!resources_.empty())
+  if (!resources_.empty() && observer_)
     observer_->OnItemsChanged(0, ResourceCount());
 
   // Schedule the next update.
@@ -697,8 +698,8 @@ void TaskManager::RegisterPrefs(PrefService* prefs) {
 }
 
 TaskManager::TaskManager()
-    : ALLOW_THIS_IN_INITIALIZER_LIST(model_(new TaskManagerModel(this))) {
-  Init();
+    : ALLOW_THIS_IN_INITIALIZER_LIST(model_(new TaskManagerModel(this))),
+      view_(NULL) {
 }
 
 TaskManager::~TaskManager() {
@@ -707,7 +708,12 @@ TaskManager::~TaskManager() {
 // static
 void TaskManager::Open() {
   TaskManager* task_manager = GetInstance();
-  task_manager->view_->OpenWindow();
+  if (task_manager->view_) {
+    task_manager->view_->ActivateWindow();
+  } else {
+    task_manager->CreateView();
+    task_manager->view_->OpenWindow();
+  }
 }
 
 // static
@@ -717,7 +723,7 @@ void TaskManager::Close() {
 }
 
 bool TaskManager::BrowserProcessIsSelected() {
-  if (!view_.get())
+  if (!view_)
     return false;
   std::vector<int> selection;
   view_->GetSelection(&selection);
@@ -793,6 +799,7 @@ void TaskManager::RemoveResource(Resource* resource) {
 void TaskManager::OnWindowClosed() {
   model_->StopUpdating();
   model_->Clear();
+  view_ = NULL;
 }
 
 // static
diff --git a/chrome/browser/task_manager.h b/chrome/browser/task_manager.h
index c0eaa8b..c0f312e 100644
--- a/chrome/browser/task_manager.h
+++ b/chrome/browser/task_manager.h
@@ -130,7 +130,7 @@ class TaskManager {
 
   ~TaskManager();
 
-  void Init();
+  void CreateView();
 
   // Returns the singleton instance (and initializes it if necessary).
   static TaskManager* GetInstance();
@@ -140,7 +140,7 @@ class TaskManager {
   scoped_refptr<TaskManagerModel> model_;
 
   // A container containing the buttons and table.
-  scoped_ptr<TaskManagerView> view_;
+  TaskManagerView* view_;
 
   DISALLOW_COPY_AND_ASSIGN(TaskManager);
 };
@@ -356,6 +356,7 @@ class TaskManagerView {
   virtual void GetFocused(std::vector<int>* focused) = 0;
 
   virtual void OpenWindow() = 0;
+  virtual void ActivateWindow() = 0;
   virtual void CloseWindow() = 0;
 };
 
diff --git a/chrome/browser/task_manager_linux.cc b/chrome/browser/task_manager_linux.cc
index 4171406..17baa3c 100644
--- a/chrome/browser/task_manager_linux.cc
+++ b/chrome/browser/task_manager_linux.cc
@@ -21,6 +21,7 @@ class TaskManagerViewImpl : public TaskManagerView,
   virtual void GetSelection(std::vector<int>* selection);
   virtual void GetFocused(std::vector<int>* focused);
   virtual void OpenWindow();
+  virtual void ActivateWindow();
   virtual void CloseWindow();
 
   // TaskManagerModelObserver
@@ -42,6 +43,10 @@ void TaskManagerViewImpl::OpenWindow() {
   NOTIMPLEMENTED();
 }
 
+void TaskManagerViewImpl::ActivateWindow() {
+  NOTIMPLEMENTED();
+}
+
 void TaskManagerViewImpl::CloseWindow() {
   NOTIMPLEMENTED();
 }
@@ -64,6 +69,7 @@ void TaskManagerViewImpl::OnItemsRemoved(int start, int length) {
 
 }  // namespace
 
-void TaskManager::Init() {
-  view_.reset(new TaskManagerViewImpl(model_.get()));
+void TaskManager::CreateView() {
+  DCHECK(!view_);
+  view_= new TaskManagerViewImpl(model_.get());
 }
diff --git a/chrome/browser/task_manager_win.cc b/chrome/browser/task_manager_win.cc
index 988d4dd..671c2ea 100644
--- a/chrome/browser/task_manager_win.cc
+++ b/chrome/browser/task_manager_win.cc
@@ -187,6 +187,7 @@ class TaskManagerViewImpl : public TaskManagerView,
   virtual void GetSelection(std::vector<int>* selection);
   virtual void GetFocused(std::vector<int>* focused);
   virtual void OpenWindow();
+  virtual void ActivateWindow();
   virtual void CloseWindow();
 
   // ButtonListener implementation.
@@ -233,9 +234,9 @@ class TaskManagerViewImpl : public TaskManagerView,
   // Restores saved always on top state from a previous session.
   bool GetSavedAlwaysOnTopState(bool* always_on_top) const;
 
-  scoped_ptr<views::NativeButton> kill_button_;
-  scoped_ptr<views::Link> about_memory_link_;
-  scoped_ptr<views::GroupTableView> tab_table_;
+  views::NativeButton* kill_button_;
+  views::Link* about_memory_link_;
+  views::GroupTableView* tab_table_;
 
   TaskManager* task_manager_;
 
@@ -297,9 +298,9 @@ void TaskManagerViewImpl::Init() {
                                         views::TableColumn::RIGHT, -1, 0));
   columns_.back().sortable = true;
 
-  tab_table_.reset(new views::GroupTableView(table_model_.get(), columns_,
-                                             views::ICON_AND_TEXT, false, true,
-                                             true));
+  tab_table_ = new views::GroupTableView(table_model_.get(), columns_,
+                                         views::ICON_AND_TEXT, false, true,
+                                         true);
   tab_table_->SetParentOwned(false);
 
   // Hide some columns by default
@@ -315,12 +316,12 @@ void TaskManagerViewImpl::Init() {
   tab_table_->AddColumn(col);
   tab_table_->SetObserver(this);
   SetContextMenuController(this);
-  kill_button_.reset(new views::NativeButton(
-      this, l10n_util::GetString(IDS_TASK_MANAGER_KILL)));
+  kill_button_ = new views::NativeButton(
+      this, l10n_util::GetString(IDS_TASK_MANAGER_KILL));
   kill_button_->AddAccelerator(views::Accelerator('E', false, false, false));
   kill_button_->SetAccessibleKeyboardShortcut(L"E");
-  about_memory_link_.reset(new views::Link(
-      l10n_util::GetString(IDS_TASK_MANAGER_ABOUT_MEMORY_LINK)));
+  about_memory_link_ = new views::Link(
+      l10n_util::GetString(IDS_TASK_MANAGER_ABOUT_MEMORY_LINK));
   about_memory_link_->SetController(this);
 
   // Makes sure our state is consistent.
@@ -360,15 +361,12 @@ void TaskManagerViewImpl::ViewHierarchyChanged(bool is_add,
   // hierarchy, we must take care to clean up those items as well.
   if (child == this) {
     if (is_add) {
-      parent->AddChildView(kill_button_.get());
-      parent->AddChildView(about_memory_link_.get());
-      AddChildView(tab_table_.get());
+      parent->AddChildView(kill_button_);
+      parent->AddChildView(about_memory_link_);
+      AddChildView(tab_table_);
     } else {
-      parent->RemoveChildView(kill_button_.get());
-      parent->RemoveChildView(about_memory_link_.get());
-      // Note that these items aren't deleted here, since this object is owned
-      // by the TaskManager, whose lifetime surpasses the window, and the next
-      // time we are inserted into a window these items will need to be valid.
+      parent->RemoveChildView(kill_button_);
+      parent->RemoveChildView(about_memory_link_);
     }
   }
 }
@@ -432,25 +430,28 @@ void TaskManagerViewImpl::GetFocused(std::vector<int>* focused) {
 }
 
 void TaskManagerViewImpl::OpenWindow() {
-  if (window()) {
-    window()->Activate();
-  } else {
-    views::Window::CreateChromeWindow(NULL, gfx::Rect(), this);
-    InitAlwaysOnTopState();
-    model_->StartUpdating();
-    window()->Show();
-  }
+  DCHECK(!window());
+  views::Window::CreateChromeWindow(NULL, gfx::Rect(), this);
+  InitAlwaysOnTopState();
+  model_->StartUpdating();
+  window()->Show();
+}
+
+void TaskManagerViewImpl::ActivateWindow() {
+  DCHECK(window());
+  window()->Activate();
 }
 
 void TaskManagerViewImpl::CloseWindow() {
   if (!window())
     return;
+  // TODO(phajdan.jr): Destroy the window, not just hide it.
   window()->HideWindow();
 }
 
 // ButtonListener implementation.
 void TaskManagerViewImpl::ButtonPressed(views::Button* sender) {
-  if (sender == kill_button_.get())
+  if (sender == kill_button_)
     task_manager_->KillSelectedProcesses();
 }
 
@@ -508,10 +509,6 @@ int TaskManagerViewImpl::GetDialogButtons() const {
 }
 
 void TaskManagerViewImpl::WindowClosing() {
-  // Remove the view from its parent to trigger the contents'
-  // ViewHierarchyChanged notification to unhook the extra buttons from the
-  // non-client view.
-  GetParent()->RemoveChildView(this);
   task_manager_->OnWindowClosed();
 }
 
@@ -630,6 +627,7 @@ bool TaskManagerViewImpl::GetSavedAlwaysOnTopState(bool* always_on_top) const {
 
 }  // namespace
 
-void TaskManager::Init() {
-  view_.reset(new TaskManagerViewImpl(this, model_.get()));
+void TaskManager::CreateView() {
+  DCHECK(!view_);
+  view_ = new TaskManagerViewImpl(this, model_.get());
 }