Refactored loading of applications / extensions icons.

chrome/browser/extensions/api/developer_private/developer_private_api.cc

Index: chrome/browser/extensions/api/developer_private/developer_private_api.cc
diff --git a/chrome/browser/extensions/api/developer_private/developer_private_api.cc b/chrome/browser/extensions/api/developer_private/developer_private_api.cc
index 3f594d60c68d4cb5ecaffbb53a6924dc3e3bfbd2..1d352aed50c72f70691996a7aaf883f1ac22be09 100644
--- a/chrome/browser/extensions/api/developer_private/developer_private_api.cc
+++ b/chrome/browser/extensions/api/developer_private/developer_private_api.cc
@@ -27,6 +27,7 @@
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/extensions/image_loader.h"
 #include "chrome/browser/extensions/management_policy.h"
 #include "chrome/browser/extensions/unpacked_installer.h"
 #include "chrome/browser/extensions/updater/extension_updater.h"
@@ -85,45 +86,6 @@ ExtensionUpdater* GetExtensionUpdater(Profile* profile) {
     return profile->GetExtensionService()->updater();
 }
 
-GURL GetImageURLFromData(std::string contents) {
-  std::string contents_base64;
-  if (!base::Base64Encode(contents, &contents_base64))
-    return GURL();
-
-  // TODO(dvh): make use of chrome::kDataScheme. Filed as crbug/297301.
-  const char kDataURLPrefix[] = "data:image;base64,";
-  return GURL(kDataURLPrefix + contents_base64);
-}
-
-GURL GetDefaultImageURL(developer_private::ItemType type) {
-  int icon_resource_id;
-  switch (type) {
-    case developer::ITEM_TYPE_LEGACY_PACKAGED_APP:
-    case developer::ITEM_TYPE_HOSTED_APP:
-    case developer::ITEM_TYPE_PACKAGED_APP:
-      icon_resource_id = IDR_APP_DEFAULT_ICON;
-      break;
-    default:
-      icon_resource_id = IDR_EXTENSION_DEFAULT_ICON;
-      break;
-  }
-
-  return GetImageURLFromData(
-      ResourceBundle::GetSharedInstance().GetRawDataResourceForScale(
-          icon_resource_id, ui::SCALE_FACTOR_100P).as_string());
-}
-
-// TODO(dvh): This code should be refactored and moved to
-// extensions::ImageLoader. Also a resize should be performed to avoid
-// potential huge URLs: crbug/297298.
-GURL ToDataURL(const base::FilePath& path, developer_private::ItemType type) {
-  std::string contents;
-  if (path.empty() || !base::ReadFileToString(path, &contents))
-    return GetDefaultImageURL(type);
-
-  return GetImageURLFromData(contents);
-}
-
 std::vector<base::FilePath> ListFolder(const base::FilePath path) {
   base::FileEnumerator files(path, false,
       base::FileEnumerator::DIRECTORIES | base::FileEnumerator::FILES);
@@ -374,27 +336,6 @@ scoped_ptr<developer::ItemInfo>
   return info.Pass();
 }
 
-void DeveloperPrivateGetItemsInfoFunction::GetIconsOnFileThread(
-    ItemInfoList item_list,
-    const std::map<std::string, ExtensionResource> idToIcon) {
-  for (ItemInfoList::iterator iter = item_list.begin();
-       iter != item_list.end(); ++iter) {
-    developer_private::ItemInfo* info = iter->get();
-    std::map<std::string, ExtensionResource>::const_iterator resource_ptr
-        = idToIcon.find(info->id);
-    if (resource_ptr != idToIcon.end()) {
-      info->icon_url =
-          ToDataURL(resource_ptr->second.GetFilePath(), info->type).spec();
-    }
-  }
-
-  results_ = developer::GetItemsInfo::Results::Create(item_list);
-  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
-      base::Bind(&DeveloperPrivateGetItemsInfoFunction::SendResponse,
-                 this,
-                 true));
-}
-
 void DeveloperPrivateGetItemsInfoFunction::
     GetInspectablePagesForExtensionProcess(
         const Extension* extension,
@@ -554,37 +495,55 @@ bool DeveloperPrivateGetItemsInfoFunction::RunImpl() {
     items.InsertAll(*service->terminated_extensions());
   }
 
-  std::map<std::string, ExtensionResource> id_to_icon;
-  ItemInfoList item_list;
-
   for (ExtensionSet::const_iterator iter = items.begin();
        iter != items.end(); ++iter) {
     const Extension& item = *iter->get();
 
-    ExtensionResource item_resource =
-        IconsInfo::GetIconResource(&item,
-                                   extension_misc::EXTENSION_ICON_MEDIUM,
-                                   ExtensionIconSet::MATCH_BIGGER);
-    id_to_icon[item.id()] = item_resource;
-
     // Don't show component extensions and invisible apps.
     if (item.ShouldNotBeVisible())
       continue;
 
-    item_list.push_back(make_linked_ptr<developer::ItemInfo>(
+    item_list_.push_back(make_linked_ptr<developer::ItemInfo>(
         CreateItemInfo(
             item, service->IsExtensionEnabled(item.id())).release()));
   }
 
-  content::BrowserThread::PostTask(content::BrowserThread::FILE, FROM_HERE,
-      base::Bind(&DeveloperPrivateGetItemsInfoFunction::GetIconsOnFileThread,
-                 this,
-                 item_list,
-                 id_to_icon));
+  // We will iterator over |item_list_| to request all the icons.

[Finnur, 2013/09/30 15:11:21]

s/iterator/iterate/
Or just:
s/We will iterator/Iterate/

[dvh-g, 2013/10/01 04:19:26]

Done.

+  RequestNextIcon();
 
   return true;
 }
 
+void DeveloperPrivateGetItemsInfoFunction::RequestNextIcon() {
+  developer::ItemInfo* info = &*item_list_[icon_to_load_];
+  ExtensionService* service = profile()->GetExtensionService();
+  ImageLoader::Get(profile())->LoadExtensionIconDataURLAsync(
+      service->GetExtensionById(info->id, true),
+      extension_misc::EXTENSION_ICON_MEDIUM,
+      false,
+      base::Bind(&DeveloperPrivateGetItemsInfoFunction::LoadIconFinished,
+                 this));
+}
+
+void DeveloperPrivateGetItemsInfoFunction::LoadIconFinished(const GURL& url) {
+  // Store the result.
+  developer::ItemInfo* info = &*item_list_[icon_to_load_];

[Finnur, 2013/09/30 15:11:21]

Hmm: &* 
What's wrong with .get() ?

[dvh-g, 2013/10/01 04:19:26]

Done.

[Finnur, 2013/10/01 11:14:15]

You changed line 518, but not 530...

[dvh, 2013/10/02 05:15:08]

Done.

+  info->icon_url = url.spec();
+  ++icon_to_load_;
+  if (icon_to_load_ < item_list_.size()) {
+    // Loop to request the next icon.

[Finnur, 2013/09/30 15:11:21]

nit: This comment is redundant.

[dvh-g, 2013/10/01 04:19:26]

Done.

+    RequestNextIcon();
+  } else {
+    // All icons have been loaded: we send the result.

[Finnur, 2013/09/30 15:11:21]

nit: The word 'we' is redundant.

[dvh-g, 2013/10/01 04:19:26]

Done.

+    results_ = developer::GetItemsInfo::Results::Create(item_list_);
+    SendResponse(true);
+  }
+}
+
+DeveloperPrivateGetItemsInfoFunction::DeveloperPrivateGetItemsInfoFunction()
+    : icon_to_load_(0) {
+}
+
 DeveloperPrivateGetItemsInfoFunction::~DeveloperPrivateGetItemsInfoFunction() {}
 
 bool DeveloperPrivateAllowFileAccessFunction::RunImpl() {