Return a new error code for multi-install updates if Group Policy settings blocking...

chrome/installer/setup/setup_main.cc

Index: chrome/installer/setup/setup_main.cc
===================================================================
--- chrome/installer/setup/setup_main.cc	(revision 88131)
+++ chrome/installer/setup/setup_main.cc	(working copy)
@@ -252,6 +252,78 @@
   return ret;
 }
 
+// For each product that is being updated (i.e., already installed at an earlier
+// version), see if that product has an update policy override that differs from
+// that for the binaries.  If any are found, fail with an error indicating that
+// the Group Policy settings are in an inconsistent state.  Do not do this test
+// for same-version installs, since it would be unkind to block attempts to
+// repair a corrupt installation.  This function returns false when installation
+// should be halted, in which case |status| contains the relevant exit code and
+// the proper installer result has been written to the registry.
+bool CheckGroupPolicySettings(const InstallationState& original_state,
+                              const InstallerState& installer_state,
+                              const Version& new_version,
+                              installer::InstallStatus* status) {
+#if !defined(GOOGLE_CHROME_BUILD)
+  // Chromium builds are not updated via Google Update, so there are no
+  // Group Policy settings to consult.
+  return true;
+#else
+  DCHECK(status);
+
+  // Single installs are always in good shape.
+  if (!installer_state.is_multi_install())
+    return true;
+
+  bool settings_are_valid = true;
+  const bool is_system_install = installer_state.system_install();
+  BrowserDistribution* const binaries_dist =
+      installer_state.multi_package_binaries_distribution();
+
+  // Get the update policy for the binaries.
+  const GoogleUpdateSettings::UpdatePolicy binaries_policy =
+      GoogleUpdateSettings::GetAppUpdatePolicy(binaries_dist->GetAppGuid(),
+                                               NULL);
+
+  // Check for differing update policies for all of the products being updated.
+  const Products& products = installer_state.products();
+  Products::const_iterator scan = products.begin();
+  for (Products::const_iterator end = products.end(); scan != end; ++scan) {
+    BrowserDistribution* dist = (*scan)->distribution();

[pastarmovj, 2011/06/15 09:32:22]

Can this be const as well? If all the getters below are fine this should be
possible too?

[grt (UTC plus 2), 2011/06/15 13:38:16]

I wish.  :-)  The BrowserDistribution interface is declared with all non-const
methods.

+    const ProductState* product_state =
+        original_state.GetProductState(is_system_install, dist->GetType());
+    // Is an earlier version of this product already installed?
+    if (product_state != NULL &&
+        product_state->version().CompareTo(new_version) < 0) {
+      bool is_overridden = false;
+      GoogleUpdateSettings::UpdatePolicy app_policy =
+          GoogleUpdateSettings::GetAppUpdatePolicy(dist->GetAppGuid(),
+                                                   &is_overridden);
+      if (is_overridden && app_policy != binaries_policy) {
+        // TODO(grt): add " See http://goo.gl/+++ for details." to the
+        // end of this log message and to the
+        // IDS_INSTALL_INCONSISTENT_UPDATE_POLICY string once we have a page
+        // that explains why this error is being reported and how to resolve it.
+        LOG(ERROR) << "Found legacy Group Policy setting for "
+                   << dist->GetAppShortCutName() << " (value: " << app_policy
+                   << ") that does not match the setting for "
+                   << binaries_dist->GetAppShortCutName()
+                   << " (value: " << binaries_policy << ").";
+        settings_are_valid = false;
+      }
+    }
+  }
+
+  if (!settings_are_valid) {
+    *status = installer::INCONSISTENT_UPDATE_POLICY;
+    installer_state.WriteInstallerResult(
+        *status, IDS_INSTALL_INCONSISTENT_UPDATE_POLICY_BASE, NULL);
+  }
+
+  return settings_are_valid;
+#endif  // defined(GOOGLE_CHROME_BUILD)
+}
+
 // The supported multi-install modes are:
 // --multi-install --chrome --chrome-frame --ready-mode
 //   - If a non-multi Chrome Frame installation is present, Chrome Frame is
@@ -526,7 +598,7 @@
       // currently installed product for the shared binary installation, should
       // (or rather must) be upgraded.
       VLOG(1) << "version to install: " << installer_version->GetString();
-      bool higher_version_installed = false;
+      bool proceed_with_installation = true;
       for (size_t i = 0; i < installer_state.products().size(); ++i) {
         const Product* product = installer_state.products()[i];
         const ProductState* product_state =
@@ -535,7 +607,7 @@
         if (product_state != NULL &&
             (product_state->version().CompareTo(*installer_version) > 0)) {
           LOG(ERROR) << "Higher version is already installed.";
-          higher_version_installed = true;
+          proceed_with_installation = false;
           install_status = installer::HIGHER_VERSION_EXISTS;
 
           if (product->is_chrome()) {
@@ -550,7 +622,12 @@
         }
       }
 
-      if (!higher_version_installed) {
+      proceed_with_installation =

[pastarmovj, 2011/06/15 09:32:22]

Why not &= instead?

[grt (UTC plus 2), 2011/06/15 13:38:16]

&= is a bitwise rather than a logical operator.

[pastarmovj, 2011/06/15 13:56:34]

True, sorry I must have been half asleep when I wrote that :)

+          proceed_with_installation &&
+          CheckGroupPolicySettings(original_state, installer_state,
+                                   *installer_version, &install_status);
+
+      if (proceed_with_installation) {
         // We want to keep uncompressed archive (chrome.7z) that we get after
         // uncompressing and binary patching. Get the location for this file.
         FilePath archive_to_copy(