Make old task manager tests work against new task manager.

chrome/browser/task_manager/task_manager_browsertest_util.cc

Index: chrome/browser/task_manager/task_manager_browsertest_util.cc
diff --git a/chrome/browser/task_manager/task_manager_browsertest_util.cc b/chrome/browser/task_manager/task_manager_browsertest_util.cc
index edeccd0f4a2dc840a22a4dd3433dfa79aa03c7b7..3f8a2030dc1dbf0eadc0208ff127476c06eb2faf 100644
--- a/chrome/browser/task_manager/task_manager_browsertest_util.cc
+++ b/chrome/browser/task_manager/task_manager_browsertest_util.cc
@@ -4,8 +4,10 @@
 
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
 
+#include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/location.h"
+#include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/pattern.h"
@@ -17,40 +19,221 @@
 #include "base/timer/timer.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/sessions/session_tab_helper.h"
+#include "chrome/browser/task_management/task_manager_interface.h"
+#include "chrome/browser/task_management/task_manager_observer.h"
 #include "chrome/browser/task_manager/resource_provider.h"
 #include "chrome/browser/task_manager/task_manager.h"
+#include "chrome/browser/ui/task_manager/task_manager_table_model.h"
+#include "chrome/browser/ui/views/new_task_manager_view.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/grit/generated_resources.h"
 #include "extensions/strings/grit/extensions_strings.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
 
+// TODO(nick): Move everything here (except for LegacyTaskManagerTesterImpl)
+// into the task_management namespace.
+namespace task_management {
+
+class TaskManagerTesterImpl
+    : public task_manager::browsertest_util::TaskManagerTester,
+      public TaskManagerObserver {
+ public:
+  // TODO(nick): We have a couple ways we could implement this class:
+  // (1) Grab the view's model directly.

[ncarter (slow), 2016/04/27 17:17:45]

This is what the code does today. It gets the job done, but has the following
issue:

 - #including ui/views from here is forbidden by checkdeps -- I assume for good
reason? (Is that even compiled on the mac?)

 - We need friend declarations in both the model and the view. All we really
need the view for, is to hijack its TaskManagerTableModel instance. (What we
really want, I guess, is a method like
TaskManagerTableModel::GetInstanceForTests())

 - We still have to implement TaskManagerObserver, because we don't have another
way to listen for updates. This is kind of lame.

 - The main reason I want to use the real Model, is so that we are looking at
rows that match what is being painted on the screen. E.g. for tests that look at
V8 memory, I'd like the visible TaskManager to actually show the columns.
However, we bypass the model anyways when grabbing the stats, because all we get
from the model is text, and parsing that out to a number seems like a bad idea.

[afakhry, 2016/04/28 00:03:13]

I think this is the best approach since we are actually testing the real model.
I like the way you implemented it in the last patch.

+  // (2) Instantiate our own TaskManagerTableModel instance and ignore

[ncarter (slow), 2016/04/27 17:17:45]

I think this is probably the best approach for what I'm trying to accomplish.

The main downside I see is that the visibility state of the real TaskManager
becomes totally irrelevant to the functioning of the tests (and a good many of
the tests occur in pairs, that only differ because of show/hide behavior. This
scenario matters so much less in the new TM, since we have the tags approach, so
maybe this is just okay.)

I guess this also means we'd need to provide a stub implementation of
TableViewDelegate? How hard will that be?

[afakhry, 2016/04/28 00:03:13]

Your interceptor approach is better.

+  //     the "real" view. This would be pretty clean, but it makes the
+  //     chrome::ShowTaskManager() steps of the tests kind of lame.
+  // (3) Some hybrid of the above (where we cross validate the contents)
+  //     of the real table model
+  // (4) Make TaskManagerTableModel support multiple observers, and

[ncarter (slow), 2016/04/27 17:17:45]

This turns out to be pretty gross, since SetObserver() is dictated by
ui::TableModel. I could still back it with an ObserverList internally, and
provide a {Add|Remove}ObserverForTesting() private methods for friends, but it's
more intrusive than I had imagined.

[afakhry, 2016/04/28 00:03:13]

Yes, this doesn't sound like a clean solution.

+  //     observe the real view's model.
+  explicit TaskManagerTesterImpl(const base::Closure& on_resource_change)
+      : TaskManagerObserver(NewTaskManagerView::GetInstanceForTests()
+                                ->table_model_->desired_refresh_time(),
+                            NewTaskManagerView::GetInstanceForTests()
+                                ->table_model_->desired_resources_flags()),
+        on_resource_change_(on_resource_change),
+        model_(NewTaskManagerView::GetInstanceForTests()->table_model_.get()) {
+    model_->observed_task_manager()->AddObserver(this);
+  }
+
+  ~TaskManagerTesterImpl() override {
+    observed_task_manager()->RemoveObserver(this);
+  }
+
+  int GetRowCount() override { return model_->RowCount(); }
+
+  base::string16 GetRowTitle(int row) override {
+    return model_->GetText(row, IDS_TASK_MANAGER_TASK_COLUMN);
+  }
+
+  void ToggleColumnVisibility(
+      task_manager::browsertest_util::ColumnSpecifier column) override {
+    int column_id = 0;
+    switch (column) {
+      case task_manager::browsertest_util::COLUMN_NONE:
+        return;
+      case task_manager::browsertest_util::SQLITE_MEMORY_USED:
+        column_id = IDS_TASK_MANAGER_SQLITE_MEMORY_USED_COLUMN;
+        break;
+      case task_manager::browsertest_util::V8_MEMORY_USED:
+      case task_manager::browsertest_util::V8_MEMORY:
+        column_id = IDS_TASK_MANAGER_JAVASCRIPT_MEMORY_ALLOCATED_COLUMN;
+        break;
+    }
+    model_->ToggleColumnVisibility(column_id);
+  }
+
+  int64_t GetColumnValue(task_manager::browsertest_util::ColumnSpecifier column,
+                         int row) override {
+    TaskId task_id = model_->tasks_[row];
+    int64_t value = 0;
+    int64_t ignored = 0;
+    bool success = false;
+
+    switch (column) {
+      case task_manager::browsertest_util::COLUMN_NONE:
+        break;
+      case task_manager::browsertest_util::V8_MEMORY:
+        success =
+            observed_task_manager()->GetV8Memory(task_id, &value, &ignored);
+        break;
+      case task_manager::browsertest_util::V8_MEMORY_USED:
+        success =
+            observed_task_manager()->GetV8Memory(task_id, &ignored, &value);
+        break;
+      case task_manager::browsertest_util::SQLITE_MEMORY_USED:
+        value = observed_task_manager()->GetSqliteMemoryUsed(task_id);
+        success = true;
+        break;
+    }
+    if (!success)
+      return 0;
+    return value;
+  }
+
+  int32_t GetTabId(int row) override {
+    TaskId task_id = model_->tasks_[row];
+    return observed_task_manager()->GetTabId(task_id);
+  }
+
+  void Kill(int row) override { model_->KillTask(row); }
+
+  // TaskManagerObserver
+  void OnTaskAdded(TaskId id) override { on_resource_change_.Run(); }
+
+  void OnTaskToBeRemoved(TaskId id) override { on_resource_change_.Run(); }
+
+  void OnTasksRefreshed(const TaskIdList& task_ids) override {
+    on_resource_change_.Run();
+  }
+
+  void OnTasksRefreshedWithBackgroundCalculations(
+      const TaskIdList& task_ids) override {
+    on_resource_change_.Run();
+  }
+
+  void OnTaskUnresponsive(TaskId id) override { on_resource_change_.Run(); }
+
+ private:
+  base::Closure on_resource_change_;
+  TaskManagerTableModel* model_;
+};
+
+}  // namespace task_management
+
 namespace task_manager {
 namespace browsertest_util {
 
 namespace {
 
-class ResourceChangeObserver : public TaskManagerModelObserver {
+class LegacyTaskManagerTesterImpl : public TaskManagerTester,
+                                    public TaskManagerModelObserver {
  public:
-  ResourceChangeObserver(const TaskManagerModel* model,
-                         int required_count,
-                         const base::string16& title_pattern,
-                         ColumnSpecifier column_specifier,
-                         size_t min_column_value)
-      : model_(model),
-        required_count_(required_count),
-        title_pattern_(title_pattern),
-        column_specifier_(column_specifier),
-        min_column_value_(min_column_value) {}
+  explicit LegacyTaskManagerTesterImpl(const base::Closure& on_resource_change)
+      : on_resource_change_(on_resource_change),
+        model_(TaskManager::GetInstance()->model()) {
+    model_->AddObserver(this);
+  }
+  ~LegacyTaskManagerTesterImpl() override { model_->RemoveObserver(this); }
 
-  void OnModelChanged() override { OnResourceChange(); }
+  // TaskManagerTester:
+  int GetRowCount() override { return model_->ResourceCount(); }
 
-  void OnItemsChanged(int start, int length) override { OnResourceChange(); }
+  base::string16 GetRowTitle(int row) override {
+    return model_->GetResourceTitle(row);
+  }
 
-  void OnItemsAdded(int start, int length) override { OnResourceChange(); }
+  int64_t GetColumnValue(ColumnSpecifier column, int row) override {
+    size_t value = 0;
+    bool success = false;
+    switch (column) {
+      case COLUMN_NONE:
+        break;
+      case V8_MEMORY:
+        success = model_->GetV8Memory(row, &value);
+        break;
+      case V8_MEMORY_USED:
+        success = model_->GetV8MemoryUsed(row, &value);
+        break;
+      case SQLITE_MEMORY_USED:
+        success = model_->GetSqliteMemoryUsedBytes(row, &value);
+        break;
+    }
+    if (!success)
+      return 0;
+    return static_cast<int64_t>(value);
+  }
+
+  void ToggleColumnVisibility(ColumnSpecifier column) override {
+    // Doing nothing is okay here; the legacy TaskManager always collects all
+    // stats.
+  }
+
+  int32_t GetTabId(int row) override {
+    if (model_->GetResourceWebContents(row)) {
+      return SessionTabHelper::IdForTab(model_->GetResourceWebContents(row));
+    }
+    return -1;
+  }
 
+  void Kill(int row) override { TaskManager::GetInstance()->KillProcess(row); }
+
+  // TaskManagerModelObserver:
+  void OnModelChanged() override { OnResourceChange(); }
+  void OnItemsChanged(int start, int length) override { OnResourceChange(); }
+  void OnItemsAdded(int start, int length) override { OnResourceChange(); }
   void OnItemsRemoved(int start, int length) override { OnResourceChange(); }
 
+ private:
+  void OnResourceChange() { on_resource_change_.Run(); }
+  base::Closure on_resource_change_;
+  TaskManagerModel* model_;
+};
+
+class ResourceChangeObserver {
+ public:
+  ResourceChangeObserver(int required_count,
+                         const base::string16& title_pattern,
+                         ColumnSpecifier column_specifier,
+                         size_t min_column_value)
+      : required_count_(required_count),
+        title_pattern_(title_pattern),
+        column_specifier_(column_specifier),
+        min_column_value_(min_column_value) {
+    base::Closure on_resource_change = base::Bind(
+        &ResourceChangeObserver::OnResourceChange, base::Unretained(this));
+
+    if (switches::NewTaskManagerEnabled()) {
+      model_.reset(
+          new task_management::TaskManagerTesterImpl(on_resource_change));
+    } else {
+      model_.reset(new LegacyTaskManagerTesterImpl(on_resource_change));
+    }
+  }
+
   void RunUntilSatisfied() {
     // See if the condition is satisfied without having to run the loop. This
     // check has to be placed after the installation of the
@@ -91,8 +274,8 @@ class ResourceChangeObserver : public TaskManagerModelObserver {
 
   int CountMatches() {
     int match_count = 0;
-    for (int i = 0; i < model_->ResourceCount(); i++) {
-      if (!base::MatchPattern(model_->GetResourceTitle(i), title_pattern_))
+    for (int i = 0; i < model_->GetRowCount(); i++) {
+      if (!base::MatchPattern(model_->GetRowTitle(i), title_pattern_))
         continue;
 
       if (GetColumnValue(i) < min_column_value_)
@@ -103,25 +286,8 @@ class ResourceChangeObserver : public TaskManagerModelObserver {
     return match_count;
   }
 
-  size_t GetColumnValue(int index) {
-    size_t value = 0;
-    bool success = false;
-    switch (column_specifier_) {
-      case COLUMN_NONE:
-        break;
-      case V8_MEMORY:
-        success = model_->GetV8Memory(index, &value);
-        break;
-      case V8_MEMORY_USED:
-        success = model_->GetV8MemoryUsed(index, &value);
-        break;
-      case SQLITE_MEMORY_USED:
-        success = model_->GetSqliteMemoryUsedBytes(index, &value);
-        break;
-    }
-    if (!success)
-      return 0;
-    return value;
+  int64_t GetColumnValue(int index) {
+    return model_->GetColumnValue(column_specifier_, index);
   }
 
   const char* GetColumnName() {
@@ -156,10 +322,9 @@ class ResourceChangeObserver : public TaskManagerModelObserver {
     task_manager_state_dump << "\nCurrently there are " << CountMatches()
                             << " matches.";
     task_manager_state_dump << "\nCurrent Task Manager Model is:";
-    for (int i = 0; i < model_->ResourceCount(); i++) {
-      task_manager_state_dump
-          << "\n  > " << std::setw(40) << std::left
-          << base::UTF16ToASCII(model_->GetResourceTitle(i));
+    for (int i = 0; i < model_->GetRowCount(); i++) {
+      task_manager_state_dump << "\n  > " << std::setw(40) << std::left
+                              << base::UTF16ToASCII(model_->GetRowTitle(i));
       if (min_column_value_ > 0) {
         task_manager_state_dump << " [" << GetColumnName()
                                 << " == " << GetColumnValue(i) << "]";
@@ -168,45 +333,42 @@ class ResourceChangeObserver : public TaskManagerModelObserver {
     return task_manager_state_dump;
   }
 
-  const TaskManagerModel* model_;
+  std::unique_ptr<TaskManagerTester> model_;
   const int required_count_;
   const base::string16 title_pattern_;
   const ColumnSpecifier column_specifier_;
-  const size_t min_column_value_;
+  const int64_t min_column_value_;
   base::RunLoop run_loop_;
   base::OneShotTimer timer_;
 };
 
 }  // namespace
 
-void EnableOldTaskManager() {
-  base::CommandLine::ForCurrentProcess()->AppendSwitch(
-      switches::kDisableNewTaskManager);
+std::unique_ptr<TaskManagerTester> GetTaskManagerTester() {
+  base::Closure do_nothing = base::Bind(&base::DoNothing);
+  if (switches::NewTaskManagerEnabled()) {
+    return base::WrapUnique(
+        new task_management::TaskManagerTesterImpl(do_nothing));
+  } else {
+    return base::WrapUnique(new LegacyTaskManagerTesterImpl(do_nothing));
+  }
 }
 
 void WaitForTaskManagerRows(int required_count,
                             const base::string16& title_pattern) {
-  TaskManagerModel* model = TaskManager::GetInstance()->model();
-
   const int column_value_dont_care = 0;
-  ResourceChangeObserver observer(model, required_count, title_pattern,
-                                  COLUMN_NONE, column_value_dont_care);
-  model->AddObserver(&observer);
+  ResourceChangeObserver observer(required_count, title_pattern, COLUMN_NONE,
+                                  column_value_dont_care);
   observer.RunUntilSatisfied();
-  model->RemoveObserver(&observer);
 }
 
 void WaitForTaskManagerStatToExceed(const base::string16& title_pattern,
                                     ColumnSpecifier column_getter,
                                     size_t min_column_value) {
-  TaskManagerModel* model = TaskManager::GetInstance()->model();
-
   const int wait_for_one_match = 1;
-  ResourceChangeObserver observer(model, wait_for_one_match, title_pattern,
+  ResourceChangeObserver observer(wait_for_one_match, title_pattern,
                                   column_getter, min_column_value);
-  model->AddObserver(&observer);
   observer.RunUntilSatisfied();
-  model->RemoveObserver(&observer);
 }
 
 base::string16 MatchTab(const char* title) {