Removing base::ThreadRestrictions::ScopedAllowIO from icon_manager_linux.cc

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 2f1444e83d0d7e999d93a9e818c95fc0ea4db953..41d746127155b6fb523d4cd4d0d7806b84d35b7e 100644
--- a/chrome/browser/icon_manager.cc
+++ b/chrome/browser/icon_manager.cc
@@ -27,7 +27,7 @@ void RunCallbackIfNotCanceled(
 
 struct IconManager::ClientRequest {
   IconRequestCallback callback;
-  IconGroupID group;
+  FilePath file_path;
   IconLoader::IconSize size;
 };
 
@@ -38,9 +38,17 @@ IconManager::~IconManager() {
   STLDeleteValues(&icon_cache_);
 }
 
-gfx::Image* IconManager::LookupIcon(const FilePath& file_name,
+gfx::Image* IconManager::LookupIconFromFilepath(const FilePath& file_name,
+                                    IconLoader::IconSize size) {

[Robert Sesek, 2013/02/12 16:49:25]

nit: this isn't wrapped correctly. It should either align with 'c' in const or
just be indented 4 spaces. Same on line 51.

[shatch, 2013/02/13 16:14:52]

Done.

+  GroupMap::iterator it = group_cache_.find(file_name);
+  if (it != group_cache_.end())
+    LookupIconFromGroup(it->second, size);
+
+  return NULL;
+}
+
+gfx::Image* IconManager::LookupIconFromGroup(const IconGroupID& group,
                                     IconLoader::IconSize size) {
-  IconGroupID group = GetGroupIDFromFilepath(file_name);
   IconMap::iterator it = icon_cache_.find(CacheKey(group, size));
   if (it != icon_cache_.end())
     return it->second;
@@ -53,9 +61,7 @@ CancelableTaskTracker::TaskId IconManager::LoadIcon(
     IconLoader::IconSize size,
     const IconRequestCallback& callback,
     CancelableTaskTracker* tracker) {
-  IconGroupID group = GetGroupIDFromFilepath(file_name);
-
-  IconLoader* loader = new IconLoader(group, size, this);
+  IconLoader* loader = new IconLoader(file_name, size, this);
   loader->AddRef();
   loader->Start();
 
@@ -64,14 +70,42 @@ CancelableTaskTracker::TaskId IconManager::LoadIcon(
   IconRequestCallback callback_runner = base::Bind(
       &RunCallbackIfNotCanceled, is_canceled, callback);
 
-  ClientRequest client_request = { callback_runner, group, size };
+  ClientRequest client_request = { callback_runner, file_name, size };
   requests_[loader] = client_request;
   return id;
 }
 
 // IconLoader::Delegate implementation -----------------------------------------
 
-bool IconManager::OnImageLoaded(IconLoader* loader, gfx::Image* result) {
+bool IconManager::OnGroupLoaded(IconLoader* loader,
+                                const IconGroupID& group) {
+  ClientRequests::iterator rit = requests_.find(loader);
+
+  if (rit == requests_.end()) {
+    NOTREACHED();
+    return false;
+  }
+
+  const ClientRequest& client_request = rit->second;
+
+  CacheKey key(group, client_request.size);
+
+  group_cache_[client_request.file_path] = group;
+
+  gfx::Image* result = LookupIconFromGroup(group, client_request.size);
+  if (!result) {
+    return false;
+  }
+
+  // Inform our client that the request has completed.
+  client_request.callback.Run(result);
+  requests_.erase(rit);
+
+  return true;

[Robert Sesek, 2013/02/12 16:49:25]

Don't you also need to IconLoader::Release() if this happens since OnImageLoaded
won't be called?

But going further, could you rewrite OnGroupLoaded to just call through to
OnImageLoaded so you don't have to duplicate so much bookkeeping logic?

[shatch, 2013/02/13 16:14:52]

Done.

+}
+
+bool IconManager::OnImageLoaded(
+    IconLoader* loader, gfx::Image* result, const IconGroupID& group) {
   ClientRequests::iterator rit = requests_.find(loader);
 
   // Balances the AddRef() in LoadIcon().
@@ -87,7 +121,7 @@ bool IconManager::OnImageLoaded(IconLoader* loader, gfx::Image* result) {
 
   // Cache the bitmap. Watch out: |result| or the cached bitmap may be NULL to
   // indicate a current or past failure.
-  CacheKey key(client_request.group, client_request.size);
+  CacheKey key(group, client_request.size);
   IconMap::iterator it = icon_cache_.find(key);
   if (it != icon_cache_.end() && result && it->second) {
     it->second->SwapRepresentations(result);
@@ -97,6 +131,8 @@ bool IconManager::OnImageLoaded(IconLoader* loader, gfx::Image* result) {
     icon_cache_[key] = result;
   }
 
+  group_cache_[client_request.file_path] = group;
+
   // Inform our client that the request has completed.
   client_request.callback.Run(result);
   requests_.erase(rit);