Create profile .ico file on profile creation

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 4620240825ae897b36b72fe078729a08d424487c..6391c240d33baf7b35f2196778b97e86d5d0f07e 100644
--- a/chrome/browser/profiles/profile_shortcut_manager_win.cc
+++ b/chrome/browser/profiles/profile_shortcut_manager_win.cc
@@ -139,14 +139,14 @@ SkBitmap BadgeIcon(const SkBitmap& app_icon_bitmap,
 // badging the browser distribution icon with the profile avatar.
 // Returns a path to the shortcut icon file on disk, which is empty if this
 // fails. Use index 0 when assigning the resulting file as the icon.
-base::FilePath CreateChromeDesktopShortcutIconForProfile(
+void CreateOrUpdateShortcutIconForProfile(
     const base::FilePath& profile_path,
     const SkBitmap& avatar_bitmap_1x,
     const SkBitmap& avatar_bitmap_2x) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   scoped_ptr<SkBitmap> app_icon_bitmap(GetAppIconForSize(kShortcutIconSize));
   if (!app_icon_bitmap.get())
-    return base::FilePath();
+    return;
 
   const SkBitmap badged_bitmap = BadgeIcon(*app_icon_bitmap,
                                            avatar_bitmap_1x, 1);
@@ -161,9 +161,7 @@ base::FilePath CreateChromeDesktopShortcutIconForProfile(
       profile_path.AppendASCII(profiles::internal::kProfileIconFileName);
   if (!IconUtil::CreateIconFileFromSkBitmap(badged_bitmap, large_badged_bitmap,
                                             icon_path))
-    return base::FilePath();
-
-  return icon_path;
+    LOG(ERROR) << "Failed to create icon at " << icon_path.value();

[Alexei Svitkine (slow), 2013/04/26 21:16:23]

Just make it a NOTREACHED()?

Also, please add {}'s to the if.

[calamity, 2013/04/30 06:45:41]

Done.

 }
 
 // Gets the user and system directories for desktop shortcuts. Parameters may
@@ -313,8 +311,6 @@ void CreateOrUpdateDesktopShortcutsForProfile(
     const base::FilePath& profile_path,
     const string16& old_profile_name,
     const string16& profile_name,
-    const SkBitmap& avatar_image_1x,
-    const SkBitmap& avatar_image_2x,
     ProfileShortcutManagerWin::CreateOrUpdateMode create_mode,
     ProfileShortcutManagerWin::NonProfileShortcutAction action) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
@@ -352,11 +348,9 @@ void CreateOrUpdateDesktopShortcutsForProfile(
   // If it is empty, it means the shortcut being created should be a regular,
   // non-profile Chrome shortcut.
   if (!profile_name.empty()) {
-    const base::FilePath shortcut_icon =
-        CreateChromeDesktopShortcutIconForProfile(profile_path,
-                                                  avatar_image_1x,
-                                                  avatar_image_2x);
-    if (!shortcut_icon.empty())
+    base::FilePath shortcut_icon =
+        profile_path.AppendASCII(profiles::internal::kProfileIconFileName);
+    if (file_util::PathExists(shortcut_icon))

[Alexei Svitkine (slow), 2013/04/26 21:16:23]

When would this be the case?

Wouldn't we want to re-create the icon if it doesn't exist?

[calamity, 2013/04/30 06:45:41]

Rolled the 2 functions back together and created the icon at the top of this
function.

       properties.set_icon(shortcut_icon, 0);
     properties.set_arguments(command_line);
   } else {
@@ -596,6 +590,7 @@ void ProfileShortcutManagerWin::HasProfileShortcuts(
 
 void ProfileShortcutManagerWin::OnProfileAdded(
     const base::FilePath& profile_path) {
+  CreateOrUpdateShortcutIconForProfileAtPath(profile_path);
   const size_t profile_count =
       profile_manager_->GetProfileInfoCache().GetNumberOfProfiles();
   if (profile_count == 1) {
@@ -621,7 +616,9 @@ void ProfileShortcutManagerWin::OnProfileWasRemoved(
   // from an existing shortcut.
   const bool deleting_down_to_last_profile = (cache.GetNumberOfProfiles() == 1);
   if (deleting_down_to_last_profile) {
-    CreateOrUpdateShortcutsForProfileAtPath(cache.GetPathOfProfileAtIndex(0),
+    base::FilePath profile_path = cache.GetPathOfProfileAtIndex(0);
+    CreateOrUpdateShortcutIconForProfileAtPath(profile_path);

[Alexei Svitkine (slow), 2013/04/26 21:16:23]

Add a comment mentioning that this is needed to "unbadge" the icon.

[calamity, 2013/04/30 06:45:41]

Done. I actually want to make an unbadged icon that overwrites the profile icon.
Should I do this in this patch? (It currently crashes on the no badge case, the
changes are above in CreateOrUpdateShortcutIconForProfile)

If you're interested, the reason I need an unbadged icon to overwrite the
profile icon is that if the user pins a profile shortcut, that shortcut points
to the profile icon. So we can either make the profile icon an unbadged chrome
icon in the final delete case or we can try to find every profile shortcut and
change the icon to point back at chrome.exe. I think the old code explicitly set
no icon which made it point to chrome.exe by default but only had to deal with
the desktop shortcuts. Let me know which you think is better.

[Alexei Svitkine (slow), 2013/05/01 15:27:30]

I think it's fine to create an un-badged icon for the profile, in that case. At
the same time, I think we should continue to create "vanilla" shortcuts when
deleting down to 1 profile. The unbadged icon would just be there for the case
of pinned shortcuts or shortcuts in other locations that we don't look for as
well as for the window decoration (which will be associated with the icon now).

+    CreateOrUpdateShortcutsForProfileAtPath(profile_path,
                                             UPDATE_EXISTING_ONLY,
                                             IGNORE_NON_PROFILE_SHORTCUTS);
   }
@@ -641,6 +638,7 @@ void ProfileShortcutManagerWin::OnProfileNameChanged(
 
 void ProfileShortcutManagerWin::OnProfileAvatarChanged(
     const base::FilePath& profile_path) {
+  CreateOrUpdateShortcutIconForProfileAtPath(profile_path);
   CreateOrUpdateShortcutsForProfileAtPath(profile_path, UPDATE_EXISTING_ONLY,

[Alexei Svitkine (slow), 2013/04/26 21:16:23]

Is this call necessary if we're already calling
CreateOrUpdateShortcutIconForProfileAtPath() to update the image with the
avatar? (please test this)

If yes, please add a comment why.

[calamity, 2013/04/30 06:45:41]

Not updating the shortcut seems to be fine.

                                           IGNORE_NON_PROFILE_SHORTCUTS);
 }
@@ -664,7 +662,7 @@ void ProfileShortcutManagerWin::CreateOrUpdateShortcutsForProfileAtPath(
   size_t profile_index = cache->GetIndexOfProfileWithPath(profile_path);
   if (profile_index == std::string::npos)
     return;
-  bool remove_badging = cache->GetNumberOfProfiles() == 1;
+  bool is_last_profile = cache->GetNumberOfProfiles() == 1;
 
   string16 old_shortcut_appended_name =
       cache->GetShortcutNameOfProfileAtIndex(profile_index);
@@ -679,9 +677,27 @@ void ProfileShortcutManagerWin::CreateOrUpdateShortcutsForProfileAtPath(
   }
 
   string16 new_shortcut_appended_name;
-  if (!remove_badging)
+  if (!is_last_profile)
     new_shortcut_appended_name = cache->GetNameOfProfileAtIndex(profile_index);
 
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&CreateOrUpdateDesktopShortcutsForProfile, profile_path,
+                 old_shortcut_appended_name, new_shortcut_appended_name,
+                 create_mode, action));
+
+  cache->SetShortcutNameOfProfileAtIndex(profile_index,
+                                         new_shortcut_appended_name);
+}
+
+void ProfileShortcutManagerWin::CreateOrUpdateShortcutIconForProfileAtPath(
+    const base::FilePath& profile_path) {
+  ProfileInfoCache* cache = &profile_manager_->GetProfileInfoCache();
+  size_t profile_index = cache->GetIndexOfProfileWithPath(profile_path);
+  if (profile_index == std::string::npos)
+    return;
+  bool remove_badging = cache->GetNumberOfProfiles() == 1;
+
   SkBitmap avatar_bitmap_copy_1x;
   SkBitmap avatar_bitmap_copy_2x;
   if (!remove_badging) {
@@ -697,11 +713,6 @@ void ProfileShortcutManagerWin::CreateOrUpdateShortcutsForProfileAtPath(
   }
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
-      base::Bind(&CreateOrUpdateDesktopShortcutsForProfile, profile_path,
-                 old_shortcut_appended_name, new_shortcut_appended_name,
-                 avatar_bitmap_copy_1x, avatar_bitmap_copy_2x, create_mode,
-                 action));
-
-  cache->SetShortcutNameOfProfileAtIndex(profile_index,
-                                         new_shortcut_appended_name);
+      base::Bind(&CreateOrUpdateShortcutIconForProfile, profile_path,
+                 avatar_bitmap_copy_1x, avatar_bitmap_copy_2x));
 }