crOS Account Manager: Add a util for running migrations

A series of async steps need to be performed for migrating existing
users to Chrome OS Account Manager. Please see the attached bug for a
Design Document.

Add a utility for sequentially running a series of async migration
steps, which are run only if the previous step was completed
successfully.

This utility allows us to specify the migration steps in a declarative
style with clear dependencies instead of creating a complex web of
callbacks.

Bug: 904128
Test: unit_tests --gtest_filter="*MigrationRunnerTest*"
Change-Id: I5bd5ddc1fb59060577e535fb115bd4689adfaaa7
Reviewed-on: https://chromium-review.googlesource.com/c/1329964Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612179}

chrome/browser/chromeos/BUILD.gn

@@ -294,6 +294,8 @@ source_set("chromeos") {
     "accessibility/switch_access_event_handler.h",
     "accessibility/switch_access_panel.cc",
     "accessibility/switch_access_panel.h",
+    "account_manager/account_migration_runner.cc",
+    "account_manager/account_migration_runner.h",
     "account_mapper_util.cc",
     "account_mapper_util.h",
     "app_mode/app_launch_utils.cc",
@@ -2067,6 +2069,7 @@ source_set("unit_tests") {
     "../resources/chromeos/zip_archiver/test/char_coding_test.cc",
     "../ui/browser_finder_chromeos_unittest.cc",
     "accessibility/ax_host_service_unittest.cc",
+    "account_manager/account_migration_runner_unittest.cc",
     "app_mode/startup_app_launcher_unittest.cc",
     "apps/intent_helper/apps_navigation_throttle_unittest.cc",
     "apps/intent_helper/page_transition_util_unittest.cc",

chrome/browser/chromeos/account_manager/account_migration_runner.cc

+// Copyright 2018 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 "chrome/browser/chromeos/account_manager/account_migration_runner.h"
+
+#include <string>
+#include <utility>
+
+#include "base/bind.h"
+
+namespace chromeos {
+
+AccountMigrationRunner::Step::Step(const std::string& id) : id_(id) {}
+
+AccountMigrationRunner::Step::~Step() = default;
+
+const std::string& AccountMigrationRunner::Step::GetId() const {
+  return id_;
+}
+
+AccountMigrationRunner::AccountMigrationRunner() : weak_factory_(this) {}
+
+AccountMigrationRunner::~AccountMigrationRunner() = default;
+
+AccountMigrationRunner::Status AccountMigrationRunner::GetStatus() const {
+  return status_;
+}
+
+void AccountMigrationRunner::AddStep(std::unique_ptr<Step> step) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  // |Step|s cannot be added after migration has begun.
+  DCHECK_EQ(Status::kNotStarted, status_);
+
+  steps_.emplace(std::move(step));
+}
+
+void AccountMigrationRunner::Run(OnMigrationDone callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  if (status_ != Status::kNotStarted) {
+    // Ignore calls to |Run| after migration has begun.
+    return;
+  }
+  status_ = Status::kRunning;
+  callback_ = std::move(callback);
+
+  RunNextStep();
+}
+
+void AccountMigrationRunner::RunNextStep() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  if (steps_.empty()) {
+    FinishWithSuccess();
+    return;
+  }
+
+  current_step_ = std::move(steps_.front());
+  steps_.pop();
+
+  current_step_->Run(base::BindOnce(&AccountMigrationRunner::OnStepCompleted,
+                                    weak_factory_.GetWeakPtr()));
+}
+
+void AccountMigrationRunner::OnStepCompleted(bool result) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  if (result) {
+    RunNextStep();
+  } else {
+    FinishWithFailure();
+  }
+}
+
+void AccountMigrationRunner::FinishWithSuccess() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  status_ = Status::kSuccess;
+  DCHECK(callback_);
+  std::move(callback_).Run(MigrationResult{Status::kSuccess /* final_status */,
+                                           std::string() /* failed_step */});
+}
+
+void AccountMigrationRunner::FinishWithFailure() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  status_ = Status::kFailure;
+  DCHECK(callback_);
+  std::move(callback_).Run(
+      MigrationResult{Status::kFailure /* final_status */,
+                      current_step_->GetId() /* failed_step */});
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/account_manager/account_migration_runner.h

+// Copyright 2018 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 CHROME_BROWSER_CHROMEOS_ACCOUNT_MANAGER_ACCOUNT_MIGRATION_RUNNER_H_
+#define CHROME_BROWSER_CHROMEOS_ACCOUNT_MANAGER_ACCOUNT_MIGRATION_RUNNER_H_
+
+#include <memory>
+#include <queue>
+#include <string>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/sequence_checker.h"
+
+namespace chromeos {
+
+// A utility class to run account migrations for |chromeos::AccountManager|. It
+// enables the specification of a series of async migration |Step|s in a
+// declarative style (see the test cases for usage examples). If any of these
+// |Step|s fail, the entire migration fails, without running subsequent steps.
+class AccountMigrationRunner {
+ public:
+  enum class Status {
+    // Migration has not started yet.
+    kNotStarted,
+    // Migration is in progress.
+    kRunning,
+    // All migration steps completed successfully.
+    kSuccess,
+    // Migration failed at some |Step|.
+    kFailure,
+  };
+
+  struct MigrationResult {
+    // Final status of migration.
+    Status final_status;
+    // If |final_status| is |FAILURE|, this field will contain the id of the
+    // step, that caused migration to fail.
+    std::string failed_step_id;
+  };
+
+  // Type alias for the callback called at the end of a migration run.
+  using OnMigrationDone = base::OnceCallback<void(const MigrationResult&)>;
+
+  // Abstract base class for a migration step.
+  class Step {
+   public:
+    // |id| is an identifier for this step. This should be unique across |Step|s
+    // added to |AccountMigrationRunner::AddStep| but maintaining this
+    // uniqueness is a responsibility of the callers and not enforced by
+    // |AccountMigrationRunner|.
+    explicit Step(const std::string& id);
+
+    virtual ~Step();
+
+    // Runs the migration step and calls |callback| with the result (|true| for
+    // success and |false| for failure) of the migration step.
+    // Note that |AccountMigrationRunner| does not guarantee anything about the
+    // lifetime of a |Step| once it has been added via
+    // |AccountMigrationRunner::AddStep|.
+    virtual void Run(base::OnceCallback<void(bool)> callback) = 0;
+
+    // Gets this |Step|'s identifier.
+    const std::string& GetId() const;
+
+   private:
+    const std::string id_;
+    DISALLOW_COPY_AND_ASSIGN(Step);
+  };
+
+  AccountMigrationRunner();
+  ~AccountMigrationRunner();
+
+  // Gets the current status of migration.
+  Status GetStatus() const;
+
+  // Adds a migration step. |AddStep| must be called before |Run| has been
+  // called. Calls to |AddStep| create a dependency between the supplied
+  // |step|s, i.e. |step|s will be executed in the order in which they were
+  // supplied via |AddStep|. If any |step| fails, none of the following steps
+  // will be executed.
+  void AddStep(std::unique_ptr<Step> step);
+
+  // Runs all the migration steps that have previously been added via |AddStep|.
+  // |Run| must be called at most once during the lifetime of |this| object.
+  // Subsequent calls to |Run| are silently ignored.
+  // |callback| is called with the final result of the migration run.
+  void Run(OnMigrationDone callback);
+
+ private:
+  // Runs the next migration step in |steps_|.
+  void RunNextStep();
+
+  // Callback from a migration |Step|.
+  void OnStepCompleted(bool result);
+
+  // Immediately finishes migration with a |Status::SUCCESS| and informs the
+  // caller of |Run| about the result of the migration.
+  void FinishWithSuccess();
+
+  // Immediately finishes migration with a |Status::FAILURE| and informs the
+  // caller of |Run| about the result of the migration.
+  void FinishWithFailure();
+
+  // Current status of migration.
+  Status status_ = Status::kNotStarted;
+
+  // A list of migration steps.
+  std::queue<std::unique_ptr<Step>> steps_;
+
+  // The current step being executed.
+  std::unique_ptr<Step> current_step_ = nullptr;
+
+  // Supplied by the caller of |Run| to get the overall result of migration.
+  OnMigrationDone callback_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+
+  base::WeakPtrFactory<AccountMigrationRunner> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(AccountMigrationRunner);
+};
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_ACCOUNT_MANAGER_ACCOUNT_MIGRATION_RUNNER_H_

chrome/browser/chromeos/account_manager/account_migration_runner_unittest.cc

+// Copyright 2018 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 "chrome/browser/chromeos/account_manager/account_migration_runner.h"
+
+#include <memory>
+#include <string>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/run_loop.h"
+#include "base/test/scoped_task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromeos {
+
+namespace {
+
+class AlwaysSucceeds : public AccountMigrationRunner::Step {
+ public:
+  AlwaysSucceeds(const std::string& id, base::RepeatingClosure closure)
+      : AccountMigrationRunner::Step(id), closure_(closure) {}
+  ~AlwaysSucceeds() override = default;
+
+  void Run(base::OnceCallback<void(bool)> callback) override {
+    closure_.Run();
+    std::move(callback).Run(true);
+  }
+
+ private:
+  base::RepeatingClosure closure_;
+  DISALLOW_COPY_AND_ASSIGN(AlwaysSucceeds);
+};
+
+class AlwaysFails : public AccountMigrationRunner::Step {
+ public:
+  AlwaysFails(const std::string& id, base::RepeatingClosure closure)
+      : AccountMigrationRunner::Step(id), closure_(closure) {}
+  ~AlwaysFails() override = default;
+
+  void Run(base::OnceCallback<void(bool)> callback) override {
+    closure_.Run();
+    std::move(callback).Run(false);
+  }
+
+ private:
+  base::RepeatingClosure closure_;
+  DISALLOW_COPY_AND_ASSIGN(AlwaysFails);
+};
+
+class MustNeverRun : public AccountMigrationRunner::Step {
+ public:
+  explicit MustNeverRun(const std::string& id)
+      : AccountMigrationRunner::Step(id) {}
+  ~MustNeverRun() override = default;
+
+  void Run(base::OnceCallback<void(bool)> callback) override {
+    EXPECT_FALSE(true);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MustNeverRun);
+};
+
+}  // namespace
+
+class AccountMigrationRunnerTest : public testing::Test {
+ protected:
+  AccountMigrationRunnerTest() : weak_factory_(this) {
+    increment_num_steps_executed_ = base::BindRepeating(
+        &AccountMigrationRunnerTest::IncrementNumStepsExecuted,
+        weak_factory_.GetWeakPtr());
+  }
+
+  ~AccountMigrationRunnerTest() override = default;
+
+  AccountMigrationRunner::MigrationResult RunMigration() {
+    AccountMigrationRunner::MigrationResult migration_result;
+    base::RunLoop run_loop;
+    AccountMigrationRunner::OnMigrationDone callback = base::BindOnce(
+        [](AccountMigrationRunner::MigrationResult* result,
+           base::OnceClosure quit_closure,
+           const AccountMigrationRunner::MigrationResult& returned_result) {
+          *result = returned_result;
+          std::move(quit_closure).Run();
+        },
+        &migration_result, run_loop.QuitClosure());
+
+    migration_runner_.Run(std::move(callback));
+    // Wait for callback from |migration_runner_|.
+    run_loop.Run();
+
+    return migration_result;
+  }
+
+  // Check base/test/scoped_task_environment.h. This must be the first member /
+  // declared before any member that cares about tasks.
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+
+  AccountMigrationRunner migration_runner_;
+
+  int num_steps_executed_ = 0;
+
+  base::RepeatingClosure increment_num_steps_executed_;
+
+ private:
+  void IncrementNumStepsExecuted() { ++num_steps_executed_; }
+
+  base::WeakPtrFactory<AccountMigrationRunnerTest> weak_factory_;
+  DISALLOW_COPY_AND_ASSIGN(AccountMigrationRunnerTest);
+};
+
+TEST_F(AccountMigrationRunnerTest, DoesNotRunUntilRunIsCalled) {
+  EXPECT_EQ(AccountMigrationRunner::Status::kNotStarted,
+            migration_runner_.GetStatus());
+}
+
+TEST_F(AccountMigrationRunnerTest, RunsSuccessfullyForZeroSteps) {
+  AccountMigrationRunner::MigrationResult migration_result = RunMigration();
+  EXPECT_EQ(AccountMigrationRunner::Status::kSuccess,
+            migration_runner_.GetStatus());
+  EXPECT_EQ(AccountMigrationRunner::Status::kSuccess,
+            migration_result.final_status);
+  EXPECT_EQ(0, num_steps_executed_);
+}
+
+TEST_F(AccountMigrationRunnerTest, RunsSuccessfullyForOneStep) {
+  migration_runner_.AddStep(
+      std::make_unique<AlwaysSucceeds>("Step1", increment_num_steps_executed_));
+
+  AccountMigrationRunner::MigrationResult migration_result = RunMigration();
+  EXPECT_EQ(AccountMigrationRunner::Status::kSuccess,
+            migration_runner_.GetStatus());
+  EXPECT_EQ(AccountMigrationRunner::Status::kSuccess,
+            migration_result.final_status);
+  EXPECT_EQ(1, num_steps_executed_);
+}
+
+TEST_F(AccountMigrationRunnerTest, FailsForOneFailingStep) {
+  migration_runner_.AddStep(
+      std::make_unique<AlwaysFails>("Step1", increment_num_steps_executed_));
+
+  AccountMigrationRunner::MigrationResult migration_result = RunMigration();
+  EXPECT_EQ(AccountMigrationRunner::Status::kFailure,
+            migration_runner_.GetStatus());
+  EXPECT_EQ(AccountMigrationRunner::Status::kFailure,
+            migration_result.final_status);
+  EXPECT_EQ("Step1", migration_result.failed_step_id);
+  EXPECT_EQ(1, num_steps_executed_);
+}
+
+TEST_F(AccountMigrationRunnerTest, RunsSuccessfullyForMoreThanOneStep) {
+  migration_runner_.AddStep(
+      std::make_unique<AlwaysSucceeds>("Step1", increment_num_steps_executed_));
+  migration_runner_.AddStep(
+      std::make_unique<AlwaysSucceeds>("Step2", increment_num_steps_executed_));
+  migration_runner_.AddStep(
+      std::make_unique<AlwaysSucceeds>("Step3", increment_num_steps_executed_));
+
+  AccountMigrationRunner::MigrationResult migration_result = RunMigration();
+  EXPECT_EQ(AccountMigrationRunner::Status::kSuccess,
+            migration_runner_.GetStatus());
+  EXPECT_EQ(AccountMigrationRunner::Status::kSuccess,
+            migration_result.final_status);
+  EXPECT_EQ(3, num_steps_executed_);
+}
+
+TEST_F(AccountMigrationRunnerTest, FailsIfFirstStepFails) {
+  migration_runner_.AddStep(
+      std::make_unique<AlwaysFails>("Step1", increment_num_steps_executed_));
+  migration_runner_.AddStep(std::make_unique<MustNeverRun>("Step2"));
+
+  AccountMigrationRunner::MigrationResult migration_result = RunMigration();
+  EXPECT_EQ(AccountMigrationRunner::Status::kFailure,
+            migration_runner_.GetStatus());
+  EXPECT_EQ(AccountMigrationRunner::Status::kFailure,
+            migration_result.final_status);
+  EXPECT_EQ("Step1", migration_result.failed_step_id);
+  EXPECT_EQ(1, num_steps_executed_);
+}
+
+TEST_F(AccountMigrationRunnerTest, FailsIfIntermediateStepFails) {
+  migration_runner_.AddStep(
+      std::make_unique<AlwaysSucceeds>("Step1", increment_num_steps_executed_));
+  migration_runner_.AddStep(
+      std::make_unique<AlwaysFails>("Step2", increment_num_steps_executed_));
+  migration_runner_.AddStep(std::make_unique<MustNeverRun>("Step3"));
+
+  AccountMigrationRunner::MigrationResult migration_result = RunMigration();
+  EXPECT_EQ(AccountMigrationRunner::Status::kFailure,
+            migration_runner_.GetStatus());
+  EXPECT_EQ(AccountMigrationRunner::Status::kFailure,
+            migration_result.final_status);
+  EXPECT_EQ("Step2", migration_result.failed_step_id);
+  EXPECT_EQ(2, num_steps_executed_);
+}
+
+}  // namespace chromeos