GuestViews should be able to specify task manager entries

chrome/browser/task_manager/guest_information.cc

 // Copyright 2014 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/guest_information.h"
 
 #include "base/strings/string16.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/task_manager/renderer_resource.h"
 #include "chrome/browser/task_manager/resource_provider.h"
 #include "chrome/browser/task_manager/task_manager.h"
 #include "chrome/browser/task_manager/task_manager_util.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #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"
 
 using content::RenderProcessHost;
 using content::RenderViewHost;
 using content::RenderWidgetHost;
 using content::WebContents;
 using extensions::Extension;
 
@@ skipping 22 matching lines @@
 
 GuestResource::~GuestResource() {
 }
 
 Resource::Type GuestResource::GetType() const {
   return GUEST;
 }
 
 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 {
   WebContents* web_contents = GetWebContents();
   if (web_contents && FaviconTabHelper::FromWebContents(web_contents)) {
     return FaviconTabHelper::FromWebContents(web_contents)->
         GetFavicon().AsImageSkia();
   }
   return gfx::ImageSkia();
 }
 
 WebContents* GuestResource::GetWebContents() const {
   return WebContents::FromRenderViewHost(render_view_host());
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // GuestInformation class
 ////////////////////////////////////////////////////////////////////////////////
 
 GuestInformation::GuestInformation() {}
 
 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) {
   scoped_ptr<content::RenderWidgetHostIterator> widgets(
       content::RenderWidgetHost::GetRenderWidgetHosts());
   while (content::RenderWidgetHost* widget = widgets->GetNextHost()) {
     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);
     }
   }
 }
 
 scoped_ptr<RendererResource> GuestInformation::MakeResource(
     WebContents* web_contents) {
   return scoped_ptr<RendererResource>(
       new GuestResource(web_contents->GetRenderViewHost()));
 }
 
 }  // namespace task_manager