Fix log type preservation across log pausing

This was causing the first ongoing log to be stored as an initial log instead of an ongoing log (if it was stored instead of being sent).

BUG=None
TEST=None


Review URL: http://codereview.chromium.org/10134051

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@134375 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 42 insertions, 2 deletions

diff --git a/chrome/common/metrics/metrics_log_manager.cc b/chrome/common/metrics/metrics_log_manager.cc
index 078f9b4..2326cc8 100644
--- a/chrome/common/metrics/metrics_log_manager.cc
+++ b/chrome/common/metrics/metrics_log_manager.cc
@@ -22,8 +22,9 @@ const char kDiscardedLog[] = "Log discarded";
 
 }  // anonymous namespace
 
-MetricsLogManager::MetricsLogManager() : current_log_type_(INITIAL_LOG),
-                                         staged_log_type_(INITIAL_LOG),
+MetricsLogManager::MetricsLogManager() : current_log_type_(NO_LOG),
+                                         paused_log_type_(NO_LOG),
+                                         staged_log_type_(NO_LOG),
                                          max_ongoing_log_store_size_(0) {}
 
 MetricsLogManager::~MetricsLogManager() {}
@@ -40,6 +41,7 @@ void MetricsLogManager::SerializedLog::swap(SerializedLog& log) {
 
 void MetricsLogManager::BeginLoggingWithLog(MetricsLogBase* log,
                                             LogType log_type) {
+  DCHECK(log_type != NO_LOG);
   DCHECK(!current_log_.get());
   current_log_.reset(log);
   current_log_type_ = log_type;
@@ -47,12 +49,14 @@ void MetricsLogManager::BeginLoggingWithLog(MetricsLogBase* log,
 
 void MetricsLogManager::FinishCurrentLog() {
   DCHECK(current_log_.get());
+  DCHECK(current_log_type_ != NO_LOG);
   current_log_->CloseLog();
   SerializedLog compressed_log;
   CompressCurrentLog(&compressed_log);
   if (!compressed_log.empty())
     StoreLog(&compressed_log, current_log_type_);
   current_log_.reset();
+  current_log_type_ = NO_LOG;
 }
 
 void MetricsLogManager::StageNextLogForUpload() {
@@ -64,6 +68,7 @@ void MetricsLogManager::StageNextLogForUpload() {
                                                                : INITIAL_LOG;
   DCHECK(!source_list->empty());
   DCHECK(staged_log_text_.empty());
+  DCHECK(staged_log_type_ == NO_LOG);
   staged_log_text_.swap(source_list->back());
   staged_log_type_ = source_type;
   source_list->pop_back();
@@ -80,6 +85,7 @@ bool MetricsLogManager::has_staged_log_proto() const {
 void MetricsLogManager::DiscardStagedLog() {
   staged_log_text_.xml.clear();
   staged_log_text_.proto.clear();
+  staged_log_type_ = NO_LOG;
 }
 
 void MetricsLogManager::DiscardStagedLogProto() {
@@ -89,16 +95,23 @@ void MetricsLogManager::DiscardStagedLogProto() {
 void MetricsLogManager::DiscardCurrentLog() {
   current_log_->CloseLog();
   current_log_.reset();
+  current_log_type_ = NO_LOG;
 }
 
 void MetricsLogManager::PauseCurrentLog() {
   DCHECK(!paused_log_.get());
+  DCHECK(paused_log_type_ == NO_LOG);
   paused_log_.reset(current_log_.release());
+  paused_log_type_ = current_log_type_;
+  current_log_type_ = NO_LOG;
 }
 
 void MetricsLogManager::ResumePausedLog() {
   DCHECK(!current_log_.get());
+  DCHECK(current_log_type_ == NO_LOG);
   current_log_.reset(paused_log_.release());
+  current_log_type_ = paused_log_type_;
+  paused_log_type_ = NO_LOG;
 }
 
 void MetricsLogManager::StoreStagedLogAsUnsent() {
@@ -113,6 +126,7 @@ void MetricsLogManager::StoreStagedLogAsUnsent() {
 }
 
 void MetricsLogManager::StoreLog(SerializedLog* log_text, LogType log_type) {
+  DCHECK(log_type != NO_LOG);
   std::vector<SerializedLog>* destination_list =
       (log_type == INITIAL_LOG) ? &unsent_initial_logs_
                                 : &unsent_ongoing_logs_;
diff --git a/chrome/common/metrics/metrics_log_manager.h b/chrome/common/metrics/metrics_log_manager.h
index d1e245e..426cadb 100644
--- a/chrome/common/metrics/metrics_log_manager.h
+++ b/chrome/common/metrics/metrics_log_manager.h
@@ -37,6 +37,7 @@ class MetricsLogManager {
   enum LogType {
     INITIAL_LOG,  // The first log of a session.
     ONGOING_LOG,  // Subsequent logs in a session.
+    NO_LOG,       // Placeholder value for when there is no log.
   };
 
   // Takes ownership of |log|, which has type |log_type|, and makes it the
@@ -156,6 +157,7 @@ class MetricsLogManager {
 
   // A paused, previously-current log.
   scoped_ptr<MetricsLogBase> paused_log_;
+  LogType paused_log_type_;
 
   // Helper class to handle serialization/deserialization of logs for persistent
   // storage. May be NULL.
diff --git a/chrome/common/metrics/metrics_log_manager_unittest.cc b/chrome/common/metrics/metrics_log_manager_unittest.cc
index d8be967..4369540 100644
--- a/chrome/common/metrics/metrics_log_manager_unittest.cc
+++ b/chrome/common/metrics/metrics_log_manager_unittest.cc
@@ -116,6 +116,30 @@ TEST(MetricsLogManagerTest, InterjectedLog) {
   EXPECT_FALSE(log_manager.has_unsent_logs());
 }
 
+TEST(MetricsLogManagerTest, InterjectedLogPreservesType) {
+  MetricsLogManager log_manager;
+
+  MetricsLogBase* ongoing_log = new MetricsLogBase("id", 0, "version");
+  MetricsLogBase* temp_log = new MetricsLogBase("id", 0, "version");
+
+  log_manager.BeginLoggingWithLog(ongoing_log, MetricsLogManager::ONGOING_LOG);
+  log_manager.PauseCurrentLog();
+  log_manager.BeginLoggingWithLog(temp_log, MetricsLogManager::INITIAL_LOG);
+  log_manager.FinishCurrentLog();
+  log_manager.ResumePausedLog();
+  log_manager.StageNextLogForUpload();
+  log_manager.DiscardStagedLog();
+
+  // Verify that the remaining log (which is the original ongoing log) still
+  // has the right type.
+  DummyLogSerializer* serializer = new DummyLogSerializer;
+  log_manager.set_log_serializer(serializer);
+  log_manager.FinishCurrentLog();
+  log_manager.PersistUnsentLogs();
+  EXPECT_EQ(0U, serializer->TypeCount(MetricsLogManager::INITIAL_LOG));
+  EXPECT_EQ(1U, serializer->TypeCount(MetricsLogManager::ONGOING_LOG));
+}
+
 TEST(MetricsLogManagerTest, StoreAndLoad) {
   std::vector<SerializedLog> initial_logs;
   std::vector<SerializedLog> ongoing_logs;