Split a MetricsLogStore object out of MetricsLogManager.

components/metrics/log_store.h

Index: components/metrics/log_store.h
diff --git a/components/metrics/log_store.h b/components/metrics/log_store.h
new file mode 100644
index 0000000000000000000000000000000000000000..9dd1a62c605ac6129777b0f4efe8c227523d67ad
--- /dev/null
+++ b/components/metrics/log_store.h
@@ -0,0 +1,49 @@
+// Copyright 2017 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.
+
+#ifndef COMPONENTS_METRICS_LOG_STORE_H_
+#define COMPONENTS_METRICS_LOG_STORE_H_
+
+#include <string>
+
+namespace metrics {
+
+// Interface for local storage of serialized logs to be reported.
+// It allows consumers to check if there are logs to consume, consume them one
+// at a time by staging and discarding logs, and persist/load the whole set.
+class LogStore {
+ public:
+  // Returns true if there are any logs waiting to be uploaded.
+  virtual bool has_unsent_logs() const = 0;

[bcwhite, 2017/02/22 16:25:08]

Is "hacker style" still appropriate for these methods?

[Steven Holte, 2017/02/22 20:35:50]

I thinks so.  These should still accessors, and the style guide says "Accessors
may be named like variables"
https://google.github.io/styleguide/cppguide.html#Function_Names

There is also
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...
but I'm pretty sure that means virtual functions shouldn't be declared inline,
not that they shouldn't be named with unix_hacker_style.

[Alexei Svitkine (slow), 2017/02/22 20:43:46]

My understanding is that virtual functions should be CamelCase because there's
no guarantee what the implementation class does with them.

Alternatively, you could make these not pure virtual and have concrete
implementations for them and have the relevant state be stored in the base
class.

[Steven Holte, 2017/02/22 22:15:53]

I have to disagree here.  A function being a accessor or not is an interface
detail, not an implementation detail.  It implies that it doesn't modify the
state of the object.  If the implementation didn't do that, it would be
violating the expectations of interface, in the same way that it would if
calling DiscardStagedLog did something completely different.

+
+  // Returns true if there is a log that needs to be, or is being, uploaded.
+  virtual bool has_staged_log() const = 0;
+
+  // The text of the staged log, as a serialized protobuf.
+  // Will trigger a DCHECK if there is no staged log.
+  virtual const std::string& staged_log() const = 0;
+
+  // The SHA1 hash of the staged log.
+  // Will trigger a DCHECK if there is no staged log.
+  virtual const std::string& staged_log_hash() const = 0;
+
+  // Populates staged_log() with the next stored log to send.
+  // The order in which logs are staged is up to the implementor.
+  // The staged_log must remain the same even if additional logs are added.
+  // Should only be called if has_unsent_logs() is true.
+  virtual void StageNextLog() = 0;
+
+  // Discards the staged log.
+  virtual void DiscardStagedLog() = 0;
+
+  // Saves any unsent logs to persistent storage.
+  virtual void PersistUnsentLogs() const = 0;
+
+  // Loads unsent logs from persistent storage.
+  virtual void LoadPersistedUnsentLogs() = 0;
+};
+
+}  // namespace metrics
+
+#endif  // COMPONENTS_METRICS_LOG_STORE_H_