Blow away BackgroundContents when RenderView goes away.

chrome/browser/task_manager/task_manager_browsertest.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/task_manager/task_manager.h"
 
 #include "app/l10n_util.h"
 #include "base/file_path.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/background_contents_service.h"
@@ skipping 16 matching lines @@
 #include "chrome/common/page_transition_types.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
 #include "grit/generated_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
 const FilePath::CharType* kTitle1File = FILE_PATH_LITERAL("title1.html");
 
-class ResourceChangeObserver : public TaskManagerModelObserver {
+class ResourceChangeObserver
+    : public TaskManagerModelObserver,
+      public base::RefCountedThreadSafe<ResourceChangeObserver> {
  public:
   ResourceChangeObserver(const TaskManagerModel* model,
                          int target_resource_count)
       : model_(model),
-        target_resource_count_(target_resource_count) {
+        target_resource_count_(target_resource_count),
+        check_pending_(false) {
   }
 
   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:
+  friend class base::RefCountedThreadSafe<ResourceChangeObserver>;
   void OnResourceChange() {
+    // The task manager can churn resources (for example, when a
+    // BackgroundContents navigates, we remove and re-add the resource to
+    // allow the UI to update properly). So check the resource count via
+    // a task to make sure we aren't triggered by a transient change.

[rafaelw, 2011/01/14 01:04:46]

I'm not familiar with the TaskManager or this test, so forgive me if the
question is silly:

I assume that this is safe because the remove & add that results from a BC
navigation always happens in a single event cycle? even so, this seems a bit
brittle. is there a safer way to approach this?

+    if (check_pending_)
+      return;
+    check_pending_ = true;
+    MessageLoopForUI::current()->PostTask(
+        FROM_HERE,
+        NewRunnableMethod(this, &ResourceChangeObserver::CheckResourceCount));
+  }
+
+  void CheckResourceCount() {
     if (model_->ResourceCount() == target_resource_count_)
       MessageLoopForUI::current()->Quit();
   }
 
   const TaskManagerModel* model_;
   const int target_resource_count_;
+  bool check_pending_;
+};
+
+// Helper class used to wait for a BackgroundContents to finish loading.
+class BackgroundContentsListener : public NotificationObserver {
+ public:
+  explicit BackgroundContentsListener(Profile* profile) {
+    registrar_.Add(this, NotificationType::BACKGROUND_CONTENTS_NAVIGATED,
+                   Source<Profile>(profile));
+  }
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) {
+    // Quit once the BackgroundContents has been loaded.
+    if (type.value == NotificationType::BACKGROUND_CONTENTS_NAVIGATED)
+      MessageLoopForUI::current()->Quit();
+  }
+ private:
+  NotificationRegistrar registrar_;
 };
 
 }  // 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);
+    scoped_refptr<ResourceChangeObserver> observer(
+        new ResourceChangeObserver(model(), target_count));
+    model()->AddObserver(observer.get());
     ui_test_utils::RunMessageLoop();
-    model()->RemoveObserver(&observer);
+    model()->RemoveObserver(observer.get());
+  }
+
+  // Wait for any pending BackgroundContents to finish starting up.
+  void WaitForBackgroundContents() {
+    BackgroundContentsListener listener(browser()->profile());
+    ui_test_utils::RunMessageLoop();
   }
 };
 
 // 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());
@@ skipping 46 matching lines @@
                                   url,
                                   ASCIIToUTF16("background_page"),
                                   application_id);
   WaitForResourceChange(3);
 
   // Close the background contents and verify that we notice.
   service->ShutdownAssociatedBackgroundContents(application_id);
   WaitForResourceChange(2);
 }
 
+IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, KillBGContents) {
+  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.
+  WaitForResourceChange(2);
+
+  // Open a new background contents and make sure we notice that.
+  GURL url(ui_test_utils::GetTestUrl(FilePath(FilePath::kCurrentDirectory),
+                                     FilePath(kTitle1File)));
+
+  BackgroundContentsService* service =
+      browser()->profile()->GetBackgroundContentsService();
+  string16 application_id(ASCIIToUTF16("test_app_id"));
+  service->LoadBackgroundContents(browser()->profile(),
+                                  url,
+                                  ASCIIToUTF16("background_page"),
+                                  application_id);
+  // Wait for the background contents process to finish loading.
+  WaitForBackgroundContents();
+  EXPECT_EQ(3, model()->ResourceCount());
+
+  // Kill the background contents process and verify that it disappears from the
+  // model.
+  bool found = false;
+  for (int i = 0; i < model()->ResourceCount(); ++i) {
+    if (model()->IsBackgroundResource(i)) {
+      TaskManager::GetInstance()->KillProcess(i);
+      found = true;
+      break;
+    }
+  }
+  ASSERT_TRUE(found);
+  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.
   WaitForResourceChange(2);
 
@@ skipping 231 matching lines @@
   WaitForResourceChange(3);
 
   // Check that we get some value for the cache columns.
   DCHECK_NE(model()->GetResourceWebCoreImageCacheSize(2),
             l10n_util::GetStringUTF16(IDS_TASK_MANAGER_NA_CELL_TEXT));
   DCHECK_NE(model()->GetResourceWebCoreScriptsCacheSize(2),
             l10n_util::GetStringUTF16(IDS_TASK_MANAGER_NA_CELL_TEXT));
   DCHECK_NE(model()->GetResourceWebCoreCSSCacheSize(2),
             l10n_util::GetStringUTF16(IDS_TASK_MANAGER_NA_CELL_TEXT));
 }