[Cleanup] Factor out a class to manage safe mode state.

R=asvitkine@chromium.org

Bug: 727984
Change-Id: Ic1993cce55beed9e00512f6fe4b001fc5ec4c591
Reviewed-on: https://chromium-review.googlesource.com/818746
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523262}

components/variations/service/BUILD.gn

@@ -4,6 +4,8 @@
 
 static_library("service") {
   sources = [
+    "safe_seed_manager.cc",
+    "safe_seed_manager.h",
     "ui_string_overrider.cc",
     "ui_string_overrider.h",
     "variations_field_trial_creator.cc",

components/variations/service/safe_seed_manager.cc

+// Copyright 2017 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 "components/variations/service/safe_seed_manager.h"
+
+#include "base/base_switches.h"
+#include "base/command_line.h"
+#include "base/metrics/histogram_functions.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/numerics/ranges.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+#include "components/variations/pref_names.h"
+
+namespace variations {
+
+SafeSeedManager::SafeSeedManager(bool did_previous_session_exit_cleanly,
+                                 PrefService* local_state)
+    : local_state_(local_state) {
+  // Increment the crash streak if the previous session crashed.
+  // Note that the streak is not cleared if the previous run didn’t crash.
+  // Instead, it’s incremented on each crash until Chrome is able to
+  // successfully fetch a new seed. This way, a seed update that mostly
+  // destabilizes Chrome will still result in a fallback to safe mode.
+  int num_crashes = local_state->GetInteger(prefs::kVariationsCrashStreak);
+  if (!did_previous_session_exit_cleanly) {
+    ++num_crashes;
+    local_state->SetInteger(prefs::kVariationsCrashStreak, num_crashes);
+  }
+
+  // After three failures in a row -- either consistent crashes or consistent
+  // failures to fetch the seed -- assume that the current seed is bad, and fall
+  // back to the safe seed. However, ignore any number of failures if the
+  // --force-fieldtrials flag is set, as this flag is only used by developers,
+  // and there's no need to make the development process flakier.
+  const int kMaxFailuresBeforeRevertingToSafeSeed = 3;
+  int num_failures_to_fetch =
+      local_state->GetInteger(prefs::kVariationsFailedToFetchSeedStreak);
+  bool fall_back_to_safe_mode =
+      (num_crashes >= kMaxFailuresBeforeRevertingToSafeSeed ||
+       num_failures_to_fetch >= kMaxFailuresBeforeRevertingToSafeSeed) &&
+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          ::switches::kForceFieldTrials);
+  UMA_HISTOGRAM_BOOLEAN("Variations.SafeMode.FellBackToSafeMode",
+                        fall_back_to_safe_mode);
+  base::UmaHistogramSparse("Variations.SafeMode.Streak.Crashes",
+                           base::ClampToRange(num_crashes, 0, 100));
+  base::UmaHistogramSparse("Variations.SafeMode.Streak.FetchFailures",
+                           base::ClampToRange(num_failures_to_fetch, 0, 100));
+}
+
+SafeSeedManager::~SafeSeedManager() = default;
+
+// static
+void SafeSeedManager::RegisterPrefs(PrefRegistrySimple* registry) {
+  // Prefs tracking failures along the way to fetching a seed.
+  registry->RegisterIntegerPref(prefs::kVariationsCrashStreak, 0);
+  registry->RegisterIntegerPref(prefs::kVariationsFailedToFetchSeedStreak, 0);
+}
+
+void SafeSeedManager::RecordFetchStarted() {
+  // Pessimistically assume the fetch will fail. The failure streak will be
+  // reset upon success.
+  int num_failures_to_fetch =
+      local_state_->GetInteger(prefs::kVariationsFailedToFetchSeedStreak);
+  local_state_->SetInteger(prefs::kVariationsFailedToFetchSeedStreak,
+                           num_failures_to_fetch + 1);
+}
+
+void SafeSeedManager::RecordSuccessfulFetch() {
+  // Note: It's important to clear the crash streak as well as the fetch
+  // failures streak. Crashes that occur after a successful seed fetch do not
+  // prevent updating to a new seed, and therefore do not necessitate falling
+  // back to a safe seed.
+  local_state_->SetInteger(prefs::kVariationsCrashStreak, 0);
+  local_state_->SetInteger(prefs::kVariationsFailedToFetchSeedStreak, 0);
+}
+
+}  // namespace variations

components/variations/service/safe_seed_manager.h

+// Copyright 2017 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.
+
+#ifndef COMPONENTS_VARIATIONS_SERVICE_SAFE_SEED_MANAGER_H_
+#define COMPONENTS_VARIATIONS_SERVICE_SAFE_SEED_MANAGER_H_
+
+#include "base/macros.h"
+
+class PrefRegistrySimple;
+class PrefService;
+
+namespace variations {
+
+// The primary class that encapsulates state for managing the safe seed.
+class SafeSeedManager {
+ public:
+  // Creates a SafeSeedManager instance, and updates safe mode prefs for
+  // bookkeeping.
+  SafeSeedManager(bool did_previous_session_exit_cleanly,
+                  PrefService* local_state);
+  ~SafeSeedManager();
+
+  // Register safe mode prefs in Local State.
+  static void RegisterPrefs(PrefRegistrySimple* registry);
+
+  // Records that a fetch has started: pessimistically increments the
+  // corresponding failure streak for safe mode.
+  void RecordFetchStarted();
+
+  // Records a successful fetch: resets the failure streaks for safe mode.
+  void RecordSuccessfulFetch();
+
+ private:
+  // The pref service used to store persist the variations seed. Weak reference;
+  // must outlive |this| instance.
+  PrefService* local_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(SafeSeedManager);
+};
+
+}  // namespace variations
+
+#endif  // COMPONENTS_VARIATIONS_SERVICE_SAFE_SEED_MANAGER_H_

components/variations/service/variations_service.cc

@@ -31,6 +31,7 @@
 #include "components/data_use_measurement/core/data_use_user_data.h"
 #include "components/encrypted_messages/encrypted_message.pb.h"
 #include "components/encrypted_messages/message_encrypter.h"
+#include "components/metrics/clean_exit_beacon.h"
 #include "components/metrics/metrics_state_manager.h"
 #include "components/network_time/network_time_tracker.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -250,44 +251,14 @@ VariationsService::VariationsService(
       disable_deltas_for_next_request_(false),
       resource_request_allowed_notifier_(std::move(notifier)),
       request_count_(0),
+      safe_seed_manager_(state_manager->clean_exit_beacon()->exited_cleanly(),
+                         local_state),
       field_trial_creator_(local_state, client_.get(), ui_string_overrider),
       weak_ptr_factory_(this) {
   DCHECK(client_.get());
   DCHECK(resource_request_allowed_notifier_.get());
 
   resource_request_allowed_notifier_->Init(this);
-
-  // Increment the crash streak if the previous session crashed.
-  // Note that the streak is not cleared if the previous run didn’t crash.
-  // Instead, it’s incremented on each crash until Chrome is able to
-  // successfully fetch a new seed. This way, a seed update that mostly
-  // destabilizes Chrome will still result in a fallback to safe mode.
-  int num_crashes = local_state->GetInteger(prefs::kVariationsCrashStreak);
-  if (!state_manager_->clean_exit_beacon()->exited_cleanly()) {
-    ++num_crashes;
-    local_state->SetInteger(prefs::kVariationsCrashStreak, num_crashes);
-  }
-
-  // After three failures in a row -- either consistent crashes or consistent
-  // failures to fetch the seed -- assume that the current seed is bad, and fall
-  // back to the safe seed. However, ignore any number of failures if the
-  // --force-fieldtrials flag is set, as this flag is only used by developers,
-  // and there's no need to make the development process flakier.
-  const int kMaxFailuresBeforeRevertingToSafeSeed = 3;
-  int num_failures_to_fetch =
-      local_state->GetInteger(prefs::kVariationsFailedToFetchSeedStreak);
-  bool fall_back_to_safe_mode =
-      (num_crashes >= kMaxFailuresBeforeRevertingToSafeSeed ||
-       num_failures_to_fetch >= kMaxFailuresBeforeRevertingToSafeSeed) &&
-      !base::CommandLine::ForCurrentProcess()->HasSwitch(
-          ::switches::kForceFieldTrials);
-  UMA_HISTOGRAM_BOOLEAN("Variations.SafeMode.FellBackToSafeMode",
-                        fall_back_to_safe_mode);
-  UMA_HISTOGRAM_SPARSE_SLOWLY("Variations.SafeMode.Streak.Crashes",
-                              std::min(std::max(num_crashes, 0), 100));
-  UMA_HISTOGRAM_SPARSE_SLOWLY(
-      "Variations.SafeMode.Streak.FetchFailures",
-      std::min(std::max(num_failures_to_fetch, 0), 100));
 }
 
 VariationsService::~VariationsService() {
@@ -409,7 +380,9 @@ std::string VariationsService::GetDefaultVariationsServerURLForTesting() {
 
 // static
 void VariationsService::RegisterPrefs(PrefRegistrySimple* registry) {
+  SafeSeedManager::RegisterPrefs(registry);
   VariationsSeedStore::RegisterPrefs(registry);
+
   registry->RegisterInt64Pref(prefs::kVariationsLastFetchTime, 0);
   // This preference will only be written by the policy service, which will fill
   // it according to a value stored in the User Policy.
@@ -418,11 +391,6 @@ void VariationsService::RegisterPrefs(PrefRegistrySimple* registry) {
   // This preference keeps track of the country code used to filter
   // permanent-consistency studies.
   registry->RegisterListPref(prefs::kVariationsPermanentConsistencyCountry);
-
-  // Prefs tracking failures along the way to fetching a seed, used to implement
-  // Safe Mode.
-  registry->RegisterIntegerPref(prefs::kVariationsCrashStreak, 0);
-  registry->RegisterIntegerPref(prefs::kVariationsFailedToFetchSeedStreak, 0);
 }
 
 // static
@@ -493,12 +461,7 @@ bool VariationsService::DoFetchFromURL(const GURL& url) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(IsFetchingEnabled());
 
-  // Pessimistically assume the fetch will fail. The failure streak will be
-  // reset upon success.
-  int num_failures_to_fetch =
-      local_state_->GetInteger(prefs::kVariationsFailedToFetchSeedStreak);
-  local_state_->SetInteger(prefs::kVariationsFailedToFetchSeedStreak,
-                           num_failures_to_fetch + 1);
+  safe_seed_manager_.RecordFetchStarted();
 
   // Normally, there shouldn't be a |pending_request_| when this fires. However
   // it's not impossible - for example if Chrome was paused (e.g. in a debugger
@@ -788,13 +751,7 @@ void VariationsService::PerformSimulationWithVersion(
 
 void VariationsService::RecordSuccessfulFetch() {
   field_trial_creator_.RecordLastFetchTime();
-
-  // Note: It's important to clear the crash streak as well as the fetch
-  // failures streak. Crashes that occur after a successful seed fetch do not
-  // prevent updating to a new seed, and therefore do not necessitate falling
-  // back to a safe seed.
-  local_state_->SetInteger(prefs::kVariationsCrashStreak, 0);
-  local_state_->SetInteger(prefs::kVariationsFailedToFetchSeedStreak, 0);
+  safe_seed_manager_.RecordSuccessfulFetch();
 }
 
 void VariationsService::GetClientFilterableStateForVersionCalledForTesting() {

components/variations/service/variations_service.h

@@ -16,6 +16,7 @@
 #include "base/observer_list.h"
 #include "base/time/time.h"
 #include "components/variations/client_filterable_state.h"
+#include "components/variations/service/safe_seed_manager.h"
 #include "components/variations/service/ui_string_overrider.h"
 #include "components/variations/service/variations_field_trial_creator.h"
 #include "components/variations/service/variations_service_client.h"
@@ -351,6 +352,9 @@ class VariationsService
   // List of observers of the VariationsService.
   base::ObserverList<Observer> observer_list_;
 
+  // The main entry point for managing safe mode state.
+  SafeSeedManager safe_seed_manager_;
+
   // Member responsible for creating trials from a variations seed.
   VariationsFieldTrialCreator field_trial_creator_;