Remove uses of BrowserDistribution::CHROME_BINARIES in installer_util.

chrome/installer/util/google_update_settings.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/util/google_update_settings.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <limits>
 #include <vector>

[huangs, 2017/01/09 21:27:08]

<vector> now unused?

[grt (UTC plus 2), 2017/01/10 08:43:56]

You bet. <limits>, too.

 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 127 matching lines @@
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   base::string16 reg_path = dist->GetStateKey();
   RegKey key(HKEY_CURRENT_USER,
              reg_path.c_str(),
              KEY_READ | KEY_WRITE | KEY_WOW64_32KEY);
   if (!key.HasValue(name))
     return true;
   return (key.DeleteValue(name) == ERROR_SUCCESS);
 }
 
-// Initializes |channel_info| based on |system_install| and |dist|. Also
-// returns whether the install is a multi-install via output parameter
-// |is_multi_install|. Returns false on failure.
+// Initializes |channel_info| based on |system_install|. Returns false on
+// failure.
 bool InitChannelInfo(bool system_install,
-                     installer::ChannelInfo* channel_info,
-                     bool* is_multi_install) {
-  // Determine whether or not chrome is multi-install. If so, updates are
-  // delivered under the binaries' app guid, so that's where the relevant
-  // channel is found.
-  installer::ProductState state;
-  ignore_result(state.Initialize(system_install));
-  if (!state.is_multi_install()) {
-    // Use the channel info that was just read for this single-install chrome.
-    *channel_info = state.channel();
-  } else {
-    // Read the channel info from the binaries' state key.
-    HKEY root_key = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
-    BrowserDistribution* dist = BrowserDistribution::GetSpecificDistribution(
-        BrowserDistribution::CHROME_BINARIES);
-    RegKey key(root_key, dist->GetStateKey().c_str(),
-               KEY_READ | KEY_WOW64_32KEY);
-
-    if (!channel_info->Initialize(key))
-      return false;
-  }
-  *is_multi_install = state.is_multi_install();
-  return true;
+                     installer::ChannelInfo* channel_info) {
+  HKEY root_key = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  RegKey key(root_key, dist->GetStateKey().c_str(),
+             KEY_QUERY_VALUE | KEY_WOW64_32KEY);
+  return channel_info->Initialize(key);
 }
 
 bool GetChromeChannelInternal(bool system_install,
-                              bool add_multi_modifier,
                               base::string16* channel) {
-  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
-
   // Shortcut in case this distribution knows what channel it is (canary).
-  if (dist->GetChromeChannel(channel))
+  if (BrowserDistribution::GetDistribution()->GetChromeChannel(channel))
     return true;
 
   installer::ChannelInfo channel_info;
-  bool is_multi_install = false;
-  if (!InitChannelInfo(system_install, &channel_info, &is_multi_install)) {
+  if (!InitChannelInfo(system_install, &channel_info)) {
     channel->assign(installer::kChromeChannelUnknown);
     return false;
   }
 
   if (!channel_info.GetChannelName(channel))
     channel->assign(installer::kChromeChannelUnknown);
 
-  // Tag the channel name if this is a multi-install.
-  if (add_multi_modifier && is_multi_install) {
-    if (!channel->empty())
-      channel->push_back(L'-');
-    channel->push_back(L'm');
-  }
-
   return true;
 }
 
 #if defined(GOOGLE_CHROME_BUILD)
 // Populates |update_policy| with the UpdatePolicy enum value corresponding to a
 // DWORD read from the registry and returns true if |value| is within range.
 // If |value| is out of range, returns false without modifying |update_policy|.
 bool GetUpdatePolicyFromDword(
     const DWORD value,
     GoogleUpdateSettings::UpdatePolicy* update_policy) {
   switch (value) {
     case GoogleUpdateSettings::UPDATES_DISABLED:
     case GoogleUpdateSettings::AUTOMATIC_UPDATES:
     case GoogleUpdateSettings::MANUAL_UPDATES_ONLY:
     case GoogleUpdateSettings::AUTO_UPDATES_ONLY:
       *update_policy = static_cast<GoogleUpdateSettings::UpdatePolicy>(value);
       return true;
     default:
       LOG(WARNING) << "Unexpected update policy override value: " << value;
   }
   return false;
 }
 #endif  // defined(GOOGLE_CHROME_BUILD)
 
-bool UpdateDidRunStateForApp(const AppRegistrationData& app_reg_data,
-                             bool did_run) {
-  return WriteGoogleUpdateStrKeyInternal(
-      app_reg_data, google_update::kRegDidRunField, did_run ? L"1" : L"0");
-}
-
-// Convenience routine: GoogleUpdateSettings::UpdateDidRunStateForApp()
-// specialized for Chrome Binaries.
-bool UpdateDidRunStateForBinaries(bool did_run) {
-  BrowserDistribution* dist = BrowserDistribution::GetSpecificDistribution(
-      BrowserDistribution::CHROME_BINARIES);
-  return UpdateDidRunStateForApp(dist->GetAppRegistrationData(), did_run);
-}
-
 }  // namespace
 
 bool GoogleUpdateSettings::IsSystemInstall() {
   bool system_install = false;
   base::FilePath module_dir;
   if (!PathService::Get(base::DIR_MODULE, &module_dir)) {
     LOG(WARNING)
         << "Failed to get directory of module; assuming per-user install.";
   } else {
     system_install = !InstallUtil::IsPerUserInstall(module_dir);
   }
   return system_install;
 }
 
 bool GoogleUpdateSettings::GetCollectStatsConsent() {
   return GetCollectStatsConsentAtLevel(IsSystemInstall());
 }
 
 bool GoogleUpdateSettings::SetCollectStatsConsent(bool consented) {
   return SetCollectStatsConsentAtLevel(IsSystemInstall(), consented);
 }
 
 bool GoogleUpdateSettings::GetCollectStatsConsentAtLevel(bool system_install) {
-  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
-
-  // Consent applies to all products in a multi-install package.
-  if (InstallUtil::IsMultiInstall(system_install)) {
-    dist = BrowserDistribution::GetSpecificDistribution(
-        BrowserDistribution::CHROME_BINARIES);
-  }
-
   return GetCollectStatsConsentForApp(system_install,
-                                      dist->GetAppRegistrationData()) ==
+                                      BrowserDistribution::GetDistribution()
+                                          ->GetAppRegistrationData()) ==
          google_update::TRISTATE_TRUE;
 }
 
 google_update::Tristate GoogleUpdateSettings::GetCollectStatsConsentForApp(
     bool system_install,
     const AppRegistrationData& reg_data) {
   RegKey key;
   DWORD value = google_update::TRISTATE_NONE;
   bool have_value = false;
   const REGSAM kAccess = KEY_QUERY_VALUE | KEY_WOW64_32KEY;
@@ skipping 20 matching lines @@
 
   return value == google_update::TRISTATE_TRUE ? google_update::TRISTATE_TRUE
                                                : google_update::TRISTATE_FALSE;
 }
 
 // static
 bool GoogleUpdateSettings::SetCollectStatsConsentAtLevel(bool system_install,
                                                          bool consented) {
   DWORD value =
       consented ? google_update::TRISTATE_TRUE : google_update::TRISTATE_FALSE;
-
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
 
-  // Consent applies to all products in a multi-install package.
-  if (InstallUtil::IsMultiInstall(system_install)) {
-    dist = BrowserDistribution::GetSpecificDistribution(
-        BrowserDistribution::CHROME_BINARIES);
-  }
-
   // Write to ClientStateMedium for system-level; ClientState otherwise.
   HKEY root_key = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
   base::string16 reg_path =
       system_install ? dist->GetStateMediumKey() : dist->GetStateKey();
   RegKey key;
   LONG result = key.Create(
       root_key, reg_path.c_str(), KEY_SET_VALUE | KEY_WOW64_32KEY);
   if (result != ERROR_SUCCESS) {
     LOG(ERROR) << "Failed opening key " << reg_path << " to set "
                << google_update::kRegUsageStatsField << "; result: " << result;
   } else {
     result = key.WriteValue(google_update::kRegUsageStatsField, value);
     LOG_IF(ERROR, result != ERROR_SUCCESS) << "Failed setting "
         << google_update::kRegUsageStatsField << " in key " << reg_path
         << "; result: " << result;
   }
 
   // When opting out, clear registry backup of client id and related values.
   if (result == ERROR_SUCCESS && !consented)
     StoreMetricsClientInfo(metrics::ClientInfo());
 
   return (result == ERROR_SUCCESS);
 }
 
 // static
 bool GoogleUpdateSettings::GetCollectStatsConsentDefault(
     bool* stats_consent_default) {
   installer::ChannelInfo channel_info;
-  bool is_multi_install = false;
-  if (InitChannelInfo(IsSystemInstall(), &channel_info, &is_multi_install)) {
+  if (InitChannelInfo(IsSystemInstall(), &channel_info)) {
     base::string16 stats_default = channel_info.GetStatsDefault();
     if (stats_default == L"0" || stats_default == L"1") {
       *stats_consent_default = (stats_default == L"1");
       return true;
     }
   }
   return false;
 }
 
 std::unique_ptr<metrics::ClientInfo>
@@ skipping 101 matching lines @@
 }
 
 bool GoogleUpdateSettings::GetReferral(base::string16* referral) {
   return ReadGoogleUpdateStrKey(google_update::kRegReferralField, referral);
 }
 
 bool GoogleUpdateSettings::ClearReferral() {
   return ClearGoogleUpdateStrKey(google_update::kRegReferralField);
 }
 
 bool GoogleUpdateSettings::UpdateDidRunState(bool did_run, bool system_level) {

[Sébastien Marchand, 2017/01/09 21:19:39]

Oops, you forgot to remove the |bool system_level| argument.

[grt (UTC plus 2), 2017/01/10 08:43:56]

Doh!

   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
-  bool result = UpdateDidRunStateForApp(dist->GetAppRegistrationData(),
-                                        did_run);
-  // Update state for binaries, even if the previous call was unsuccessful.
-  if (InstallUtil::IsMultiInstall(system_level))
-    result = UpdateDidRunStateForBinaries(did_run) && result;
-  return result;
+  return WriteGoogleUpdateStrKeyInternal(dist->GetAppRegistrationData(),
+                                         google_update::kRegDidRunField,
+                                         did_run ? L"1" : L"0");
 }
 
 base::string16 GoogleUpdateSettings::GetChromeChannel(bool system_install) {
   base::string16 channel;
-  GetChromeChannelInternal(system_install, false, &channel);
+  GetChromeChannelInternal(system_install, &channel);
   return channel;
 }
 
-bool GoogleUpdateSettings::GetChromeChannelAndModifiers(
-    bool system_install,
-    base::string16* channel) {
-  return GetChromeChannelInternal(system_install, true, channel);
-}
-
 void GoogleUpdateSettings::UpdateInstallStatus(bool system_install,
     installer::ArchiveType archive_type, int install_return_code,
     const base::string16& product_guid) {
   DCHECK(archive_type != installer::UNKNOWN_ARCHIVE_TYPE ||
          install_return_code != 0);
   HKEY reg_root = (system_install) ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
 
   RegKey key;
   installer::ChannelInfo channel_info;
   base::string16 reg_key(google_update::kRegPathClientState);
@@ skipping 66 matching lines @@
     } else {
       VLOG(1) << "Incremental installer failure; already on channel: "
               << value->value();
     }
   } else {
     // It's okay if we don't know the archive type.  In this case, leave the
     // "-full" suffix as we found it.
     DCHECK_EQ(installer::UNKNOWN_ARCHIVE_TYPE, archive_type);
   }
 
   if (value->SetMultiFailSuffix(false)) {

[huangs, 2017/01/09 21:27:08]

Should this be addressed? If we should leave this, maybe add brief comment on
why (e.g., clean up legacy)?

[grt (UTC plus 2), 2017/01/10 08:43:56]

Done.

     VLOG(1) << "Removed multi-install failure key; switching to channel: "
             << value->value();
     modified = true;
   }
 
   if (value->ClearStage()) {
     VLOG(1) << "Removed (legacy) stage information; switching to channel: "
             << value->value();
     modified = true;
   }
@@ skipping 88 matching lines @@
   RegKey policy_key;
   DWORD value = 0;
   if (policy_key.Open(HKEY_LOCAL_MACHINE, kPoliciesKey,
                       KEY_QUERY_VALUE) == ERROR_SUCCESS &&
       policy_key.ReadValueDW(kCheckPeriodOverrideMinutes,
                              &value) == ERROR_SUCCESS &&
       (value == 0 || value > kCheckPeriodOverrideMinutesMax)) {
     return false;
   }
 
-  // Auto updates are subtly broken when Chrome and the binaries have different
-  // overrides in place. If this Chrome cannot possibly be multi-install by
-  // virtue of being a side-by-side installation, simply check Chrome's policy.
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   UpdatePolicy app_policy = GetAppUpdatePolicy(dist->GetAppGuid(), nullptr);
-  if (InstallUtil::IsChromeSxSProcess())
-    return app_policy == AUTOMATIC_UPDATES || app_policy == AUTO_UPDATES_ONLY;
-
-  // Otherwise, check for consistency between Chrome and the binaries regardless
-  // of whether or not this Chrome is multi-install since the next update likely
-  // will attempt to migrate it to such.
-  BrowserDistribution* binaries = BrowserDistribution::GetSpecificDistribution(
-      BrowserDistribution::CHROME_BINARIES);
-  return (GetAppUpdatePolicy(binaries->GetAppGuid(), nullptr) == app_policy &&
-          (app_policy == AUTOMATIC_UPDATES || app_policy == AUTO_UPDATES_ONLY));
+  return app_policy == AUTOMATIC_UPDATES || app_policy == AUTO_UPDATES_ONLY;
 #else  // defined(GOOGLE_CHROME_BUILD)
   // Chromium does not auto update.
   return false;
 #endif  // !defined(GOOGLE_CHROME_BUILD)
 }
 
 // static
 bool GoogleUpdateSettings::ReenableAutoupdates() {
 #if defined(GOOGLE_CHROME_BUILD)
   int needs_reset_count = 0;
   int did_reset_count = 0;
 
-  // Reset overrides for Chrome and for the binaries if this Chrome supports
-  // multi-install.
-  std::vector<base::string16> app_guids;
-  app_guids.push_back(BrowserDistribution::GetDistribution()->GetAppGuid());
-  if (!InstallUtil::IsChromeSxSProcess()) {
-    app_guids.push_back(BrowserDistribution::GetSpecificDistribution(
-        BrowserDistribution::CHROME_BINARIES)->GetAppGuid());
-  }
-
   UpdatePolicy update_policy = kDefaultUpdatePolicy;
   RegKey policy_key;
   if (policy_key.Open(HKEY_LOCAL_MACHINE, kPoliciesKey,
                       KEY_SET_VALUE | KEY_QUERY_VALUE) == ERROR_SUCCESS) {
     // Set to true while app-specific overrides are present that allow automatic
     // updates. When this is the case, the defaults are irrelevant and don't
     // need to be checked or reset.
     bool automatic_updates_allowed_by_overrides = true;
     DWORD value = 0;
-    for (const base::string16& app_guid : app_guids) {
-      // First check the app-specific override value and reset that if needed.
-      // Note that this intentionally sets the override to AUTOMATIC_UPDATES
-      // even if it was previously AUTO_UPDATES_ONLY. The thinking is that
-      // AUTOMATIC_UPDATES is marginally more likely to let a user update and
-      // this code is only called when a stuck user asks for updates.
-      base::string16 app_update_override(kUpdateOverrideValuePrefix);
-      app_update_override.append(app_guid);
-      if (policy_key.ReadValueDW(app_update_override.c_str(),
-                                 &value) != ERROR_SUCCESS) {
-        automatic_updates_allowed_by_overrides = false;
-      } else if (!GetUpdatePolicyFromDword(value, &update_policy) ||
-                 update_policy != GoogleUpdateSettings::AUTOMATIC_UPDATES) {
-        automatic_updates_allowed_by_overrides = false;
-        ++needs_reset_count;
-        if (policy_key.WriteValue(
-                app_update_override.c_str(),
-                static_cast<DWORD>(GoogleUpdateSettings::AUTOMATIC_UPDATES)) ==
-            ERROR_SUCCESS) {
-          ++did_reset_count;
-        }
+
+    // First check the app-specific override value and reset that if needed.
+    // Note that this intentionally sets the override to AUTOMATIC_UPDATES even
+    // if it was previously AUTO_UPDATES_ONLY. The thinking is that
+    // AUTOMATIC_UPDATES is marginally more likely to let a user update and this
+    // code is only called when a stuck user asks for updates.
+    base::string16 app_update_override(kUpdateOverrideValuePrefix);
+    app_update_override.append(
+        BrowserDistribution::GetDistribution()->GetAppGuid());
+    if (policy_key.ReadValueDW(app_update_override.c_str(), &value) !=
+        ERROR_SUCCESS) {
+      automatic_updates_allowed_by_overrides = false;
+    } else if (!GetUpdatePolicyFromDword(value, &update_policy) ||
+               update_policy != GoogleUpdateSettings::AUTOMATIC_UPDATES) {
+      automatic_updates_allowed_by_overrides = false;
+      ++needs_reset_count;
+      if (policy_key.WriteValue(
+              app_update_override.c_str(),
+              static_cast<DWORD>(GoogleUpdateSettings::AUTOMATIC_UPDATES)) ==
+          ERROR_SUCCESS) {
+        ++did_reset_count;
       }
     }
 
     // If there were no app-specific override policies, see if there's a global
     // policy preventing updates and delete it if so.
     if (!automatic_updates_allowed_by_overrides &&
         policy_key.ReadValueDW(kUpdatePolicyValue, &value) == ERROR_SUCCESS &&
         (!GetUpdatePolicyFromDword(value, &update_policy) ||
          update_policy != GoogleUpdateSettings::AUTOMATIC_UPDATES)) {
       ++needs_reset_count;
@@ skipping 13 matching lines @@
 
     // Return whether the number of successful resets is the same as the
     // number of things that appeared to need resetting.
     return (needs_reset_count == did_reset_count);
   } else {
     // For some reason we couldn't open the policy key with the desired
     // permissions to make changes (the most likely reason is that there is no
     // policy set). Simply return whether or not we think updates are enabled.
     return AreAutoupdatesEnabled();
   }
-
 #endif
   // Non Google Chrome isn't going to autoupdate.
   return true;
 }
 
 // Reads and sanitizes the value of
 // "HKLM\SOFTWARE\Policies\Google\Update\DownloadPreference". A valid
 // group policy option must be a single alpha numeric word of up to 32
 // characters.
 base::string16 GoogleUpdateSettings::GetDownloadPreference() {
@@ skipping 10 matching lines @@
     for (auto ch : value) {
       if (!base::IsAsciiAlpha(ch))
         return base::string16();
     }
     return value;
   }
   return base::string16();
 }
 
 void GoogleUpdateSettings::RecordChromeUpdatePolicyHistograms() {
-  const bool is_multi_install = InstallUtil::IsMultiInstall(IsSystemInstall());
   const base::string16 app_guid =
-      BrowserDistribution::GetSpecificDistribution(
-          is_multi_install ? BrowserDistribution::CHROME_BINARIES :
-                             BrowserDistribution::CHROME_BROWSER)->GetAppGuid();
+      BrowserDistribution::GetDistribution()->GetAppGuid();
 
   bool is_overridden = false;
   const UpdatePolicy update_policy = GetAppUpdatePolicy(app_guid,
                                                         &is_overridden);
   UMA_HISTOGRAM_BOOLEAN("GoogleUpdate.UpdatePolicyIsOverridden", is_overridden);
   UMA_HISTOGRAM_ENUMERATION("GoogleUpdate.EffectivePolicy", update_policy,
                             UPDATE_POLICIES_COUNT);
 }
 
 base::string16 GoogleUpdateSettings::GetUninstallCommandLine(
@@ skipping 182 matching lines @@
   }
 
   // If the key or value was not present, return the empty string.
   if (result == ERROR_FILE_NOT_FOUND || result == ERROR_PATH_NOT_FOUND) {
     experiment_labels->clear();
     return true;
   }
 
   return result == ERROR_SUCCESS;
 }