[Win] Profile desktop shortcuts should use the Gaia avatar if it's in use.

chrome/browser/profiles/profile_shortcut_manager_win.cc

Index: chrome/browser/profiles/profile_shortcut_manager_win.cc
diff --git a/chrome/browser/profiles/profile_shortcut_manager_win.cc b/chrome/browser/profiles/profile_shortcut_manager_win.cc
index 72256196a964929786ec37c968159783a6a13dde..0854b93bd5f92e159261c8ab308de71fd819d254 100644
--- a/chrome/browser/profiles/profile_shortcut_manager_win.cc
+++ b/chrome/browser/profiles/profile_shortcut_manager_win.cc
@@ -71,7 +71,7 @@ const int kMaxProfileShortcutFileNameLength = 64;
 const int kShortcutIconSize = 48;
 const int kProfileAvatarBadgeSize = kShortcutIconSize / 2;
 
-const int kCurrentProfileIconVersion = 2;
+const int kCurrentProfileIconVersion = 3;
 
 // 2x sized profile avatar icons. Mirrors |kDefaultAvatarIconResources| in
 // profile_info_cache.cc.
@@ -589,17 +589,22 @@ base::string16 SanitizeShortcutProfileNameString(
   return sanitized;
 }
 
+// Returns a copied SkBitmap for the given image that can be safely passed to
+// another thread.
+SkBitmap GetSkBitmapCopy(const gfx::Image& image) {
+  const SkBitmap* image_bitmap = image.ToSkBitmap();
+  SkBitmap bitmap_copy;
+  image_bitmap->deepCopyTo(&bitmap_copy);
+  return bitmap_copy;
+}
+
 // Returns a copied SkBitmap for the given resource id that can be safely passed
 // to another thread.
 SkBitmap GetImageResourceSkBitmapCopy(int resource_id) {
   const gfx::Image image =
       ResourceBundle::GetSharedInstance().GetNativeImageNamed(resource_id);
   DCHECK(!image.IsEmpty());

[Alexei Svitkine (slow), 2014/08/12 14:54:07]

Nit: Move this check to GetSkBitmapCopy() since this is what image.ToSkBitmap()
documents as being required.

[noms (inactive), 2014/08/12 18:52:53]

Done.

-
-  const SkBitmap* image_bitmap = image.ToSkBitmap();
-  SkBitmap bitmap_copy;
-  image_bitmap->deepCopyTo(&bitmap_copy);
-  return bitmap_copy;
+  return GetSkBitmapCopy(image);
 }
 
 }  // namespace
@@ -808,15 +813,24 @@ void ProfileShortcutManagerWin::CreateOrUpdateShortcutsForProfileAtPath(
   if (!remove_badging) {
     params.profile_name = cache->GetNameOfProfileAtIndex(profile_index);
 
-    const size_t icon_index =
-        cache->GetAvatarIconIndexOfProfileAtIndex(profile_index);
-    const int resource_id_1x =
-        profiles::GetDefaultAvatarIconResourceIDAtIndex(icon_index);
-    const int resource_id_2x = kProfileAvatarIconResources2x[icon_index];
-    // Make a copy of the SkBitmaps to ensure that we can safely use the image
-    // data on the FILE thread.
-    params.avatar_image_1x = GetImageResourceSkBitmapCopy(resource_id_1x);
-    params.avatar_image_2x = GetImageResourceSkBitmapCopy(resource_id_2x);
+    // The profile might be using the Gaia avatar, which is not in the
+    // resources array.
+    if (cache->IsUsingGAIAPictureOfProfileAtIndex(profile_index)) {
+      const gfx::Image* image =
+          cache->GetGAIAPictureOfProfileAtIndex(profile_index);

[Alexei Svitkine (slow), 2014/08/12 14:54:07]

According to the docs for that method:

  // Returns the GAIA picture for the given profile. This may return NULL
  // if the profile does not have a GAIA picture or if the picture must be
  // loaded from disk.

I don't see a NULL check in your code, but the comment for the method sounds
like we should have one (though it's not clear to me what's the best way to
handle it - I guess if there's no GAIA picture than using the other icon is
fine, but if it's just not loaded... perhaps it needs to load it?). Or perhaps
the comment on that API method isn't quite accurate?

[noms (inactive), 2014/08/12 18:52:53]

Done.

+      params.avatar_image_1x = GetSkBitmapCopy(*image);
+      params.avatar_image_2x = params.avatar_image_1x;

[gab, 2014/08/11 20:49:34]

Maybe add a comment about explaining why it's okay to re-use the same picture?

Even better, a DCHECK that the size is bigger then what's required for the 2x
picture?

[noms (inactive), 2014/08/12 18:52:53]

Done.

+    } else {
+      const size_t icon_index =
+          cache->GetAvatarIconIndexOfProfileAtIndex(profile_index);
+      const int resource_id_1x =
+          profiles::GetDefaultAvatarIconResourceIDAtIndex(icon_index);
+      const int resource_id_2x = kProfileAvatarIconResources2x[icon_index];
+      // Make a copy of the SkBitmaps to ensure that we can safely use the image
+      // data on the FILE thread.
+      params.avatar_image_1x = GetImageResourceSkBitmapCopy(resource_id_1x);
+      params.avatar_image_2x = GetImageResourceSkBitmapCopy(resource_id_2x);
+    }
   }
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,