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

chrome/installer/setup/install.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.
 
 #include "chrome/installer/setup/install.h"
 
 #include <shlobj.h>
 #include <time.h>
 #include <winuser.h>
 
@@ skipping 238 matching lines @@
               << " to " << src_path.value();
       return true;
     } else {
       PLOG(ERROR) << "Error writing " << installer::kVisualElementsManifest
                   << " to " << src_path.value();
       return false;
     }
   }
 }
 
-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());
 
   // Information used for all shortcut types
   BrowserDistribution* browser_dist = product.distribution();
   const string16 product_name(browser_dist->GetAppShortCutName());
   const string16 product_desc(browser_dist->GetAppDescription());
   // Chrome link target
   FilePath chrome_exe(
       installer_state.target_path().Append(installer::kChromeExe));
 
   bool create_always = ((options & ShellUtil::SHORTCUT_CREATE_ALWAYS) != 0);
   const char* operation = create_always ? "Creating" : "Updating";
 
   // Create Start Menu shortcuts.
   // The location of Start->Programs->Google Chrome folder
   FilePath start_menu_folder_path;
   int dir_enum = installer_state.system_install() ?
       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);
 
   // Create/update Chrome link (points to chrome.exe) & Uninstall Chrome link
   // (which points to setup.exe) under |start_menu_folder_path|.
 
   // Chrome link (launches Chrome)
   FilePath chrome_link(start_menu_folder_path.Append(product_name + L".lnk"));
 
   if (create_always && !file_util::PathExists(start_menu_folder_path))
       file_util::CreateDirectoryW(start_menu_folder_path);
 
   VLOG(1) << operation << " shortcut to " << chrome_exe.value() << " at "
           << chrome_link.value();
   if (!ShellUtil::UpdateChromeShortcut(browser_dist, chrome_exe.value(),
           chrome_link.value(), string16(), product_desc, chrome_exe.value(),
           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|,
     // proceed to pin the Start Menu shortcut to the taskbar on Win7+.
     VLOG(1) << "Pinning new shortcut at " << chrome_link.value()
             << " to taskbar";
     if (!file_util::TaskbarPinShortcutLink(chrome_link.value().c_str())) {
       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
   // installations are, for the time being, managed only through the
   // Add/Remove Programs dialog.
   // TODO(robertshield): We could add a shortcut to msiexec /X {GUID} here.
   if (!installer_state.is_msi()) {
     // Uninstall Chrome link
     FilePath uninstall_link(start_menu_folder_path.Append(
         browser_dist->GetUninstallLinkName() + L".lnk"));
 
     CommandLine arguments(CommandLine::NO_PROGRAM);
     AppendUninstallCommandLineFlags(installer_state, product, &arguments);
     VLOG(1) << operation << " uninstall link at " << uninstall_link.value();
     if (!file_util::CreateOrUpdateShortcutLink(setup_exe.value().c_str(),
             uninstall_link.value().c_str(), NULL,
             arguments.GetCommandLineString().c_str(), NULL,
             setup_exe.value().c_str(), 0, NULL,
             create_always ? file_util::SHORTCUT_CREATE_ALWAYS :
                             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());
 
   // Information used for all shortcut types
   BrowserDistribution* browser_dist = product.distribution();
   const string16 product_name(browser_dist->GetAppShortCutName());
   const string16 product_desc(browser_dist->GetAppDescription());
   // Chrome link target
   FilePath chrome_exe(
       installer_state.target_path().Append(installer::kChromeExe));
 
   bool create_always = ((options & ShellUtil::SHORTCUT_CREATE_ALWAYS) != 0);
   const char* operation = create_always ? "Creating" : "Updating";
 
   ShellUtil::ShellChange desktop_level = ShellUtil::CURRENT_USER;
   int quick_launch_levels = ShellUtil::CURRENT_USER;
   if (installer_state.system_install()) {
     desktop_level = ShellUtil::SYSTEM_LEVEL;
     quick_launch_levels |= ShellUtil::SYSTEM_LEVEL;
   }
 
   VLOG(1) << operation << " desktop shortcut for " << chrome_exe.value();
   if (!ShellUtil::CreateChromeDesktopShortcut(
           browser_dist, chrome_exe.value(), product_desc, string16(),
           string16(), chrome_exe.value(), browser_dist->GetIconIndex(),
           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();
   if (!ShellUtil::CreateChromeQuickLaunchShortcut(
           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());
 
   // Try to add Chrome to Media Player shim inclusion list. We don't do any
   // error checking here because this operation will fail if user doesn't
   // have admin rights and we want to ignore the error.
   AddChromeToMediaPlayerList();
 
   // Make Chrome the default browser if desired when possible. Otherwise, only
   // register it with Windows.
   BrowserDistribution* dist = product.distribution();
   const string16 chrome_exe(
       installer_state.target_path().Append(installer::kChromeExe).value());
   VLOG(1) << "Registering Chrome as browser: " << chrome_exe;
   if (make_chrome_default) {
     if (ShellUtil::CanMakeChromeDefaultUnattended()) {
       int level = ShellUtil::CURRENT_USER;
       if (installer_state.system_install())
         level = level | ShellUtil::SYSTEM_LEVEL;
       ShellUtil::MakeChromeDefault(dist, level, chrome_exe, true);
     } else if (IsInteractiveProcess()) {
       ShellUtil::ShowMakeChromeDefaultSystemUI(dist, chrome_exe);
     } else {
       ShellUtil::RegisterChromeBrowser(dist, chrome_exe, string16(), false);
     }
   } 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(
     const InstallationState& original_state,
     const InstallerState& installer_state,
     const FilePath& setup_path,
     const FilePath& archive_path,
     const FilePath& install_temp_path,
     const FilePath& prefs_path,
     const MasterPreferences& prefs,
@@ skipping 121 matching lines @@
 
     installer_state.RemoveOldVersionDirectories(
         new_version,
         existing_version.get(),
         install_temp_path);
   }
 
   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