Extensions: Resize favicons to 16x16.

This changes the ExtensionDOMUI favicon loading to use a ImageLoadingTracker which handles reading the file in the correct thread as well as resizing it.

BUG=47964
TEST=Install one of the extensions in the bug. Open the options page for the extension. The icon in the tab should be 16x16 pixels.

Review URL: http://codereview.chromium.org/2918007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@52376 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 117 insertions, 52 deletions

diff --git a/chrome/browser/dom_ui/dom_ui_factory.cc b/chrome/browser/dom_ui/dom_ui_factory.cc
index 224cd091..9ff2137 100644
--- a/chrome/browser/dom_ui/dom_ui_factory.cc
+++ b/chrome/browser/dom_ui/dom_ui_factory.cc
@@ -172,6 +172,25 @@ DOMUI* DOMUIFactory::CreateDOMUIForURL(TabContents* tab_contents,
 }
 
 // static
+void DOMUIFactory::GetFaviconForURL(Profile* profile,
+                                    FaviconService::GetFaviconRequest* request,
+                                    const GURL& page_url) {
+  // All extensions but the bookmark manager get their favicon from the icons
+  // part of the manifest.
+  if (page_url.SchemeIs(chrome::kExtensionScheme) &&
+      page_url.host() != extension_misc::kBookmarkManagerId) {
+    ExtensionDOMUI::GetFaviconForURL(profile, request, page_url);
+  } else {
+    scoped_refptr<RefCountedMemory> icon_data =
+        DOMUIFactory::GetFaviconResourceBytes(profile, page_url);
+    bool know_icon = icon_data.get() != NULL && icon_data->size() > 0;
+    request->ForwardResultAsync(
+        FaviconService::FaviconDataCallback::TupleType(request->handle(),
+            know_icon, icon_data, false, GURL()));
+  }
+}
+
+// static
 RefCountedMemory* DOMUIFactory::GetFaviconResourceBytes(Profile* profile,
                                                         const GURL& page_url) {
   // The bookmark manager is a chrome extension, so we have to check for it
@@ -179,8 +198,11 @@ RefCountedMemory* DOMUIFactory::GetFaviconResourceBytes(Profile* profile,
   if (page_url.host() == extension_misc::kBookmarkManagerId)
     return BookmarksUI::GetFaviconResourceBytes();
 
-  if (page_url.SchemeIs(chrome::kExtensionScheme))
-    return ExtensionDOMUI::GetFaviconResourceBytes(profile, page_url);
+  // The extension scheme is handled in GetFaviconForURL.
+  if (page_url.SchemeIs(chrome::kExtensionScheme)) {
+    NOTREACHED();
+    return NULL;
+  }
 
   if (!HasDOMUIScheme(page_url))
     return NULL;
diff --git a/chrome/browser/dom_ui/dom_ui_factory.h b/chrome/browser/dom_ui/dom_ui_factory.h
index 7b84867..781b7f38 100644
--- a/chrome/browser/dom_ui/dom_ui_factory.h
+++ b/chrome/browser/dom_ui/dom_ui_factory.h
@@ -7,6 +7,8 @@
 
 #include <vector>
 
+#include "chrome/browser/favicon_service.h"
+
 class DOMUI;
 class GURL;
 class Profile;
@@ -42,14 +44,20 @@ class DOMUIFactory {
   // the returned pointer is passed to the caller.
   static DOMUI* CreateDOMUIForURL(TabContents* tab_contents, const GURL& url);
 
-  // Gets the data for the favicon for a DOMUI page. Returns false if the DOMUI
-  // does not have a favicon.
-  static RefCountedMemory* GetFaviconResourceBytes(Profile* profile,
-                                                   const GURL& page_url);
+  // Get the favicon for |page_url| and forward the result to the |request|
+  // when loaded.
+  static void GetFaviconForURL(Profile* profile,
+                               FaviconService::GetFaviconRequest* request,
+                               const GURL& page_url);
 
  private:
   // Class is for scoping only.
   DOMUIFactory() {}
+
+  // Gets the data for the favicon for a DOMUI page. Returns NULL if the DOMUI
+  // does not have a favicon.
+  static RefCountedMemory* GetFaviconResourceBytes(Profile* profile,
+                                                   const GURL& page_url);
 };
 
 #endif  // CHROME_BROWSER_DOM_UI_DOM_UI_FACTORY_H_
diff --git a/chrome/browser/extensions/extension_dom_ui.cc b/chrome/browser/extensions/extension_dom_ui.cc
index cacf094..7cbe08f 100644
--- a/chrome/browser/extensions/extension_dom_ui.cc
+++ b/chrome/browser/extensions/extension_dom_ui.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/extensions/extension_bookmark_manager_api.h"
 #include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/browser/pref_service.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/render_widget_host_view.h"
@@ -24,27 +25,11 @@
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "chrome/common/url_constants.h"
+#include "gfx/codec/png_codec.h"
+#include "gfx/favicon_size.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 
 namespace {
-// Returns a piece of memory with the contents of the file |path|.
-RefCountedMemory* ReadFileData(const FilePath& path) {
-  // TODO(arv): We currently read this on the UI thread since extension objects
-  // can only safely be accessed on the UI thread. Read the file on the FILE
-  // thread and cache the result on the UI thread instead.
-  if (path.empty())
-    return NULL;
-
-  RefCountedBytes* result = new RefCountedBytes;
-  std::string content;
-  if (!file_util::ReadFileToString(path, &content))
-    return NULL;
-
-  result->data.resize(content.size());
-  std::copy(content.begin(), content.end(),
-            result->data.begin());
-
-  return result;
-}
 
 // De-dupes the items in |list|. Assumes the values are strings.
 void CleanUpDuplicates(ListValue* list) {
@@ -65,6 +50,70 @@ void CleanUpDuplicates(ListValue* list) {
   }
 }
 
+// Helper class that is used to track the loading of the favicon of an
+// extension.
+class ExtensionDOMUIImageLoadingTracker : public ImageLoadingTracker::Observer {
+ public:
+  ExtensionDOMUIImageLoadingTracker(Profile* profile,
+                                    FaviconService::GetFaviconRequest* request,
+                                    const GURL& page_url)
+      : ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)),
+        request_(request),
+        extension_(NULL) {
+    // Even when the extensions service is enabled by default, it's still
+    // disabled in incognito mode.
+    ExtensionsService* service = profile->GetExtensionsService();
+    if (service)
+      extension_ = service->GetExtensionByURL(page_url);
+  }
+
+  void Init() {
+    if (extension_) {
+      ExtensionResource icon_resource =
+          extension_->GetIconPath(Extension::EXTENSION_ICON_BITTY);
+
+      tracker_.LoadImage(extension_, icon_resource,
+                         gfx::Size(kFavIconSize, kFavIconSize),
+                         ImageLoadingTracker::DONT_CACHE);
+    } else {
+      ForwardResult(NULL);
+    }
+  }
+
+  virtual void OnImageLoaded(SkBitmap* image, ExtensionResource resource,
+                             int index) {
+    if (image) {
+      std::vector<unsigned char> image_data;
+      if (!gfx::PNGCodec::EncodeBGRASkBitmap(*image, false, &image_data)) {
+        NOTREACHED() << "Could not encode extension favicon";
+      }
+      ForwardResult(RefCountedBytes::TakeVector(&image_data));
+    } else {
+      ForwardResult(NULL);
+    }
+  }
+
+ private:
+  ~ExtensionDOMUIImageLoadingTracker() {}
+
+  // Forwards the result on the request. If no favicon was available then
+  // |icon_data| may be backed by NULL. Once the result has been forwarded the
+  // instance is deleted.
+  void ForwardResult(scoped_refptr<RefCountedMemory> icon_data) {
+    bool know_icon = icon_data.get() != NULL && icon_data->size() > 0;
+    request_->ForwardResultAsync(
+        FaviconService::FaviconDataCallback::TupleType(request_->handle(),
+            know_icon, icon_data, false, GURL()));
+    delete this;
+  }
+
+  ImageLoadingTracker tracker_;
+  scoped_refptr<FaviconService::GetFaviconRequest> request_;
+  Extension* extension_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionDOMUIImageLoadingTracker);
+};
+
 }  // namespace
 
 const wchar_t ExtensionDOMUI::kExtensionURLOverrides[] =
@@ -335,23 +384,10 @@ void ExtensionDOMUI::UnregisterChromeURLOverrides(
 }
 
 // static
-RefCountedMemory* ExtensionDOMUI::GetFaviconResourceBytes(Profile* profile,
-                                                          GURL page_url) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)) << "The extension "
-      "objects should only be accessed on the UI thread.";
-
-  // Even when the extensions service is enabled by default, it's still
-  // disabled in incognito mode.
-  ExtensionsService* service = profile->GetExtensionsService();
-  if (!service)
-    return NULL;
-
-  Extension* extension = service->GetExtensionByURL(page_url);
-  if (!extension)
-    return NULL;
-
-  // TODO(arv): Move this off of the UI thread and onto the File thread. If
-  //            possible to do this asynchronously, use ImageLoadingTracker.
-  return ReadFileData(extension->GetIconPath(
-      Extension::EXTENSION_ICON_BITTY).GetFilePathOnAnyThreadHack());
+void ExtensionDOMUI::GetFaviconForURL(Profile* profile,
+    FaviconService::GetFaviconRequest* request, const GURL& page_url) {
+  // tracker deletes itself when done.
+  ExtensionDOMUIImageLoadingTracker* tracker =
+      new ExtensionDOMUIImageLoadingTracker(profile, request, page_url);
+  tracker->Init();
 }
diff --git a/chrome/browser/extensions/extension_dom_ui.h b/chrome/browser/extensions/extension_dom_ui.h
index 7e66a4a..a89592e 100644
--- a/chrome/browser/extensions/extension_dom_ui.h
+++ b/chrome/browser/extensions/extension_dom_ui.h
@@ -11,6 +11,7 @@
 #include "chrome/browser/dom_ui/dom_ui.h"
 #include "chrome/browser/extensions/extension_bookmark_manager_api.h"
 #include "chrome/browser/extensions/extension_function_dispatcher.h"
+#include "chrome/browser/favicon_service.h"
 #include "chrome/common/extensions/extension.h"
 
 class ListValue;
@@ -72,8 +73,10 @@ class ExtensionDOMUI
   // Called from BrowserPrefs
   static void RegisterUserPrefs(PrefService* prefs);
 
-  static RefCountedMemory* GetFaviconResourceBytes(Profile* profile,
-                                                   GURL page_url);
+  // Get the favicon for the extension by getting an icon from the manifest.
+  static void GetFaviconForURL(Profile* profile,
+                               FaviconService::GetFaviconRequest* request,
+                               const GURL& page_url);
 
  private:
   // Unregister the specified override, and if it's the currently active one,
diff --git a/chrome/browser/favicon_service.cc b/chrome/browser/favicon_service.cc
index 0c24c80..94baaf7 100644
--- a/chrome/browser/favicon_service.cc
+++ b/chrome/browser/favicon_service.cc
@@ -51,12 +51,8 @@ FaviconService::Handle FaviconService::GetFaviconForURL(
   AddRequest(request, consumer);
   FaviconService::Handle handle = request->handle();
   if (page_url.SchemeIs(chrome::kChromeUIScheme) ||
-    page_url.SchemeIs(chrome::kExtensionScheme)) {
-    scoped_refptr<RefCountedMemory> icon_data =
-        DOMUIFactory::GetFaviconResourceBytes(profile_, page_url);
-    bool know_icon = icon_data.get() != NULL && icon_data->size() > 0;
-    request->ForwardResultAsync(FaviconDataCallback::TupleType(handle,
-        know_icon, icon_data, false, GURL()));
+      page_url.SchemeIs(chrome::kExtensionScheme)) {
+    DOMUIFactory::GetFaviconForURL(profile_, request, page_url);
   } else {
     HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
     if (hs)