Clean up the IconLoader.

chrome/browser/icon_manager.cc

Index: chrome/browser/icon_manager.cc
diff --git a/chrome/browser/icon_manager.cc b/chrome/browser/icon_manager.cc
index 6596ab311234c4660a1a7b0e7a54ee8becf43d26..d2fd939364c327464e9fbdace316618a7fa3857e 100644
--- a/chrome/browser/icon_manager.cc
+++ b/chrome/browser/icon_manager.cc
@@ -8,7 +8,6 @@
 #include <tuple>
 
 #include "base/bind.h"
-#include "base/stl_util.h"
 #include "base/task_runner.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkCanvas.h"
@@ -28,7 +27,7 @@ void RunCallbackIfNotCanceled(
 
 struct IconManager::ClientRequest {
   IconRequestCallback callback;
-  base::FilePath file_path;
+  IconLoader::IconGroup group;
   IconLoader::IconSize size;
 };
 
@@ -36,33 +35,25 @@ IconManager::IconManager() {
 }
 
 IconManager::~IconManager() {
-  base::STLDeleteValues(&icon_cache_);
 }
 
-gfx::Image* IconManager::LookupIconFromFilepath(const base::FilePath& file_name,
+gfx::Image* IconManager::LookupIconFromFilepath(const base::FilePath& file_path,
                                                 IconLoader::IconSize size) {
-  GroupMap::iterator it = group_cache_.find(file_name);
-  if (it != group_cache_.end())
-    return LookupIconFromGroup(it->second, size);
-
-  return NULL;
-}
-
-gfx::Image* IconManager::LookupIconFromGroup(const IconGroupID& group,
-                                             IconLoader::IconSize size) {
-  IconMap::iterator it = icon_cache_.find(CacheKey(group, size));
+  IconLoader::IconGroup group = IconLoader::GroupForFilepath(file_path);
+  auto it = icon_cache_.find(CacheKey(group, size));
   if (it != icon_cache_.end())
-    return it->second;
+    return it->second.get();
 
-  return NULL;
+  return nullptr;
 }
 
 base::CancelableTaskTracker::TaskId IconManager::LoadIcon(
-    const base::FilePath& file_name,
+    const base::FilePath& file_path,
     IconLoader::IconSize size,
     const IconRequestCallback& callback,
     base::CancelableTaskTracker* tracker) {
-  IconLoader* loader = new IconLoader(file_name, size, this);
+  IconLoader::IconGroup group = IconLoader::GroupForFilepath(file_path);
+  IconLoader* loader = new IconLoader(group, size, this);
   loader->AddRef();
   loader->Start();
 
@@ -72,76 +63,41 @@ base::CancelableTaskTracker::TaskId IconManager::LoadIcon(
   IconRequestCallback callback_runner = base::Bind(
       &RunCallbackIfNotCanceled, is_canceled, callback);
 
-  ClientRequest client_request = { callback_runner, file_name, size };
-  requests_[loader] = client_request;
+  requests_[loader] = {callback_runner, group, size};
   return id;
 }
 
 // IconLoader::Delegate implementation -----------------------------------------
 
-bool IconManager::OnGroupLoaded(IconLoader* loader,
-                                const IconGroupID& group) {
-  ClientRequests::iterator rit = requests_.find(loader);
-  if (rit == requests_.end()) {
-    NOTREACHED();
-    return false;
-  }
-
-  gfx::Image* result = LookupIconFromGroup(group, rit->second.size);
-  if (!result) {
-    return false;
-  }
-
-  return OnImageLoaded(loader, result, group);
-}
-
-bool IconManager::OnImageLoaded(
-    IconLoader* loader, gfx::Image* result, const IconGroupID& group) {
-  ClientRequests::iterator rit = requests_.find(loader);
+void IconManager::OnImageLoaded(IconLoader* loader,
+                                std::unique_ptr<gfx::Image> result,
+                                const IconLoader::IconGroup& group) {
+  auto rit = requests_.find(loader);
+  DCHECK(rit != requests_.end());
 
   // Balances the AddRef() in LoadIcon().
   loader->Release();
 
-  // Look up our client state.
-  if (rit == requests_.end()) {
-    NOTREACHED();
-    return false;  // Return false to indicate result should be deleted.
-  }
-
   const ClientRequest& client_request = rit->second;
 
-  // Cache the bitmap. Watch out: |result| may be NULL to indicate a current
+  // Cache the bitmap. Watch out: |result| may be null to indicate a current
   // failure. We assume that if we have an entry in |icon_cache_|
-  // it must not be NULL.
+  // it must not be null.
   CacheKey key(group, client_request.size);
-  IconMap::iterator it = icon_cache_.find(key);
-  if (it != icon_cache_.end()) {
-    if (!result) {
-      delete it->second;
-      icon_cache_.erase(it);
-    } else if (result != it->second) {
-      it->second->SwapRepresentations(result);
-      delete result;
-      result = it->second;
-    }
-  } else if (result) {
-    icon_cache_[key] = result;
+  if (result) {
+    client_request.callback.Run(result.get());
+    icon_cache_[key] = std::move(result);
+  } else {
+    client_request.callback.Run(nullptr);
+    icon_cache_.erase(key);
   }
 
-  group_cache_[client_request.file_path] = group;
-
-  // Inform our client that the request has completed.
-  client_request.callback.Run(result);
   requests_.erase(rit);
-
-  return true;  // Indicates we took ownership of result.
 }
 
-IconManager::CacheKey::CacheKey(const IconGroupID& group,
+IconManager::CacheKey::CacheKey(const IconLoader::IconGroup& group,
                                 IconLoader::IconSize size)
-    : group(group),
-      size(size) {
-}
+    : group(group), size(size) {}
 
 bool IconManager::CacheKey::operator<(const CacheKey &other) const {
   return std::tie(group, size) < std::tie(other.group, other.size);