Allow in-memory operation of crOS Account Manager

For some browser tests, it is useful to create an in-memory instance of
Account Manager.
Allow this by checking the provided cryptohome root directory during
initialization. If it is an empty path, then Account Manager should not
perform any disk I/O.

Bug: 1068240
Change-Id: I1e0278bf65631e01f246ea85074af4e2ca5e4807
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2254780Reviewed-by: Anastasiia N <anastasiian@chromium.org>
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781331}

chromeos/components/account_manager/account_manager.cc

@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/important_file_writer.h"
 #include "base/location.h"
@@ -152,7 +153,7 @@ AccountManager::Observer::Observer() = default;
 
 AccountManager::Observer::~Observer() = default;
 
-AccountManager::AccountManager() {}
+AccountManager::AccountManager() = default;
 
 // static
 void AccountManager::RegisterPrefs(PrefRegistrySimple* registry) {
@@ -201,22 +202,28 @@ void AccountManager::Initialize(
     // conditions, check whether the |home_dir| parameter provided by the first
     // invocation of |Initialize| matches the one it is currently being called
     // with.
-    DCHECK_EQ(home_dir, writer_->path().DirName());
+    DCHECK_EQ(home_dir, home_dir_);
     std::move(initialization_callback).Run();
     return;
   }
 
+  home_dir_ = home_dir;
   init_state_ = InitializationState::kInProgress;
   url_loader_factory_ = url_loader_factory;
   delay_network_call_runner_ = std::move(delay_network_call_runner);
   task_runner_ = task_runner;
-  writer_ = std::make_unique<base::ImportantFileWriter>(
-      home_dir.Append(kTokensFileName), task_runner_);
+
+  base::FilePath tokens_file_path;
+  if (!home_dir_.empty()) {
+    tokens_file_path = home_dir_.Append(kTokensFileName);
+    writer_ = std::make_unique<base::ImportantFileWriter>(tokens_file_path,
+                                                          task_runner_);
+  }
   initialization_callbacks_.emplace_back(std::move(initialization_callback));
 
   PostTaskAndReplyWithResult(
       task_runner_.get(), FROM_HERE,
-      base::BindOnce(&AccountManager::LoadAccountsFromDisk, writer_->path()),
+      base::BindOnce(&AccountManager::LoadAccountsFromDisk, tokens_file_path),
       base::BindOnce(
           &AccountManager::InsertAccountsAndRunInitializationCallbacks,
           weak_factory_.GetWeakPtr(), initialization_start_time));
@@ -228,6 +235,12 @@ AccountManager::AccountMap AccountManager::LoadAccountsFromDisk(
   AccountManager::AccountMap accounts;
 
   VLOG(1) << "AccountManager::LoadTokensFromDisk";
+
+  if (tokens_file_path.empty()) {
+    RecordTokenLoadStatus(TokenLoadStatus::kSuccess);
+    return accounts;
+  }
+
   std::string token_file_data;
   bool success = base::ReadFileToStringWithMaxSize(
       tokens_file_path, &token_file_data, kTokensFileMaxSizeInBytes);
@@ -293,9 +306,8 @@ void AccountManager::InsertAccountsAndRunInitializationCallbacks(
   RecordNumAccountsMetric(accounts_.size());
 }
 
-AccountManager::~AccountManager() {
-  // AccountManager is supposed to be used as a leaky global.
-}
+// AccountManager is supposed to be used as a leaky global.
+AccountManager::~AccountManager() = default;
 
 bool AccountManager::IsInitialized() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -507,6 +519,10 @@ void AccountManager::UpsertAccountInternal(const AccountKey& account_key,
 }
 
 void AccountManager::PersistAccountsAsync() {
+  if (!writer_) {
+    return;
+  }
+
   // Schedule (immediately) a non-blocking write.
   writer_->WriteNow(std::make_unique<std::string>(GetSerializedAccounts()));
 }

chromeos/components/account_manager/account_manager.h

@@ -126,7 +126,8 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER) AccountManager {
   void SetPrefService(PrefService* pref_service);
 
   // |home_dir| is the path of the Device Account's home directory (root of the
-  // user's cryptohome).
+  // user's cryptohome). If |home_dir| is |base::FilePath::empty()|, then |this|
+  // |AccountManager| does not persist any data to disk.
   // |request_context| is a non-owning pointer.
   // |delay_network_call_runner| is basically a wrapper for
   // |chromeos::DelayNetworkCall|. Cannot use |chromeos::DelayNetworkCall| due
@@ -371,8 +372,14 @@ class COMPONENT_EXPORT(ACCOUNT_MANAGER) AccountManager {
 
   // A task runner for disk I/O.
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
+  // Writes |AccountManager|'s state to disk. Will be |nullptr| if
+  // |AccountManager| is operating in-memory only.
   std::unique_ptr<base::ImportantFileWriter> writer_;
 
+  // Cryptohome root.
+  base::FilePath home_dir_;
+
   // A map from |AccountKey|s to |AccountInfo|.
   AccountMap accounts_;
 

chromeos/components/account_manager/account_manager_unittest.cc

@@ -11,6 +11,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
@@ -61,8 +62,8 @@ class AccountManagerSpy : public AccountManager {
 
 class AccountManagerTest : public testing::Test {
  public:
-  AccountManagerTest() {}
-  ~AccountManagerTest() override {}
+  AccountManagerTest() = default;
+  ~AccountManagerTest() override = default;
 
  protected:
   void SetUp() override {
@@ -73,10 +74,16 @@ class AccountManagerTest : public testing::Test {
 
   // Gets the list of accounts stored in |account_manager_|.
   std::vector<AccountManager::Account> GetAccountsBlocking() {
+    return GetAccountsBlocking(account_manager_.get());
+  }
+
+  // Gets the list of accounts stored in |account_manager|.
+  std::vector<AccountManager::Account> GetAccountsBlocking(
+      AccountManager* const account_manager) {
     std::vector<AccountManager::Account> accounts;
 
     base::RunLoop run_loop;
-    account_manager_->GetAccounts(base::BindLambdaForTesting(
+    account_manager->GetAccounts(base::BindLambdaForTesting(
         [&accounts, &run_loop](
             const std::vector<AccountManager::Account>& stored_accounts) {
           accounts = stored_accounts;
@@ -113,10 +120,21 @@ class AccountManagerTest : public testing::Test {
   }
 
   // |account_manager| is a non-owning pointer.
+  // |initialization_callback| is called after initialization is complete.
+  void InitializeAccountManager(AccountManager* account_manager,
+                                base::OnceClosure initialization_callback) {
+    InitializeAccountManager(account_manager, tmp_dir_.GetPath(),
+                             std::move(initialization_callback));
+  }
+
+  // |account_manager| is a non-owning pointer.
+  // |home_dir| is the cryptohome root.
+  // |initialization_callback| is called after initialization is complete.
   void InitializeAccountManager(AccountManager* account_manager,
+                                const base::FilePath& home_dir,
                                 base::OnceClosure initialization_callback) {
     account_manager->Initialize(
-        tmp_dir_.GetPath(), test_url_loader_factory_.GetSafeWeakWrapper(),
+        home_dir, test_url_loader_factory_.GetSafeWeakWrapper(),
         immediate_callback_runner_, base::SequencedTaskRunnerHandle::Get(),
         std::move(initialization_callback));
     account_manager->SetPrefService(&pref_service_);
@@ -237,6 +255,7 @@ TEST_F(AccountManagerTest, TestUpsert) {
   EXPECT_EQ(kRawUserEmail, accounts[0].raw_email);
 }
 
+// Test that |AccountManager| saves its tokens to disk.
 TEST_F(AccountManagerTest, TestTokenPersistence) {
   account_manager_->UpsertAccount(kGaiaAccountKey_, kRawUserEmail, kGaiaToken);
   task_environment_.RunUntilIdle();
@@ -250,6 +269,30 @@ TEST_F(AccountManagerTest, TestTokenPersistence) {
   EXPECT_EQ(kGaiaToken, account_manager_->accounts_[kGaiaAccountKey_].token);
 }
 
+// Test that |AccountManager| does not save its tokens to disk if an empty
+// cryptohome root path is provided during initialization.
+TEST_F(AccountManagerTest, TestTokenTransience) {
+  const base::FilePath home_dir;
+
+  {
+    // Create a scoped |AccountManager|.
+    AccountManager account_manager;
+    InitializeAccountManager(&account_manager, home_dir,
+                             /* initialization_callback= */ base::DoNothing());
+    account_manager.UpsertAccount(kGaiaAccountKey_, kRawUserEmail, kGaiaToken);
+    task_environment_.RunUntilIdle();
+  }
+
+  // Create another |AccountManager| at the same path.
+  AccountManager account_manager;
+  InitializeAccountManager(&account_manager, home_dir,
+                           /* initialization_callback= */ base::DoNothing());
+
+  std::vector<AccountManager::Account> accounts =
+      GetAccountsBlocking(&account_manager);
+  EXPECT_EQ(0UL, accounts.size());
+}
+
 TEST_F(AccountManagerTest, TestAccountEmailPersistence) {
   account_manager_->UpsertAccount(kGaiaAccountKey_, kRawUserEmail, kGaiaToken);
   task_environment_.RunUntilIdle();