Suffix Chrome's appid on user-level installs

chrome/installer/util/shell_util.cc

Index: chrome/installer/util/shell_util.cc
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index 432806542c210d2d4a5a1234a2011ac898a579de..3ca001864c03e2107a4e8e4916637f2cac1eb32c 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -135,7 +135,7 @@ class RegistryEntry {
     string16 delegate_command(ShellUtil::GetChromeDelegateCommand(chrome_exe));
     // For user-level installs: entries for the app id and DelegateExecute verb
     // handler will be in HKCU; thus we do not need a suffix on those entries.
-    string16 app_id(dist->GetBrowserAppId());
+    string16 app_id(ShellUtil::GetAppId(dist, chrome_exe));
     string16 delegate_guid;
     // TODO(grt): remove HasDelegateExecuteHandler when the exe is ever-present;
     // see also install_worker.cc's AddDelegateExecuteWorkItems.
@@ -662,7 +662,7 @@ void RemoveBadWindows8RegistrationIfNeeded(
     // suffix.
     const string16 installation_suffix(
         ShellUtil::GetCurrentInstallationSuffix(dist, chrome_exe));
-    const string16 app_id(dist->GetBrowserAppId());
+    const string16 app_id(ShellUtil::GetAppId(dist, chrome_exe));
 
     // <root hkey>\Software\Classes\<app_id>
     string16 key(ShellUtil::kRegClasses);
@@ -1073,6 +1073,17 @@ string16 ShellUtil::GetApplicationName(BrowserDistribution* dist,
   return app_name;
 }
 
+string16 ShellUtil::GetAppId(BrowserDistribution* dist,
+                             const string16& chrome_exe) {
+  string16 app_id = dist->GetBaseAppId();

[grt (UTC plus 2), 2012/06/15 03:03:03]

ctor-style rather than assignment-style

[gab, 2012/06/15 19:01:04]

Done.

+  string16 suffix;
+  if (InstallUtil::IsPerUserInstall(chrome_exe.c_str()) &&
+      !GetUserSpecificRegistrySuffix(&suffix)) {
+    NOTREACHED();
+  }
+  return app_id + suffix;

[grt (UTC plus 2), 2012/06/15 03:03:03]

return app_id += suffix or app_id.append(suffix) to avoid the risk of an
unnecessary temporary (which the optimizer may remove, but why chance it)

[gab, 2012/06/15 19:01:04]

Done.

+}
+
 bool ShellUtil::MakeChromeDefault(BrowserDistribution* dist,
                                   int shell_change,
                                   const string16& chrome_exe,
@@ -1432,7 +1443,14 @@ bool ShellUtil::UpdateChromeShortcut(BrowserDistribution* dist,
                                      const string16& icon_path,
                                      int icon_index,
                                      uint32 options) {
-  string16 chrome_path = FilePath(chrome_exe).DirName().value();
+  // Make sure Chrome's ProgIds are registered (this is important because the
+  // registered appid is embedded in the shortcut).
+  DCHECK(QuickIsChromeRegistered(dist,

[grt (UTC plus 2), 2012/06/15 03:03:03]

please explain why the shortcut can't be created before registration.  will the
shortcut simply not work at all otherwise?

[gab, 2012/06/15 19:01:04]

See comment on shell_util.h

+                                 chrome_exe,
+                                 GetCurrentInstallationSuffix(dist, chrome_exe),
+                                 CONFIRM_PROGID_REGISTRATION));
+
+  const string16 chrome_path = FilePath(chrome_exe).DirName().value();

[grt (UTC plus 2), 2012/06/15 03:03:03]

while you're here, would you mind tightening up this func a bit with the
suggested changes below?
first change:
const FilePath chrome_path(FilePath(chrome_exe).DirName());

[gab, 2012/06/15 19:01:04]

Done.

 
   FilePath prefs_path(chrome_path);

[grt (UTC plus 2), 2012/06/15 03:03:03]

second change:
FilePath prefs_path(chrome_path.AppendASCII(installer::kDefaultMasterPrefs))
(and delete line 1456)

[gab, 2012/06/15 19:01:04]

Even better, remove prefs_path altogether, inlining it below.

   prefs_path = prefs_path.AppendASCII(installer::kDefaultMasterPrefs);
@@ -1442,6 +1460,8 @@ bool ShellUtil::UpdateChromeShortcut(BrowserDistribution* dist,
                  &icon_index);
   }
 
+  const string16 app_id(GetAppId(dist, chrome_exe));
+
   return file_util::CreateOrUpdateShortcutLink(
       chrome_exe.c_str(),
       shortcut.c_str(),
@@ -1450,6 +1470,6 @@ bool ShellUtil::UpdateChromeShortcut(BrowserDistribution* dist,
       description.c_str(),
       icon_path.c_str(),
       icon_index,
-      dist->GetBrowserAppId().c_str(),
+      app_id.c_str(),
       ConvertShellUtilShortcutOptionsToFileUtil(options));
 }