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

chrome/installer/setup/install_worker.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 contains the definitions of the installer functions that build
 // the WorkItemList used to install the application.
 
 #include "chrome/installer/setup/install_worker.h"
 
 #include <oaidl.h>
@@ skipping 462 matching lines @@
   AddOemInstallWorkItems(original_state, installer_state, install_list);
 
   AddEulaAcceptedWorkItems(original_state, installer_state, install_list);
 
   AddUsageStatsWorkItems(original_state, installer_state, install_list);
 
   // TODO(grt): check for other keys/values we should put in the package's
   // ClientState and/or Clients key.
 }
 
+void AddOsUpgradeWorkItems(const InstallationState& original_state,

[grt (UTC plus 2), 2012/08/21 20:32:55]

Since this does work for the browser only, I think it's cleaner to 1) add a
clause to AddProductSpecificWorkItems that calls this function, and 2) put
Chrome in the function name in some way.  This will let you get rid of the
installer_state.state_type() call, which doesn't do what you think it does.

[huangs, 2012/08/21 22:00:43]

Renaming to AddCromeBrowserOsUpgradeWorkItems().  Added "Browser" because
"AddChromeOsUpgradeWorkItems()" is confusing, owing to "ChromeOs".

I'm also moving routine to the bottom of the file.

+                           const InstallerState& installer_state,
+                           const FilePath& setup_path,
+                           const Version& new_version,
+                           WorkItemList* install_list) {
+  const HKEY root_key = installer_state.root_key();
+
+  // In Chrome Browser, add registry entries so that whenOS upgrade occurs,

[grt (UTC plus 2), 2012/08/21 20:32:55]

"whenOS" -> "when an OS"

[huangs, 2012/08/21 22:00:43]

Done.

+  // Google Update would call setup.exe with --on-os-upgrade switch, which

[grt (UTC plus 2), 2012/08/21 20:32:55]

"would" -> "will"

[huangs, 2012/08/21 22:00:43]

Done.

+  // leads to OnOsUpgrade() being called.
+  if (installer_state.state_type() == BrowserDistribution::CHROME_BROWSER) {
+    const Product* product = installer_state.FindProduct(
+        BrowserDistribution::CHROME_BROWSER);
+    DCHECK(product != NULL);
+    std::wstring browser_key(product->distribution()->GetVersionKey());
+    browser_key.append(1, L'\\').append(google_update::kRegCommandsKey)
+        .append(1, L'\\').append(kCmdOnOsUpgrade);
+
+    if (installer_state.operation() != InstallerState::UNINSTALL) {
+      FilePath installer_path(installer_state
+          .GetInstallerDirectory(new_version).Append(setup_path.BaseName()));
+      CommandLine command(installer_path);
+      command.AppendSwitch(installer::switches::kOnOsUpgrade);

[grt (UTC plus 2), 2012/08/21 20:32:55]

my gut tells me that you want to add kMultiInstall to the command-line when this
is a multi-install of Chrome.

[huangs, 2012/08/21 22:00:43]

Done.

+      if (installer_state.verbose_logging())
+        command.AppendSwitch(installer::switches::kVerboseLogging);
+
+      install_list->set_log_message("adding OS upgrade command");
+
+      install_list->AddCreateRegKeyWorkItem(root_key, browser_key)
+          ->set_log_message("creating AppCommand command registry key");

[grt (UTC plus 2), 2012/08/21 20:32:55]

does this use of set_log_message add useful meaning to the logs?  i don't think
this is standard practice in the installer.  should it be?

[huangs, 2012/08/21 22:00:43]

This was modelled after
http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/installer/util/app_...
which I just realized is  out-of-date, so I'll remove redundant comments.

+      install_list->AddSetRegValueWorkItem(root_key, browser_key,
+                                           google_update::kRegCommandLineField,
+                                           command.GetCommandLineString(),
+                                           true)
+          ->set_log_message("setting AppCommand CommandLine registry value");
+      install_list->AddSetRegValueWorkItem(
+          root_key, browser_key, google_update::kRegAutoRunOnOSUpgrade,
+          (DWORD) 1, true)
+          ->set_log_message("setting AutoRunOnOSUpgrade registry value");
+
+    } else {
+      install_list->AddDeleteRegKeyWorkItem(root_key,

[grt (UTC plus 2), 2012/08/21 20:32:55]

is this necessary?  the whole version key is deleted on uninstall, right?

[huangs, 2012/08/21 22:00:43]

I'm following the general style of the file, where add/remove come in pairs
(remove is idempotent anyway).  Will remove if you still don't like that.  :)

+                                            browser_key)->set_log_message(
+          "Removing OS upgrade command");
+    }
+  }
+}
+
 void AddUsageStatsWorkItems(const InstallationState& original_state,
                             const InstallerState& installer_state,
                             WorkItemList* install_list) {
   DCHECK(installer_state.operation() == InstallerState::MULTI_INSTALL ||
          installer_state.operation() == InstallerState::MULTI_UPDATE);
 
   HKEY root_key = installer_state.root_key();
   bool value_found = false;
   DWORD usagestats = 0;
   const Products& products = installer_state.products();
@@ skipping 424 matching lines @@
   }
 
   // Add any remaining work items that involve special settings for
   // each product.
   AddProductSpecificWorkItems(original_state, installer_state, setup_path,
                               new_version, install_list);
 
   // Copy over brand, usagestats, and other values.
   AddGoogleUpdateWorkItems(original_state, installer_state, install_list);
 
+  AddOsUpgradeWorkItems(original_state, installer_state, setup_path,
+                        new_version, install_list);
+
   AddQuickEnableApplicationHostWorkItems(installer_state, original_state,
                                          &setup_path, &new_version,
                                          install_list);
 
   AddQuickEnableChromeFrameWorkItems(installer_state, original_state,
                                      &setup_path, &new_version, install_list);
 
   // Append the tasks that run after the installation.
   AppendPostInstallTasks(installer_state,
                          setup_path,
@@ skipping 650 matching lines @@
                                  machine_state,
                                  setup_path,
                                  new_version,
                                  work_item_list,
                                  false,  // have_child_product
                                  cmd_line,
                                  kCmdQuickEnableApplicationHost);
 }
 
 }  // namespace installer