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

chrome/browser/profiles/profile_shortcut_manager_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/profiles/profile_shortcut_manager_win.h"
 
 #include <shlobj.h>  // For SHChangeNotify().
 
 #include <string>
 #include <vector>
@@ skipping 53 matching lines @@
 // differently than it was when a shortcut was originally created.
 const int kMaxProfileShortcutFileNameLength = 64;
 
 // The avatar badge size needs to be half of the shortcut icon size because
 // the Windows taskbar icon is 32x32 and the avatar icon overlay is 16x16. So to
 // get the shortcut avatar badge and the avatar icon overlay to match up, we
 // need to preserve those ratios when creating the shortcut icon.
 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.
 const int kProfileAvatarIconResources2x[] = {
   IDR_PROFILE_AVATAR_2X_0,
   IDR_PROFILE_AVATAR_2X_1,
   IDR_PROFILE_AVATAR_2X_2,
   IDR_PROFILE_AVATAR_2X_3,
   IDR_PROFILE_AVATAR_2X_4,
   IDR_PROFILE_AVATAR_2X_5,
@@ skipping 497 matching lines @@
   }
 
   base::TrimWhitespace(sanitized, base::TRIM_LEADING, &sanitized);
   if (sanitized.size() > kMaxProfileShortcutFileNameLength)
     sanitized.erase(kMaxProfileShortcutFileNameLength);
   base::TrimWhitespace(sanitized, base::TRIM_TRAILING, &sanitized);
 
   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
 
 namespace profiles {
 namespace internal {
 
 base::FilePath GetProfileIconPath(const base::FilePath& profile_path) {
   return profile_path.AppendASCII(kProfileIconFileName);
 }
@@ skipping 188 matching lines @@
   // set in the profile info cache.
   if (params.old_profile_name.empty() &&
       create_mode == UPDATE_EXISTING_ONLY &&
       action == IGNORE_NON_PROFILE_SHORTCUTS) {
     return;
   }
 
   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,
       base::Bind(&CreateOrUpdateDesktopShortcutsAndIconForProfile, params));
 
   cache->SetShortcutNameOfProfileAtIndex(profile_index,
                                          params.profile_name);
 }
 
 void ProfileShortcutManagerWin::Observe(
@@ skipping 10 matching lines @@
         // Ensure the profile's icon file has been created.
         CreateOrUpdateProfileIcon(profile->GetPath());
       }
       break;
     }
     default:
       NOTREACHED();
       break;
   }
 }