Replaced GetAppIconForSize with GetAppIconImageFamily.

chrome/browser/app_icon_win.cc

Index: chrome/browser/app_icon_win.cc
diff --git a/chrome/browser/app_icon_win.cc b/chrome/browser/app_icon_win.cc
index 6f1424e4977b5c54e3ad1045be78dc010264c421..33b66f4aebb96f88f3fa4b407db26dd85ead31e7 100644
--- a/chrome/browser/app_icon_win.cc
+++ b/chrome/browser/app_icon_win.cc
@@ -7,7 +7,9 @@
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/common/chrome_constants.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/gfx/geometry/size.h"
 #include "ui/gfx/icon_util.h"
+#include "ui/gfx/image/image_family.h"
 
 #if defined(GOOGLE_CHROME_BUILD)
 #include "chrome/installer/util/install_util.h"
@@ -28,6 +30,8 @@ int GetAppIconResourceId() {
 }  // namespace
 
 HICON GetAppIcon() {
+  // TODO(mgiuca): Use GetAppIconImageFamily/CreateExact instead of LoadIcon, to
+  // get correct scaling. (See http://crbug.com/551256)
   const int icon_id = GetAppIconResourceId();
   // HICON returned from LoadIcon do not leak and do not have to be destroyed.
   return LoadIcon(GetModuleHandle(chrome::kBrowserResourcesDll),
@@ -35,16 +39,35 @@ HICON GetAppIcon() {
 }
 
 HICON GetSmallAppIcon() {
+  // TODO(mgiuca): Use GetAppIconImageFamily/CreateExact instead of LoadIcon, to
+  // get correct scaling. (See http://crbug.com/551256)
   const int icon_id = GetAppIconResourceId();
+  gfx::Size size = GetSmallAppIconSize();
   // HICON returned from LoadImage must be released using DestroyIcon.
   return static_cast<HICON>(LoadImage(
       GetModuleHandle(chrome::kBrowserResourcesDll), MAKEINTRESOURCE(icon_id),
-      IMAGE_ICON, GetSystemMetrics(SM_CXSMICON), GetSystemMetrics(SM_CYSMICON),
-      LR_DEFAULTCOLOR | LR_SHARED));
+      IMAGE_ICON, size.width(), size.height(), LR_DEFAULTCOLOR | LR_SHARED));
 }
 
-scoped_ptr<SkBitmap> GetAppIconForSize(int size) {
+gfx::Size GetAppIconSize() {
+  return gfx::Size(GetSystemMetrics(SM_CXICON), GetSystemMetrics(SM_CYICON));
+}
+
+gfx::Size GetSmallAppIconSize() {
+  return gfx::Size(GetSystemMetrics(SM_CXSMICON),
+                   GetSystemMetrics(SM_CYSMICON));
+}
+
+scoped_ptr<gfx::ImageFamily> GetAppIconImageFamily() {
   const int icon_id = GetAppIconResourceId();
-  return IconUtil::CreateSkBitmapFromIconResource(
-      GetModuleHandle(chrome::kBrowserResourcesDll), icon_id, size);
+  // Get the icon from chrome.dll (not chrome.exe, which has different resource
+  // IDs). If chrome.dll is not loaded, we are probably in a unit test, so fall
+  // back to getting the icon from the current module (assuming it is
+  // unit_tests.exe, that has the same resource IDs as chrome.dll).

[Nico, 2015/11/16 19:04:28]

why wasn't this needed previously? or am i just missing where this is from?

[Matt Giuca, 2015/11/17 07:11:45]

That's a great question. It took me some time to figure this one out. The answer
is "it just happened to work before" :/

This would call GetModuleHandle(kBrowserResourcesDll) which returned nullptr (in
tests). CreateSkBitmapFromIconResource passes this nullptr Win32's LoadImage
which, according to its docs "load[s] an OEM image" [1]. I'm not sure what that
is but it returns a valid 48x48 icon.

With my new code, it goes through base::win::GetResourceFromModule which
explicitly checks for a null module.

So yeah, this new code here is correct in explicitly checking for null.

[1]
https://msdn.microsoft.com/en-us/library/windows/desktop/ms648045(v=vs.85).aspx

+  HMODULE module = GetModuleHandle(chrome::kBrowserResourcesDll);
+  if (!module)
+    module = GetModuleHandle(nullptr);
+  DCHECK(module);
+
+  return IconUtil::CreateImageFamilyFromIconResource(module, icon_id);
 }