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 534 matching lines @@
 
     installer_state.RemoveOldVersionDirectories(
         new_version,
         existing_version.get(),
         install_temp_path);
   }
 
   return result;
 }
 
+InstallStatus ChromeBrowserOnOsUpgrade(const InstallationState& original_state,
+                                       const InstallerState& installer_state,
+                                       const CommandLine& cmd_line) {

[gab, 2012/08/28 16:08:19]

Since all you need is setup_exe here, take setup_exe as an argument instead of
cmd_line.

[huangs, 2012/08/29 17:02:54]

Done.

+  const Product* chrome_install =
+      installer_state.FindProduct(BrowserDistribution::CHROME_BROWSER);
+  DCHECK(chrome_install);
+
+  const FilePath& setup_exe = cmd_line.GetProgram();

[gab, 2012/08/28 16:08:19]

Chromium style is to initialize non-POD types using:
const foo& bar(myFoo);

instead of:
const foo& bar = myFoo;
(this style is for POD types only, i.e. int, char, pointers, etc.)

[huangs, 2012/08/29 17:02:54]

Done.

+  base::win::Version win_version = base::win::GetVersion();

[gab, 2012/08/28 16:08:19]

inline in if condition

+
+  // 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 (win_version >= base::win::VERSION_WIN8) {
+    // Trigger error on next OS upgrade, so we can review if this is needed.
+    DCHECK(win_version == base::win::VERSION_WIN8);

[gab, 2012/08/28 16:08:19]

Remove this comment/DCHECK, everything will need to be revisited on the next OS
upgrade, registration and shortcuts would already be done this way on the next
auto-update we are simply forcing it early here.

[huangs, 2012/08/29 17:02:54]

Done.

+
+    VLOG(1) << "Updating and registering shortcut for Windows 8.";
+    uint32 shortcut_options = ShellUtil::SHORTCUT_DUAL_MODE;
+    CreateOrUpdateDesktopAndQuickLaunchShortcuts(
+        installer_state, *chrome_install, shortcut_options);
+    CreateOrUpdateStartMenuAndTaskbarShortcuts(
+        installer_state, setup_exe, *chrome_install, shortcut_options);
+    RegisterChromeOnMachine(installer_state, *chrome_install, false);
+  }
+
+  return installer::ON_OS_UPGRADE_SUCCESSFUL;

[gab, 2012/08/28 16:08:19]

I don't think this has any value as there is no other return statement possible,
having a void method accomplishes the same thing here.

[grt (UTC plus 2), 2012/08/28 19:35:39]

should the CreateOrUpdate... and Register... funcs return their status so that
it can be propagated here?

[huangs, 2012/08/29 17:02:54]

The caller HandleNonInstallCmdLineOptions() expects some exit_code to be
supplied.  Since there is no generic "SUCCESS", so I created the extra flag
ON_OS_UPGRADE_SUCCESSFUL to be returned here.

[grt (UTC plus 2), 2012/08/29 17:39:21]

as mentioned in my comment in util_constants.h, InstallerStatus is a
special-purpose enum used to convey information to Google Update.  this new
value you've added isn't passed to Google Update, so it doesn't belong.  do you
lose anything by having this return bool and using that to set *exit_code in
HandleNonInstallCmdLineOptions?

+}
+
 }  // namespace installer