Begin to separate the MetricsService logic for creating vs uploading logs

chrome/browser/metrics/metrics_service.cc

Index: chrome/browser/metrics/metrics_service.cc
diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index 634ef6a86d7574528d6c3f1ed2d28443ea86009a..571bcfaff7a9005490a93499d55b818fa05968ac 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -797,7 +797,8 @@ void MetricsService::StartRecording() {
   if (log_manager_.current_log())
     return;
 
-  log_manager_.BeginLoggingWithLog(new MetricsLog(client_id_, session_id_));
+  log_manager_.BeginLoggingWithLog(new MetricsLog(client_id_, session_id_),
+                                   MetricsLogManager::ONGOING_LOG);
   if (state_ == INITIALIZED) {
     // We only need to schedule that run once.
     state_ = INIT_TASK_SCHEDULED;
@@ -842,7 +843,7 @@ void MetricsService::StopRecording() {
   current_log->RecordIncrementalStabilityElements();
   RecordCurrentHistograms();
 
-  log_manager_.StageCurrentLogForUpload();
+  log_manager_.FinishCurrentLog();
 }
 
 void MetricsService::PushPendingLogsToPersistentStorage() {
@@ -850,19 +851,13 @@ void MetricsService::PushPendingLogsToPersistentStorage() {
     return;  // We didn't and still don't have time to get plugin list etc.
 
   if (log_manager_.has_staged_log()) {
-    if (state_ == INITIAL_LOG_READY) {
-      // We may race here, and send second copy of initial log later.
-      log_manager_.StoreStagedLogAsUnsent(MetricsLogManager::INITIAL_LOG);
+    // We may race here, and send second copy of initial log later.
+    if (state_ == INITIAL_LOG_READY)
       state_ = SENDING_OLD_LOGS;
-    } else {
-      // TODO(jar): Verify correctness in other states, including sending unsent
-      // initial logs.
-      log_manager_.StoreStagedLogAsUnsent(MetricsLogManager::ONGOING_LOG);
-    }
+    log_manager_.StoreStagedLogAsUnsent();
   }
   DCHECK(!log_manager_.has_staged_log());
   StopRecording();
-  log_manager_.StoreStagedLogAsUnsent(MetricsLogManager::ONGOING_LOG);
   StoreUnsentLogs();
 }
 
@@ -882,6 +877,15 @@ void MetricsService::StartScheduledUpload() {
     return;
   }
 
+  StartFinalLogInfoCollection();
+}
+
+void MetricsService::StartFinalLogInfoCollection() {
+  // Begin the multi-step process of collecting memory usage histograms:
+  // First spawn a task to collect the memory details; when that task is
+  // finished, it will call OnMemoryDetailCollectionDone. That will in turn
+  // call HistogramSynchronization to collect histograms from all renderers and
+  // then call OnHistogramSynchronizationDone to continue processing.
   DCHECK(!waiting_for_asynchronus_reporting_step_);
   waiting_for_asynchronus_reporting_step_ = true;
 
@@ -906,13 +910,6 @@ void MetricsService::OnMemoryDetailCollectionDone() {
   // step.
   DCHECK(waiting_for_asynchronus_reporting_step_);
 
-  // Right before the UMA transmission gets started, there's one more thing we'd
-  // like to record: the histogram of memory usage, so we spawn a task to
-  // collect the memory details and when that task is finished, it will call
-  // OnMemoryDetailCollectionDone, which will call HistogramSynchronization to
-  // collect histograms from all renderers and then we will call
-  // OnHistogramSynchronizationDone to continue processing.
-
   // Create a callback_task for OnHistogramSynchronizationDone.
   base::Closure callback = base::Bind(
       &MetricsService::OnHistogramSynchronizationDone,
@@ -930,7 +927,15 @@ void MetricsService::OnMemoryDetailCollectionDone() {
 
 void MetricsService::OnHistogramSynchronizationDone() {
   DCHECK(IsSingleThreaded());
+  // This function should only be called as the callback from an ansynchronous
+  // step.
+  DCHECK(waiting_for_asynchronus_reporting_step_);
 
+  waiting_for_asynchronus_reporting_step_ = false;
+  OnFinalLogInfoCollectionDone();
+}
+
+void MetricsService::OnFinalLogInfoCollectionDone() {
   // If somehow there is a fetch in progress, we return and hope things work
   // out. The scheduler isn't informed since if this happens, the scheduler
   // will get a response from the upload.
@@ -938,11 +943,6 @@ void MetricsService::OnHistogramSynchronizationDone() {
   if (current_fetch_.get())
     return;
 
-  // This function should only be called as the callback from an ansynchronous
-  // step.
-  DCHECK(waiting_for_asynchronus_reporting_step_);
-  waiting_for_asynchronus_reporting_step_ = false;
-
   // If we're getting no notifications, then the log won't have much in it, and
   // it's possible the computer is about to go to sleep, so don't upload and
   // stop the scheduler.
@@ -963,26 +963,9 @@ void MetricsService::OnHistogramSynchronizationDone() {
     return;
   }
 
-  PrepareFetchWithStagedLog();
-
-  if (!current_fetch_.get()) {
-    // Compression failed, and log discarded :-/.
-    log_manager_.DiscardStagedLog();
-    scheduler_->UploadCancelled();
-    // TODO(jar): If compression failed, we should have created a tiny log and
-    // compressed that, so that we can signal that we're losing logs.
-    return;
-  }
-
-  DCHECK(!waiting_for_asynchronus_reporting_step_);
-
-  waiting_for_asynchronus_reporting_step_ = true;
-  current_fetch_->Start();
-
-  HandleIdleSinceLastTransmission(true);
+  SendStagedLog();
 }
 
-
 void MetricsService::MakeStagedLog() {
   if (log_manager_.has_staged_log())
     return;
@@ -1005,7 +988,7 @@ void MetricsService::MakeStagedLog() {
 
     case SENDING_OLD_LOGS:
       if (log_manager_.has_unsent_logs()) {
-        log_manager_.StageNextStoredLogForUpload();
+        log_manager_.StageNextLogForUpload();
         break;
       }
       state_ = SENDING_CURRENT_LOGS;
@@ -1014,6 +997,7 @@ void MetricsService::MakeStagedLog() {
     case SENDING_CURRENT_LOGS:
       StopRecording();
       StartRecording();
+      log_manager_.StageNextLogForUpload();
       break;
 
     default:
@@ -1034,12 +1018,13 @@ void MetricsService::PrepareInitialLog() {
   // Histograms only get written to the current log, so make the new log current
   // before writing them.
   log_manager_.PauseCurrentLog();
-  log_manager_.BeginLoggingWithLog(log);
+  log_manager_.BeginLoggingWithLog(log, MetricsLogManager::INITIAL_LOG);
   RecordCurrentHistograms();
+  log_manager_.FinishCurrentLog();
+  log_manager_.ResumePausedLog();
 
   DCHECK(!log_manager_.has_staged_log());
-  log_manager_.StageCurrentLogForUpload();
-  log_manager_.ResumePausedLog();
+  log_manager_.StageNextLogForUpload();
 }
 
 void MetricsService::StoreUnsentLogs() {
@@ -1049,6 +1034,28 @@ void MetricsService::StoreUnsentLogs() {
   log_manager_.PersistUnsentLogs();
 }
 
+void MetricsService::SendStagedLog() {
+  DCHECK(log_manager_.has_staged_log());
+
+  PrepareFetchWithStagedLog();
+
+  if (!current_fetch_.get()) {
+    // Compression failed, and log discarded :-/.
+    log_manager_.DiscardStagedLog();
+    scheduler_->UploadCancelled();
+    // TODO(jar): If compression failed, we should have created a tiny log and
+    // compressed that, so that we can signal that we're losing logs.
+    return;
+  }
+
+  DCHECK(!waiting_for_asynchronus_reporting_step_);
+
+  waiting_for_asynchronus_reporting_step_ = true;
+  current_fetch_->Start();
+
+  HandleIdleSinceLastTransmission(true);
+}
+
 void MetricsService::PrepareFetchWithStagedLog() {
   DCHECK(!log_manager_.staged_log_text().empty());
   DCHECK(!current_fetch_.get());