Consolidate icon loading on ChromeOS

chrome/browser/ui/webui/fileicon_source.cc

Index: chrome/browser/ui/webui/fileicon_source.cc
diff --git a/chrome/browser/ui/webui/fileicon_source.cc b/chrome/browser/ui/webui/fileicon_source.cc
index 30ee3dac6131c4397e61fa0a637d1e8922306cbb..d725a3306c282e7bab76c4b888ab7589193a3b70 100644
--- a/chrome/browser/ui/webui/fileicon_source.cc
+++ b/chrome/browser/ui/webui/fileicon_source.cc
@@ -4,45 +4,101 @@
 
 #include "chrome/browser/ui/webui/fileicon_source.h"
 
+#include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/file_path.h"
 #include "base/memory/ref_counted_memory.h"
+#include "base/string_split.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/common/time_format.h"
+#include "googleurl/src/gurl.h"
 #include "grit/generated_resources.h"
 #include "net/base/escape.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/image/image.h"
 
-// The path used in internal URLs to file icon data.
-static const char kFileIconPath[] = "fileicon";
+namespace {
 
-FileIconSource::FileIconSource()
-    : DataSource(kFileIconPath, MessageLoop::current()) {}
+typedef std::map<std::string, IconLoader::IconSize> QueryIconSizeMap;
 
-FileIconSource::~FileIconSource() {
-  cancelable_consumer_.CancelAllRequests();
-}
+// The path used in internal URLs to file icon data.
+const char kFileIconPath[] = "fileicon";
 
-void FileIconSource::StartDataRequest(const std::string& path,
-                                      bool is_incognito,
-                                      int request_id) {
-  std::string escaped_path = net::UnescapeURLComponent(path,
-      net::UnescapeRule::SPACES);
+// URL parameter specifying icon size.
+const char kIconSize[] = "iconsize";
+
+// Assuming the url is of the form '/path?query', convert the path portion into
+// a FilePath and return the resulting |file_path| and |query|.  The path
+// portion may have been encoded using encodeURIComponent().
+void GetFilePathAndQuery(const std::string& url,

[achuithb, 2012/01/23 20:59:27]

I assume GetFilePathAndQuery and QueryToIconSize are exercised by
fileicon_source_unittest?

[asanka, 2012/01/26 00:49:35]

Yes.

+                         FilePath* file_path,
+                         std::string* query) {
+  std::string path;
+  // We receive the url with chrome://fileicon/ stripped but GURL expects it.
+  const GURL gurl("chrome://fileicon/" + url);
+  path.assign(net::UnescapeURLComponent(
+      gurl.path().substr(1), (net::UnescapeRule::URL_SPECIAL_CHARS |
+                              net::UnescapeRule::SPACES)));
 #if defined(OS_WIN)
   // The path we receive has the wrong slashes and escaping for what we need;
   // this only appears to matter for getting icons from .exe files.
-  std::replace(escaped_path.begin(), escaped_path.end(), '/', '\\');
-  FilePath escaped_filepath(UTF8ToWide(escaped_path));
+  std::replace(path.begin(), path.end(), '/', '\\');
+  *file_path = FilePath(UTF8ToWide(path));
 #elif defined(OS_POSIX)
   // The correct encoding on Linux may not actually be UTF8.
-  FilePath escaped_filepath(escaped_path);
+  *file_path = FilePath(path);
 #endif
+  query->assign(gurl.query());
+}
+
+QueryIconSizeMap BuildQueryIconSizeMap() {
+  QueryIconSizeMap::value_type kQueryIconSizeData[] = {
+    std::make_pair("small", IconLoader::SMALL),
+    std::make_pair("normal", IconLoader::NORMAL),
+    std::make_pair("large", IconLoader::LARGE)
+  };
+
+  size_t kQSize = arraysize(kQueryIconSizeData);
+  return QueryIconSizeMap(&kQueryIconSizeData[0],
+                          &kQueryIconSizeData[kQSize]);
+}
+
+// Simple parser for data on the query.
+IconLoader::IconSize QueryToIconSize(const std::string& query) {
+  CR_DEFINE_STATIC_LOCAL(

[achuithb, 2012/01/23 20:59:27]

Do you think a comment would be useful here? I had to look up
CR_DEFINE_STATIC_LOCAL to figure out why you needed BuildQueryIconSizeMap(). I
realize we also do this in other places, but it's uncommon enough that a comment
wouldn't hurt.

[asanka, 2012/01/26 00:49:35]

Done. This code actually changed a bit, and I'm not using this macro anymore.

+      QueryIconSizeMap, kQueryIconSizeMap, (BuildQueryIconSizeMap()));
+  typedef std::pair<std::string, std::string> KVPair;
+  std::vector<KVPair> parameters;
+  if (base::SplitStringIntoKeyValuePairs(query, '=', '&', &parameters)) {
+    for (std::vector<KVPair>::const_iterator itk = parameters.begin();
+         itk != parameters.end(); ++itk) {
+      if (itk->first == kIconSize) {
+        QueryIconSizeMap::const_iterator itq(
+            kQueryIconSizeMap.find(itk->second));
+        if (itq != kQueryIconSizeMap.end())
+          return itq->second;
+      }
+    }
+  }
+  return IconLoader::NORMAL;
+}
+
+}
 
+FileIconSource::FileIconSource()
+    : DataSource(kFileIconPath, MessageLoop::current()) {}
+
+FileIconSource::~FileIconSource() {
+  cancelable_consumer_.CancelAllRequests();
+}
+
+void FileIconSource::FetchFileIcon(const FilePath& path,
+                                   IconLoader::IconSize icon_size,
+                                   int request_id) {
   IconManager* im = g_browser_process->icon_manager();
-  gfx::Image* icon = im->LookupIcon(escaped_filepath, IconLoader::NORMAL);
+  gfx::Image* icon = im->LookupIcon(path, icon_size);
 
   if (icon) {
     scoped_refptr<RefCountedBytes> icon_data(new RefCountedBytes);
@@ -51,9 +107,8 @@ void FileIconSource::StartDataRequest(const std::string& path,
     SendResponse(request_id, icon_data);
   } else {
     // Icon was not in cache, go fetch it slowly.
-    IconManager::Handle h = im->LoadIcon(escaped_filepath,
-        IconLoader::NORMAL,
-        &cancelable_consumer_,
+    IconManager::Handle h = im->LoadIcon(
+        path, icon_size, &cancelable_consumer_,
         base::Bind(&FileIconSource::OnFileIconDataAvailable,
                    base::Unretained(this)));
 
@@ -62,6 +117,15 @@ void FileIconSource::StartDataRequest(const std::string& path,
   }
 }
 
+void FileIconSource::StartDataRequest(const std::string& url_path,
+                                      bool is_incognito,
+                                      int request_id) {
+  std::string query;
+  FilePath file_path;
+  GetFilePathAndQuery(url_path, &file_path, &query);
+  FetchFileIcon(file_path, QueryToIconSize(query), request_id);
+}
+
 std::string FileIconSource::GetMimeType(const std::string&) const {
   // Rely on image decoder inferring the correct type.
   return std::string();