Begin to separate the MetricsService logic for creating vs uploading logs

chrome/common/metrics/metrics_log_manager.h

Index: chrome/common/metrics/metrics_log_manager.h
diff --git a/chrome/common/metrics/metrics_log_manager.h b/chrome/common/metrics/metrics_log_manager.h
index 3eb04f00d755047be6eff068b3e6685ec612866f..46d92d0a0ee8a383d23dcc9b6eb646385e36709e 100644
--- a/chrome/common/metrics/metrics_log_manager.h
+++ b/chrome/common/metrics/metrics_log_manager.h
@@ -22,18 +22,32 @@ class MetricsLogManager {
   MetricsLogManager();
   ~MetricsLogManager();
 
-  // Takes ownership of |log|, and makes it the current_log.
-  // This should only be called if there is not a current log.
-  void BeginLoggingWithLog(MetricsLogBase* log);
+  enum LogType {
+    INITIAL_LOG,  // The first log of a session.
+    ONGOING_LOG,  // Subsequent logs in a session.
+  };
+
+  // Takes ownership of |log|, which has type |log_type|, and makes it the
+  // current_log. This should only be called if there is not a current log.
+  void BeginLoggingWithLog(MetricsLogBase* log, LogType log_type);
 
   // Returns the in-progress log.
   MetricsLogBase* current_log() { return current_log_.get(); }
 
-  // Closes |current_log| and stages it for upload, leaving |current_log| NULL.
-  void StageCurrentLogForUpload();
+  // Closes |current_log|, compresses it, and stores the compressed log for
+  // later upload as |log_type|, leaving |current_log| NULL.

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

nit: current_log -> current_log_; log_type -> log_type_

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

Changed to current_log() (I prefer to document public methods in terms of public
API rather than private members when it's reasonable to do so).

+  void FinishCurrentLog();
+
+  // Returns true if there are any logs waiting to be uploaded.
+  bool has_unsent_logs() const {
+    return !unsent_initial_logs_.empty() || !unsent_ongoing_logs_.empty();
+  }
+
+  // Populates staged_log_text with the next stored log to send.

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

nit: staged_log_text -> |staged_log_text_|

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

Same.

+  // Should only be called if has_unsent_logs is true.

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

nit: has_unsent_logs -> has_unsent_logs()

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

Done.

+  void StageNextLogForUpload();
 
   // Returns true if there is a log that needs to be, or is being, uploaded.
-  // Note that this returns true even if compressing the log text failed.
   bool has_staged_log() const;
 
   // The compressed text of the staged log. Empty if there is no staged log,
@@ -57,28 +71,14 @@ class MetricsLogManager {
   // This should only be called if there is not a current log.
   void ResumePausedLog();
 
-  // Returns true if there are any logs left over from previous sessions that
-  // need to be uploaded.
-  bool has_unsent_logs() const {
-    return !unsent_initial_logs_.empty() || !unsent_ongoing_logs_.empty();
-  }
-
-  enum LogType {
-    INITIAL_LOG,  // The first log of a session.
-    ONGOING_LOG,  // Subsequent logs in a session.
-  };
-
-  // Saves the staged log as the given type (or discards it in accordance with
+  // Saves the staged log (or discards it in accordance with
   // set_max_ongoing_log_store_size), then clears the staged log.

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

nit: set_max_ongoing_log_store_size -> |max_ongoing_log_store_size_|

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

That part of the comment was actually no longer accurate, so I've removed it.

-  // This can only be called after StageCurrentLogForUpload.
-  void StoreStagedLogAsUnsent(LogType log_type);
-
-  // Populates staged_log_text with the next stored log to send.
-  void StageNextStoredLogForUpload();
+  // This can only be called if has_staged_log() is true.
+  void StoreStagedLogAsUnsent();
 
-  // Sets the threshold for how large an onging log can be and still be stored.
-  // Ongoing logs larger than this will be discarded. 0 is interpreted as no
-  // limit.
+  // Sets the threshold for how large an onging log can be and still be written
+  // to persistant storage. Ongoing logs larger than this will be discarded
+  // before persisting. 0 is interpreted as no limit.
   void set_max_ongoing_log_store_size(size_t max_size) {
     max_ongoing_log_store_size_ = max_size;
   }
@@ -115,30 +115,30 @@ class MetricsLogManager {
   void LoadPersistedUnsentLogs();
 
  private:
-  // Compresses staged_log_ and stores the result in
-  // compressed_staged_log_text_.
-  void CompressStagedLog();
+  // Saves |log_text| as the given type (or discards it in accordance with
+  // set_max_ongoing_log_store_size).
+  void StoreLog(const std::string& log_text, LogType log_type);
+
+  // Compresses current_log_ into compressed_log.
+  void CompressCurrentLog(std::string* compressed_log);
 
   // Compresses the text in |input| using bzip2, store the result in |output|.
   static bool Bzip2Compress(const std::string& input, std::string* output);
 
   // The log that we are still appending to.
   scoped_ptr<MetricsLogBase> current_log_;
+  LogType current_log_type_;
 
   // A paused, previously-current log.
   scoped_ptr<MetricsLogBase> paused_log_;
 
-  // The log that we are currently transmiting, or about to try to transmit.
-  // Note that when using StageNextStoredLogForUpload, this can be NULL while
-  // compressed_staged_log_text_ is non-NULL.
-  scoped_ptr<MetricsLogBase> staged_log_;
-
   // Helper class to handle serialization/deserialization of logs for persistent
   // storage. May be NULL.
   scoped_ptr<LogSerializer> log_serializer_;
 
   // The compressed text of the staged log, ready for upload to the server.
   std::string compressed_staged_log_text_;
+  LogType staged_log_type_;
 
   // Logs from a previous session that have not yet been sent.
   // Note that the vector has the oldest logs listed first (early in the