Suffix Chrome's appid on user-level installs

chrome/installer/util/shell_util.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.
 //
 // This file defines functions that integrate Chrome in Windows shell. These
 // functions can be used by Chrome as well as Chrome installer. All of the
 // work is done by the local functions defined in anonymous namespace in
 // this class.
 
 #include "chrome/installer/util/shell_util.h"
@@ skipping 117 matching lines @@
   // HKLM for system-level installs.
   static bool GetProgIdEntries(BrowserDistribution* dist,
                                const string16& chrome_exe,
                                const string16& suffix,
                                std::list<RegistryEntry*>* entries) {
     string16 icon_path(ShellUtil::GetChromeIcon(dist, chrome_exe));
     string16 open_cmd(ShellUtil::GetChromeShellOpenCmd(chrome_exe));
     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.
     bool set_delegate_execute =
         IsChromeMetroSupported() &&
         dist->GetDelegateExecuteHandlerData(&delegate_guid, NULL, NULL, NULL) &&
         InstallUtil::HasDelegateExecuteHandler(dist, chrome_exe);
 
     // DelegateExecute ProgId. Needed for Chrome Metro in Windows 8.
     if (set_delegate_execute) {
@@ skipping 506 matching lines @@
       (!InstallUtil::HasDelegateExecuteHandler(dist, chrome_exe) ||
        !IsChromeMetroSupported())) {
     // There's no need to rollback, so forgo the usual work item lists and just
     // remove the values from the registry.
     const HKEY root_key = InstallUtil::IsPerUserInstall(chrome_exe.c_str()) ?
         HKEY_CURRENT_USER : HKEY_LOCAL_MACHINE;
     // Use the current installation's suffix, not the about-to-be-installed
     // 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);
     key.push_back(FilePath::kSeparators[0]);
     key.append(app_id);
     InstallUtil::DeleteRegistryKey(root_key, key);
 
     // <root hkey>\Software\Classes\ChromiumHTML[.user]\shell\open\command
     key = ShellUtil::kRegClasses;
     key.push_back(FilePath::kSeparators[0]);
@@ skipping 390 matching lines @@
   return tested_suffix;
 }
 
 string16 ShellUtil::GetApplicationName(BrowserDistribution* dist,
                                        const string16& chrome_exe) {
   string16 app_name = dist->GetBaseAppName();
   app_name += GetCurrentInstallationSuffix(dist, chrome_exe);
   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,
                                   bool elevate_if_not_admin) {
   DCHECK(!(shell_change & ShellUtil::SYSTEM_LEVEL) || IsUserAnAdmin());
 
   if (!dist->CanSetAsDefault())
     return false;
 
   // Windows 8 does not permit making a browser default just like that.
@@ skipping 339 matching lines @@
 }
 
 bool ShellUtil::UpdateChromeShortcut(BrowserDistribution* dist,
                                      const string16& chrome_exe,
                                      const string16& shortcut,
                                      const string16& arguments,
                                      const string16& description,
                                      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);
   installer::MasterPreferences prefs(prefs_path);
   if (FilePath::CompareEqualIgnoreCase(icon_path, chrome_exe)) {
     prefs.GetInt(installer::master_preferences::kChromeShortcutIconIndex,
                  &icon_index);
   }
 
+  const string16 app_id(GetAppId(dist, chrome_exe));
+
   return file_util::CreateOrUpdateShortcutLink(
       chrome_exe.c_str(),
       shortcut.c_str(),
       chrome_path.c_str(),

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

third change:
chrome_path.value().c_str(),

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

Done.

       arguments.c_str(),
       description.c_str(),
       icon_path.c_str(),
       icon_index,
-      dist->GetBrowserAppId().c_str(),
+      app_id.c_str(),
       ConvertShellUtilShortcutOptionsToFileUtil(options));
 }