ProfileShortcutManagerWin: Don't create user level shortcut when system level one exists.

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 176381)
+++ chrome/browser/profiles/profile_shortcut_manager_win.cc	(working copy)
@@ -199,6 +199,20 @@
   if (!file_util::PathExists(old_shortcut_path))
     return;
 
+  // If a system shortcut exists at the destination, then simply delete the
+  // old shortcut.
+  FilePath system_shortcuts_directory;
+  DCHECK(ShellUtil::GetShortcutPath(ShellUtil::SHORTCUT_LOCATION_DESKTOP,

[gab, 2013/01/14 21:49:30]

Take the call out of the DCHECK, store the result and DCHECK on that.

Or instead do:
if (!ShellUtil::GetShortcutPath...)
  NOTREACHED();

Otherwise this code will not run in Release builds..

Also if this fails you probably don't want to keep going with the verification.

[Alexei Svitkine (slow), 2013/01/15 22:03:02]

Doh, thanks for catching this. Fixed.

+                                    BrowserDistribution::GetDistribution(),
+                                    ShellUtil::SYSTEM_LEVEL,
+                                    &system_shortcuts_directory));
+  const FilePath possible_system_shortcut =

[sail, 2013/01/14 21:45:19]

new_system_shortcut_path

[Alexei Svitkine (slow), 2013/01/15 22:03:02]

I've changed it to |possible_new_system_shortcut|, to still have the name
indicate that it may or may not exist.

+      system_shortcuts_directory.Append(new_shortcut_file);
+  if (file_util::PathExists(possible_system_shortcut)) {
+    file_util::Delete(old_shortcut_path, false);
+    return;
+  }
+
   FilePath new_shortcut_path = shortcuts_directory.Append(new_shortcut_file);

[sail, 2013/01/14 21:45:19]

new_user_shortcut_path

[Alexei Svitkine (slow), 2013/01/15 22:03:02]

All the paths here are assumed to be user paths except for
|possible_new_system_shortcut|, which is named to indicate that. I suppose I
could change things to be more explicit (e.g. rename old_shortcut_path to
old_user_shortcut_path), but it seems unnecessary to me. WDYT?

   if (!file_util::Move(old_shortcut_path, new_shortcut_path))
     LOG(ERROR) << "Could not rename Windows profile desktop shortcut.";
@@ -262,7 +276,7 @@
         profiles::internal::GetShortcutFilenameForProfile(profile_name,
                                                           distribution);
     shortcuts.push_back(FilePath(shortcut_name));
-    operation = ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS;
+    operation = ShellUtil::SHELL_SHORTCUT_CREATE_IF_NO_SYSTEM_LEVEL;

[gab, 2013/01/14 21:49:30]

This will prevent creation of all profile shortcuts when a system-level shortcut
is present, I think you want to only prevent creation of the shortcut when it is
unbadged (i.e. only 1 profile).

It is unfortunate that the global shortcut is still visible when you have 2
profiles, but I think you still want to show the both profile's badged/named
shortcut in that case as it feels otherwise confusing... I guess the global
shortcut will keep the current behavior of opening the last profile.

[Alexei Svitkine (slow), 2013/01/14 22:57:14]

Makes sense. Just to clarify, SHELL_SHORTCUT_CREATE_IF_NO_SYSTEM_LEVEL will
prevent the creation of a user-level shortcut "Foo.lnk" when there's a system
level shortcut "Google Chrome.lnk"? When I wrote this, I did it under the
impression that it would check for a system-level shortcut with the same name,
in which case this code should be correct, I think. Please clarify.

[gab, 2013/01/14 23:04:42]

Ah, you're right, I wrongly assumed it would prevent all creations of
user-level, but just looked at the details of the implementation and you're
right; this was not really intended when I wrote it, but I guess it has
clever/useful side-effects now :). Could you add to the documentation for
SHELL_SHORTCUT_CREATE_IF_NO_SYSTEM_LEVEL to be clear that this is the intended
behavior for it to stick?

Cheers!
Gab

[Alexei Svitkine (slow), 2013/01/15 22:03:02]

Done.

   }
 
   for (size_t i = 0; i < shortcuts.size(); ++i) {
@@ -340,9 +354,9 @@
     properties.set_shortcut_name(
         profiles::internal::GetShortcutFilenameForProfile(string16(),
                                                           distribution));
-    ShellUtil::CreateOrUpdateShortcut(ShellUtil::SHORTCUT_LOCATION_DESKTOP,
-                                      distribution, properties,
-                                      ShellUtil::SHELL_SHORTCUT_CREATE_ALWAYS);
+    ShellUtil::CreateOrUpdateShortcut(
+        ShellUtil::SHORTCUT_LOCATION_DESKTOP, distribution, properties,
+        ShellUtil::SHELL_SHORTCUT_CREATE_IF_NO_SYSTEM_LEVEL);
   }
 }