Add more browser tests for Task Manager.

TEST=Covered by browser_tests.
http://crbug.com/12127

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

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

12 files changed, 162 insertions, 43 deletions

diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc
index 6602ca7..dd46e07 100644
--- a/chrome/browser/extensions/extension_host.cc
+++ b/chrome/browser/extensions/extension_host.cc
@@ -130,12 +130,19 @@ void ExtensionHost::CreateRenderView(RenderWidgetHostView* host_view) {
   render_view_host_->set_view(host_view);
   render_view_host_->CreateRenderView();
   render_view_host_->NavigateToURL(url_);
+  DCHECK(IsRenderViewLive());
+  NotificationService::current()->Notify(
+      NotificationType::EXTENSION_PROCESS_CREATED,
+      Source<Profile>(profile_),
+      Details<ExtensionHost>(this));
 }
 
 void ExtensionHost::RecoverCrashedExtension() {
   DCHECK(!IsRenderViewLive());
 #if defined(TOOLKIT_VIEWS)
   if (view_.get()) {
+    // The view should call us back to CreateRenderView, which is the place
+    // where we create the render process and fire notification.
     view_->RecoverCrashedExtension();
   } else {
     CreateRenderView(NULL);
@@ -143,12 +150,6 @@ void ExtensionHost::RecoverCrashedExtension() {
 #else
   CreateRenderView(NULL);
 #endif
-  if (IsRenderViewLive()) {
-    NotificationService::current()->Notify(
-        NotificationType::EXTENSION_PROCESS_RESTORED,
-        Source<Profile>(profile_),
-        Details<ExtensionHost>(this));
-  }
 }
 
 void ExtensionHost::UpdatePreferredWidth(int pref_width) {
diff --git a/chrome/browser/gtk/task_manager_gtk.cc b/chrome/browser/gtk/task_manager_gtk.cc
index 5f64c45..94d06d9 100644
--- a/chrome/browser/gtk/task_manager_gtk.cc
+++ b/chrome/browser/gtk/task_manager_gtk.cc
@@ -231,6 +231,7 @@ TaskManagerGtk* TaskManagerGtk::instance_ = NULL;
 
 TaskManagerGtk::~TaskManagerGtk() {
   task_manager_->OnWindowClosed();
+  model_->RemoveObserver(this);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -363,7 +364,7 @@ void TaskManagerGtk::Init() {
   gtk_window_resize(GTK_WINDOW(dialog_), kDefaultWidth, kDefaultHeight);
   gtk_widget_show_all(dialog_);
 
-  model_->SetObserver(this);
+  model_->AddObserver(this);
 }
 
 void TaskManagerGtk::ConnectAccelerators() {
diff --git a/chrome/browser/task_manager.cc b/chrome/browser/task_manager.cc
index d80d51c..b7de95e 100644
--- a/chrome/browser/task_manager.cc
+++ b/chrome/browser/task_manager.cc
@@ -44,8 +44,7 @@ static int ValueCompare(T value1, T value2) {
 int TaskManagerModel::goats_teleported_ = 0;
 
 TaskManagerModel::TaskManagerModel(TaskManager* task_manager)
-    : observer_(NULL),
-      ui_loop_(MessageLoop::current()),
+    : ui_loop_(MessageLoop::current()),
       update_state_(IDLE) {
 
   TaskManagerBrowserProcessResourceProvider* browser_provider =
@@ -77,8 +76,12 @@ int TaskManagerModel::ResourceCount() const {
   return resources_.size();
 }
 
-void TaskManagerModel::SetObserver(TaskManagerModelObserver* observer) {
-  observer_ = observer;
+void TaskManagerModel::AddObserver(TaskManagerModelObserver* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void TaskManagerModel::RemoveObserver(TaskManagerModelObserver* observer) {
+  observer_list_.RemoveObserver(observer);
 }
 
 std::wstring TaskManagerModel::GetResourceTitle(int index) const {
@@ -443,8 +446,8 @@ void TaskManagerModel::AddResource(TaskManager::Resource* resource) {
   }
 
   // Notify the table that the contents have changed for it to redraw.
-  if (observer_)
-    observer_->OnItemsAdded(new_entry_index, 1);
+  FOR_EACH_OBSERVER(TaskManagerModelObserver, observer_list_,
+                    OnItemsAdded(new_entry_index, 1));
 }
 
 void TaskManagerModel::RemoveResource(TaskManager::Resource* resource) {
@@ -496,8 +499,8 @@ void TaskManagerModel::RemoveResource(TaskManager::Resource* resource) {
     displayed_network_usage_map_.erase(net_iter);
 
   // Notify the table that the contents have changed.
-  if (observer_)
-    observer_->OnItemsRemoved(index, 1);
+  FOR_EACH_OBSERVER(TaskManagerModelObserver, observer_list_,
+                    OnItemsRemoved(index, 1));
 }
 
 void TaskManagerModel::Clear() {
@@ -524,8 +527,8 @@ void TaskManagerModel::Clear() {
     current_byte_count_map_.clear();
     displayed_network_usage_map_.clear();
 
-    if (observer_)
-      observer_->OnItemsRemoved(0, size);
+    FOR_EACH_OBSERVER(TaskManagerModelObserver, observer_list_,
+                      OnItemsRemoved(0, size));
   }
 }
 
@@ -571,8 +574,10 @@ void TaskManagerModel::Refresh() {
     // Then we reset the current byte count.
     iter->second = 0;
   }
-  if (!resources_.empty() && observer_)
-    observer_->OnItemsChanged(0, ResourceCount());
+  if (!resources_.empty()) {
+    FOR_EACH_OBSERVER(TaskManagerModelObserver, observer_list_,
+                      OnItemsChanged(0, ResourceCount()));
+  }
 
   // Schedule the next update.
   MessageLoop::current()->PostDelayedTask(FROM_HERE,
diff --git a/chrome/browser/task_manager.h b/chrome/browser/task_manager.h
index c574b4a..3ca3774 100644
--- a/chrome/browser/task_manager.h
+++ b/chrome/browser/task_manager.h
@@ -12,6 +12,7 @@
 
 #include "base/basictypes.h"
 #include "base/lock.h"
+#include "base/observer_list.h"
 #include "base/process_util.h"
 #include "base/ref_counted.h"
 #include "base/singleton.h"
@@ -156,8 +157,8 @@ class TaskManagerModel : public URLRequestJobTracker::JobObserver,
   explicit TaskManagerModel(TaskManager* task_manager);
   ~TaskManagerModel();
 
-  // Set object to be notified on model changes.
-  void SetObserver(TaskManagerModelObserver* observer);
+  void AddObserver(TaskManagerModelObserver* observer);
+  void RemoveObserver(TaskManagerModelObserver* observer);
 
   // Returns number of registered resources.
   int ResourceCount() const;
@@ -322,7 +323,7 @@ class TaskManagerModel : public URLRequestJobTracker::JobObserver,
   // A map that contains the CPU usage (in %) for a process since last refresh.
   CPUUsageMap cpu_usage_map_;
 
-  TaskManagerModelObserver* observer_;
+  ObserverList<TaskManagerModelObserver> observer_list_;
 
   MessageLoop* ui_loop_;
 
diff --git a/chrome/browser/task_manager_browsertest.cc b/chrome/browser/task_manager_browsertest.cc
index 8aa343f..5439043 100644
--- a/chrome/browser/task_manager_browsertest.cc
+++ b/chrome/browser/task_manager_browsertest.cc
@@ -6,12 +6,112 @@
 
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_window.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/common/page_transition_types.h"
 #include "chrome/test/in_process_browser_test.h"
+#include "chrome/test/ui_test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-class TaskManagerBrowserTest : public InProcessBrowserTest {
+namespace {
+
+class ResourceChangeObserver : public TaskManagerModelObserver {
+ public:
+  ResourceChangeObserver(const TaskManagerModel* model,
+                         int target_resource_count)
+      : model_(model),
+        target_resource_count_(target_resource_count) {
+  }
+
+  virtual void OnModelChanged() {
+    OnResourceChange();
+  }
+
+  virtual void OnItemsChanged(int start, int length) {
+    OnResourceChange();
+  }
+
+  virtual void OnItemsAdded(int start, int length) {
+    OnResourceChange();
+  }
+
+  virtual void OnItemsRemoved(int start, int length) {
+    OnResourceChange();
+  }
+
+ private:
+  void OnResourceChange() {
+    if (model_->ResourceCount() == target_resource_count_)
+      MessageLoopForUI::current()->Quit();
+  }
+
+  const TaskManagerModel* model_;
+  const int target_resource_count_;
 };
 
+}  // namespace
+
+class TaskManagerBrowserTest : public ExtensionBrowserTest {
+ public:
+  TaskManagerModel* model() const {
+    return TaskManager::GetInstance()->model();
+  }
+
+  void WaitForResourceChange(int target_count) {
+    if (model()->ResourceCount() == target_count)
+      return;
+    ResourceChangeObserver observer(model(), target_count);
+    model()->AddObserver(&observer);
+    ui_test_utils::RunMessageLoop();
+    model()->RemoveObserver(&observer);
+  }
+};
+
+// Regression test for http://crbug.com/13361
 IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, ShutdownWhileOpen) {
   browser()->window()->ShowTaskManager();
 }
+
+IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeTabContentsChanges) {
+  EXPECT_EQ(0, model()->ResourceCount());
+
+  // Show the task manager. This populates the model, and helps with debugging
+  // (you see the task manager).
+  browser()->window()->ShowTaskManager();
+
+  // Browser and the New Tab Page.
+  EXPECT_EQ(2, model()->ResourceCount());
+
+  // Open a new tab and make sure we notice that.
+  GURL url(ui_test_utils::GetTestUrl(L".", L"title1.html"));
+  browser()->AddTabWithURL(url, GURL(), PageTransition::TYPED,
+                           true, 0, false, NULL);
+  WaitForResourceChange(3);
+
+  // Close the tab and verify that we notice.
+  TabContents* first_tab = browser()->GetTabContentsAt(0);
+  ASSERT_TRUE(first_tab);
+  browser()->CloseTabContents(first_tab);
+  WaitForResourceChange(2);
+}
+
+IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeExtensionChanges) {
+  EXPECT_EQ(0, model()->ResourceCount());
+
+  // Show the task manager. This populates the model, and helps with debugging
+  // (you see the task manager).
+  browser()->window()->ShowTaskManager();
+
+  // Browser and the New Tab Page.
+  EXPECT_EQ(2, model()->ResourceCount());
+
+  // Loading an extension should result in a new resource being
+  // created for it.
+  ASSERT_TRUE(LoadExtension(
+      test_data_dir_.AppendASCII("common").AppendASCII("one_in_shelf")));
+  WaitForResourceChange(3);
+
+  // Make sure we also recognize extensions with just background pages.
+  ASSERT_TRUE(LoadExtension(
+      test_data_dir_.AppendASCII("common").AppendASCII("background_page")));
+  WaitForResourceChange(4);
+}
diff --git a/chrome/browser/task_manager_resource_providers.cc b/chrome/browser/task_manager_resource_providers.cc
index 77cbf10..8a9713b 100644
--- a/chrome/browser/task_manager_resource_providers.cc
+++ b/chrome/browser/task_manager_resource_providers.cc
@@ -541,14 +541,10 @@ void TaskManagerExtensionProcessResourceProvider::StartUpdating() {
   }
 
   // Register for notifications about extension process changes.
-  registrar_.Add(this, NotificationType::EXTENSION_HOST_CREATED,
-                 NotificationService::AllSources());
-  registrar_.Add(this, NotificationType::EXTENSION_HOST_DESTROYED,
+  registrar_.Add(this, NotificationType::EXTENSION_PROCESS_CREATED,
                  NotificationService::AllSources());
   registrar_.Add(this, NotificationType::EXTENSION_PROCESS_CRASHED,
                  NotificationService::AllSources());
-  registrar_.Add(this, NotificationType::EXTENSION_PROCESS_RESTORED,
-                 NotificationService::AllSources());
 }
 
 void TaskManagerExtensionProcessResourceProvider::StopUpdating() {
@@ -556,14 +552,10 @@ void TaskManagerExtensionProcessResourceProvider::StopUpdating() {
   updating_ = false;
 
   // Unregister for notifications about extension process changes.
-  registrar_.Remove(this, NotificationType::EXTENSION_HOST_CREATED,
-                    NotificationService::AllSources());
-  registrar_.Remove(this, NotificationType::EXTENSION_HOST_DESTROYED,
+  registrar_.Remove(this, NotificationType::EXTENSION_PROCESS_CREATED,
                     NotificationService::AllSources());
   registrar_.Remove(this, NotificationType::EXTENSION_PROCESS_CRASHED,
                     NotificationService::AllSources());
-  registrar_.Remove(this, NotificationType::EXTENSION_PROCESS_RESTORED,
-                    NotificationService::AllSources());
 
   // Delete all the resources.
   STLDeleteContainerPairSecondPointers(resources_.begin(), resources_.end());
@@ -577,11 +569,9 @@ void TaskManagerExtensionProcessResourceProvider::Observe(
     const NotificationSource& source,
     const NotificationDetails& details) {
   switch (type.value) {
-    case NotificationType::EXTENSION_HOST_CREATED:
-    case NotificationType::EXTENSION_PROCESS_RESTORED:
+    case NotificationType::EXTENSION_PROCESS_CREATED:
       AddToTaskManager(Details<ExtensionHost>(details).ptr());
       break;
-    case NotificationType::EXTENSION_HOST_DESTROYED:
     case NotificationType::EXTENSION_PROCESS_CRASHED:
       RemoveFromTaskManager(Details<ExtensionHost>(details).ptr());
       break;
diff --git a/chrome/browser/views/task_manager_view.cc b/chrome/browser/views/task_manager_view.cc
index a9cd9e6..52a14bd 100644
--- a/chrome/browser/views/task_manager_view.cc
+++ b/chrome/browser/views/task_manager_view.cc
@@ -52,9 +52,12 @@ class TaskManagerTableModel : public views::GroupTableModel,
   explicit TaskManagerTableModel(TaskManagerModel* model)
       : model_(model),
         observer_(NULL) {
-    model->SetObserver(this);
+    model_->AddObserver(this);
+  }
+
+  ~TaskManagerTableModel() {
+    model_->RemoveObserver(this);
   }
-  ~TaskManagerTableModel() {}
 
   // GroupTableModel.
   int RowCount();
@@ -71,7 +74,7 @@ class TaskManagerTableModel : public views::GroupTableModel,
   virtual void OnItemsRemoved(int start, int length);
 
  private:
-  const TaskManagerModel* model_;
+  TaskManagerModel* model_;
   TableModelObserver* observer_;
 };
 
diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h
index d56a0b0..d691f66 100644
--- a/chrome/common/notification_type.h
+++ b/chrome/common/notification_type.h
@@ -600,14 +600,14 @@ class NotificationType {
     // an ExtensionHost*.
     EXTENSION_HOST_DESTROYED,
 
+    // Sent after an extension render process is created and fully functional.
+    // The details are an ExtensionHost*.
+    EXTENSION_PROCESS_CREATED,
+
     // Sent when extension render process crashes. The details are
     // an ExtensionHost*.
     EXTENSION_PROCESS_CRASHED,
 
-    // Sent after an extension render process is restarted after a crash
-    // and is fully functional. The details are an ExtensionHost*.
-    EXTENSION_PROCESS_RESTORED,
-
     // Sent when the contents or order of toolstrips in the shelf model change.
     EXTENSION_SHELF_MODEL_CHANGED,
 
diff --git a/chrome/test/data/extensions/common/background_page/background.html b/chrome/test/data/extensions/common/background_page/background.html
new file mode 100644
index 0000000..e2a5d5a
--- /dev/null
+++ b/chrome/test/data/extensions/common/background_page/background.html
@@ -0,0 +1,2 @@
+<div class="toolstrip-button">Hello World!</div>
+
diff --git a/chrome/test/data/extensions/common/background_page/manifest.json b/chrome/test/data/extensions/common/background_page/manifest.json
new file mode 100644
index 0000000..074e7c65
--- /dev/null
+++ b/chrome/test/data/extensions/common/background_page/manifest.json
@@ -0,0 +1,6 @@
+{
+  "description": "Extension which has just a background page",
+  "name": "background_page",
+  "background_page": "background.html",
+  "version": "0.1"
+}
\ No newline at end of file
diff --git a/chrome/test/data/extensions/common/one_in_shelf/manifest.json b/chrome/test/data/extensions/common/one_in_shelf/manifest.json
new file mode 100644
index 0000000..f714a17
--- /dev/null
+++ b/chrome/test/data/extensions/common/one_in_shelf/manifest.json
@@ -0,0 +1,8 @@
+{
+  "description": "Extension which has exactly one item in the shelf",
+  "name": "one_in_shelf",
+  "toolstrips": [
+    "shelf.html"
+  ],
+  "version": "0.1"
+}
\ No newline at end of file
diff --git a/chrome/test/data/extensions/common/one_in_shelf/shelf.html b/chrome/test/data/extensions/common/one_in_shelf/shelf.html
new file mode 100644
index 0000000..e2a5d5a
--- /dev/null
+++ b/chrome/test/data/extensions/common/one_in_shelf/shelf.html
@@ -0,0 +1,2 @@
+<div class="toolstrip-button">Hello World!</div>
+