Add stub implementations for state keys and VPD

chromeos/dbus/session_manager_client.cc

Index: chromeos/dbus/session_manager_client.cc
diff --git a/chromeos/dbus/session_manager_client.cc b/chromeos/dbus/session_manager_client.cc
index 9e87be84e0a371296c29620fa4930b9e484d6049..07a8a5ef9fd819b0d137bb2d97c8111f6de11b29 100644
--- a/chromeos/dbus/session_manager_client.cc
+++ b/chromeos/dbus/session_manager_client.cc
@@ -42,6 +42,7 @@ constexpr char kArcLowDiskError[] =
 
 constexpr char kStubPolicyFile[] = "stub_policy";
 constexpr char kStubDevicePolicyFile[] = "stub_device_policy";
+constexpr char kStubStateKeysFile[] = "stub_state_keys";
 
 // Returns a location for |file| that is specific to the given |cryptohome_id|.
 // These paths will be relative to DIR_USER_POLICY_KEYS, and can be used only
@@ -74,6 +75,38 @@ void StoreFile(const base::FilePath& path, const std::string& data) {
   }
 }
 
+// Helper to asynchronously read (or if missing create) state key stubs.
+std::vector<std::string> ReadCreateStateKeys(const base::FilePath& path) {

[Daniel Erat, 2017/03/23 23:35:23]

please update this function's name to make it clear that it's only used by the
stub implementation.

[Thiemo Nagel, 2017/04/11 15:29:32]

Done.

(I really dislike the practice of keeping multiple classes in a single .cc file
which gives rise to such ambiguities.)

+  std::string contents;
+  if (base::PathExists(path)) {
+    contents = GetFileContent(path);
+  } else {
+    // Create stub state keys on the fly.
+    for (int i = 0; i < 5; ++i) {
+      contents += crypto::SHA256HashString(
+          base::IntToString(i) +
+          base::Int64ToString(base::Time::Now().ToJavaTime()));
+    }
+    StoreFile(path, contents);
+  }
+
+  std::vector<std::string> state_keys;
+  for (size_t i = 0; i < contents.size() / 32; ++i) {
+    state_keys.push_back(contents.substr(i * 32, 32));
+  }
+  return state_keys;
+}
+
+std::vector<std::string> g_state_keys;

[Daniel Erat, 2017/03/23 23:35:23]

i don't think that non-POD static variables are allowed:
https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables

[Thiemo Nagel, 2017/04/11 15:29:32]

Sorry for that momentary lapse of reason.

+
+// This wrapper is required because StateKeysCallback takes a reference. This is
+// thread-safe as long as all callers are on the same thread.

[Daniel Erat, 2017/03/23 23:35:23]

can you just fix the callback to not take a reference? alternately, can you
store the keys inside of the stub client instead of in a global?

[Thiemo Nagel, 2017/04/11 15:29:32]

It's not even necessary to store them anywhere.  As the callback runs
synchronously, passing the reference of a stack variable is fine.  I just had
overlooked that in the first version of the CL.  :\

+void RunStateKeysCallback(SessionManagerClient::StateKeysCallback callback,
+                          std::vector<std::string> state_keys) {
+  g_state_keys = state_keys;
+  callback.Run(g_state_keys);
+}
+
 }  // namespace
 
 // The SessionManagerClient implementation used in production.
@@ -991,12 +1024,18 @@ class SessionManagerClientStubImpl : public SessionManagerClient {
                        const std::vector<std::string>& flags) override {}
 
   void GetServerBackedStateKeys(const StateKeysCallback& callback) override {
-    std::vector<std::string> state_keys;
-    for (int i = 0; i < 5; ++i)
-      state_keys.push_back(crypto::SHA256HashString(base::IntToString(i)));
-
-    if (!callback.is_null())
-      callback.Run(state_keys);
+    base::FilePath owner_key_path;
+    CHECK(PathService::Get(chromeos::FILE_OWNER_KEY, &owner_key_path));
+    const base::FilePath state_keys_path =
+        owner_key_path.DirName().AppendASCII(kStubStateKeysFile);
+    base::PostTaskWithTraitsAndReplyWithResult(
+        FROM_HERE,
+        base::TaskTraits()
+            .WithShutdownBehavior(
+                base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
+            .MayBlock(),
+        base::Bind(&ReadCreateStateKeys, state_keys_path),
+        base::Bind(&RunStateKeysCallback, callback));
   }
 
   void CheckArcAvailability(const ArcCallback& callback) override {