Allow MetricsProviders to request an initial stability log.

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 3c499c14e671415bfd2414e05389a73f4720fd9d..f2fd2e31e4d5bd57bbcd5005e2e24255b38922d2 100644
--- a/components/metrics/metrics_service.cc
+++ b/components/metrics/metrics_service.cc
@@ -566,11 +566,14 @@ void MetricsService::InitializeMetricsState() {
 
   session_id_ = local_state_->GetInteger(metrics::prefs::kMetricsSessionID);
 
-  if (!local_state_->GetBoolean(metrics::prefs::kStabilityExitedCleanly)) {
-    IncrementPrefValue(metrics::prefs::kStabilityCrashCount);
-    // Reset flag, and wait until we call LogNeedForCleanShutdown() before
-    // monitoring.
-    local_state_->SetBoolean(metrics::prefs::kStabilityExitedCleanly, true);
+  if (ShouldPrepareStabilityLog()) {
+    if (!local_state_->GetBoolean(metrics::prefs::kStabilityExitedCleanly)) {

[Alexei Svitkine (slow), 2014/09/09 15:34:28]

It's kind of confusing to have the check for this to be both in
ShouldPrepareStabilityLog() and here, since it relies on
ShouldPrepareStabilityLog() to check this pref which is not obvious by just
looking at the code here.

How about using the following code structure instead:

(in this function)

boolean exited_cleanly = true;
if (!local_state_->GetBoolean(metrics::prefs::kStabilityExitedCleanly)) {
  IncrementPrefValue(metrics::prefs::kStabilityCrashCount);
  local_state_->SetBoolean(metrics::prefs::kStabilityExitedCleanly, true);
  exited_cleanly = false;
}

if (!exited_cleanly || ProvidersHaveStabilityMetrics()) {
   ...
}

[Sigurður Ásgeirsson, 2014/09/09 17:13:40]

Yups, that's much clearer.

+      IncrementPrefValue(metrics::prefs::kStabilityCrashCount);
+
+      // Reset flag, and wait until we call LogNeedForCleanShutdown() before
+      // monitoring.
+      local_state_->SetBoolean(metrics::prefs::kStabilityExitedCleanly, true);
+    }
 
     // TODO(rtenneti): On windows, consider saving/getting execution_phase from
     // the registry.
@@ -909,9 +912,22 @@ void MetricsService::StageNewLog() {
   DCHECK(log_manager_.has_staged_log());
 }
 
+bool MetricsService::ShouldPrepareStabilityLog() {
+  // Prepare a stability log if we had a crash in the previous session.
+  if (!local_state_->GetBoolean(metrics::prefs::kStabilityExitedCleanly))
+    return true;
+
+  // Or if any of the registered providers request it.
+  for (size_t i = 0; i < metrics_providers_.size(); ++i) {
+    if (metrics_providers_[i]->HasStabilityMetrics())
+      return true;
+  }
+
+  return false;
+}
+
 void MetricsService::PrepareInitialStabilityLog() {
   DCHECK_EQ(INITIALIZED, state_);
-  DCHECK_NE(0, local_state_->GetInteger(metrics::prefs::kStabilityCrashCount));
 
   scoped_ptr<MetricsLog> initial_stability_log(
       CreateLog(MetricsLog::INITIAL_STABILITY_LOG));