Create profile .ico file on profile creation

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 120 matching lines @@
   const SkBitmap& badged_bitmap =
       offscreen_canvas->getDevice()->accessBitmap(false);
   SkBitmap badged_bitmap_copy;
   badged_bitmap.deepCopyTo(&badged_bitmap_copy, badged_bitmap.getConfig());
   return badged_bitmap_copy;
 }
 
 // Creates a desktop shortcut icon file (.ico) on the disk for a given profile,
 // 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(
+// fails. Use index 0 when assigning the resulting file as the icon. If both
+// given bitmaps are empty, an unbadged icon is created.

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

Please add a TODO that ideally we'd just want to copy the app icon verbatim from
the exe's resources in this case.

[calamity, 2013/05/03 04:40:28]

Done.

+base::FilePath CreateOrUpdateShortcutIconForProfile(
     const base::FilePath& profile_path,
     const SkBitmap& avatar_bitmap_1x,
     const SkBitmap& avatar_bitmap_2x) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));

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

Add: DCHECK_EQ(avatar_bitmap_1x.empty(), avatar_bitmap_1x.empty());

[calamity, 2013/05/03 04:40:28]

I'm assuming this is to facilitate the change below. If so, this may be voided.

   scoped_ptr<SkBitmap> app_icon_bitmap(GetAppIconForSize(kShortcutIconSize));
   if (!app_icon_bitmap.get())
     return base::FilePath();
 
-  const SkBitmap badged_bitmap = BadgeIcon(*app_icon_bitmap,
-                                           avatar_bitmap_1x, 1);
+  SkBitmap badged_bitmap;

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

How about:

SkBitmap badged_bitmap = avatar_bitmap_1x.empty() ? *app_icon_bitmap :
                                            BadgeIcon(*app_icon_bitmap,
avatar_bitmap_1x, 1);

Same thing for large_badged_bitmap.

Then you can avoid the if later on.

[calamity, 2013/05/03 04:40:28]

I think that making these changes decreases the robustness of this icon
creation? What should happen if only the 2x bitmap is available? The ImageFamily
icon creation should handle only having one size of icon to work with.

I've refactored this a little so the logic is clearer. Let me know if this is
sufficient.

On 2013/05/01 15:27:30, Alexei Svitkine wrote:

+  if (!avatar_bitmap_1x.empty())
+    badged_bitmap = BadgeIcon(*app_icon_bitmap, avatar_bitmap_1x, 1);
 
   SkBitmap large_badged_bitmap;
-  app_icon_bitmap = GetAppIconForSize(IconUtil::kLargeIconSize);
-  if (app_icon_bitmap.get())
-    large_badged_bitmap = BadgeIcon(*app_icon_bitmap, avatar_bitmap_2x, 2);
+  scoped_ptr<SkBitmap> large_app_icon_bitmap(
+      GetAppIconForSize(IconUtil::kLargeIconSize));
+  if (large_app_icon_bitmap.get() && !avatar_bitmap_2x.empty()) {
+    large_badged_bitmap =
+        BadgeIcon(*large_app_icon_bitmap, avatar_bitmap_2x, 2);
+  }
 
   // Finally, write the .ico file containing this new bitmap.
   const base::FilePath icon_path =
-      profile_path.AppendASCII(profiles::internal::kProfileIconFileName);
+      profiles::internal::GetProfileIconPath(profile_path);
+  if (avatar_bitmap_1x.empty() && avatar_bitmap_2x.empty()) {
+    badged_bitmap = *app_icon_bitmap;
+    if (large_app_icon_bitmap.get())
+      large_badged_bitmap = *large_app_icon_bitmap;
+  }
   if (!IconUtil::CreateIconFileFromSkBitmap(badged_bitmap, large_badged_bitmap,
-                                            icon_path))
-    return base::FilePath();
-
+                                            icon_path)) {
+    NOTREACHED();

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

Re-add the return base::FilePath() to propagate the failure, just in case.

[calamity, 2013/05/03 04:40:28]

Done.

+  }
+  SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST | SHCNF_FLUSHNOWAIT,
+                 NULL, NULL);
   return icon_path;
 }
 
 // Gets the user and system directories for desktop shortcuts. Parameters may
 // be NULL if a directory type is not needed. Returns true on success.
 bool GetDesktopShortcutsDirectories(
     base::FilePath* user_shortcuts_directory,
     base::FilePath* system_shortcuts_directory) {
   BrowserDistribution* distribution = BrowserDistribution::GetDistribution();
   if (user_shortcuts_directory &&
@@ skipping 114 matching lines @@
     if (file_util::PathExists(possible_new_system_shortcut))
       file_util::Delete(old_shortcut_path, false);
     else if (!RenameDesktopShortcut(old_shortcut_path, new_shortcut_path))
       DLOG(ERROR) << "Could not rename Windows profile desktop shortcut.";
   } else {
     // If the shortcut does not exist, it may have been renamed by the user. In
     // that case, its name should not be changed.
     // It's also possible that a system-level shortcut exists instead - this
     // should only be the case for the original Chrome shortcut from an
     // installation. If that's the case, copy that one over - it will get its
-    // properties updated by |CreateOrUpdateDesktopShortcutsForProfile()|.
+    // properties updated by
+    // |CreateOrUpdateDesktopShortcutsAndIconForProfile()|.
     const base::FilePath possible_old_system_shortcut =
         system_shortcuts_directory.Append(old_shortcut_filename);
     if (file_util::PathExists(possible_old_system_shortcut))
       file_util::CopyFile(possible_old_system_shortcut, new_shortcut_path);
   }
 }
 
 // Updates all desktop shortcuts for the given profile to have the specified
 // parameters. If |create_mode| is CREATE_WHEN_NONE_FOUND, a new shortcut is
 // created if no existing ones were found. Whether non-profile shortcuts should
 // be updated is specified by |action|. Must be called on the FILE thread.
-void CreateOrUpdateDesktopShortcutsForProfile(
+void CreateOrUpdateDesktopShortcutsAndIconForProfile(
     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));
 
+  base::FilePath shortcut_icon =
+      CreateOrUpdateShortcutIconForProfile(profile_path,
+                                           avatar_image_1x,
+                                           avatar_image_2x);
+  if (shortcut_icon.empty()) {
+    NOTREACHED();

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

Add a return.

[calamity, 2013/05/03 04:40:28]

Done.

+  }
+  if (create_mode == ProfileShortcutManagerWin::CREATE_ICON_ONLY) {

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

No need for {}'s.

[calamity, 2013/05/03 04:40:28]

Done.

+    return;
+  }
+
   base::FilePath chrome_exe;
   if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
     NOTREACHED();
     return;
   }
 
   BrowserDistribution* distribution = BrowserDistribution::GetDistribution();
   // Ensure that the distribution supports creating shortcuts. If it doesn't,
   // the following code may result in NOTREACHED() being hit.
   DCHECK(distribution->CanCreateDesktopShortcuts());
@@ skipping 13 matching lines @@
   installer::Product product(distribution);
   product.AddDefaultShortcutProperties(chrome_exe, &properties);
 
   const string16 command_line =
       profiles::internal::CreateProfileShortcutFlags(profile_path);
 
   // Only set the profile-specific properties when |profile_name| is non empty.
   // 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())
       properties.set_icon(shortcut_icon, 0);
     properties.set_arguments(command_line);
   } else {
     // Set the arguments explicitly to the empty string to ensure that
     // |ShellUtil::CreateOrUpdateShortcut| updates that part of the shortcut.
     properties.set_arguments(string16());
   }
 
   ShellUtil::ShortcutOperation operation =
@@ skipping 36 matching lines @@
       return true;
   }
 
   return false;
 }
 
 // Deletes all desktop shortcuts for the specified profile and also removes the
 // corresponding icon file. If |ensure_shortcuts_remain| is true, then a regular
 // non-profile shortcut will be created if this function would otherwise delete
 // the last Chrome desktop shortcut(s). Must be called on the FILE thread.
-void DeleteDesktopShortcutsAndIconFile(const base::FilePath& profile_path,
+void DeleteDesktopShortcuts(const base::FilePath& profile_path,
                                        bool ensure_shortcuts_remain) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   base::FilePath chrome_exe;
   if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
     NOTREACHED();
     return;
   }
 
   const string16 command_line =
       profiles::internal::CreateProfileShortcutFlags(profile_path);
   std::vector<base::FilePath> shortcuts;
   ListDesktopShortcutsWithCommandLine(chrome_exe, command_line, false,
                                       &shortcuts);
 
   BrowserDistribution* distribution = BrowserDistribution::GetDistribution();
   for (size_t i = 0; i < shortcuts.size(); ++i) {
     // Use file_util::Delete() instead of ShellUtil::RemoveShortcut(), as the
     // latter causes non-profile taskbar shortcuts to be unpinned.
     file_util::Delete(shortcuts[i], false);
     // Notify the shell that the shortcut was deleted to ensure desktop refresh.
     SHChangeNotify(SHCNE_DELETE, SHCNF_PATH, shortcuts[i].value().c_str(),
                    NULL);
   }
 
-  const base::FilePath icon_path =
-      profile_path.AppendASCII(profiles::internal::kProfileIconFileName);
-  file_util::Delete(icon_path, false);
-
   // If |ensure_shortcuts_remain| is true and deleting this profile caused the
   // last shortcuts to be removed, re-create a regular non-profile shortcut.
   const bool had_shortcuts = !shortcuts.empty();
   if (ensure_shortcuts_remain && had_shortcuts &&
       !ChromeDesktopShortcutsExist(chrome_exe)) {
     BrowserDistribution* distribution = BrowserDistribution::GetDistribution();
     // Ensure that the distribution supports creating shortcuts. If it doesn't,
     // the following code may result in NOTREACHED() being hit.
     DCHECK(distribution->CanCreateDesktopShortcuts());
     installer::Product product(distribution);
@@ skipping 61 matching lines @@
   SkBitmap bitmap_copy;
   image_bitmap->deepCopyTo(&bitmap_copy, image_bitmap->getConfig());
   return bitmap_copy;
 }
 
 }  // namespace
 
 namespace profiles {
 namespace internal {
 
+base::FilePath GetProfileIconPath(const base::FilePath& profile_path) {
+  return profile_path.AppendASCII(profiles::internal::kProfileIconFileName);
+}
+
 const char kProfileIconFileName[] = "Google Profile.ico";
 
 string16 GetShortcutFilenameForProfile(const string16& profile_name,
                                        BrowserDistribution* distribution) {
   string16 shortcut_name;
   if (!profile_name.empty()) {
     shortcut_name.append(SanitizeShortcutProfileNameString(profile_name));
     shortcut_name.append(L" - ");
     shortcut_name.append(l10n_util::GetStringUTF16(IDS_SHORT_PRODUCT_NAME));
   } else {
@@ skipping 40 matching lines @@
 void ProfileShortcutManagerWin::CreateProfileShortcut(
     const base::FilePath& profile_path) {
   CreateOrUpdateShortcutsForProfileAtPath(profile_path, CREATE_WHEN_NONE_FOUND,
                                           IGNORE_NON_PROFILE_SHORTCUTS);
 }
 
 void ProfileShortcutManagerWin::RemoveProfileShortcuts(
     const base::FilePath& profile_path) {
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
-      base::Bind(&DeleteDesktopShortcutsAndIconFile, profile_path, false));
+      base::Bind(&DeleteDesktopShortcuts, profile_path, false));
 }
 
 void ProfileShortcutManagerWin::HasProfileShortcuts(
     const base::FilePath& profile_path,
     const base::Callback<void(bool)>& callback) {
   BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&HasAnyProfileShortcuts, profile_path), callback);
 }
 
 void ProfileShortcutManagerWin::OnProfileAdded(
     const base::FilePath& profile_path) {
   const size_t profile_count =
       profile_manager_->GetProfileInfoCache().GetNumberOfProfiles();
   if (profile_count == 1) {
     CreateOrUpdateShortcutsForProfileAtPath(profile_path,
                                             CREATE_WHEN_NONE_FOUND,
                                             UPDATE_NON_PROFILE_SHORTCUTS);
-  } else if (profile_count == 2) {
-    CreateOrUpdateShortcutsForProfileAtPath(GetOtherProfilePath(profile_path),
-                                            UPDATE_EXISTING_ONLY,
-                                            UPDATE_NON_PROFILE_SHORTCUTS);
+  } else {
+    CreateOrUpdateShortcutsForProfileAtPath(profile_path, CREATE_ICON_ONLY,
+                                            IGNORE_NON_PROFILE_SHORTCUTS);
+    if (profile_count == 2) {
+      CreateOrUpdateShortcutsForProfileAtPath(GetOtherProfilePath(profile_path),
+                                              UPDATE_EXISTING_ONLY,
+                                              UPDATE_NON_PROFILE_SHORTCUTS);
+    }
   }
 }
 
 void ProfileShortcutManagerWin::OnProfileWillBeRemoved(
     const base::FilePath& profile_path) {
 }
 
 void ProfileShortcutManagerWin::OnProfileWasRemoved(
     const base::FilePath& profile_path,
     const string16& profile_name) {
   const ProfileInfoCache& cache = profile_manager_->GetProfileInfoCache();
   // If there is only one profile remaining, remove the badging information
   // from an existing shortcut.
   const bool deleting_down_to_last_profile = (cache.GetNumberOfProfiles() == 1);
   if (deleting_down_to_last_profile) {
-    CreateOrUpdateShortcutsForProfileAtPath(cache.GetPathOfProfileAtIndex(0),
+    // This is needed to unbadge the icon.
+    base::FilePath profile_path = cache.GetPathOfProfileAtIndex(0);
+    CreateOrUpdateShortcutsForProfileAtPath(profile_path,
                                             UPDATE_EXISTING_ONLY,
                                             IGNORE_NON_PROFILE_SHORTCUTS);
   }
 
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-                          base::Bind(&DeleteDesktopShortcutsAndIconFile,
+                          base::Bind(&DeleteDesktopShortcuts,
                                      profile_path,
                                      deleting_down_to_last_profile));
 }
 
 void ProfileShortcutManagerWin::OnProfileNameChanged(
     const base::FilePath& profile_path,
     const string16& old_profile_name) {
   CreateOrUpdateShortcutsForProfileAtPath(profile_path, UPDATE_EXISTING_ONLY,
                                           IGNORE_NON_PROFILE_SHORTCUTS);
 }
 
 void ProfileShortcutManagerWin::OnProfileAvatarChanged(
     const base::FilePath& profile_path) {
-  CreateOrUpdateShortcutsForProfileAtPath(profile_path, UPDATE_EXISTING_ONLY,
+  CreateOrUpdateShortcutsForProfileAtPath(profile_path, CREATE_ICON_ONLY,
                                           IGNORE_NON_PROFILE_SHORTCUTS);
 }
 
 base::FilePath ProfileShortcutManagerWin::GetOtherProfilePath(
     const base::FilePath& profile_path) {
   const ProfileInfoCache& cache = profile_manager_->GetProfileInfoCache();
   DCHECK_EQ(2U, cache.GetNumberOfProfiles());
   // Get the index of the current profile, in order to find the index of the
   // other profile.
   size_t current_profile_index = cache.GetIndexOfProfileWithPath(profile_path);
@@ skipping 35 matching lines @@
     const int resource_id_1x =
         cache->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.
     avatar_bitmap_copy_1x = GetImageResourceSkBitmapCopy(resource_id_1x);
     avatar_bitmap_copy_2x = GetImageResourceSkBitmapCopy(resource_id_2x);
   }
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
-      base::Bind(&CreateOrUpdateDesktopShortcutsForProfile, profile_path,
+      base::Bind(&CreateOrUpdateDesktopShortcutsAndIconForProfile, 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);
 }