Callback flow to register Chrome and update shortcuts after OS upgrade to Windows 8

chrome/installer/setup/install.cc

Index: chrome/installer/setup/install.cc
diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc
index bd7ab6ff49ef95e607eb51f31d95e09e4a8cae39..9f5fead80a8f68a1f4d4edb5f816b0a901f58b2f 100644
--- a/chrome/installer/setup/install.cc
+++ b/chrome/installer/setup/install.cc
@@ -256,11 +256,12 @@ bool CreateVisualElementsManifest(const FilePath& src_path,
   }
 }
 
-void CreateOrUpdateStartMenuAndTaskbarShortcuts(
+bool CreateOrUpdateStartMenuAndTaskbarShortcuts(
     const InstallerState& installer_state,
     const FilePath& setup_exe,
     const Product& product,
     uint32 options) {
+  bool success = true;
   // TODO(tommi): Change this function to use WorkItemList.
   DCHECK(product.is_chrome());
 
@@ -282,7 +283,7 @@ void CreateOrUpdateStartMenuAndTaskbarShortcuts(
       base::DIR_COMMON_START_MENU : base::DIR_START_MENU;
   if (!PathService::Get(dir_enum, &start_menu_folder_path)) {
     LOG(ERROR) << "Failed to get start menu path.";
-    return;
+    return false;
   }
 
   start_menu_folder_path = start_menu_folder_path.Append(product_name);
@@ -303,6 +304,7 @@ void CreateOrUpdateStartMenuAndTaskbarShortcuts(
           browser_dist->GetIconIndex(), options)) {
     LOG(WARNING) << operation << " shortcut at " << chrome_link.value()
                  << " failed.";
+    success = false;

[grt (UTC plus 2), 2012/08/31 00:41:36]

UpdateChromeShortcut will return false in certain success cases.  i think we
need to back this change out.  it's safer to go with gab's less-prefered option
of making the new os upgrade fn return void for now.  perhaps it'll make sense
to propagate errors correctly once gab refactors shortcut creation so that it's
more sensible.

[gab, 2012/08/31 01:31:06]

Ah yes indeed, UpdateChromeShortcut() currently fails if the shortcut has been
deleted by the user and this shouldn't be considered an install/upgrade failure
over all.

[huangs, 2012/08/31 20:50:17]

Removing all these, per discussion.

   } else if (create_always &&
              base::win::GetVersion() >= base::win::VERSION_WIN7) {
     // If the Start Menu shortcut was successfully created and |create_always|,
@@ -313,6 +315,7 @@ void CreateOrUpdateStartMenuAndTaskbarShortcuts(
       LOG(ERROR) << "Failed to pin shortcut to taskbar: "
                  << chrome_link.value();
     }
+    success = false;
   }
 
   // Create/update uninstall link if we are not an MSI install. MSI
@@ -335,14 +338,17 @@ void CreateOrUpdateStartMenuAndTaskbarShortcuts(
                             file_util::SHORTCUT_NO_OPTIONS)) {
       LOG(WARNING) << operation << " uninstall link at "
                    << uninstall_link.value() << " failed.";
+      success = false;
     }
   }
+  return success;
 }
 
-void CreateOrUpdateDesktopAndQuickLaunchShortcuts(
+bool CreateOrUpdateDesktopAndQuickLaunchShortcuts(
     const InstallerState& installer_state,
     const Product& product,
     uint32 options) {
+  bool success = true;
   // TODO(tommi): Change this function to use WorkItemList.
   DCHECK(product.is_chrome());
 
@@ -371,6 +377,7 @@ void CreateOrUpdateDesktopAndQuickLaunchShortcuts(
           desktop_level, options)) {
     LOG(WARNING) << operation << " desktop shortcut for " << chrome_exe.value()
                  << " failed.";
+    success = false;
   }
 
   VLOG(1) << operation << " quick launch shortcut for " << chrome_exe.value();
@@ -378,10 +385,12 @@ void CreateOrUpdateDesktopAndQuickLaunchShortcuts(
           browser_dist, chrome_exe.value(), quick_launch_levels, options)) {
     LOG(WARNING) << operation << " quick launch shortcut for "
                  << chrome_exe.value() << " failed.";
+    success = false;
   }
+  return success;
 }
 
-void RegisterChromeOnMachine(const InstallerState& installer_state,
+bool RegisterChromeOnMachine(const InstallerState& installer_state,
                              const Product& product,
                              bool make_chrome_default) {
   DCHECK(product.is_chrome());
@@ -411,6 +420,7 @@ void RegisterChromeOnMachine(const InstallerState& installer_state,
   } else {
     ShellUtil::RegisterChromeBrowser(dist, chrome_exe, string16(), false);
   }
+  return true;

[gab, 2012/08/30 21:12:03]

A bunch of things can fail above, simply returning true here is not enough...

[robertshield, 2012/08/31 02:24:17]

We never cared about failure here before. This change is a no-op in practice.

[huangs, 2012/08/31 20:50:17]

Done.

 }
 
 InstallStatus InstallOrUpdateProduct(
@@ -552,4 +562,24 @@ InstallStatus InstallOrUpdateProduct(
   return result;
 }
 
+bool HandleOsUpgradeForBrowser(const InstallerState& installer_state,
+                               const Product& chrome,
+                               const FilePath& setup_exe) {
+  DCHECK(chrome.is_chrome());
+  bool success = true;
+  // Upon upgrading to Windows 8, we need to fix Chrome shortcuts and register
+  // Chrome, so that Metro Chrome would work if Chrome is the default browser.
+  if (base::win::GetVersion() >= base::win::VERSION_WIN8) {
+    VLOG(1) << "Updating and registering shortcuts.";
+    uint32 shortcut_options = ShellUtil::SHORTCUT_DUAL_MODE;
+    success = CreateOrUpdateDesktopAndQuickLaunchShortcuts(
+        installer_state, chrome, shortcut_options) && success;
+    success = CreateOrUpdateStartMenuAndTaskbarShortcuts(
+        installer_state, setup_exe, chrome, shortcut_options) && success;
+    success = RegisterChromeOnMachine(installer_state, chrome, false)
+        && success;

[gab, 2012/08/30 20:51:07]

nit: && should be on the line above and the 2nd condition below
alternatively you can do:
success =
    RegisterChromeOnMachine(installer_state, chrome, false) && success;
if that fits.

[huangs, 2012/08/31 20:50:17]

Okay, but removing this.

+  }
+  return success;
+}
+
 }  // namespace installer