Attempt to load component extension favicon from the resources first.

Special handling of CWS icon.
Also correctly handle URL rewrites in favicon requests.

BUG=chromium-os:28314,chromium:120471
TEST=Observe the right favicon for CWS and FileManager component extension. Bookmark them and see the right favicon.
Review URL: https://chromiumcodereview.appspot.com/9979001

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

chrome/browser/extensions/extension_icon_manager_unittest.cc

@@ -137,3 +137,45 @@ TEST_F(ExtensionIconManagerTest, LoadRemoveLoad) {
 
   EXPECT_TRUE(gfx::BitmapsAreEqual(first_icon, second_icon));
 }
+
+#if defined(FILE_MANAGER_EXTENSION)
+// Tests loading an icon for a component extension.
+TEST_F(ExtensionIconManagerTest, LoadComponentExtensionResource) {
+  SkBitmap default_icon = GetDefaultIcon();
+
+  FilePath test_dir;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir));
+  FilePath manifest_path = test_dir.AppendASCII(
+      "extensions/file_manager/app.json");
+
+  JSONFileValueSerializer serializer(manifest_path);
+  scoped_ptr<DictionaryValue> manifest(
+      static_cast<DictionaryValue*>(serializer.Deserialize(NULL, NULL)));
+  ASSERT_TRUE(manifest.get() != NULL);
+
+  std::string error;
+  scoped_refptr<Extension> extension(Extension::Create(
+      manifest_path.DirName(), Extension::COMPONENT, *manifest.get(),
+      Extension::STRICT_ERROR_CHECKS, &error));
+  ASSERT_TRUE(extension.get());
+
+  TestIconManager icon_manager(this);
+  // Load the icon and grab the bitmap.
+  icon_manager.LoadIcon(extension.get());
+  WaitForImageLoad();
+  SkBitmap first_icon = icon_manager.GetIcon(extension->id());
+  EXPECT_FALSE(gfx::BitmapsAreEqual(first_icon, default_icon));
+
+  // Remove the icon from the manager.
+  icon_manager.RemoveIcon(extension->id());
+
+  // Now re-load the icon - we should get the same result bitmap (and not the
+  // default icon).
+  icon_manager.LoadIcon(extension.get());
+  WaitForImageLoad();
+  SkBitmap second_icon = icon_manager.GetIcon(extension->id());
+  EXPECT_FALSE(gfx::BitmapsAreEqual(second_icon, default_icon));
+
+  EXPECT_TRUE(gfx::BitmapsAreEqual(first_icon, second_icon));
+}
+#endif

chrome/browser/extensions/image_loading_tracker.cc

@@ -6,11 +6,15 @@
 
 #include "base/bind.h"
 #include "base/file_util.h"
+#include "chrome/browser/ui/webui/extensions/extension_icon_source.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
+#include "grit/component_extension_resources_map.h"
+#include "grit/theme_resources.h"
 #include "skia/ext/image_operations.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/image/image.h"
@@ -114,6 +118,28 @@ class ImageLoadingTracker::ImageLoader
     ReportBack(decoded.release(), resource, original_size, id);
   }
 
+  // Instructs the loader to load a resource on the File thread.
+  void LoadResource(const ExtensionResource& resource,
+                    const gfx::Size& max_size,
+                    int id,
+                    int resource_id) {
+    DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE,
+        base::Bind(&ImageLoader::LoadResourceOnFileThread, this, resource,
+                   max_size, id, resource_id));
+  }
+
+  void LoadResourceOnFileThread(const ExtensionResource& resource,
+                                const gfx::Size& max_size,
+                                int id,
+                                int resource_id) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+    SkBitmap* image = ExtensionIconSource::LoadImageByResourceId(
+        resource_id);
+    ReportBack(image, resource, max_size, id);
+  }
+
   void ReportBack(SkBitmap* image, const ExtensionResource& resource,
                   const gfx::Size& original_size, int id) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
@@ -200,12 +226,47 @@ void ImageLoadingTracker::LoadImages(const Extension* extension,
       continue;
     }
 
-    // Instruct the ImageLoader to load this on the File thread. LoadImage does
-    // not block.
+    // Instruct the ImageLoader to load this on the File thread. LoadImage and
+    // LoadResource do not block.
     if (!loader_)
       loader_ = new ImageLoader(this);
-    loader_->LoadImage(it->resource, it->max_size, id);
+
+    // Load resources for WebStore component extension.
+    if (load_info.extension_id == extension_misc::kWebStoreAppId) {
+      loader_->LoadResource(it->resource, it->max_size, id, IDR_WEBSTORE_ICON);
+      continue;
+    }
+
+    int resource_id;
+    if (IsComponentExtensionResource(extension, it->resource, resource_id))
+      loader_->LoadResource(it->resource, it->max_size, id, resource_id);
+    else
+      loader_->LoadImage(it->resource, it->max_size, id);
+  }
+}
+
+bool ImageLoadingTracker::IsComponentExtensionResource(
+    const Extension* extension,
+    const ExtensionResource& resource,
+    int& resource_id) const {
+  if (extension->location() != Extension::COMPONENT)
+    return false;
+
+  FilePath directory_path = extension->path();
+  FilePath relative_path = directory_path.BaseName().Append(
+      resource.relative_path());
+
+  for (size_t i = 0; i < kComponentExtensionResourcesSize; ++i) {
+    FilePath resource_path =
+        FilePath().AppendASCII(kComponentExtensionResources[i].name);
+    resource_path = resource_path.NormalizePathSeparators();
+
+    if (relative_path == resource_path) {
+      resource_id = kComponentExtensionResources[i].value;
+      return true;
+    }
   }
+  return false;
 }
 
 void ImageLoadingTracker::OnImageLoaded(

chrome/browser/extensions/image_loading_tracker.h

@@ -9,6 +9,7 @@
 #include <map>
 
 #include "base/compiler_specific.h"
+#include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "content/public/browser/notification_observer.h"
@@ -122,6 +123,13 @@ class ImageLoadingTracker : public content::NotificationObserver {
   void OnImageLoaded(SkBitmap* image, const ExtensionResource& resource,
                      const gfx::Size& original_size, int id, bool should_cache);
 
+  // Checks whether image is a component extension resource. Returns false
+  // if a given |resource| does not have a corresponding image in bundled
+  // resources. Otherwise fills |resource_id|.
+  bool IsComponentExtensionResource(const Extension* extension,
+                                    const ExtensionResource& resource,
+                                    int& resource_id) const;
+
   // content::NotificationObserver method. If an extension is uninstalled while
   // we're waiting for the image we remove the entry from load_map_.
   virtual void Observe(int type,
@@ -143,6 +151,9 @@ class ImageLoadingTracker : public content::NotificationObserver {
 
   content::NotificationRegistrar registrar_;
 
+  FRIEND_TEST_ALL_PREFIXES(ImageLoadingTrackerTest,
+                           IsComponentExtensionResource);
+
   DISALLOW_COPY_AND_ASSIGN(ImageLoadingTracker);
 };
 

chrome/browser/extensions/image_loading_tracker_unittest.cc

@@ -13,6 +13,7 @@
 #include "chrome/common/extensions/extension_resource.h"
 #include "content/public/browser/notification_service.h"
 #include "content/test/test_browser_thread.h"
+#include "grit/component_extension_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/image/image.h"
@@ -52,7 +53,8 @@ class ImageLoadingTrackerTest : public testing::Test,
     return result;
   }
 
-  scoped_refptr<Extension> CreateExtension() {
+  scoped_refptr<Extension> CreateExtension(const char* name,
+                                           Extension::Location location) {
     // Create and load an extension.
     FilePath test_file;
     if (!PathService::Get(chrome::DIR_TEST_DATA, &test_file)) {
@@ -60,7 +62,7 @@ class ImageLoadingTrackerTest : public testing::Test,
       return NULL;
     }
     test_file = test_file.AppendASCII("extensions")
-                         .AppendASCII("image_loading_tracker");
+                         .AppendASCII(name);
     int error_code = 0;
     std::string error;
     JSONFileValueSerializer serializer(test_file.AppendASCII("app.json"));
@@ -75,7 +77,7 @@ class ImageLoadingTrackerTest : public testing::Test,
     if (!valid_value.get())
       return NULL;
 
-    return Extension::Create(test_file, Extension::INVALID, *valid_value,
+    return Extension::Create(test_file, location, *valid_value,
         Extension::STRICT_ERROR_CHECKS, &error);
   }
 
@@ -97,7 +99,8 @@ class ImageLoadingTrackerTest : public testing::Test,
 
 // Tests asking ImageLoadingTracker to cache pushes the result to the Extension.
 TEST_F(ImageLoadingTrackerTest, Cache) {
-  scoped_refptr<Extension> extension(CreateExtension());
+  scoped_refptr<Extension> extension(CreateExtension(
+      "image_loading_tracker", Extension::INVALID));
   ASSERT_TRUE(extension.get() != NULL);
 
   ExtensionResource image_resource =
@@ -146,7 +149,8 @@ TEST_F(ImageLoadingTrackerTest, Cache) {
 // Tests deleting an extension while waiting for the image to load doesn't cause
 // problems.
 TEST_F(ImageLoadingTrackerTest, DeleteExtensionWhileWaitingForCache) {
-  scoped_refptr<Extension> extension(CreateExtension());
+  scoped_refptr<Extension> extension(CreateExtension(
+      "image_loading_tracker", Extension::INVALID));
   ASSERT_TRUE(extension.get() != NULL);
 
   ExtensionResource image_resource =
@@ -187,7 +191,8 @@ TEST_F(ImageLoadingTrackerTest, DeleteExtensionWhileWaitingForCache) {
 
 // Tests loading multiple dimensions of the same image.
 TEST_F(ImageLoadingTrackerTest, MultipleImages) {
-  scoped_refptr<Extension> extension(CreateExtension());
+  scoped_refptr<Extension> extension(CreateExtension(
+      "image_loading_tracker", Extension::INVALID));
   ASSERT_TRUE(extension.get() != NULL);
 
   std::vector<ImageLoadingTracker::ImageInfo> info_list;
@@ -221,3 +226,24 @@ TEST_F(ImageLoadingTrackerTest, MultipleImages) {
   EXPECT_EQ(ExtensionIconSet::EXTENSION_ICON_BITTY, bmp1->width());
   EXPECT_EQ(ExtensionIconSet::EXTENSION_ICON_SMALLISH, bmp2->width());
 }
+
+// Tests IsComponentExtensionResource function.
+TEST_F(ImageLoadingTrackerTest, IsComponentExtensionResource) {
+  scoped_refptr<Extension> extension(CreateExtension(
+      "file_manager", Extension::COMPONENT));
+  ASSERT_TRUE(extension.get() != NULL);
+
+  ExtensionResource resource =
+      extension->GetIconResource(ExtensionIconSet::EXTENSION_ICON_BITTY,
+                                 ExtensionIconSet::MATCH_EXACTLY);
+
+#if defined(FILE_MANAGER_EXTENSION)
+  ImageLoadingTracker loader(this);
+  int resource_id;
+  ASSERT_EQ(true,
+            loader.IsComponentExtensionResource(extension.get(),
+                                                resource,
+                                                resource_id));
+  ASSERT_EQ(IDR_FILE_MANAGER_ICON_16, resource_id);
+#endif
+}

chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc

@@ -431,15 +431,21 @@ void ChromeWebUIControllerFactory::GetFaviconForURL(
     Profile* profile,
     FaviconService::GetFaviconRequest* request,
     const GURL& page_url) const {
+  // Before determining whether page_url is an extension url, we must handle
+  // overrides. This changes urls in |kChromeUIScheme| to extension urls, and
+  // allows to use ExtensionWebUI::GetFaviconForURL.
+  GURL url(page_url);
+  ExtensionWebUI::HandleChromeURLOverride(&url, profile);
+
   // 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) {
-    ExtensionWebUI::GetFaviconForURL(profile, request, page_url);
+  if (url.SchemeIs(chrome::kExtensionScheme) &&
+      url.host() != extension_misc::kBookmarkManagerId) {
+    ExtensionWebUI::GetFaviconForURL(profile, request, url);
   } else {
     history::FaviconData favicon;
     favicon.image_data = scoped_refptr<RefCountedMemory>(
-        GetFaviconResourceBytes(page_url));
+        GetFaviconResourceBytes(url));
     favicon.known_icon = favicon.image_data.get() != NULL &&
                              favicon.image_data->size() > 0;
     favicon.icon_type = history::FAVICON;

chrome/browser/ui/webui/extensions/extension_icon_source.cc

@@ -126,10 +126,6 @@ void ExtensionIconSource::StartDataRequest(const std::string& path,
   if (icon.relative_path().empty()) {
     LoadIconFailed(request_id);
   } else {
-    if (request->extension->location() == Extension::COMPONENT &&
-        TryLoadingComponentExtensionImage(icon, request_id)) {
-      return;
-    }
     LoadExtensionImage(icon, request_id);
   }
 }
@@ -145,13 +141,6 @@ void ExtensionIconSource::LoadIconFailed(int request_id) {
     LoadDefaultImage(request_id);
 }
 
-const SkBitmap* ExtensionIconSource::GetWebStoreImage() {
-  if (!web_store_icon_data_.get())
-    web_store_icon_data_.reset(LoadImageByResourceId(IDR_WEBSTORE_ICON));
-
-  return web_store_icon_data_.get();
-}
-
 const SkBitmap* ExtensionIconSource::GetDefaultAppImage() {
   if (!default_app_data_.get())
     default_app_data_.reset(LoadImageByResourceId(IDR_APP_DEFAULT_ICON));
@@ -184,9 +173,7 @@ void ExtensionIconSource::LoadDefaultImage(int request_id) {
   ExtensionIconRequest* request = GetData(request_id);
   const SkBitmap* default_image = NULL;
 
-  if (request->extension->id() == extension_misc::kWebStoreAppId)
-    default_image = GetWebStoreImage();
-  else if (request->extension->is_app())
+  if (request->extension->is_app())
     default_image = GetDefaultAppImage();
   else
     default_image = GetDefaultExtensionImage();
@@ -204,26 +191,6 @@ void ExtensionIconSource::LoadDefaultImage(int request_id) {
   FinalizeImage(&resized_image, request_id);
 }
 
-bool ExtensionIconSource::TryLoadingComponentExtensionImage(
-    const ExtensionResource& icon, int request_id) {
-  ExtensionIconRequest* request = GetData(request_id);
-  FilePath directory_path = request->extension->path();
-  FilePath relative_path = directory_path.BaseName().Append(
-      icon.relative_path());
-  for (size_t i = 0; i < kComponentExtensionResourcesSize; ++i) {
-    FilePath bm_resource_path =
-        FilePath().AppendASCII(kComponentExtensionResources[i].name);
-    bm_resource_path = bm_resource_path.NormalizePathSeparators();
-    if (relative_path == bm_resource_path) {
-      scoped_ptr<SkBitmap> decoded(LoadImageByResourceId(
-          kComponentExtensionResources[i].value));
-      FinalizeImage(decoded.get(), request_id);
-      return true;
-    }
-  }
-  return false;
-}
-
 void ExtensionIconSource::LoadExtensionImage(const ExtensionResource& icon,
                                              int request_id) {
   ExtensionIconRequest* request = GetData(request_id);

chrome/browser/ui/webui/extensions/extension_icon_source.h

@@ -78,9 +78,6 @@ class ExtensionIconSource : public ChromeURLDataManager::DataSource,
   // Encapsulates the request parameters for |request_id|.
   struct ExtensionIconRequest;
 
-  // Returns the bitmap for the webstore icon.
-  const SkBitmap* GetWebStoreImage();
-
   // Returns the bitmap for the default app image.
   const SkBitmap* GetDefaultAppImage();
 
@@ -95,12 +92,6 @@ class ExtensionIconSource : public ChromeURLDataManager::DataSource,
   // Loads the default image for |request_id| and returns to the client.
   void LoadDefaultImage(int request_id);
 
-  // Tries loading component extension image. These usually come from resources
-  // instead of file system. Returns false if a given |icon| does not have
-  // a corresponding image in bundled resources.
-  bool TryLoadingComponentExtensionImage(const ExtensionResource& icon,
-                                         int request_id);
-
   // Loads the extension's |icon| for the given |request_id| and returns the
   // image to the client.
   void LoadExtensionImage(const ExtensionResource& icon, int request_id);

chrome/test/data/extensions/file_manager/app.json

+{
+  "name": "test",
+  "version": "1",
+  "manifest_version": 2,
+  "icons": { "16": "images/icon16.png" }
+}