Fix some problems with TaskManagerBrowserTest.PopulateWebCacheFields:

- wait for an actual update to occur; otherwise we're not testing the real thing
- change DCHECKs to EXPECT_EQs; that's what we should use in tests

This change should also fix the crashiness of this test.

TEST=browser_tests
BUG=42301

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

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

6 files changed, 40 insertions, 18 deletions

diff --git a/chrome/browser/task_manager.cc b/chrome/browser/task_manager.cc
index 8677620..79d2e9d 100644
--- a/chrome/browser/task_manager.cc
+++ b/chrome/browser/task_manager.cc
@@ -24,6 +24,7 @@
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/task_manager_resource_providers.h"
+#include "chrome/common/notification_service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "grit/app_resources.h"
@@ -179,7 +180,7 @@ std::wstring TaskManagerModel::GetResourceGoatsTeleported(int index) const {
 std::wstring TaskManagerModel::GetResourceWebCoreImageCacheSize(
     int index) const {
   DCHECK(index < ResourceCount());
-  if (!resources_[index]->ReportsCacheStats())
+  if (!resources_[index]->HasCacheStats())
     return l10n_util::GetString(IDS_TASK_MANAGER_NA_CELL_TEXT);
   const WebKit::WebCache::ResourceTypeStats stats(
       resources_[index]->GetWebCoreCacheStats());
@@ -189,7 +190,7 @@ std::wstring TaskManagerModel::GetResourceWebCoreImageCacheSize(
 std::wstring TaskManagerModel::GetResourceWebCoreScriptsCacheSize(
     int index) const {
   DCHECK(index < ResourceCount());
-  if (!resources_[index]->ReportsCacheStats())
+  if (!resources_[index]->HasCacheStats())
     return l10n_util::GetString(IDS_TASK_MANAGER_NA_CELL_TEXT);
   const WebKit::WebCache::ResourceTypeStats stats(
       resources_[index]->GetWebCoreCacheStats());
@@ -199,7 +200,7 @@ std::wstring TaskManagerModel::GetResourceWebCoreScriptsCacheSize(
 std::wstring TaskManagerModel::GetResourceWebCoreCSSCacheSize(
     int index) const {
   DCHECK(index < ResourceCount());
-  if (!resources_[index]->ReportsCacheStats())
+  if (!resources_[index]->HasCacheStats())
     return l10n_util::GetString(IDS_TASK_MANAGER_NA_CELL_TEXT);
   const WebKit::WebCache::ResourceTypeStats stats(
       resources_[index]->GetWebCoreCacheStats());
@@ -338,9 +339,9 @@ int TaskManagerModel::CompareValues(int row1, int row2, int col_id) const {
     case IDS_TASK_MANAGER_WEBCORE_CSS_CACHE_COLUMN: {
       WebKit::WebCache::ResourceTypeStats stats1 = { { 0 } };
       WebKit::WebCache::ResourceTypeStats stats2 = { { 0 } };
-      if (resources_[row1]->ReportsCacheStats())
+      if (resources_[row1]->HasCacheStats())
         stats1 = resources_[row1]->GetWebCoreCacheStats();
-      if (resources_[row2]->ReportsCacheStats())
+      if (resources_[row2]->HasCacheStats())
         stats2 = resources_[row2]->GetWebCoreCacheStats();
       if (IDS_TASK_MANAGER_WEBCORE_IMAGE_CACHE_COLUMN == col_id)
         return ValueCompare<size_t>(stats1.images.size, stats2.images.size);
@@ -687,6 +688,10 @@ void TaskManagerModel::NotifyResourceTypeStats(
       (*it)->NotifyResourceTypeStats(stats);
     }
   }
+  NotificationService::current()->Notify(
+      NotificationType::TASK_MANAGER_RESOURCE_TYPE_STATS_UPDATED,
+      Source<TaskManagerModel>(this),
+      NotificationService::NoDetails());
 }
 
 void TaskManagerModel::NotifyV8HeapStats(base::ProcessId renderer_id,
diff --git a/chrome/browser/task_manager.h b/chrome/browser/task_manager.h
index 4a92582..d078cb4 100644
--- a/chrome/browser/task_manager.h
+++ b/chrome/browser/task_manager.h
@@ -45,7 +45,7 @@ class TaskManager {
     virtual SkBitmap GetIcon() const = 0;
     virtual base::ProcessHandle GetProcess() const = 0;
 
-    virtual bool ReportsCacheStats() const { return false; }
+    virtual bool HasCacheStats() const { return false; }
     virtual WebKit::WebCache::ResourceTypeStats GetWebCoreCacheStats() const {
       return WebKit::WebCache::ResourceTypeStats();
     }
diff --git a/chrome/browser/task_manager_browsertest.cc b/chrome/browser/task_manager_browsertest.cc
index 8751db0..1712468 100644
--- a/chrome/browser/task_manager_browsertest.cc
+++ b/chrome/browser/task_manager_browsertest.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/tab_contents/infobar_delegate.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/notification_type.h"
 #include "chrome/common/page_transition_types.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
@@ -235,9 +236,7 @@ IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, DISABLED_ReloadExtension) {
   WaitForResourceChange(3);
 }
 
-// Crashy, http://crbug.com/42301.
-IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest,
-                       DISABLED_PopulateWebCacheFields) {
+IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, PopulateWebCacheFields) {
   EXPECT_EQ(0, model()->ResourceCount());
 
   // Show the task manager. This populates the model, and helps with debugging
@@ -254,11 +253,19 @@ IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest,
                            TabStripModel::ADD_SELECTED, NULL, std::string());
   WaitForResourceChange(3);
 
+  // Make sure that we have received updated stats from the renderer. An update
+  // could be pending before we added the new tab, so wait for two updates.
+  // This way at least one of them must have occurred after adding the tab.
+  ui_test_utils::WaitForNotification(
+      NotificationType::TASK_MANAGER_RESOURCE_TYPE_STATS_UPDATED);
+  ui_test_utils::WaitForNotification(
+      NotificationType::TASK_MANAGER_RESOURCE_TYPE_STATS_UPDATED);
+
   // Check that we get some value for the cache columns.
-  DCHECK_NE(model()->GetResourceWebCoreImageCacheSize(2),
+  EXPECT_NE(model()->GetResourceWebCoreImageCacheSize(2),
             l10n_util::GetString(IDS_TASK_MANAGER_NA_CELL_TEXT));
-  DCHECK_NE(model()->GetResourceWebCoreScriptsCacheSize(2),
+  EXPECT_NE(model()->GetResourceWebCoreScriptsCacheSize(2),
             l10n_util::GetString(IDS_TASK_MANAGER_NA_CELL_TEXT));
-  DCHECK_NE(model()->GetResourceWebCoreCSSCacheSize(2),
+  EXPECT_NE(model()->GetResourceWebCoreCSSCacheSize(2),
             l10n_util::GetString(IDS_TASK_MANAGER_NA_CELL_TEXT));
 }
diff --git a/chrome/browser/task_manager_resource_providers.cc b/chrome/browser/task_manager_resource_providers.cc
index d3fa808..65b107a 100644
--- a/chrome/browser/task_manager_resource_providers.cc
+++ b/chrome/browser/task_manager_resource_providers.cc
@@ -54,6 +54,7 @@ TaskManagerTabContentsResource::TaskManagerTabContentsResource(
     TabContents* tab_contents)
     : tab_contents_(tab_contents),
       pending_stats_update_(false),
+      had_stats_update_(false),
       v8_memory_allocated_(0),
       v8_memory_used_(0),
       pending_v8_memory_allocated_update_(false) {
@@ -61,11 +62,7 @@ TaskManagerTabContentsResource::TaskManagerTabContentsResource(
   // becomes NULL and the TaskManager still needs it.
   process_ = tab_contents_->GetRenderProcessHost()->GetHandle();
   pid_ = base::GetProcId(process_);
-  stats_.images.size = 0;
-  stats_.cssStyleSheets.size = 0;
-  stats_.scripts.size = 0;
-  stats_.xslStyleSheets.size = 0;
-  stats_.fonts.size = 0;
+  memset(&stats_, 0, sizeof(stats_));
 }
 
 TaskManagerTabContentsResource::~TaskManagerTabContentsResource() {
@@ -122,6 +119,7 @@ void TaskManagerTabContentsResource::NotifyResourceTypeStats(
     const WebKit::WebCache::ResourceTypeStats& stats) {
   stats_ = stats;
   pending_stats_update_ = false;
+  had_stats_update_ = true;
 }
 
 void TaskManagerTabContentsResource::NotifyV8HeapStats(
diff --git a/chrome/browser/task_manager_resource_providers.h b/chrome/browser/task_manager_resource_providers.h
index b96083c..2029a54 100644
--- a/chrome/browser/task_manager_resource_providers.h
+++ b/chrome/browser/task_manager_resource_providers.h
@@ -35,7 +35,7 @@ class TaskManagerTabContentsResource : public TaskManager::Resource {
   base::ProcessHandle GetProcess() const;
   TabContents* GetTabContents() const;
 
-  virtual bool ReportsCacheStats() const { return true; }
+  virtual bool HasCacheStats() const { return had_stats_update_; }
   virtual WebKit::WebCache::ResourceTypeStats GetWebCoreCacheStats() const;
 
   virtual bool ReportsV8MemoryStats() const { return true; }
@@ -58,12 +58,18 @@ class TaskManagerTabContentsResource : public TaskManager::Resource {
   TabContents* tab_contents_;
   base::ProcessHandle process_;
   int pid_;
+
   // The stats_ field holds information about resource usage in the renderer
   // process and so it is updated asynchronously by the Refresh() call.
   WebKit::WebCache::ResourceTypeStats stats_;
+
   // This flag is true if we are waiting for the renderer to report its stats.
   bool pending_stats_update_;
 
+  // This flag is true after we received at least one stats report
+  // from the renderer.
+  bool had_stats_update_;
+
   // We do a similar dance to gather the V8 memory usage in a process.
   size_t v8_memory_allocated_;
   size_t v8_memory_used_;
diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h
index a30fb27..87d1550 100644
--- a/chrome/common/notification_type.h
+++ b/chrome/common/notification_type.h
@@ -1051,6 +1051,12 @@ class NotificationType {
     // TokenRequestFailedDetails object.
     TOKEN_REQUEST_FAILED,
 
+    // Task Manager ------------------------------------------------------------
+
+    // Sent after the Task Manager has received updated ResourceTypeStats
+    // from a renderer. Source is a TaskManagerModel.
+    TASK_MANAGER_RESOURCE_TYPE_STATS_UPDATED,
+
     // Misc --------------------------------------------------------------------
 
 #if defined(OS_CHROMEOS)