Standardize persisted log loading

components/metrics/metrics_service.cc

 // Copyright 2014 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.
 
 //------------------------------------------------------------------------------
 // Description of the life cycle of a instance of MetricsService.
 //
 //  OVERVIEW
 //
 // A MetricsService instance is typically created at application startup.  It is
@@ skipping 533 matching lines @@
 
 //------------------------------------------------------------------------------
 // Initialization methods
 
 void MetricsService::InitializeMetricsState() {
   local_state_->SetString(metrics::prefs::kStabilityStatsVersion,
                           client_->GetVersionString());
   local_state_->SetInt64(metrics::prefs::kStabilityStatsBuildTime,
                          MetricsLog::GetBuildTime());
 
+  log_manager_.LoadPersistedUnsentLogs();
+
   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);
 
     // TODO(rtenneti): On windows, consider saving/getting execution_phase from
     // the registry.
@@ skipping 310 matching lines @@
     case INITIALIZED:
     case INIT_TASK_SCHEDULED:  // We should be further along by now.
       NOTREACHED();
       return;
 
     case INIT_TASK_DONE:
       if (has_initial_stability_log_) {
         // There's an initial stability log, ready to send.
         log_manager_.StageNextLogForUpload();
         has_initial_stability_log_ = false;
-        // Note: No need to call LoadPersistedUnsentLogs() here because unsent
-        // logs have already been loaded by PrepareInitialStabilityLog().
         state_ = SENDING_INITIAL_STABILITY_LOG;
       } else {
         PrepareInitialMetricsLog();
-        // Load unsent logs (if any) from local state.
-        log_manager_.LoadPersistedUnsentLogs();
         state_ = SENDING_INITIAL_METRICS_LOG;
       }
       break;
 
     case SENDING_OLD_LOGS:
       NOTREACHED();  // Shouldn't be staging a new log during old log sending.
       return;
 
     case SENDING_CURRENT_LOGS:
       CloseCurrentLog();
@@ skipping 15 matching lines @@
 
   scoped_ptr<MetricsLog> initial_stability_log(
       CreateLog(MetricsLog::INITIAL_STABILITY_LOG));
 
   // Do not call NotifyOnDidCreateMetricsLog here because the stability
   // log describes stats from the _previous_ session.
 
   if (!initial_stability_log->LoadSavedEnvironmentFromPrefs())
     return;
 
-  log_manager_.LoadPersistedUnsentLogs();
-
   log_manager_.PauseCurrentLog();
   log_manager_.BeginLoggingWithLog(initial_stability_log.Pass());
 
   // Note: Some stability providers may record stability stats via histograms,
   //       so this call has to be after BeginLoggingWithLog().
   log_manager_.current_log()->RecordStabilityMetrics(
       metrics_providers_.get(), base::TimeDelta(), base::TimeDelta());
   RecordCurrentStabilityHistograms();
 
   // Note: RecordGeneralMetrics() intentionally not called since this log is for
@@ skipping 21 matching lines @@
   GetUptimes(local_state_, &incremental_uptime, &uptime);
 
   // Histograms only get written to the current log, so make the new log current
   // before writing them.
   log_manager_.PauseCurrentLog();
   log_manager_.BeginLoggingWithLog(initial_metrics_log_.Pass());
 
   // Note: Some stability providers may record stability stats via histograms,
   //       so this call has to be after BeginLoggingWithLog().
   MetricsLog* current_log =
       static_cast<MetricsLog*>(log_manager_.current_log());

[Alexei Svitkine (slow), 2014/07/14 18:46:54]

Nit: While you're here, you may as well remove this unnecessary cast now that
we've unified things.

[Steven Holte, 2014/07/15 01:22:01]

Done. x3

   current_log->RecordStabilityMetrics(metrics_providers_.get(),
                                       base::TimeDelta(), base::TimeDelta());
   RecordCurrentHistograms();
 
   current_log->RecordGeneralMetrics(metrics_providers_.get());
 
   log_manager_.FinishCurrentLog();
   log_manager_.ResumePausedLog();
 
+  // Store unsent logs, including the initial log that was just saved, so
+  // that they're not lost in case of a crash before upload time.
+  log_manager_.PersistUnsentLogs();
+
   DCHECK(!log_manager_.has_staged_log());
   log_manager_.StageNextLogForUpload();
 }
 
 void MetricsService::SendStagedLog() {
   DCHECK(log_manager_.has_staged_log());
   if (!log_manager_.has_staged_log())
     return;
 
   DCHECK(!log_upload_in_progress_);
@@ skipping 43 matching lines @@
   const size_t log_size = log_manager_.staged_log().length();
   if (!upload_succeeded && log_size > kUploadLogAvoidRetransmitSize) {
     UMA_HISTOGRAM_COUNTS("UMA.Large Rejected Log was Discarded",
                          static_cast<int>(log_size));
     discard_log = true;
   } else if (response_code == 400) {
     // Bad syntax.  Retransmission won't work.
     discard_log = true;
   }
 
-  if (upload_succeeded || discard_log)
+  if (upload_succeeded || discard_log) {
     log_manager_.DiscardStagedLog();
+    // Store the updated list to disk now that the removed log is uploaded.
+    log_manager_.PersistUnsentLogs();
+  }
 
   if (!log_manager_.has_staged_log()) {
     switch (state_) {
       case SENDING_INITIAL_STABILITY_LOG:
-        // Store the updated list to disk now that the removed log is uploaded.
-        log_manager_.PersistUnsentLogs();
         PrepareInitialMetricsLog();
         SendStagedLog();
         state_ = SENDING_INITIAL_METRICS_LOG;
         break;
 
       case SENDING_INITIAL_METRICS_LOG:
-        // The initial metrics log never gets persisted to local state, so it's
-        // not necessary to call log_manager_.PersistUnsentLogs() here.
-        // TODO(asvitkine): It should be persisted like the initial stability
-        // log and old unsent logs. http://crbug.com/328417
         state_ = log_manager_.has_unsent_logs() ? SENDING_OLD_LOGS
                                                 : SENDING_CURRENT_LOGS;
         break;
 
       case SENDING_OLD_LOGS:
-        // Store the updated list to disk now that the removed log is uploaded.
-        log_manager_.PersistUnsentLogs();
         if (!log_manager_.has_unsent_logs())
           state_ = SENDING_CURRENT_LOGS;
         break;
 
       case SENDING_CURRENT_LOGS:
         break;
 
       default:
         NOTREACHED();
         break;
@@ skipping 120 matching lines @@
   RecordCurrentState(local_state_);
 }
 
 void MetricsService::RecordCurrentState(PrefService* pref) {
   pref->SetInt64(metrics::prefs::kStabilityLastTimestampSec,
                  Time::Now().ToTimeT());
 
   for (size_t i = 0; i < metrics_providers_.size(); ++i)
     metrics_providers_[i]->RecordCurrentState();
 }