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

chrome/browser/prerender/prerender_contents.cc

Index: chrome/browser/prerender/prerender_contents.cc
diff --git a/chrome/browser/prerender/prerender_contents.cc b/chrome/browser/prerender/prerender_contents.cc
index fb7daa97816557294ef6be57a1d4ed3e29eaf13e..74a7742f5db3bc624b26d97583cb4dbb8a891f12 100644
--- a/chrome/browser/prerender/prerender_contents.cc
+++ b/chrome/browser/prerender/prerender_contents.cc
@@ -19,6 +19,7 @@
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/prerender/prerender_resource_throttle.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/task_management/web_contents_tags.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tab_helpers.h"
 #include "chrome/browser/ui/web_contents_sizer.h"
@@ -279,6 +280,13 @@ void PrerenderContents::StartPrerendering(
   TabHelpers::AttachTabHelpers(prerender_contents_.get());
   content::WebContentsObserver::Observe(prerender_contents_.get());
 
+#if defined(ENABLE_TASK_MANAGER)

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

This is okay, but I wonder, would it be more elegant if we put these #defines
inside of the bodies of the methods of WebContentsTags. Basically, all of
WebContentsTags would be a no-op if ENABLE_TASK_MANAGER were false?

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

Unfortunately, it has to be done at the call site or otherwise there will be a
linking error ... task_manager_source_files are only included if
enable_task_manager==1 in the gyp files. enable_task_manager is 0 for ios and
android on the gn builds and I think is 0 only for android on the gyp builds.
See the build bots that failed the build in the previous patch.

See also:
https://code.google.com/p/chromium/codesearch#search/&q=%22enable_task_manage...

[ncarter (slow), 2015/06/20 04:34:47]

What I was advocating, was to add the web_content_tags.cc (and just that file)
back so that it's compiled on the ios/android build. But I am okay with the
#defines as they are.

[gavinp, 2015/06/22 18:40:15]

I like Nick's suggestion. That way, simpler callers like prerender_contents.cc
won't need to change when we remove the parameter and launch! It removes the
conditionals from many call sites that aren't particularly dependent on the new
messaging for the task manager.

+  // Tag the prerender contents with the task-manager specific prerender tag, so

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

"task-manager" -> "task manager". Also below.

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

Done.

+  // that it shows up in the task manager.
+  task_management::WebContentsTags::CreateForPrerenderContents(
+      prerender_contents_.get());
+#endif  // defined(ENABLE_TASK_MANAGER)
+
   web_contents_delegate_.reset(new WebContentsDelegateImpl(this));
   prerender_contents_.get()->SetDelegate(web_contents_delegate_.get());
   // Set the size of the prerender WebContents.
@@ -671,6 +679,13 @@ void PrerenderContents::DestroyWhenUsingTooManyResources() {
 WebContents* PrerenderContents::ReleasePrerenderContents() {
   prerender_contents_->SetDelegate(NULL);
   content::WebContentsObserver::Observe(NULL);
+
+#if defined(ENABLE_TASK_MANAGER)
+  // Here we have to clear the task-manager tag we added earlier to our
+  // WebContents since it's no longer a prerender content.
+  task_management::WebContentsTags::ClearTag(prerender_contents_.get());
+#endif  // defined(ENABLE_TASK_MANAGER)
+
   return prerender_contents_.release();
 }