Task manager's apps and extensions should show the correct favicon

chrome/browser/task_management/providers/web_contents/extension_task.cc

Index: chrome/browser/task_management/providers/web_contents/extension_task.cc
diff --git a/chrome/browser/task_management/providers/web_contents/extension_task.cc b/chrome/browser/task_management/providers/web_contents/extension_task.cc
index a63781a20de87823ad8deef3d4ab707e2676cd8a..2d4701158884f815a86f7cb37f4117bfaf730471 100644
--- a/chrome/browser/task_management/providers/web_contents/extension_task.cc
+++ b/chrome/browser/task_management/providers/web_contents/extension_task.cc
@@ -8,7 +8,9 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/view_type_utils.h"
+#include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/manifest_handlers/icons_handler.h"
 #include "extensions/common/view_type.h"
 #include "grit/theme_resources.h"
 #include "ui/base/resource/resource_bundle.h"
@@ -40,6 +42,7 @@ ExtensionTask::ExtensionTask(content::WebContents* web_contents,
                    GetDefaultIcon(),
                    web_contents,
                    web_contents->GetRenderProcessHost()) {
+  LoadExtensionIcon(web_contents, extension);
 }
 
 ExtensionTask::~ExtensionTask() {
@@ -51,15 +54,23 @@ void ExtensionTask::OnTitleChanged(content::NavigationEntry* entry) {
 }
 
 void ExtensionTask::OnFaviconChanged() {
-  // For now we never change the favicon of the extension, we always use the
-  // default one.
-  // TODO(afakhry): In the future use the extensions' favicons.
+  // We don't care about the favicon of the WebContents but rather of the
+  // extension.
 }
 
 Task::Type ExtensionTask::GetType() const {
   return Task::EXTENSION;
 }
 
+void ExtensionTask::OnExtensionIconImageChanged(extensions::IconImage* image) {
+  DCHECK_EQ(extension_icon_.get(), image);
+
+  if (image->image_skia().isNull())
+    return;

[Devlin, 2015/08/27 16:08:55]

nit: it'll depend on who you ask, but I generally prefer to only early-return if
it saves code/eliminates a big indent.  So in a case like this, I'd just do:
if (!image->image_skia().isNull())
  set_icon(image->image_skia());

[afakhry, 2015/08/27 21:48:15]

Done.

+
+  set_icon(image->image_skia());
+}
+
 base::string16 ExtensionTask::GetExtensionTitle(
     content::WebContents* web_contents,
     const extensions::Extension* extension,
@@ -81,4 +92,24 @@ base::string16 ExtensionTask::GetExtensionTitle(
       is_background);
 }
 
+void ExtensionTask::LoadExtensionIcon(content::WebContents* web_contents,

[Devlin, 2015/08/27 16:08:55]

instead of passing in web_contents, just use web_contents().

[afakhry, 2015/08/27 21:48:15]

Done.

+                                      const extensions::Extension* extension) {
+  DCHECK(web_contents);
+
+  if (!extension)
+    return;
+
+  extension_icon_.reset(
+      new extensions::IconImage(web_contents->GetBrowserContext(),
+                                extension,
+                                extensions::IconsInfo::GetIcons(extension),
+                                extension_misc::EXTENSION_ICON_SMALL,
+                                icon(),
+                                this));
+
+  // Triggers actual image loading with 1x resources.
+  extension_icon_->image_skia().GetRepresentation(1.0f);
+  set_icon(extension_icon_->image_skia());
+}
+
 }  // namespace task_management