Remove the old task manager view

chrome/browser/task_management/task_manager_tester.cc

 // Copyright 2016 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_management/task_manager_tester.h"
 
 #include "base/memory/ptr_util.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_manager/legacy_task_manager_tester.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/task_manager/task_manager_table_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/grit/generated_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/models/table_model_observer.h"
 
+#if defined(OS_MACOSX)
+#include "chrome/browser/task_manager/legacy_task_manager_tester.h"
+#endif  // defined(OS_MACOSX)
+
 namespace task_management {
 
 namespace {
 
-// Returns whether chrome::ShowTaskManager() will, for the current platform and
-// command line, show a view backed by a task_management::TaskManagerTableModel.
-bool IsNewTaskManagerViewEnabled() {
-#if defined(OS_MACOSX)
-  if (!chrome::ToolkitViewsDialogsEnabled())
-    return false;
-#endif
-  return switches::NewTaskManagerEnabled();
-}
-
 // Temporarily intercepts the calls between a TableModel and its Observer,
 // running |callback| whenever anything happens.
 class ScopedInterceptTableModelObserver : public ui::TableModelObserver {
  public:
   ScopedInterceptTableModelObserver(
       ui::TableModel* model_to_intercept,
       ui::TableModelObserver* real_table_model_observer,
       const base::Closure& callback)
       : model_to_intercept_(model_to_intercept),
         real_table_model_observer_(real_table_model_observer),
@@ skipping 105 matching lines @@
 
   void Kill(int row) override { model_->KillTask(row); }
 
  private:
   task_management::TaskManagerInterface* task_manager() {
     return model_->observed_task_manager();
   }
 
   // Returns the TaskManagerTableModel for the the visible NewTaskManagerView.
   static task_management::TaskManagerTableModel* GetRealModel() {
-    CHECK(IsNewTaskManagerViewEnabled());
     // This downcast is safe, as long as the new task manager is enabled.
     task_management::TaskManagerTableModel* result =
         static_cast<task_management::TaskManagerTableModel*>(
             chrome::ShowTaskManager(nullptr));
     return result;
   }
 
   task_management::TaskManagerTableModel* model_;
   std::unique_ptr<ScopedInterceptTableModelObserver> interceptor_;
 };
 
 // static
 std::unique_ptr<TaskManagerTester> TaskManagerTester::Create(
     const base::Closure& callback) {
-  if (IsNewTaskManagerViewEnabled())
-    return base::WrapUnique(new TaskManagerTesterImpl(callback));
-  else
+#if defined(OS_MACOSX)
+  if (!TaskManagerInterface::IsNewTaskManagerEnabled())
     return task_manager::CreateLegacyTaskManagerTester(callback);

[afakhry, 2016/05/20 02:32:50]

I suspect that this might cause problems since the
legacy_task_manager_tester.cc/h are part of the cocoa_sources. Any suggestions?

[tapted, 2016/05/20 09:52:33]

Hm - you're quite right. For some reason when I locally built I didn't get a
link error here, but I did when I ran a tryjob in
https://codereview.chromium.org/1924683003/ . So unfortunately I ran out of time
to probe this further.

I think part of the fix is just to create a version of TaskManagerTester::Create
in legacy_task_manager_tester.cc - one that's just for Cocoa.

Then I .. think this file can just be omitted from Cocoa builds altogether? The
stuff here between #if MAC can just be removed.

Anyway, you should be able to test it out but uploading a CL branched of this
that looks just like https://codereview.chromium.org/1924683003/#ps260001 - if
that gets a green 'compile' run (don't worry about the rest), then I don't think
I have any other objections.

[tapted, 2016/05/23 04:34:56]

OK - I've updated https://codereview.chromium.org/1924683003/#ps320001 with a
fix. If you patch that in (minus the common.gypi change), I think everything
will be good.

[afakhry, 2016/05/23 18:21:52]

You need to build browsertests in order to test this.

I tried to fix this by adding a new file:
legacy_task_manager_tester_mac_views.cc which has a default implementation of
task_manager::CreateLegacyTaskManagerTester(). This file is added to the
chrome_browser_tests_views_non_mac_sources.

Let's see what the trybots will say.

[afakhry, 2016/05/23 18:21:52]

Done.

+#endif  // defined(OS_MACOSX)
+
+  return base::WrapUnique(new TaskManagerTesterImpl(callback));
 }
 
 }  // namespace task_management