Begin to separate the MetricsService logic for creating vs uploading logs

chrome_frame/metrics_service.cc

 // Copyright (c) 2012 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 created at ChromeFrame startup in
@@ skipping 348 matching lines @@
 }
 
 //------------------------------------------------------------------------------
 // Recording control methods
 
 void MetricsService::StartRecording() {
   DCHECK_EQ(thread_, base::PlatformThread::CurrentId());
   if (log_manager_.current_log())
     return;
 
+  MetricsLogManager::LogType log_type = (state_ == INITIALIZED) ?
+      MetricsLogManager::INITIAL_LOG : MetricsLogManager::ONGOING_LOG;
   log_manager_.BeginLoggingWithLog(new MetricsLogBase(client_id_, session_id_,
-                                                      GetVersionString()));
+                                                      GetVersionString()),
+                                   log_type);
   if (state_ == INITIALIZED)
     state_ = ACTIVE;
 }
 
 void MetricsService::StopRecording(bool save_log) {
   DCHECK_EQ(thread_, base::PlatformThread::CurrentId());
   if (!log_manager_.current_log())
     return;
 
   // Put incremental histogram deltas at the end of all log transmissions.
   // Don't bother if we're going to discard current_log.
   if (save_log) {
     CrashMetricsReporter::GetInstance()->RecordCrashMetrics();
     RecordCurrentHistograms();
   }
 
-  if (save_log)
-    log_manager_.StageCurrentLogForUpload();
-  else
+  if (save_log) {
+    log_manager_.FinishCurrentLog();
+    log_manager_.StageNextLogForUpload();
+  } else {
     log_manager_.DiscardCurrentLog();
+  }
 }
 
 void MetricsService::MakePendingLog() {
   DCHECK_EQ(thread_, base::PlatformThread::CurrentId());
   if (log_manager_.has_staged_log())
     return;
 
   switch (state_) {
     case INITIALIZED:  // We should be further along by now.
       DCHECK(false);
       return;
 
     case ACTIVE:
       StopRecording(true);
       StartRecording();
       break;
 
     default:
       DCHECK(false);
       return;
   }
-
-  DCHECK(log_manager_.has_staged_log());
 }
 
 bool MetricsService::TransmissionPermitted() const {
   // If the user forbids uploading that's their business, and we don't upload
   // anything.
   return user_permits_upload_;
 }
 
 bool MetricsService::UploadData() {
   DCHECK_EQ(thread_, base::PlatformThread::CurrentId());
 
   if (!GetInstance()->TransmissionPermitted())
     return false;
 
   static long currently_uploading = 0;
   if (InterlockedCompareExchange(&currently_uploading, 1, 0)) {
     DVLOG(1) << "Contention for uploading metrics data. Backing off";
     return false;
   }
 
   MakePendingLog();
-  DCHECK(log_manager_.has_staged_log());
 
   bool ret = true;
 
-  if (log_manager_.staged_log_text().empty()) {
-    NOTREACHED() << "Failed to compress log for transmission.";
-    ret = false;
-  } else {
+  if (log_manager_.has_staged_log()) {
     HRESULT hr = ChromeFrameMetricsDataUploader::UploadDataHelper(
         log_manager_.staged_log_text());
     DCHECK(SUCCEEDED(hr));
+    log_manager_.DiscardStagedLog();
+  } else {
+    NOTREACHED() << "Failed to prepare log for transmission.";

[Ilya Sherman, 2012/02/29 01:25:41]

nit: There was a thread recently about not printing easily inferable log lines
corresponding to DCHECK() and NOTREACHED(), for the sake of reducing the binary
size.  Could this text be moved into a comment instead?

[stuartmorgan, 2012/02/29 13:26:15]

Yep, should have noticed that while I was moving the code. Done.

+    ret = false;
   }
-  log_manager_.DiscardStagedLog();
 
   currently_uploading = 0;
   return ret;
 }
 
 // static
 std::string MetricsService::GetVersionString() {
   chrome::VersionInfo version_info;
   if (version_info.is_valid()) {
     std::string version = version_info.Version();
     // Add the -F extensions to ensure that UMA data uploaded by ChromeFrame
     // lands in the ChromeFrame bucket.
     version += "-F";
     if (!version_info.IsOfficialBuild())
       version.append("-devel");
     return version;
   } else {
     NOTREACHED() << "Unable to retrieve version string.";
   }
 
   return std::string();
 }