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

chrome/installer/setup/setup_main.cc

 // Copyright (c) 2011 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 <windows.h>
 #include <msi.h>
 #include <shellapi.h>
 #include <shlobj.h>
 
 #include <string>
@@ skipping 234 matching lines @@
     install_list->Rollback();
     ret = installer::RENAME_FAILED;
   }
   // temp_path's dtor will take care of deleting or scheduling itself for
   // deletion at reboot when this scope closes.
   VLOG(1) << "Deleting temporary directory " << temp_path.path().value();
 
   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
 //     removed from |installer_state|'s list of products (thereby preserving
 //     the existing SxS install).
 //   - If a multi Chrome Frame installation is present, its options are
 //     preserved (i.e., the --ready-mode command-line option is ignored).
 // --multi-install --chrome-frame
 //   - If a non-multi Chrome Frame installation is present, fail.
 //   - If --ready-mode and no Chrome installation is present, fail.
@@ skipping 254 matching lines @@
       install_status = installer::INVALID_ARCHIVE;
       installer_state.WriteInstallerResult(install_status,
           IDS_INSTALL_INVALID_ARCHIVE_BASE, NULL);
     } else {
       // TODO(tommi): Move towards having only a single version that is common
       // to all products.  Only the package should have a version since it
       // represents all the binaries.  When a single product is upgraded, all
       // 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 =
             original_state.GetProductState(system_install,
                                            product->distribution()->GetType());
         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()) {
             // TODO(robertshield): We should take the installer result text
             // strings from the Product.
             installer_state.WriteInstallerResult(install_status,
                 IDS_INSTALL_HIGHER_VERSION_BASE, NULL);
           } else {
             installer_state.WriteInstallerResult(install_status,
                 IDS_INSTALL_HIGHER_VERSION_CF_BASE, NULL);
           }
         }
       }
 
-      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(
             temp_path.path().Append(installer::kChromeArchive));
         FilePath prefs_source_path(cmd_line.GetSwitchValueNative(
             installer::switches::kInstallerData));
         install_status = installer::InstallOrUpdateProduct(original_state,
             installer_state, cmd_line.GetProgram(), archive_to_copy,
             temp_path.path(), prefs_source_path, prefs, *installer_version);
 
@@ skipping 658 matching lines @@
   if (!(installer_state.is_msi() && is_uninstall))
     // Note that we allow the status installer::UNINSTALL_REQUIRES_REBOOT
     // to pass through, since this is only returned on uninstall which is
     // never invoked directly by Google Update.
     return_code = InstallUtil::GetInstallReturnCode(install_status);
 
   VLOG(1) << "Installation complete, returning: " << return_code;
 
   return return_code;
 }