MetricsService: Send a hash of the UMA log in a header.

chrome/common/metrics/metrics_log_manager.h

Index: chrome/common/metrics/metrics_log_manager.h
===================================================================
--- chrome/common/metrics/metrics_log_manager.h	(revision 228712)
+++ chrome/common/metrics/metrics_log_manager.h	(working copy)
@@ -27,6 +27,34 @@
     NO_LOG,       // Placeholder value for when there is no log.
   };
 
+  class SerializedLog {
+   public:
+    SerializedLog();
+    ~SerializedLog();
+
+    const std::string& log_text() const { return log_text_; }
+    const std::string& log_hash() const { return log_hash_; }
+
+    // Returns true if the log is empty.
+    bool IsEmpty() const;
+
+    // Sets the log text and updates the hash.
+    void SetLogText(const std::string& log_text);

[Ilya Sherman, 2013/10/17 01:29:03]

Can we pass |log_text| as a pointer, and swap() the contents?  The code
previously did this for "efficiency"; it might be that the performance
difference actually matters, since the strings can indeed be pretty large.

[Alexei Svitkine (slow), 2013/10/17 19:47:27]

I added a SwapLogText() function that does that now and changed the production
code to use it. I've kept SetLogText() since a lot of the test code sets the
text via a string literal (which doesn't work well when the function takes a
pointer).

[Ilya Sherman, 2013/10/17 22:06:25]

Thanks.  If SetLogText() is only used in tests, can you rename it to something
like SetLogTextForTesting()?  Even better might be to define a wrapper function
in the test file that looks something like this:

void SetLogText(const std::string& log_text, SerializedLog* log) {
  std::string log_text_copy = log_text;
  log->SwapLogText(&log_text_copy);
}

[Alexei Svitkine (slow), 2013/10/18 18:51:06]

Done.

+
+    // Clears the log.
+    void Clear();
+
+    // Swaps log contents with |other|.
+    void Swap(SerializedLog* other);
+
+   private:
+    // Non-human readable log text (serialized proto).
+    std::string log_text_;
+
+    // Non-human readable SHA1 of |log_text| or empty if |log_text| is empty.
+    std::string log_hash_;
+  };

[Ilya Sherman, 2013/10/17 01:29:03]

nit: Please either add a DISALLOW_COPY_AND_ASSIGN, or add an explicit comment
indicating that it's intentionally omitted.

[Alexei Svitkine (slow), 2013/10/17 19:47:27]

Done.

+
   enum StoreType {
     NORMAL_STORE,       // A standard store operation.
     PROVISIONAL_STORE,  // A store operation that can be easily reverted later.
@@ -57,8 +85,12 @@
 
   // The text of the staged log, as a serialized protobuf. Empty if there is no
   // staged log, or if compression of the staged log failed.
-  const std::string& staged_log_text() const { return staged_log_text_; }
+  const std::string& staged_log_text() const { return staged_log_.log_text(); }
 
+  // The SHA1 hash (non-human readable) of the staged log or empty if there is
+  // no staged log.
+  const std::string& staged_log_hash() const { return staged_log_.log_hash(); }
+
   // Discards the staged log.
   void DiscardStagedLog();
 
@@ -105,13 +137,13 @@
 
     // Serializes |logs| to persistent storage, replacing any previously
     // serialized logs of the same type.
-    virtual void SerializeLogs(const std::vector<std::string>& logs,
+    virtual void SerializeLogs(const std::vector<SerializedLog>& logs,
                                LogType log_type) = 0;
 
     // Populates |logs| with logs of type |log_type| deserialized from
     // persistent storage.
     virtual void DeserializeLogs(LogType log_type,
-                                 std::vector<std::string>* logs) = 0;
+                                 std::vector<SerializedLog>* logs) = 0;
   };
 
   // Sets the serializer to use for persisting and loading logs; takes ownership
@@ -129,16 +161,16 @@
   void LoadPersistedUnsentLogs();
 
  private:
-  // Saves |log_text| as the given type (or discards it in accordance with
+  // Saves |log| as the given type (or discards it in accordance with
   // |max_ongoing_log_store_size_|).
-  // NOTE: This clears the contents of |log_text| (to avoid an expensive
-  // string copy), so the log should be discarded after this call.
-  void StoreLog(std::string* log_text,
+  // NOTE: This clears the contents of |log| (to avoid an expensive copy),
+  // so the log should be discarded after this call.
+  void StoreLog(SerializedLog* log,
                 LogType log_type,
                 StoreType store_type);
 
-  // Compresses current_log_ into compressed_log.
-  void CompressCurrentLog(std::string* compressed_log);
+  // Compresses |current_log_| into |compressed_log|.
+  void CompressCurrentLog(SerializedLog* compressed_log);
 
   // The log that we are still appending to.
   scoped_ptr<MetricsLogBase> current_log_;
@@ -152,15 +184,15 @@
   // storage. May be NULL.
   scoped_ptr<LogSerializer> log_serializer_;
 
-  // The text representations of the staged log, ready for upload to the server.
-  std::string staged_log_text_;
+  // The current staged log, ready for upload to the server.
+  SerializedLog staged_log_;
   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
   // vector), and we'll discard old logs if we have gathered too many logs.
-  std::vector<std::string> unsent_initial_logs_;
-  std::vector<std::string> unsent_ongoing_logs_;
+  std::vector<SerializedLog> unsent_initial_logs_;
+  std::vector<SerializedLog> unsent_ongoing_logs_;
 
   size_t max_ongoing_log_store_size_;