New Task Manager - Phase 1.3.2.B: Implement Tab Contents Task Providing (Prerender)

chrome/browser/prerender/prerender_browsertest.cc

Index: chrome/browser/prerender/prerender_browsertest.cc
diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc
index 7b742f03bd6e354df0f5faaac73475686f9834d7..7f98eb86f2d38362cf61ba5604f3a3bd70c24829 100644
--- a/chrome/browser/prerender/prerender_browsertest.cc
+++ b/chrome/browser/prerender/prerender_browsertest.cc
@@ -48,6 +48,9 @@
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/safe_browsing_util.h"
 #include "chrome/browser/safe_browsing/test_database_manager.h"
+#include "chrome/browser/task_management/providers/task_provider_observer.h"
+#include "chrome/browser/task_management/providers/web_contents/web_contents_tags_manager.h"
+#include "chrome/browser/task_management/providers/web_contents/web_contents_task_provider.h"
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
 #include "chrome/browser/ui/browser.h"
@@ -4080,4 +4083,101 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTestWithNaCl,
 }
 #endif  // !defined(DISABLE_NACL)
 
+#if defined(ENABLE_TASK_MANAGER)
+
+// Defines a test class for testing that prerender WebContents are correctly
+// tagged by their task-manager specific tag. It also tests that a task-manager
+// PrerenderTask will be correctly recorded.
+class PrerenderTagTest
+    : public PrerenderBrowserTest,

[ncarter (slow), 2015/06/19 19:50:51]

I would probably structure this class as a helper utility you create on the
stack as needed, rather than extending the test fixture. This class should
derive only from TaskProviderObserver, and get a name like a MockTaskManager.
Tests can then just instantiate it at the top of their test bodies.

Since there's nothing prerender-specific here, you could also put this in a
browsertest_util.h file and use it elsewhere.

[afakhry, 2015/06/19 21:49:21]

Done.

+      public task_management::TaskProviderObserver {
+ public:
+  PrerenderTagTest() {}
+  ~PrerenderTagTest() override {}
+
+  // task_management::Task_providerObserver:
+  void TaskAdded(task_management::Task* task) override {
+    CHECK(task);
+    ASSERT_FALSE(provided_tasks_.count(task));
+    provided_tasks_.insert(task);

[ncarter (slow), 2015/06/19 19:50:50]

FWIW (this is just an idea / food for thought) I've had very good luck with the
pattern of having test observer classes like this create a text log of what
happens, and writing ASSERTs in terms of those test logs. It's a lame version of
gmock; which would work well for this too, though a lot of people find gmock to
be a headache.

An example of what I'm talking about is here:
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

+  }
+
+  void TaskRemoved(task_management::Task* task) override {
+    CHECK(task);
+    ASSERT_TRUE(provided_tasks_.count(task));
+    provided_tasks_.erase(task);
+  }
+
+  base::string16 GetPrerenderTitlePrefix() const {
+    return l10n_util::GetStringFUTF16(IDS_TASK_MANAGER_PRERENDER_PREFIX,
+                                      base::string16());
+  }
+
+  task_management::WebContentsTagsManager* tags_manager() const {
+    return task_management::WebContentsTagsManager::GetInstance();
+  }
+
+  const std::set<task_management::Task*>& provided_tasks() const {
+    return provided_tasks_;
+  }
+
+ private:
+  std::set<task_management::Task*> provided_tasks_;
+
+  DISALLOW_COPY_AND_ASSIGN(PrerenderTagTest);
+};
+
+namespace {
+const char kPrerenderPage[] = "files/prerender/prerender_page.html";
+}  // namespace
+
+// Tests the correct recording of tags for the prerender WebContents.
+IN_PROC_BROWSER_TEST_F(PrerenderTagTest, Basic) {
+  EXPECT_TRUE(tags_manager()->tracked_tags().empty());
+
+  // Start prerendering a page and make sure it's correctly tagged.
+  PrerenderTestURL(kPrerenderPage, FINAL_STATUS_USED, 1);
+  EXPECT_FALSE(tags_manager()->tracked_tags().empty());
+  EXPECT_EQ(1U, tags_manager()->tracked_tags().size());

[ncarter (slow), 2015/06/19 19:50:51]

FYI, this assert may come back to bite you, once you add tags for normal
TabContents tabs, since a browsertest, by default, starts with a browser window
with a tab.

[afakhry, 2015/06/19 21:49:22]

Yes, thanks. I'm aware of that and hence the below (misplaced) TODO.

+
+  // Swap in the prerendered content and make sure its tag is removed.
+  // TODO(afakhry): Once we start tagging the tab contents the below test
+  //                must be changed.
+  NavigateToDestURL();
+  EXPECT_TRUE(tags_manager()->tracked_tags().empty());
+}
+
+IN_PROC_BROWSER_TEST_F(PrerenderTagTest, PrerenderTaskIsProvided) {
+  EXPECT_TRUE(provided_tasks().empty());
+  EXPECT_TRUE(tags_manager()->tracked_tags().empty());
+
+  task_management::WebContentsTaskProvider provider;
+  provider.SetObserver(this);
+
+  // Still empty, no pre-existing tasks.
+  EXPECT_TRUE(provided_tasks().empty());
+
+  // Start prerendering a page.
+  PrerenderTestURL(kPrerenderPage, FINAL_STATUS_USED, 1);
+  EXPECT_FALSE(tags_manager()->tracked_tags().empty());
+  EXPECT_EQ(1U, tags_manager()->tracked_tags().size());
+  ASSERT_FALSE(provided_tasks().empty());
+
+  // TODO(afakhry): The below may not be true after we support more tags.
+  ASSERT_EQ(1U, provided_tasks().size());
+
+  const task_management::Task* task = *provided_tasks().begin();
+  EXPECT_EQ(task_management::Task::RENDERER, task->GetType());
+  const base::string16 title = task->title();
+  const base::string16 expected_prefix = GetPrerenderTitlePrefix();
+  EXPECT_TRUE(title.find(expected_prefix) != base::string16::npos);
+  EXPECT_TRUE(title.find(expected_prefix) == 0);

[ncarter (slow), 2015/06/19 19:50:51]

[Option A] EXPECT_TRUE(base::StartsWith(title, expected_prefix,
base::CompareCase::INSENSITIVE));

https://code.google.com/p/chromium/codesearch#chromium/src/base/strings/strin...

(strings can be implicitly converted to StringPieces)


[Option B]
ASSERT_TRUE(base::MatchPattern(title,
task_manager::browsertest_util::MatchAnyPrerender());

(you'd have to define MatchAnyPrerender(), but it ought to exist)

[afakhry, 2015/06/19 21:49:21]

Done [Option A].

+
+  NavigateToDestURL();
+  // TODO(afakhry): The below may not be true after we support more tags.
+  EXPECT_TRUE(provided_tasks().empty());
+}
+
+#endif  // defined(ENABLE_TASK_MANAGER)
+
 }  // namespace prerender