GuestViews should be able to specify task manager entries

chrome/browser/task_manager/guest_information.cc

Index: chrome/browser/task_manager/guest_information.cc
diff --git a/chrome/browser/task_manager/guest_information.cc b/chrome/browser/task_manager/guest_information.cc
index 131503557b08857d161284c1554b4bf58136c5e4..3f0f1a72146054accbcc097f390dd9b198f4d6e1 100644
--- a/chrome/browser/task_manager/guest_information.cc
+++ b/chrome/browser/task_manager/guest_information.cc
@@ -20,6 +20,7 @@
 #include "content/public/browser/render_widget_host_iterator.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/browser/guest_view/guest_view_base.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
@@ -62,12 +63,12 @@ Resource::Type GuestResource::GetType() const {
 
 base::string16 GuestResource::GetTitle() const {
   WebContents* web_contents = GetWebContents();
-  const int message_id = IDS_TASK_MANAGER_WEBVIEW_TAG_PREFIX;
-  if (web_contents) {
-    base::string16 title = util::GetTitleFromWebContents(web_contents);
-    return l10n_util::GetStringFUTF16(message_id, title);
+  if (!web_contents) {
+    const int message_id = IDS_TASK_MANAGER_WEBVIEW_TAG_PREFIX;
+    return l10n_util::GetStringFUTF16(message_id, base::string16());
   }
-  return l10n_util::GetStringFUTF16(message_id, base::string16());
+  return extensions::GuestViewBase::FromWebContents(web_contents)->
+      GetTaskName();

[Charlie Reis, 2014/08/22 21:11:43]

Who determines this value?  I'm skeptical about allowing apps to declare a
prefix in the task manager, since they could list "Browser," "Tab," or other
confusing values that claim to be more important than they are.  It also
wouldn't send the prefix through localization.

[Fady Samuel, 2014/08/22 21:15:31]

Apps don't: however, there are multiple GuestView elements now:

extensionoptions, appview, webview, and soon mimetypehandler, etc

We would like different strings for different GuestView types.
Internationalization is still possible as we still use IDS strings

[Fady Samuel, 2014/08/22 21:16:24]

For example: Here is what appview does:
https://codereview.chromium.org/497843002/patch/1/10004

[Charlie Reis, 2014/08/22 21:23:20]

Ah, thanks.  That sounds fine.  I just didn't want app developers to be able to
specify it.

Does it ever have a different format than Prefix: title?  If so, maybe we should
just get |message_id| from the GuestViewBase rather than duplicating the
l10n_util call everywhere?  That would also ensure we have a consistent format
for each row.

[Fady Samuel, 2014/08/22 21:48:54]

MIMETypeHandler (in a separate CL) is used to load a chrome extension page that
can handle resources on the web (such as PDF), we probably want to specify the
URL of the resource rather than the extension loading the resource in that case.

[Charlie Reis, 2014/08/22 21:57:47]

Could we do that by setting the title of that WebContents to the URL?

We can make this free-form if it's necessary, but I'd really like to have it
follow the "prefix: title" pattern and avoid code duplication if possible.

[Charlie Reis, 2014/08/22 22:40:01]

Is this suggestion feasible?  (Not sure if you saw it when the other message
came in.)

 }
 
 gfx::ImageSkia GuestResource::GetIcon() const {
@@ -93,7 +94,7 @@ GuestInformation::~GuestInformation() {}
 
 bool GuestInformation::CheckOwnership(WebContents* web_contents) {
   // Guest WebContentses are created and owned internally by the content layer.
-  return web_contents->IsSubframe();
+  return extensions::GuestViewBase::IsGuest(web_contents);
 }
 
 void GuestInformation::GetAll(const NewWebContentsCallback& callback) {
@@ -103,7 +104,7 @@ void GuestInformation::GetAll(const NewWebContentsCallback& callback) {
     if (widget->IsRenderView()) {
       content::RenderViewHost* rvh = content::RenderViewHost::From(widget);
       WebContents* web_contents = WebContents::FromRenderViewHost(rvh);
-      if (web_contents && web_contents->IsSubframe())
+      if (web_contents && extensions::GuestViewBase::IsGuest(web_contents))
         callback.Run(web_contents);
     }
   }