Fix bugs related to making Chrome default browser on Vista and also do some cleanup.

- Stupid bug where we were failing if there was a space or '-' in the path to setup.exe
- Some application still read default browser from Software\Classes\http even on Vista so set that
- Some refactoring of apppaths changes to make a method private that doesn't need to be public.
- Move constants in shell_util where rest of the shell related constants are.
- Remove duplicate attempts to create ChromeHTML key

BUG=7568,6732,6504

Review URL: http://codereview.chromium.org/21259

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@9667 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 57 insertions, 82 deletions

diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index 6e2c40d..cdc2f78 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -257,7 +257,9 @@ installer_util::InstallStatus installer_setup::UninstallChrome(
                         dist->GetApplicationName());
 
     // Delete the App Paths key that lets explorer find Chrome.
-    DeleteRegistryKey(hklm_key, installer_util::kAppPathsRegistryKey);
+    std::wstring app_path_key(ShellUtil::kAppPathsRegistryKey);
+    file_util::AppendToPath(&app_path_key, installer_util::kChromeExe);
+    DeleteRegistryKey(hklm_key, app_path_key);
 
     // Delete media player registry key that exists only in HKLM.
     std::wstring reg_path(installer::kMediaPlayerRegPath);
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index 7353f64..96f73cf 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -13,6 +13,7 @@
 
 #include "chrome/installer/util/shell_util.h"
 
+#include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
@@ -219,35 +220,13 @@ bool IsChromeRegistered(const std::wstring& chrome_exe) {
   return registered;
 }
 
-bool CreateChromeRegKeysForXP(HKEY root_key, const std::wstring& chrome_exe) {
+bool BindChromeAssociations(HKEY root_key, const std::wstring& chrome_exe) {
   // Create a list of registry entries to create so that we can rollback
   // in case of problem.
   scoped_ptr<WorkItemList> items(WorkItem::CreateWorkItemList());
-  std::wstring classes_path(ShellUtil::kRegClasses);
-
-  std::wstring exe_name = file_util::GetFilenameFromPath(chrome_exe);
-  std::wstring chrome_open = ShellUtil::GetChromeShellOpenCmd(chrome_exe);
-  std::wstring chrome_icon(chrome_exe);
-  ShellUtil::GetChromeIcon(chrome_icon);
-
-  // Create Software\Classes\ChromeHTML
-  std::wstring html_prog_id = classes_path + L"\\" +
-                              ShellUtil::kChromeHTMLProgId;
-  items->AddCreateRegKeyWorkItem(root_key, html_prog_id);
-  items->AddSetRegValueWorkItem(root_key, html_prog_id,
-                                L"", ShellUtil::kChromeHTMLProgIdDesc, true);
-  items->AddSetRegValueWorkItem(root_key, html_prog_id,
-                                ShellUtil::kRegUrlProtocol, L"", true);
-  std::wstring default_icon = html_prog_id + ShellUtil::kRegDefaultIcon;
-  items->AddCreateRegKeyWorkItem(root_key, default_icon);
-  items->AddSetRegValueWorkItem(root_key, default_icon, L"",
-                                chrome_icon, true);
-  std::wstring open_cmd = html_prog_id + ShellUtil::kRegShellOpen;
-  items->AddCreateRegKeyWorkItem(root_key, open_cmd);
-  items->AddSetRegValueWorkItem(root_key, open_cmd, L"",
-                                chrome_open, true);
 
   // file extension associations
+  std::wstring classes_path(ShellUtil::kRegClasses);
   for (int i = 0; ShellUtil::kFileAssociations[i] != NULL; i++) {
     std::wstring key_path = classes_path + L"\\" +
                             ShellUtil::kFileAssociations[i];
@@ -257,6 +236,9 @@ bool CreateChromeRegKeysForXP(HKEY root_key, const std::wstring& chrome_exe) {
   }
 
   // protocols associations
+  std::wstring chrome_open = ShellUtil::GetChromeShellOpenCmd(chrome_exe);
+  std::wstring chrome_icon(chrome_exe);
+  ShellUtil::GetChromeIcon(chrome_icon);
   for (int i = 0; ShellUtil::kProtocolAssociations[i] != NULL; i++) {
     std::wstring key_path = classes_path + L"\\" +
                             ShellUtil::kProtocolAssociations[i];
@@ -281,6 +263,7 @@ bool CreateChromeRegKeysForXP(HKEY root_key, const std::wstring& chrome_exe) {
   }
 
   // start->Internet shortcut.
+  std::wstring exe_name = file_util::GetFilenameFromPath(chrome_exe);
   std::wstring start_internet(ShellUtil::kRegStartMenuInternet);
   items->AddCreateRegKeyWorkItem(root_key, start_internet);
   items->AddSetRegValueWorkItem(root_key, start_internet, L"",
@@ -295,6 +278,25 @@ bool CreateChromeRegKeysForXP(HKEY root_key, const std::wstring& chrome_exe) {
   return true;
 }
 
+// Populate work_item_list with WorkItem entries that will add chrome.exe to
+// the set of App Paths registry keys so that ShellExecute can find it. Note
+// that this is done in HKLM, regardless of whether this is a single-user
+// install or not. For non-admin users, this will fail.
+// chrome_exe: full path to chrome.exe
+// work_item_list: pointer to the WorkItemList that will be populated
+void AddChromeAppPathWorkItems(const std::wstring& chrome_exe,
+                               WorkItemList* item_list) {
+  FilePath chrome_path(chrome_exe);
+  std::wstring app_path_key(ShellUtil::kAppPathsRegistryKey);
+  file_util::AppendToPath(&app_path_key, chrome_path.BaseName().value());
+  item_list->AddCreateRegKeyWorkItem(HKEY_LOCAL_MACHINE, app_path_key);
+  item_list->AddSetRegValueWorkItem(HKEY_LOCAL_MACHINE, app_path_key, L"",
+                                    chrome_exe, true);
+  item_list->AddSetRegValueWorkItem(HKEY_LOCAL_MACHINE, app_path_key,
+                                    ShellUtil::kAppPathsRegistryPathName,
+                                    chrome_path.DirName().value(), true);
+}
+
 // This method creates the registry entries required for Add/Remove Programs->
 // Set Program Access and Defaults, Start->Default Programs on Windows Vista
 // and Chrome ProgIds for file extension and protocol handler. root_key is
@@ -316,7 +318,7 @@ bool SetAccessDefaultRegEntries(HKEY root_key,
   // Append the App Paths registry entries. Do this only if we are an admin,
   // since they are always written to HKLM.
   if (IsUserAnAdmin())
-    ShellUtil::AddChromeAppPathWorkItems(chrome_exe, items.get());
+    AddChromeAppPathWorkItems(chrome_exe, items.get());
 
   // Apply all the registry changes and if there is a problem, rollback.
   if (!items->Do()) {
@@ -347,8 +349,9 @@ ShellUtil::RegisterStatus RegisterOnVista(const std::wstring& chrome_exe,
       BrowserDistribution* dist = BrowserDistribution::GetDistribution();
       RegKey key(HKEY_CURRENT_USER, dist->GetUninstallRegPath().c_str());
       key.ReadValue(installer_util::kUninstallStringField, &exe_path);
-      exe_path = exe_path.substr(0, exe_path.find_first_of(L" --"));
-      TrimString(exe_path, L" \"", &exe_path);
+      CommandLine command_line(L"");
+      command_line.ParseFromString(exe_path);
+      exe_path = command_line.program();
     }
     if (file_util::PathExists(exe_path)) {
       std::wstring params(L"--");
@@ -376,6 +379,9 @@ const wchar_t* ShellUtil::kRegRegisteredApplications =
     L"Software\\RegisteredApplications";
 const wchar_t* ShellUtil::kRegVistaUrlPrefs =
     L"Software\\Microsoft\\Windows\\Shell\\Associations\\UrlAssociations\\http\\UserChoice";
+const wchar_t* ShellUtil::kAppPathsRegistryKey =
+    L"Software\\Microsoft\\Windows\\CurrentVersion\\App Paths";
+const wchar_t* ShellUtil::kAppPathsRegistryPathName = L"Path";
 
 const wchar_t* ShellUtil::kChromeHTMLProgId = L"ChromeHTML";
 const wchar_t* ShellUtil::kChromeHTMLProgIdDesc = L"Chrome HTML";
@@ -471,29 +477,6 @@ bool ShellUtil::GetQuickLaunchPath(bool system_level, std::wstring* path) {
   return true;
 }
 
-void ShellUtil::AddChromeAppPathWorkItems(
-    const std::wstring& chrome_exe, WorkItemList* item_list) {
-  WorkItem* create_work_item = WorkItem::CreateCreateRegKeyWorkItem(
-      HKEY_LOCAL_MACHINE, installer_util::kAppPathsRegistryKey);
-
-  item_list->AddWorkItem(create_work_item);
-
-  WorkItem* set_default_value_work_item =
-      WorkItem::CreateSetRegValueWorkItem(HKEY_LOCAL_MACHINE,
-          installer_util::kAppPathsRegistryKey,
-          installer_util::kAppPathsRegistryDefaultName,
-          chrome_exe, true);
-  item_list->AddWorkItem(set_default_value_work_item);
-
-  FilePath chrome_path(chrome_exe);
-  WorkItem* set_path_value_work_item =
-      WorkItem::CreateSetRegValueWorkItem(HKEY_LOCAL_MACHINE,
-          installer_util::kAppPathsRegistryKey,
-          installer_util::kAppPathsRegistryPathName,
-          chrome_path.DirName().value(), true);
-  item_list->AddWorkItem(set_path_value_work_item);
-}
-
 bool ShellUtil::CreateChromeDesktopShortcut(const std::wstring& chrome_exe,
                                             int shell_change,
                                             bool create_new) {
@@ -566,6 +549,8 @@ bool ShellUtil::CreateChromeQuickLaunchShortcut(const std::wstring& chrome_exe,
 bool ShellUtil::MakeChromeDefault(int shell_change,
                                   const std::wstring chrome_exe) {
   bool ret = true;
+  // First use the new "recommended" way on Vista to make Chrome default
+  // browser.
   if (win_util::GetWinVersion() == win_util::WINVERSION_VISTA) {
     LOG(INFO) << "Registering Chrome as default browser on Vista.";
     IApplicationAssociationRegistration* pAAR;
@@ -581,18 +566,22 @@ bool ShellUtil::MakeChromeDefault(int shell_change,
       ret = false;
       LOG(ERROR) << "Could not make Chrome default browser.";
     }
-  } else {
-    // Change the default browser for current user.
-    if ((shell_change & ShellUtil::CURRENT_USER) &&
-        !CreateChromeRegKeysForXP(HKEY_CURRENT_USER, chrome_exe))
-      ret = false;
-
-    // Chrome as default browser at system level.
-    if ((shell_change & ShellUtil::SYSTEM_LEVEL) &&
-        !CreateChromeRegKeysForXP(HKEY_LOCAL_MACHINE, chrome_exe))
-      ret = false;
   }
 
+  // Now use the old way to associate Chrome with supported protocols and file
+  // associations. This should not be required on Vista but since some
+  // applications still read Software\Classes\http key directly, we have to do
+  // this on Vista also.
+  // Change the default browser for current user.
+  if ((shell_change & ShellUtil::CURRENT_USER) &&
+      !BindChromeAssociations(HKEY_CURRENT_USER, chrome_exe))
+    ret = false;
+
+  // Chrome as default browser at system level.
+  if ((shell_change & ShellUtil::SYSTEM_LEVEL) &&
+      !BindChromeAssociations(HKEY_LOCAL_MACHINE, chrome_exe))
+    ret = false;
+
   // Send Windows notification event so that it can update icons for
   // file associations.
   SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST, NULL, NULL);
diff --git a/chrome/installer/util/shell_util.h b/chrome/installer/util/shell_util.h
index 5f8300c..8fd9ae6 100644
--- a/chrome/installer/util/shell_util.h
+++ b/chrome/installer/util/shell_util.h
@@ -54,6 +54,11 @@ class ShellUtil {
   // we add Chrome as a Windows application
   static const wchar_t* kRegRegisteredApplications;
 
+  // The key path and key name required to register Chrome on Windows such
+  // that it can be launched from Start->Run just by name (chrome.exe).
+  static const wchar_t* kAppPathsRegistryKey;
+  static const wchar_t* kAppPathsRegistryPathName;
+
   // Name that we give to Chrome file association handler ProgId.
   static const wchar_t* kChromeHTMLProgId;
 
@@ -78,15 +83,6 @@ class ShellUtil {
   // Description of Chrome file/URL association handler ProgId.
   static const wchar_t* kChromeExtProgIdDesc;
 
-  // Populate work_item_list with WorkItem entries that will add chrome.exe to
-  // the set of App Paths registry keys so that ShellExecute can find it. Note
-  // that this is done in HKLM, regardless of whether this is a single-user
-  // install or not. For non-admin users, this will fail.
-  // chrome_exe: full path to chrome.exe
-  // work_item_list: pointer to the WorkItemList that will be populated
-  static void AddChromeAppPathWorkItems(const std::wstring& chrome_exe,
-                                        WorkItemList* work_item_list);
-
   // This method adds Chrome to the list that shows up in Add/Remove Programs->
   // Set Program Access and Defaults and also creates Chrome ProgIds under
   // Software\Classes. This method requires write access to HKLM so is just
@@ -204,4 +200,3 @@ class ShellUtil {
 
 
 #endif  // CHROME_INSTALLER_UTIL_SHELL_UTIL_H__
-
diff --git a/chrome/installer/util/util_constants.cc b/chrome/installer/util/util_constants.cc
index 3fa7d21..ab695e1 100755
--- a/chrome/installer/util/util_constants.cc
+++ b/chrome/installer/util/util_constants.cc
@@ -79,13 +79,6 @@ const wchar_t kChromeDll[] = L"chrome.dll";
 const wchar_t kSetupExe[] = L"setup.exe";
 const wchar_t kInstallerDir[] = L"Installer";
 
-// Note that the following value must be kept in sync with kChromeExe
-const wchar_t kAppPathsRegistryKey[] =
-    L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\App Paths\\chrome.exe";
-// Use the empty string to set the key default value.
-const wchar_t kAppPathsRegistryDefaultName[] = L"";
-const wchar_t kAppPathsRegistryPathName[] = L"Path";
-
 const wchar_t kUninstallStringField[] = L"UninstallString";
 const wchar_t kUninstallDisplayNameField[] = L"DisplayName";
 }  // namespace installer_util
diff --git a/chrome/installer/util/util_constants.h b/chrome/installer/util/util_constants.h
index a8cbe9f..aa2b27f 100755
--- a/chrome/installer/util/util_constants.h
+++ b/chrome/installer/util/util_constants.h
@@ -90,10 +90,6 @@ extern const wchar_t kChromeDll[];
 extern const wchar_t kSetupExe[];
 extern const wchar_t kInstallerDir[];
 
-extern const wchar_t kAppPathsRegistryKey[];
-extern const wchar_t kAppPathsRegistryDefaultName[];
-extern const wchar_t kAppPathsRegistryPathName[];
-
 extern const wchar_t kUninstallStringField[];
 extern const wchar_t kUninstallDisplayNameField[];
 }  // namespace installer_util