Platform-specific prune state storage.

chrome/browser/safe_browsing/incident_reporting/platform_state_store.h

Index: chrome/browser/safe_browsing/incident_reporting/platform_state_store.h
diff --git a/chrome/browser/safe_browsing/incident_reporting/platform_state_store.h b/chrome/browser/safe_browsing/incident_reporting/platform_state_store.h
new file mode 100644
index 0000000000000000000000000000000000000000..1662ba815e1fc784f3452aef79ed2be9301c6c22
--- /dev/null
+++ b/chrome/browser/safe_browsing/incident_reporting/platform_state_store.h
@@ -0,0 +1,85 @@
+// Copyright 2015 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.
+
+// An interface to platform-specific storage of IncidentReportingService prune
+// state.
+
+#ifndef CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_PLATFORM_STATE_STORE_H_
+#define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_PLATFORM_STATE_STORE_H_
+
+#include <stdint.h>
+#include <string>
+
+#include "base/memory/scoped_ptr.h"
+#include "build/build_config.h"
+
+// Certain platforms provide their own storage of protobuf-serialized prune
+// state. On platforms where it is not supported, Load() and Store() are noops.
+#if defined(OS_WIN)
+// Store the state in the registry on Windows.
+#define USE_PLATFORM_STATE_STORE
+#endif
+
+class Profile;
+
+namespace base {
+class DictionaryValue;
+}
+
+namespace safe_browsing {
+namespace platform_state_store {
+
+// Loads the platform-specific storage for |profile|. Returns null if there is
+// no such storage for the current platform or in case of error; otherwise, a
+// (possibly empty) dictionary.
+scoped_ptr<base::DictionaryValue> Load(Profile* profile);
+
+// Stores the state for |profile| in |incidents_sent| into platform-specific
+// storage if there is such for the current platform.
+void Store(Profile* profile, const base::DictionaryValue* incidents_sent);
+
+#if defined(USE_PLATFORM_STATE_STORE)

[robertshield, 2015/09/01 14:20:12]

I thought it more conventional to provide empty stub implementations for methods
on platforms that don't implement them, rather than preprocess away the
declarations. Has the zeitgeist shifted?

[grt (UTC plus 2), 2015/09/03 18:12:05]

Load/Store above are the public functions, and exist (and empty) for platforms
that don't use a platform-specific state store. The other declarations in this
file are exposed only for the sake of tests (as mentioned in the comment on line
44).

+
+// All declarations and definitions from this point forward are for use by
+// implementations in platform-specific source files, or are exposed for the
+// sake of testing.

[robertshield, 2015/09/01 14:20:12]

could these be placed in an internal namespace? If I missed this comment, I
would think that they're part of the public interface.

[grt (UTC plus 2), 2015/09/03 18:12:05]

I have been told explicitly not to do that:
https://codereview.chromium.org/729273002/diff/180001/chrome/browser/google/g...

[robertshield, 2015/09/05 03:23:24]

Hrm, ok. Not sure I agree with the premise of that thread that clearly
separating out things that aren't intended for other code to use is
over-designing, but the difference here is too small to be worth arguing about.

They could be moved to a platform_state_store_internal.h which avoids the
"namespace" objection, but would probably still get labeled as over-designing.
Let's leave it as it is I suppose, the chances of this ever being a real problem
are low.

[Peter Kasting, 2015/09/05 03:31:55]

I don't have any problem with that solution.

+
+// The result of loading platform-specific state. This is a histogram type; do
+// not reorder.
+enum class PlatformStateStoreLoadResult : int32_t {
+  SUCCESS = 0,
+  CLEARED_DATA = 1,
+  CLEARED_NO_DATA = 2,
+  DATA_CLEAR_FAILED = 3,
+  OPEN_FAILED = 4,
+  READ_FAILED = 5,
+  PARSE_ERROR = 6,
+  NUM_RESULTS
+};
+
+// A platform-specific function to read store data for |profile| into |data|.
+// Returns SUCCESS if |data| was populated, or a load result value indicating
+// why no data was read.
+PlatformStateStoreLoadResult ReadStoreData(Profile* profile, std::string* data);
+
+// A platform-specific function to write store data for |profile| from |data|.
+void WriteStoreData(Profile* profile, const std::string& data);
+
+// Serializes the |incidents_sent| preference into |data|, replacing its
+// contents. Exposed for testing.
+void SerializeIncidentsSent(const base::DictionaryValue* incidents_sent,
+                            std::string* data);
+
+// Deserializes |data| into |value_dict|. Returns SUCCESS if |data| is empty or
+// fully processed. Exposed for testing.
+PlatformStateStoreLoadResult DeserializeIncidentsSent(
+    const std::string& data,
+    base::DictionaryValue* value_dict);
+
+#endif  // USE_PLATFORM_STATE_STORE
+
+}  // namespace platform_state_store
+}  // namespace safe_browsing
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_PLATFORM_STATE_STORE_H_