Fix bugs in the new task manager Impl so that ported old browsertests pass

chrome/browser/task_management/task_manager_browsertest.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_manager/task_manager.h"
 
 #include <stddef.h>
 
 #include "base/files/file_path.h"
 #include "base/macros.h"
@@ skipping 98 matching lines @@
   }
 
   int FindResourceIndex(const base::string16& title) {
     for (int i = 0; i < model_->GetRowCount(); ++i) {
       if (title == model_->GetRowTitle(i))
         return i;
     }
     return -1;
   }
 
-  // TODO(nick, afakhry): Remove this function. https://crbug.com/606963
-  void DisableNewTaskManagerForBrokenTest() {
-    base::CommandLine::ForCurrentProcess()->AppendSwitch(
-        switches::kDisableNewTaskManager);
-  }
-
  protected:
   void SetUpCommandLine(base::CommandLine* command_line) override {
     ExtensionBrowserTest::SetUpCommandLine(command_line);
 
     // Do not launch device discovery process.
     command_line->AppendSwitch(switches::kDisableDeviceDiscoveryNotifications);
   }
 
   void TearDownOnMainThread() override { model_.reset(); }
 
@@ skipping 436 matching lines @@
   int extension_tab = FindResourceIndex(MatchExtension("Foobar"));
   ASSERT_NE(-1, extension_tab);
   ASSERT_NE(-1, model()->GetTabId(extension_tab));
 
   int background_page = FindResourceIndex(MatchExtension("My extension 1"));
   ASSERT_NE(-1, background_page);
   ASSERT_EQ(-1, model()->GetTabId(background_page));
 }
 
 IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeAppTabChanges) {
-  // TODO(nick, afakhry): Broken on new task manager because we show
-  // "Extension: Packaged App Test" instead of "App: Packaged App Test".
-  DisableNewTaskManagerForBrokenTest();
-
   ShowTaskManager();
 
   ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("packaged_app")));
   ExtensionService* service = extensions::ExtensionSystem::Get(
                                   browser()->profile())->extension_service();
   const extensions::Extension* extension =
       service->GetExtensionById(last_loaded_extension_id(), false);
 
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAboutBlankTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyTab()));
@@ skipping 23 matching lines @@
   UnloadExtension(last_loaded_extension_id());
 
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnyApp()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnyExtension()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAboutBlankTab()));
   ASSERT_EQ(1, browser()->tab_strip_model()->count());
 }
 
 IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeAppTab) {
-  // TODO(nick, afakhry): Broken on new task manager because we show
-  // "Extension: Packaged App Test" instead of "App: Packaged App Test".
-  DisableNewTaskManagerForBrokenTest();
-
   ASSERT_TRUE(LoadExtension(
       test_data_dir_.AppendASCII("packaged_app")));
   ExtensionService* service = extensions::ExtensionSystem::Get(
       browser()->profile())->extension_service();
   const extensions::Extension* extension =
       service->GetExtensionById(last_loaded_extension_id(), false);
 
   // Open a new tab to the app's launch URL and make sure we notice that.
   GURL url(extension->GetResourceURL("main.html"));
   AddTabAtIndex(0, url, ui::PAGE_TRANSITION_TYPED);
 
   ShowTaskManager();
 
   ASSERT_NO_FATAL_FAILURE(
       WaitForTaskManagerRows(1, MatchApp("Packaged App Test")));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnyExtension()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyApp()));
 
   // Check that the third entry (main.html) is of type extension and has both
   // a tab contents and an extension.
   int app_tab = FindResourceIndex(MatchApp("Packaged App Test"));
   ASSERT_NE(-1, app_tab);
   ASSERT_NE(-1, model()->GetTabId(app_tab));
 }
 
 IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeHostedAppTabChanges) {
-  // TODO(nick, afakhry): Broken on new task manager because we show
-  // "Tab: Unmodified" instead of "App: ".
-  DisableNewTaskManagerForBrokenTest();
-
   ShowTaskManager();
 
   // The app under test acts on URLs whose host is "localhost",
   // so the URLs we navigate to must have host "localhost".
   host_resolver()->AddRule("*", "127.0.0.1");
   ASSERT_TRUE(embedded_test_server()->Start());
   GURL::Replacements replace_host;
   replace_host.SetHostStr("localhost");
   GURL base_url = embedded_test_server()->GetURL(
       "/extensions/api_test/app_process/");
@@ skipping 24 matching lines @@
   // Now reload and check that the last entry's title now starts with "App:".
   ui_test_utils::NavigateToURL(browser(), url);
 
   // Force the TaskManager to query the title.
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAboutBlankTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyApp()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchApp("Unmodified")));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnyExtension()));
 
-  // Disable extension.
+  // Disable extension and relaod.

[ncarter (slow), 2016/05/18 22:21:01]

reload is misspelled.

[afakhry, 2016/05/18 22:32:49]

Done.

   DisableExtension(last_loaded_extension_id());
+  ui_test_utils::NavigateToURL(browser(), url);

[afakhry, 2016/05/18 21:53:29]

The new task manager doesn't recalculate the titles repeatedly like the old one,
so the task classification in the title won't change unless we force a reload.
Is that OK?

[ncarter (slow), 2016/05/18 22:21:01]

I think that's okay. I imagine the page will still actually think it's an app
until reload, anyhow.

 
   // The hosted app should now show up as a normal "Tab: ".
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(2, MatchAnyTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAboutBlankTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchTab("Unmodified")));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnyExtension()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnyApp()));
 
   // Reload the page.
   ui_test_utils::NavigateToURL(browser(), url);
 
   // No change expected.
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(2, MatchAnyTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAboutBlankTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchTab("Unmodified")));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnyExtension()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnyApp()));

[ncarter (slow), 2016/05/18 22:22:33]

Actually, I think we can remove this last section -- it looks redundant now?

[afakhry, 2016/05/18 22:32:49]

Yes, it it. Done.

 }
 
 IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeHostedAppTabAfterReload) {
-  // TODO(nick, afakhry): This fails on the new task manager (we never
-  // reclassify the tab as an app). Remove when fixed.
-  DisableNewTaskManagerForBrokenTest();
-
   // The app under test acts on URLs whose host is "localhost",
   // so the URLs we navigate to must have host "localhost".
   host_resolver()->AddRule("*", "127.0.0.1");
   ASSERT_TRUE(embedded_test_server()->Start());
   GURL::Replacements replace_host;
   replace_host.SetHostStr("localhost");
   GURL base_url =
       embedded_test_server()->GetURL("/extensions/api_test/app_process/");
   base_url = base_url.ReplaceComponents(replace_host);
 
@@ skipping 158 matching lines @@
       minimal_heap_size));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerStatToExceed(
       MatchUtility(proxy_resolver_name), ColumnSpecifier::V8_MEMORY_USED,
       minimal_heap_size));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyUtility()));
   ASSERT_NO_FATAL_FAILURE(
       WaitForTaskManagerRows(1, MatchUtility(proxy_resolver_name)));
 }
 
 IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, DevToolsNewDockedWindow) {
-  // TODO(nick, afakhry): Broken on new task manager because we show
-  // a long chrome-devtools:// URL without a prefix (expecting "Tab: *").
-  DisableNewTaskManagerForBrokenTest();
-
   ShowTaskManager();  // Task manager shown BEFORE dev tools window.
 
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyTab()));
   DevToolsWindow* devtools =
       DevToolsWindowTesting::OpenDevToolsWindowSync(browser(), true);
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(2, MatchAnyTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(2, MatchAnyTab()));
   DevToolsWindowTesting::CloseDevToolsWindowSync(devtools);
 }
 
 IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, DevToolsNewUndockedWindow) {
-  // TODO(nick, afakhry): Broken on new task manager because we show
-  // a long chrome-devtools:// URL without a prefix (expecting "Tab: *").
-  DisableNewTaskManagerForBrokenTest();
-
   ShowTaskManager();  // Task manager shown BEFORE dev tools window.
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyTab()));
   DevToolsWindow* devtools =
       DevToolsWindowTesting::OpenDevToolsWindowSync(browser(), false);
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(3, MatchAnyTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(3, MatchAnyTab()));
   DevToolsWindowTesting::CloseDevToolsWindowSync(devtools);
 }
 
 IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, DevToolsOldDockedWindow) {
-  // TODO(nick, afakhry): Broken on new task manager because we show
-  // a long chrome-devtools:// URL without a prefix (expecting "Tab: *").
-  DisableNewTaskManagerForBrokenTest();
-
   DevToolsWindow* devtools =
       DevToolsWindowTesting::OpenDevToolsWindowSync(browser(), true);
   ShowTaskManager();  // Task manager shown AFTER dev tools window.
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(2, MatchAnyTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(2, MatchAnyTab()));
   DevToolsWindowTesting::CloseDevToolsWindowSync(devtools);
 }
 
 IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, DevToolsOldUndockedWindow) {
-  // TODO(nick, afakhry): Broken on new task manager because we show
-  // a long chrome-devtools:// URL without a prefix (expecting "Tab: *").
-  DisableNewTaskManagerForBrokenTest();
-
   DevToolsWindow* devtools =
       DevToolsWindowTesting::OpenDevToolsWindowSync(browser(), false);
   ShowTaskManager();  // Task manager shown AFTER dev tools window.
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(3, MatchAnyTab()));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(3, MatchAnyTab()));
   DevToolsWindowTesting::CloseDevToolsWindowSync(devtools);
 }
 
 IN_PROC_BROWSER_TEST_P(TaskManagerOOPIFBrowserTest, KillSubframe) {
   ShowTaskManager();
@@ skipping 265 matching lines @@
       WaitForTaskManagerRows(1, MatchTab("Title Of Awesomeness")));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnySubframe()));
 
   HideTaskManager();
   ShowTaskManager();
 
   ASSERT_NO_FATAL_FAILURE(
       WaitForTaskManagerRows(1, MatchTab("Title Of Awesomeness")));
   ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnySubframe()));
 }