Update profile desktop shortcuts

chrome/browser/profiles/profile_shortcut_manager_win.cc

Index: chrome/browser/profiles/profile_shortcut_manager_win.cc
===================================================================
--- chrome/browser/profiles/profile_shortcut_manager_win.cc	(revision 152316)
+++ chrome/browser/profiles/profile_shortcut_manager_win.cc	(working copy)
@@ -115,6 +115,58 @@
   bool_function.Run();
 }
 
+// Renames an existing Chrome desktop profile shortcut. Must be called on the
+// FILE thread.
+void RenameChromeDesktopShortcutForProfile(
+    const string16& old_shortcut,
+    const string16& new_shortcut) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  FilePath shortcut_path;
+  if (ShellUtil::GetDesktopPath(false,  // User's directory instead of system.
+                                &shortcut_path)) {
+    FilePath old_profile = shortcut_path.Append(old_shortcut);
+    FilePath new_profile = shortcut_path.Append(new_shortcut);
+    file_util::Move(old_profile, new_profile);
+  }
+}
+
+// Updates the arguments to a Chrome desktop shortcut for a profile. Must be
+// called on the FILE thread.
+void UpdateChromeDesktopShortcutForProfile(
+    const string16& shortcut,
+    const string16& arguments,
+    const FilePath& profile_path,
+    const gfx::Image* avatar_image) {

[sail, 2012/08/21 21:59:26]

looks like the avatar_image is leaked here?

[Halli, 2012/08/23 17:59:20]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  FilePath shortcut_path;
+  if (!ShellUtil::GetDesktopPath(false, &shortcut_path))
+    return;
+
+  shortcut_path = shortcut_path.Append(shortcut);
+  FilePath chrome_exe;
+  if (!PathService::Get(base::FILE_EXE, &chrome_exe))
+    return;
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  string16 description;
+  if (!dist)
+    return;
+  else
+    description = WideToUTF16(dist->GetAppDescription());
+  FilePath icon_path = avatar_image ?
+      CreateChromeDesktopShortcutIconForProfile(profile_path, *avatar_image) :
+      FilePath();
+
+  ShellUtil::UpdateChromeShortcut(
+      dist,
+      chrome_exe.value(),
+      shortcut_path.value(),
+      arguments,
+      description,
+      icon_path.empty() ? chrome_exe.value() : icon_path.value(),
+      icon_path.empty() ? dist->GetIconIndex() : 0,
+      ShellUtil::SHORTCUT_NO_OPTIONS);
+}
+
 }  // namespace
 
 class ProfileShortcutManagerWin : public ProfileShortcutManager {
@@ -125,34 +177,17 @@
   virtual void CreateChromeDesktopShortcut(
       const FilePath& profile_path, const string16& profile_name,
       const gfx::Image& avatar_image) OVERRIDE;
-  virtual void DeleteChromeDesktopShortcut(const FilePath& profile_path)
-      OVERRIDE;
+  virtual void DeleteChromeDesktopShortcut(const FilePath& profile_path,
+      const string16& profile_name) OVERRIDE;
 
- private:
-  struct ProfileShortcutInfo {
-    string16 flags;
-    string16 profile_name;
-    gfx::Image avatar_image;
+  virtual void OnProfileAdded(const FilePath& profile_path) OVERRIDE;
+  virtual void OnProfileWillBeRemoved(const FilePath& profile_path) OVERRIDE;
+  virtual void OnProfileWasRemoved(const FilePath& profile_path,
+                                   const string16& profile_name) OVERRIDE;
+  virtual void OnProfileNameChanged(const FilePath& profile_path,
+                                    const string16& old_profile_name) OVERRIDE;
+  virtual void OnProfileAvatarChanged(const FilePath& profile_path) OVERRIDE;
 
-    ProfileShortcutInfo()
-        : flags(string16()),
-          profile_name(string16()),
-          avatar_image(gfx::Image()) {
-    }
-
-    ProfileShortcutInfo(
-        string16 p_flags,
-        string16 p_profile_name,
-        gfx::Image p_avatar_image)
-        : flags(p_flags),
-          profile_name(p_profile_name),
-          avatar_image(p_avatar_image) {
-    }
-  };
-
-  // TODO(hallielaine): Repopulate this map on chrome session startup
-  typedef std::map<FilePath, ProfileShortcutInfo> ProfileShortcutsMap;
-  ProfileShortcutsMap profile_shortcuts_;
 };
 
 // static
@@ -190,20 +225,12 @@
     return;
   description = WideToUTF16(dist->GetAppDescription());
 
-  // Add the profile to the map if it doesn't exist already
-  if (!profile_shortcuts_.count(profile_path)) {
-    string16 flags = CreateProfileShortcutFlags(profile_path);
-    profile_shortcuts_.insert(std::make_pair(profile_path,
-                              ProfileShortcutInfo(flags, profile_name,
-                                  avatar_image)));
-  }
-
   ShellUtil::CreateChromeDesktopShortcut(
       dist,
       chrome_exe.value(),
       description,
-      profile_shortcuts_[profile_path].profile_name,
-      profile_shortcuts_[profile_path].flags,
+      profile_name,
+      CreateProfileShortcutFlags(profile_path),
       shortcut_icon.empty() ? chrome_exe.value() : shortcut_icon.value(),
       shortcut_icon.empty() ? dist->GetIconIndex() : 0,
       ShellUtil::CURRENT_USER,
@@ -211,19 +238,89 @@
 }
 
 void ProfileShortcutManagerWin::DeleteChromeDesktopShortcut(
-    const FilePath& profile_path) {
+    const FilePath& profile_path, const string16& profile_name) {
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   string16 shortcut;
   // If we can find the shortcut, delete it
   if (ShellUtil::GetChromeShortcutName(dist, false,
-      profile_shortcuts_[profile_path].profile_name, &shortcut)) {
+       profile_name, &shortcut)) {
     std::vector<string16> appended_names(1, shortcut);
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
             base::Bind(&CallShellUtilBoolFunction, base::Bind(
                 &ShellUtil::RemoveChromeDesktopShortcutsWithAppendedNames,
                 appended_names)));
-    profile_shortcuts_.erase(profile_path);
   }
 }
 
+void ProfileShortcutManagerWin::OnProfileAdded(const FilePath& profile_path) {
+
+}
+
+void ProfileShortcutManagerWin::OnProfileWillBeRemoved(
+    const FilePath& profile_path) {
+
+}
+
+void ProfileShortcutManagerWin::OnProfileWasRemoved(
+    const FilePath& profile_path,
+    const string16& profile_name) {
+}
+
+void ProfileShortcutManagerWin::OnProfileNameChanged(
+    const FilePath& profile_path,
+    const string16& old_profile_name) {
+  ProfileInfoCache& cache =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+
+  string16 new_profile_name =
+      cache.GetNameOfProfileAtIndex(
+      cache.GetIndexOfProfileWithPath(profile_path));

[sail, 2012/08/21 21:59:26]

any time you call GetIndexOfProfileWithPath() you should get that the return
value is not std::string::npos.

[Halli, 2012/08/23 17:59:20]

Done.

+
+  string16 old_shortcut;
+  string16 new_shortcut;
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  if (ShellUtil::GetChromeShortcutName(
+          dist, false, old_profile_name, &old_shortcut) &&
+      ShellUtil::GetChromeShortcutName(
+          dist, false, new_profile_name, &new_shortcut)) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE,
+        base::Bind(&RenameChromeDesktopShortcutForProfile,
+                   old_shortcut,
+                   new_shortcut));
+  }
+}
+
+void ProfileShortcutManagerWin::OnProfileAvatarChanged(
+    const FilePath& profile_path) {
+  ProfileInfoCache& cache =
+      g_browser_process->profile_manager()->GetProfileInfoCache();
+
+  size_t profile_index = cache.GetIndexOfProfileWithPath(profile_path);
+  string16 profile_name =
+      cache.GetNameOfProfileAtIndex(profile_index);
+  string16 shortcut;
+
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  if (ShellUtil::GetChromeShortcutName(
+         dist, false,  profile_name, &shortcut)) {
+
+    size_t new_icon_index = cache.GetAvatarIconIndexOfProfileAtIndex(
+        profile_index);
+    gfx::Image avatar_image =
+         ResourceBundle::GetSharedInstance().GetNativeImageNamed(
+         cache.GetDefaultAvatarIconResourceIDAtIndex(new_icon_index));
+
+    // We make a copy of the Image to ensure that the underlying image data is
+    // AddRef'd, in case the original copy gets deleted.
+    gfx::Image* avatar_copy = new gfx::Image(avatar_image);

[sail, 2012/08/21 21:59:26]

I don't think this is thread safe. There was a recent discussion about this on
chromium-dev:
https://groups.google.com/a/chromium.org/forum/?fromgroups=#!topic/chromium-d...

Could you ask oshima about this?

[Halli, 2012/08/23 17:59:20]

Oshima confirmed that gfx::Image and ToSkBitmap() are only thread safe on the UI
thread - will do the refactoring in a separate CL.

On 2012/08/21 21:59:26, sail wrote:

+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE,
+        base::Bind(&UpdateChromeDesktopShortcutForProfile,
+                   shortcut,
+                   CreateProfileShortcutFlags(profile_path),
+                   profile_path,
+                   base::Owned(avatar_copy)));
+  }
+}