Add stub implementations for state keys and VPD

chromeos/dbus/session_manager_client.cc

 // Copyright (c) 2012 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.
 
 #include "chromeos/dbus/session_manager_client.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <memory>
@@ skipping 24 matching lines @@
 namespace chromeos {
 
 namespace {
 
 // TODO(hidehiko): Share the constant between Chrome and ChromeOS.
 constexpr char kArcLowDiskError[] =
     "org.chromium.SessionManagerInterface.LowFreeDisk";
 
 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
 // to store stub files.
 base::FilePath GetUserFilePath(const cryptohome::Identification& cryptohome_id,
                                const char* file) {
   base::FilePath keys_path;
   if (!PathService::Get(chromeos::DIR_USER_POLICY_KEYS, &keys_path))
     return base::FilePath();
   const std::string sanitized =
@@ skipping 12 matching lines @@
 // Helper to write a file in a background thread.
 void StoreFile(const base::FilePath& path, const std::string& data) {
   const int size = static_cast<int>(data.size());
   if (path.empty() ||
       !base::CreateDirectory(path.DirName()) ||
       base::WriteFile(path, data.data(), size) != size) {
     LOG(WARNING) << "Failed to write to " << path.value();
   }
 }
 
+// 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.
 class SessionManagerClientImpl : public SessionManagerClient {
  public:
   SessionManagerClientImpl()
       : session_manager_proxy_(NULL),
         screen_is_locked_(false),
         weak_ptr_factory_(this) {}
 
@@ skipping 897 matching lines @@
     StorePolicyForUser(cryptohome::Identification::FromString(account_id),
                        policy_blob, callback);
   }
 
   bool SupportsRestartToApplyUserFlags() const override { return false; }
 
   void SetFlagsForUser(const cryptohome::Identification& cryptohome_id,
                        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 {
     callback.Run(false);
   }
 
   void StartArcInstance(const cryptohome::Identification& cryptohome_id,
                         bool disable_boot_completed_broadcast,
                         const StartArcInstanceCallback& callback) override {
     callback.Run(StartArcInstanceResult::UNKNOWN_ERROR);
@@ skipping 43 matching lines @@
 
 SessionManagerClient* SessionManagerClient::Create(
     DBusClientImplementationType type) {
   if (type == REAL_DBUS_CLIENT_IMPLEMENTATION)
     return new SessionManagerClientImpl();
   DCHECK_EQ(FAKE_DBUS_CLIENT_IMPLEMENTATION, type);
   return new SessionManagerClientStubImpl();
 }
 
 }  // namespace chromeos