Revise system profile prefs clearing

components/metrics/metrics_service.cc

Index: components/metrics/metrics_service.cc
diff --git a/components/metrics/metrics_service.cc b/components/metrics/metrics_service.cc
index 2acc9cb0b88cbec1f0538b42147f7c800af98958..ac1ae4d39fffb13414190154db3501cb2ff749da 100644
--- a/components/metrics/metrics_service.cc
+++ b/components/metrics/metrics_service.cc
@@ -584,11 +584,11 @@ void MetricsService::InitializeMetricsState() {
     clean_exit_beacon_.WriteBeaconValue(true);
   }
 
-  bool has_initial_stability_log = false;
-  bool providers_have_initial_stability_metrics =
+  bool is_initial_stability_log_required =

[Alexei Svitkine (slow), 2016/10/20 16:47:26]

Nit: const

[manzagop (departed), 2016/10/20 18:38:37]

Done.

+      !clean_exit_beacon_.exited_cleanly() ||
       ProvidersHaveInitialStabilityMetrics();
-  if (!clean_exit_beacon_.exited_cleanly() ||
-      providers_have_initial_stability_metrics) {
+  bool has_initial_stability_log = false;
+  if (is_initial_stability_log_required) {
     // TODO(rtenneti): On windows, consider saving/getting execution_phase from
     // the registry.
     int execution_phase =
@@ -606,17 +606,24 @@ void MetricsService::InitializeMetricsState() {
     }
   }
 
-  // If no initial stability log was generated and there was a version upgrade,
-  // clear the stability stats from the previous version (so that they don't get
+  // If the version changed, but no initial stability log was generated, clear
+  // the stability stats from the previous version (so that they don't get
   // attributed to the current version). This could otherwise happen due to a
   // number of different edge cases, such as if the last version crashed before
   // it could save off a system profile or if UMA reporting is disabled (which
   // normally results in stats being accumulated).
-  if (!has_initial_stability_log && version_changed) {
+  if (version_changed && !has_initial_stability_log) {
     ClearSavedStabilityMetrics();
     IncrementPrefValue(prefs::kStabilityDiscardCount);
   }
 
+  // If the version changed, the system profile is obsolete and needs to be
+  // cleared.

[Alexei Svitkine (slow), 2016/10/20 16:47:26]

I think I do agree with this logic, but the comment should be expanded to
provide more context since originally it wasn't obvious to me that this is
desired.

The reason i think this is OK is because by this point if there stability stats
that warranted creating an initial stability log, those stats would already have
been included in that log. So we're not losing those. But the reason we want to
clear it is if we crash before we have a chance to save the new system profile,
so that those new crashes don't get attributed to the old version.

Can you make the comment include the above reasoning? Thanks!

[manzagop (departed), 2016/10/20 18:38:37]

Done.

+  if (version_changed) {
+    local_state->ClearPref(prefs::kStabilitySavedSystemProfile);
+    local_state->ClearPref(prefs::kStabilitySavedSystemProfileHash);
+  }
+
   // Update session ID.
   ++session_id_;
   local_state_->SetInteger(prefs::kMetricsSessionID, session_id_);