Clean up icon loader/manager in preparation for porting.

Remove a bunch of unused functions and change wstrings to filepaths and remove an obsolete enum.

Review URL: http://codereview.chromium.org/73007

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

7 files changed, 62 insertions, 128 deletions

diff --git a/chrome/browser/dom_ui/downloads_ui.cc b/chrome/browser/dom_ui/downloads_ui.cc
index cd3ac91..1bc5794 100644
--- a/chrome/browser/dom_ui/downloads_ui.cc
+++ b/chrome/browser/dom_ui/downloads_ui.cc
@@ -329,8 +329,7 @@ void DownloadsDOMHandler::HandleDrag(const Value* value) {
   DownloadItem* file = GetDownloadByValue(value);
   if (file) {
     IconManager* im = g_browser_process->icon_manager();
-    SkBitmap* icon = im->LookupIcon(file->full_path().ToWStringHack(),
-        IconLoader::NORMAL);
+    SkBitmap* icon = im->LookupIcon(file->full_path(), IconLoader::NORMAL);
     download_util::DragDownload(file, icon);
   }
 }
diff --git a/chrome/browser/dom_ui/fileicon_source.cc b/chrome/browser/dom_ui/fileicon_source.cc
index d1e507c..7e8ef9b 100644
--- a/chrome/browser/dom_ui/fileicon_source.cc
+++ b/chrome/browser/dom_ui/fileicon_source.cc
@@ -31,7 +31,13 @@ void FileIconSource::StartDataRequest(const std::string& path,
   std::replace(escaped_path.begin(), escaped_path.end(), '/', '\\');
 
   // Fast look up.
-  SkBitmap* icon = im->LookupIcon(UTF8ToWide(escaped_path), IconLoader::NORMAL);
+#if defined(OS_WIN)
+  FilePath escaped_filepath(UTF8ToWide(escaped_path));
+#else if defined(OS_POSIX)
+  // The correct encoding on Linux may not actually be UTF8.
+  FilePath escaped_filepath(escaped_path);
+#endif
+  SkBitmap* icon = im->LookupIcon(escaped_filepath, IconLoader::NORMAL);
 
   if (icon) {
     std::vector<unsigned char> png_bytes;
@@ -41,7 +47,7 @@ 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(UTF8ToWide(escaped_path),
+    IconManager::Handle h = im->LoadIcon(escaped_filepath,
         IconLoader::NORMAL,
         &cancelable_consumer_,
         NewCallback(this, &FileIconSource::OnFileIconDataAvailable));
diff --git a/chrome/browser/icon_loader.cc b/chrome/browser/icon_loader.cc
index 64a45034..5004e5e 100644
--- a/chrome/browser/icon_loader.cc
+++ b/chrome/browser/icon_loader.cc
@@ -20,6 +20,7 @@
 #include "SkBitmap.h"
 
 namespace {
+
 class IconLoaderProcessor :
     public base::RefCountedThreadSafe<IconLoaderProcessor> {
  public:
@@ -29,7 +30,6 @@ class IconLoaderProcessor :
         small_icon_(NULL),
         large_icon_(NULL),
         loading_from_resource_(target->loading_from_resource_),
-        icon_type_(target->icon_type_),
         icon_size_(target->icon_size_) {
     DCHECK(target);
     path_ = target->path_;
@@ -48,7 +48,7 @@ class IconLoaderProcessor :
   // Loads the icon with the specified dimensions.
   HICON LoadSizedIcon(int width, int height) {
     return static_cast<HICON>(LoadImage(NULL,
-                                        path_.c_str(),
+                                        path_.value().c_str(),
                                         width, height,
                                         IMAGE_ICON,
                                         LR_LOADTRANSPARENT | LR_LOADFROMFILE));
@@ -105,14 +105,12 @@ class IconLoaderProcessor :
     // itself.
     HICON icon_to_convert = NULL;
     gfx::Size s;
-    if (icon_type_ == IconLoader::SK_BITMAP) {
-      if (large_icon_) {
-        icon_to_convert = large_icon_;
-        s.SetSize(large_width, large_height);
-      } else if (small_icon_) {
-        icon_to_convert = small_icon_;
-        s.SetSize(small_width, small_height);
-      }
+    if (large_icon_) {
+      icon_to_convert = large_icon_;
+      s.SetSize(large_width, large_height);
+    } else if (small_icon_) {
+      icon_to_convert = small_icon_;
+      s.SetSize(small_width, small_height);
     }
 
     if (icon_to_convert) {
@@ -144,7 +142,7 @@ class IconLoaderProcessor :
         NOTREACHED();
     }
     SHFILEINFO file_info = { 0 };
-    if (!SHGetFileInfo(path_.c_str(), FILE_ATTRIBUTE_NORMAL, &file_info,
+    if (!SHGetFileInfo(path_.value().c_str(), FILE_ATTRIBUTE_NORMAL, &file_info,
                        sizeof(SHFILEINFO),
                        SHGFI_ICON | size | SHGFI_USEFILEATTRIBUTES))
       return;
@@ -163,24 +161,9 @@ class IconLoaderProcessor :
 
   // Invoked in the target thread.
   void NotifyFetcher() {
-    if (target_) {
-      switch (target_->icon_type_) {
-        case IconLoader::SK_BITMAP:
-          if (target_->OnLoadComplete(bitmap_)) {
-            // Receiver took ownership of the bitmap.
-            bitmap_ = NULL;
-          }
-          break;
-        case IconLoader::WINDOWS_HICON:
-          if (target_->OnLoadComplete(small_icon_, large_icon_)) {
-            // Receiver took ownership of the icons.
-            small_icon_ = NULL;
-            large_icon_ = NULL;
-          }
-          break;
-        default:
-          NOTREACHED();
-      }
+    if (target_ && target_->OnLoadComplete(bitmap_)) {
+      // Receiver took ownership of the bitmap.
+      bitmap_ = NULL;
     }
   }
 
@@ -192,13 +175,12 @@ class IconLoaderProcessor :
   IconLoader* target_;
 
   // The path of the file.
-  std::wstring path_;
+  FilePath path_;
 
   // Fields from IconLoader that we need to copy as we cannot access them
   // directly from the target_ in a thread-safe way.
   bool loading_from_resource_;
   IconLoader::IconSize icon_size_;
-  IconLoader::IconType icon_type_;
 
   // The result bitmap.
   SkBitmap* bitmap_;
@@ -209,31 +191,22 @@ class IconLoaderProcessor :
   // The result large icon.
   HICON large_icon_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(IconLoaderProcessor);
+  DISALLOW_COPY_AND_ASSIGN(IconLoaderProcessor);
 };
-}
 
-// static
-IconLoader* IconLoader::CreateIconLoaderForFile(const std::wstring& path,
-                                                IconType icon_type,
-                                                Delegate* delegate) {
-  // Note: the icon size is unused in this case.
-  return new IconLoader(path, icon_type, false, IconLoader::NORMAL, delegate);
-}
+}  // namespace
 
 // static
 IconLoader* IconLoader::CreateIconLoaderForFileResource(
-    const std::wstring& path, IconSize size, Delegate* delegate) {
-  return new IconLoader(path, IconLoader::SK_BITMAP, true, size, delegate);
+    const FilePath& path, IconSize size, Delegate* delegate) {
+  return new IconLoader(path, true, size, delegate);
 }
 
-IconLoader::IconLoader(const std::wstring& path,
-                       IconType type,
+IconLoader::IconLoader(const FilePath& path,
                        bool from_resource,
                        IconSize size,
                        Delegate* delegate)
     : path_(path),
-      icon_type_(type),
       loading_from_resource_(from_resource),
       icon_size_(size),
       delegate_(delegate),
@@ -268,11 +241,3 @@ bool IconLoader::OnLoadComplete(SkBitmap* bitmap) {
   }
   return false;
 }
-
-bool IconLoader::OnLoadComplete(HICON small_icon, HICON large_icon) {
-  if (delegate_) {
-    return delegate_->OnHICONLoaded(this, small_icon, large_icon);
-    // We are likely deleted after this point.
-  }
-  return false;
-}
diff --git a/chrome/browser/icon_loader.h b/chrome/browser/icon_loader.h
index 3505aae..ff1c5a4 100644
--- a/chrome/browser/icon_loader.h
+++ b/chrome/browser/icon_loader.h
@@ -2,13 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_ICON_LOADER_H__
-#define CHROME_BROWSER_ICON_LOADER_H__
+#ifndef CHROME_BROWSER_ICON_LOADER_H_
+#define CHROME_BROWSER_ICON_LOADER_H_
 
 #include <string>
-#include <windows.h>
 
 #include "base/basictypes.h"
+#include "base/file_path.h"
 
 namespace {
 class IconLoaderProcessor;
@@ -29,11 +29,6 @@ class SkBitmap;
 ////////////////////////////////////////////////////////////////////////////////
 class IconLoader {
  public:
-  enum IconType {
-    SK_BITMAP = 0,
-    WINDOWS_HICON
-  };
-
   enum IconSize {
     SMALL = 0,  // 16x16
     NORMAL,     // 32x32
@@ -45,34 +40,19 @@ class IconLoader {
     // Invoked when an icon has been read. |source| is the IconLoader. If the
     // icon has been successfully loaded, result is non-null. This method must
     // return true if it is taking ownership of the returned bitmap.
-    //
-    // This method is only called when GetIconType() above returns SK_BITMAP.
     virtual bool OnSkBitmapLoaded(IconLoader* source, SkBitmap* result) = 0;
-
-    // Invoked when the small and the large HICONS have been read. |source| is
-    // the IconLoader. If the small icon has been successfully loaded,
-    // small_icon is non-null. The same applies to the large_icon. This method
-    // must return true if it is taking ownership of the returned icon handles.
-    //
-    // This method is only called when GetIconType() above returns
-    // WINDOWS_HICON.
-    virtual bool OnHICONLoaded(IconLoader* source,
-                               HICON small_icon,
-                               HICON large_icon) = 0;
   };
 
   // Create a new IconLoader that loads the icon from the data at contained in
-  // the file at |path|.  |icon_type| specifies which format to generate and
-  // which method is invoked on the |delegate| once the icon was loaded.
-  static IconLoader* CreateIconLoaderForFile(const std::wstring& path,
-                                             IconType icon_type,
+  // the file at |path|.
+  static IconLoader* CreateIconLoaderForFile(const FilePath& path,
                                              Delegate* delegate);
 
   // Create a new IconLoader that loads the icon in the resource of the file
   // at |path|.  This is used with .exe/.dll files.
   // Note that this generates a SkBitmap (and consequently OnSkBitmapLoaded is
   // invoked on the delegate once the load has completed).
-  static IconLoader* CreateIconLoaderForFileResource(const std::wstring& path,
+  static IconLoader* CreateIconLoaderForFileResource(const FilePath& path,
                                                      IconSize size,
                                                      Delegate* delegate);
 
@@ -89,8 +69,7 @@ class IconLoader {
   friend class IconLoaderProcessor;
 
   // Use the factory methods CreateIconLoader* instead of using this constructor
-  IconLoader(const std::wstring& path,
-             IconType type,
+  IconLoader(const FilePath& path,
              bool from_resource,
              IconSize size,
              Delegate* delegate);
@@ -99,12 +78,8 @@ class IconLoader {
   // object was requested.
   bool OnLoadComplete(SkBitmap* result);
 
-  // Invoked by the processor when the file has been read and HICON handles
-  // (small and large) were requested.
-  bool OnLoadComplete(HICON small_icon, HICON large_icon);
-
   // The path.
-  std::wstring path_;
+  FilePath path_;
 
   // The delegate.
   Delegate* delegate_;
@@ -117,14 +92,10 @@ class IconLoader {
   // Not used if loading_from_resource_ is false.
   IconSize icon_size_;
 
-  // The type of icon that should be generated.
-  // Not used if loading_from_resource_ is true.
-  IconType icon_type_;
-
   // The underlying object performing the read.
   IconLoaderProcessor* processor_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(IconLoader);
+  DISALLOW_COPY_AND_ASSIGN(IconLoader);
 };
 
-#endif  // CHROME_BROWSER_ICON_LOADER_H__
+#endif  // CHROME_BROWSER_ICON_LOADER_H_
diff --git a/chrome/browser/icon_manager.cc b/chrome/browser/icon_manager.cc
index f75e93b..0513a81 100644
--- a/chrome/browser/icon_manager.cc
+++ b/chrome/browser/icon_manager.cc
@@ -17,12 +17,14 @@ IconManager::~IconManager() {
   STLDeleteValues(&icon_cache_);
 }
 
-SkBitmap* IconManager::LookupIcon(const std::wstring& file_name,
+SkBitmap* IconManager::LookupIcon(const FilePath& file_name,
                                   IconLoader::IconSize size) {
-  std::wstring path = file_name;
-  std::wstring extension = file_util::GetFileExtensionFromPath(path);
+  FilePath path = file_name;
+  FilePath::StringType extension = file_util::GetFileExtensionFromPath(path);
+#if defined(OS_WIN)
   if (extension != L"exe" && extension != L"dll" && extension != L"ico")
-    path = L'.' + extension;
+    path = FilePath(L'.' + extension);
+#endif
 
   IconMap::iterator it = icon_cache_.find(CacheKey(path, size));
   if (it != icon_cache_.end())
@@ -32,14 +34,16 @@ SkBitmap* IconManager::LookupIcon(const std::wstring& file_name,
 }
 
 IconManager::Handle IconManager::LoadIcon(
-    const std::wstring& file_name,
+    const FilePath& file_name,
     IconLoader::IconSize size,
     CancelableRequestConsumerBase* consumer,
     IconRequestCallback* callback) {
-  std::wstring path = file_name;
-  std::wstring extension = file_util::GetFileExtensionFromPath(path);
+  FilePath path = file_name;
+  FilePath::StringType extension = file_util::GetFileExtensionFromPath(path);
+#if defined(OS_WIN)
   if (extension != L"exe" && extension != L"dll" && extension != L"ico")
-    path = L'.' + extension;
+    path = FilePath(L'.' + extension);
+#endif
 
   IconRequest* request = new IconRequest(callback);
   AddRequest(request, consumer);
@@ -91,14 +95,7 @@ bool IconManager::OnSkBitmapLoaded(IconLoader* source, SkBitmap* result) {
   return true;  // Indicates we took ownership of result.
 }
 
-bool IconManager::OnHICONLoaded(IconLoader* source,
-                                HICON small_icon,
-                                HICON large_icon) {
-  NOTREACHED();
-  return false;
-}
-
-IconManager::CacheKey::CacheKey(std::wstring file_name,
+IconManager::CacheKey::CacheKey(const FilePath& file_name,
                                 IconLoader::IconSize size)
     : file_name(file_name),
       size(size) {
diff --git a/chrome/browser/icon_manager.h b/chrome/browser/icon_manager.h
index 6840ac1..4e44cf4 100644
--- a/chrome/browser/icon_manager.h
+++ b/chrome/browser/icon_manager.h
@@ -39,8 +39,8 @@
 // Icon bitmaps returned should be treated as const since they may be referenced
 // by other clients. Make a copy of the icon if you need to modify it.
 
-#ifndef CHROME_BROWSER_ICON_MANAGER_H__
-#define CHROME_BROWSER_ICON_MANAGER_H__
+#ifndef CHROME_BROWSER_ICON_MANAGER_H_
+#define CHROME_BROWSER_ICON_MANAGER_H_
 
 #include <map>
 #include <set>
@@ -63,7 +63,7 @@ public:
   // it via 'LoadIcon'. The returned bitmap is owned by the IconManager and must
   // not be free'd by the caller. If the caller needs to modify the icon, it
   // must make a copy and modify the copy.
-  SkBitmap* LookupIcon(const std::wstring& file_name,
+  SkBitmap* LookupIcon(const FilePath& file_name,
                        IconLoader::IconSize size);
 
   // Asynchronous call to lookup and return the icon associated with file. The
@@ -75,25 +75,22 @@ public:
   typedef CancelableRequestProvider::Handle Handle;
   typedef Callback2<Handle, SkBitmap*>::Type IconRequestCallback;
 
-  Handle LoadIcon(const std::wstring& file_name,
+  Handle LoadIcon(const FilePath& file_name,
                   IconLoader::IconSize size,
                   CancelableRequestConsumerBase* consumer,
                   IconRequestCallback* callback);
 
   // IconLoader::Delegate interface.
   virtual bool OnSkBitmapLoaded(IconLoader* source, SkBitmap* result);
-  virtual bool OnHICONLoaded(IconLoader* source,
-                             HICON small_icon,
-                             HICON large_icon);
 
 private:
   struct CacheKey {
-    CacheKey(std::wstring file_name, IconLoader::IconSize size);
+    CacheKey(const FilePath& file_name, IconLoader::IconSize size);
 
     // Used as a key in the map below, so we need this comparator.
     bool operator<(const CacheKey &other) const;
 
-    std::wstring file_name;
+    FilePath file_name;
     IconLoader::IconSize size;
   };
 
@@ -103,7 +100,7 @@ private:
   typedef CancelableRequest<IconRequestCallback> IconRequest;
   typedef struct {
     scoped_refptr<IconRequest> request;
-    std::wstring file_name;
+    FilePath file_name;
     IconLoader::IconSize size;
   } ClientRequest;
 
@@ -111,7 +108,7 @@ private:
   typedef base::hash_map<IconLoader*, ClientRequest> ClientRequests;
   ClientRequests requests_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(IconManager);
+  DISALLOW_COPY_AND_ASSIGN(IconManager);
 };
 
-#endif  // #ifndef CHROME_BROWSER_ICON_MANAGER_H__
+#endif  // #ifndef CHROME_BROWSER_ICON_MANAGER_H_
diff --git a/chrome/browser/views/download_item_view.cc b/chrome/browser/views/download_item_view.cc
index d66a53e..fb2c1a7 100644
--- a/chrome/browser/views/download_item_view.cc
+++ b/chrome/browser/views/download_item_view.cc
@@ -571,7 +571,7 @@ void DownloadItemView::Paint(ChromeCanvas* canvas) {
   // Paint the icon.
   IconManager* im = g_browser_process->icon_manager();
   SkBitmap* icon = IsDangerousMode() ? warning_icon_ :
-      im->LookupIcon(download_->full_path().ToWStringHack(), IconLoader::SMALL);
+      im->LookupIcon(download_->full_path(), IconLoader::SMALL);
 
   // We count on the fact that the icon manager will cache the icons and if one
   // is available, it will be cached here. We *don't* want to request the icon
@@ -809,7 +809,7 @@ bool DownloadItemView::OnMouseDragged(const views::MouseEvent& event) {
   if (dragging_) {
     if (download_->state() == DownloadItem::COMPLETE) {
       IconManager* im = g_browser_process->icon_manager();
-      SkBitmap* icon = im->LookupIcon(download_->full_path().ToWStringHack(),
+      SkBitmap* icon = im->LookupIcon(download_->full_path(),
                                       IconLoader::SMALL);
       if (icon)
         download_util::DragDownload(download_, icon);
@@ -843,8 +843,7 @@ void DownloadItemView::OnExtractIconComplete(IconManager::Handle handle,
 
 void DownloadItemView::LoadIcon() {
   IconManager* im = g_browser_process->icon_manager();
-  im->LoadIcon(download_->full_path().ToWStringHack(), IconLoader::SMALL,
-               &icon_consumer_,
+  im->LoadIcon(download_->full_path(), IconLoader::SMALL, &icon_consumer_,
                NewCallback(this, &DownloadItemView::OnExtractIconComplete));
 }