Refactor BrowserDataMigrator.
Refactors BrowserDataMigrator and turns some methods into instance
methods. This is in preparation to implementing move migration
(go/lacros-move-migration) which requires multiple hops between UI
thread and worker thread.
Test: unit_tests
Bug: 1261730
Change-Id: I2a3f7a0126f738130aac08ec8b140d48b8748faf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3350149
Reviewed-by: Denis Kuznetsov <[email protected]>
Reviewed-by: Dominic Battré <[email protected]>
Reviewed-by: Stefan Kuhne <[email protected]>
Reviewed-by: Alexander Alekseev <[email protected]>
Commit-Queue: Yuta Hijikata <[email protected]>
Cr-Commit-Position: refs/heads/main@{#953773}
diff --git a/chrome/browser/ash/chrome_browser_main_parts_ash.cc b/chrome/browser/ash/chrome_browser_main_parts_ash.cc
index 4ea240e..ad3e041 100644
--- a/chrome/browser/ash/chrome_browser_main_parts_ash.cc
+++ b/chrome/browser/ash/chrome_browser_main_parts_ash.cc
@@ -927,7 +927,8 @@
std::string user_id_hash =
parsed_command_line().GetSwitchValueASCII(switches::kLoginProfile);
- if (BrowserDataMigrator::MaybeRestartToMigrate(account_id, user_id_hash)) {
+ if (BrowserDataMigratorImpl::MaybeRestartToMigrate(account_id,
+ user_id_hash)) {
LOG(WARNING) << "Restarting chrome to run profile migration.";
return;
}
diff --git a/chrome/browser/ash/crosapi/browser_data_migrator.cc b/chrome/browser/ash/crosapi/browser_data_migrator.cc
index 425ed13..a6b0ad5 100644
--- a/chrome/browser/ash/crosapi/browser_data_migrator.cc
+++ b/chrome/browser/ash/crosapi/browser_data_migrator.cc
@@ -19,7 +19,6 @@
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"
#include "base/notreached.h"
-#include "base/path_service.h"
#include "base/strings/stringprintf.h"
#include "base/system/sys_info.h"
#include "base/task/post_task.h"
@@ -28,11 +27,9 @@
#include "base/values.h"
#include "chrome/browser/ash/crosapi/browser_data_migrator_util.h"
#include "chrome/browser/ash/crosapi/browser_util.h"
-#include "chrome/browser/ash/profiles/profile_helper.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/lifetime/application_lifetime.h"
#include "chrome/common/chrome_constants.h"
-#include "chrome/common/chrome_paths.h"
#include "chromeos/cryptohome/cryptohome_parameters.h"
#include "chromeos/dbus/session_manager/session_manager_client.h"
#include "components/prefs/pref_service.h"
@@ -189,35 +186,36 @@
CancelFlag::CancelFlag() : cancelled_(false) {}
CancelFlag::~CancelFlag() = default;
-BrowserDataMigrator::TargetInfo::TargetInfo()
+BrowserDataMigratorImpl::TargetInfo::TargetInfo()
: ash_data_size(0),
no_copy_data_size(0),
lacros_data_size(0),
common_data_size(0) {}
-BrowserDataMigrator::TargetInfo::TargetInfo(TargetInfo&&) = default;
-BrowserDataMigrator::TargetInfo::~TargetInfo() = default;
+BrowserDataMigratorImpl::TargetInfo::TargetInfo(TargetInfo&&) = default;
+BrowserDataMigratorImpl::TargetInfo::~TargetInfo() = default;
-BrowserDataMigrator::TargetItem::TargetItem(base::FilePath path,
- int64_t size,
- ItemType item_type)
+BrowserDataMigratorImpl::TargetItem::TargetItem(base::FilePath path,
+ int64_t size,
+ ItemType item_type)
: path(path), size(size), is_directory(item_type == ItemType::kDirectory) {}
-bool BrowserDataMigrator::TargetItem::operator==(const TargetItem& rhs) const {
+bool BrowserDataMigratorImpl::TargetItem::operator==(
+ const TargetItem& rhs) const {
return this->path == rhs.path && this->size == rhs.size &&
this->is_directory == rhs.is_directory;
}
-int64_t BrowserDataMigrator::TargetInfo::TotalCopySize() const {
+int64_t BrowserDataMigratorImpl::TargetInfo::TotalCopySize() const {
return lacros_data_size + common_data_size;
}
-int64_t BrowserDataMigrator::TargetInfo::TotalDirSize() const {
+int64_t BrowserDataMigratorImpl::TargetInfo::TotalDirSize() const {
return no_copy_data_size + ash_data_size + lacros_data_size +
common_data_size;
}
// static
-bool BrowserDataMigrator::MaybeRestartToMigrate(
+bool BrowserDataMigratorImpl::MaybeRestartToMigrate(
const AccountId& account_id,
const std::string& user_id_hash) {
// TODO(crbug.com/1277848): Once `BrowserDataMigrator` stabilises, remove this
@@ -349,8 +347,9 @@
}
// static
-bool BrowserDataMigrator::RestartToMigrate(const AccountId& account_id,
- const std::string& user_id_hash) {
+bool BrowserDataMigratorImpl::RestartToMigrate(
+ const AccountId& account_id,
+ const std::string& user_id_hash) {
SetMigrationStep(g_browser_process->local_state(),
MigrationStep::kRestartCalled);
@@ -375,54 +374,57 @@
return true;
}
-// static
-base::OnceClosure BrowserDataMigrator::Migrate(
+BrowserDataMigratorImpl::BrowserDataMigratorImpl(
+ const base::FilePath& original_profile_dir,
const std::string& user_id_hash,
const ProgressCallback& progress_callback,
- base::OnceClosure completion_callback) {
+ base::OnceClosure completion_callback,
+ PrefService* local_state)
+ : original_profile_dir_(original_profile_dir),
+ user_id_hash_(user_id_hash),
+ progress_tracker_(
+ std::make_unique<MigrationProgressTrackerImpl>(progress_callback)),
+ completion_callback_(std::move(completion_callback)),
+ cancel_flag_(base::MakeRefCounted<CancelFlag>()),
+ local_state_(local_state),
+ final_status_(ResultValue::kSkipped) {
+ DCHECK(local_state_);
+}
+
+BrowserDataMigratorImpl::~BrowserDataMigratorImpl() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
+
+void BrowserDataMigratorImpl::Migrate() {
+ DCHECK(local_state_);
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// TODO(crbug.com/1178702): Once BrowserDataMigrator stabilises, reduce the
// log level to VLOG(1).
- LOG(WARNING) << "BrowserDataMigrator::Migrate() is called.";
- DCHECK(GetMigrationStep(g_browser_process->local_state()) ==
- MigrationStep::kRestartCalled);
- SetMigrationStep(g_browser_process->local_state(), MigrationStep::kStarted);
- base::FilePath user_data_dir;
- if (!base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
- LOG(ERROR)
- << "Could not get the original user data dir path. Aborting migration.";
- RecordStatus(FinalStatus::kGetPathFailed);
- std::move(completion_callback).Run();
- return base::DoNothing();
- }
+ LOG(WARNING) << "BrowserDataMigratorImpl::Migrate() is called.";
- // The pointer will be shared by `cancel_callback` and `MigrateInternal()`.
- scoped_refptr<CancelFlag> cancel_flag = base::MakeRefCounted<CancelFlag>();
- // Create a callback that can be called to cancel the migration.
- base::OnceClosure cancel_callback = base::BindOnce(
- [](scoped_refptr<CancelFlag> cancel_flag) { cancel_flag->Set(); },
- cancel_flag);
+ DCHECK(GetMigrationStep(local_state_) == MigrationStep::kRestartCalled);
+ SetMigrationStep(local_state_, MigrationStep::kStarted);
- std::unique_ptr<MigrationProgressTracker> progress_tracker =
- std::make_unique<MigrationProgressTrackerImpl>(progress_callback);
-
- base::FilePath profile_data_dir =
- user_data_dir.Append(ProfileHelper::GetUserProfileDir(user_id_hash));
base::ThreadPool::PostTaskAndReplyWithResult(
FROM_HERE,
{base::MayBlock(), base::TaskPriority::USER_VISIBLE,
base::TaskShutdownBehavior::BLOCK_SHUTDOWN},
- base::BindOnce(&BrowserDataMigrator::MigrateInternal, profile_data_dir,
- std::move(progress_tracker), cancel_flag),
- base::BindOnce(&BrowserDataMigrator::MigrateInternalFinishedUIThread,
- std::move(completion_callback), user_id_hash));
+ base::BindOnce(&BrowserDataMigratorImpl::MigrateInternal,
+ original_profile_dir_, std::move(progress_tracker_),
+ cancel_flag_),
+ base::BindOnce(&BrowserDataMigratorImpl::MigrateInternalFinishedUIThread,
+ weak_factory_.GetWeakPtr()));
+}
- return cancel_callback;
+void BrowserDataMigratorImpl::Cancel() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ cancel_flag_->Set();
}
// static
-void BrowserDataMigrator::RecordStatus(const FinalStatus& final_status,
- const TargetInfo* target_info,
- const base::ElapsedTimer* timer) {
+void BrowserDataMigratorImpl::RecordStatus(const FinalStatus& final_status,
+ const TargetInfo* target_info,
+ const base::ElapsedTimer* timer) {
// Record final status enum.
UMA_HISTOGRAM_ENUMERATION(kFinalStatus, final_status);
@@ -455,7 +457,8 @@
// only web browser, update the underlying logic of migration from copy to move.
// Note that during testing phase we are copying files and leaving files in
// original location intact. We will allow these two states to diverge.
-BrowserDataMigrator::MigrationResult BrowserDataMigrator::MigrateInternal(
+BrowserDataMigratorImpl::MigrationResult
+BrowserDataMigratorImpl::MigrateInternal(
const base::FilePath& original_user_dir,
std::unique_ptr<MigrationProgressTracker> progress_tracker,
scoped_refptr<CancelFlag> cancel_flag) {
@@ -521,50 +524,52 @@
return {data_wipe_result, ResultValue::kFailed};
}
- LOG(WARNING) << "BrowserDataMigrator::Migrate took "
+ LOG(WARNING) << "BrowserDataMigratorImpl::Migrate took "
<< timer.Elapsed().InMilliseconds() << " ms and migrated "
<< target_info.TotalCopySize() / (1024 * 1024) << " MB.";
RecordStatus(FinalStatus::kSuccess, &target_info, &timer);
return {data_wipe_result, ResultValue::kSucceeded};
}
-// static
-void BrowserDataMigrator::MigrateInternalFinishedUIThread(
- base::OnceClosure callback,
- const std::string& user_id_hash,
+void BrowserDataMigratorImpl::MigrateInternalFinishedUIThread(
MigrationResult result) {
- DCHECK(GetMigrationStep(g_browser_process->local_state()) ==
- MigrationStep::kStarted);
- SetMigrationStep(g_browser_process->local_state(), MigrationStep::kEnded);
+ DCHECK(local_state_);
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ DCHECK(GetMigrationStep(local_state_) == MigrationStep::kStarted);
+ SetMigrationStep(local_state_, MigrationStep::kEnded);
+
+ final_status_ = result.data_migration;
if (result.data_wipe != ResultValue::kFailed) {
// kSkipped means that the directory did not exist so record the current
// version as the data version.
- crosapi::browser_util::RecordDataVer(g_browser_process->local_state(),
- user_id_hash,
+ crosapi::browser_util::RecordDataVer(local_state_, user_id_hash_,
version_info::GetVersion());
}
if (result.data_migration == ResultValue::kSucceeded) {
- crosapi::browser_util::SetProfileMigrationCompletedForUser(
- g_browser_process->local_state(), user_id_hash);
+ crosapi::browser_util::SetProfileMigrationCompletedForUser(local_state_,
+ user_id_hash_);
- ClearMigrationAttemptCountForUser(g_browser_process->local_state(),
- user_id_hash);
+ ClearMigrationAttemptCountForUser(local_state_, user_id_hash_);
}
// If migration has failed or skipped, we silently relaunch ash and send them
// to their home screen. In that case lacros will be disabled.
- g_browser_process->local_state()->CommitPendingWrite();
+ local_state_->CommitPendingWrite();
- std::move(callback).Run();
+ std::move(completion_callback_).Run();
+}
+
+BrowserDataMigratorImpl::ResultValue BrowserDataMigratorImpl::GetFinalStatus() {
+ return final_status_;
}
// static
// Copies `item` to location pointed by `dest`. Returns true on success and
// false on failure.
-bool BrowserDataMigrator::CopyTargetItem(
- const BrowserDataMigrator::TargetItem& item,
+bool BrowserDataMigratorImpl::CopyTargetItem(
+ const BrowserDataMigratorImpl::TargetItem& item,
const base::FilePath& dest,
CancelFlag* cancel_flag,
MigrationProgressTracker* progress_tracker) {
@@ -586,7 +591,7 @@
}
// static
-BrowserDataMigrator::TargetInfo BrowserDataMigrator::GetTargetInfo(
+BrowserDataMigratorImpl::TargetInfo BrowserDataMigratorImpl::GetTargetInfo(
const base::FilePath& original_user_dir) {
TargetInfo target_info;
const base::span<const char* const> no_copy_data_paths = GetNoCopyDataPaths();
@@ -640,7 +645,7 @@
}
// static
-bool BrowserDataMigrator::HasEnoughDiskSpace(
+bool BrowserDataMigratorImpl::HasEnoughDiskSpace(
const TargetInfo& target_info,
const base::FilePath& original_user_dir,
Mode mode) {
@@ -677,7 +682,7 @@
}
// static
-bool BrowserDataMigrator::CopyDirectory(
+bool BrowserDataMigratorImpl::CopyDirectory(
const base::FilePath& from_path,
const base::FilePath& to_path,
CancelFlag* cancel_flag,
@@ -719,7 +724,8 @@
return true;
}
-bool BrowserDataMigrator::CopyTargetItems(
+// static
+bool BrowserDataMigratorImpl::CopyTargetItems(
const base::FilePath& to_dir,
const std::vector<TargetItem>& items,
CancelFlag* cancel_flag,
@@ -755,7 +761,7 @@
}
// static
-bool BrowserDataMigrator::SetupTmpDir(
+bool BrowserDataMigratorImpl::SetupTmpDir(
const TargetInfo& target_info,
const base::FilePath& from_dir,
const base::FilePath& tmp_dir,
@@ -793,7 +799,7 @@
}
// static
-void BrowserDataMigrator::RegisterLocalStatePrefs(
+void BrowserDataMigratorImpl::RegisterLocalStatePrefs(
PrefRegistrySimple* registry) {
registry->RegisterIntegerPref(kMigrationStep,
static_cast<int>(MigrationStep::kCheckStep));
@@ -802,24 +808,25 @@
}
// static
-void BrowserDataMigrator::SetMigrationStep(
+void BrowserDataMigratorImpl::SetMigrationStep(
PrefService* local_state,
- BrowserDataMigrator::MigrationStep step) {
+ BrowserDataMigratorImpl::MigrationStep step) {
local_state->SetInteger(kMigrationStep, static_cast<int>(step));
}
// static
-void BrowserDataMigrator::ClearMigrationStep(PrefService* local_state) {
+void BrowserDataMigratorImpl::ClearMigrationStep(PrefService* local_state) {
local_state->ClearPref(kMigrationStep);
}
// static
-BrowserDataMigrator::MigrationStep BrowserDataMigrator::GetMigrationStep(
- PrefService* local_state) {
+BrowserDataMigratorImpl::MigrationStep
+BrowserDataMigratorImpl::GetMigrationStep(PrefService* local_state) {
return static_cast<MigrationStep>(local_state->GetInteger(kMigrationStep));
}
-void BrowserDataMigrator::UpdateMigrationAttemptCountForUser(
+// static
+void BrowserDataMigratorImpl::UpdateMigrationAttemptCountForUser(
PrefService* local_state,
const std::string& user_id_hash) {
int count = GetMigrationAttemptCountForUser(local_state, user_id_hash);
@@ -829,7 +836,8 @@
dict->SetKey(user_id_hash, base::Value(count));
}
-int BrowserDataMigrator::GetMigrationAttemptCountForUser(
+// static
+int BrowserDataMigratorImpl::GetMigrationAttemptCountForUser(
PrefService* local_state,
const std::string& user_id_hash) {
return local_state->GetDictionary(kMigrationAttemptCountPref)
@@ -837,7 +845,8 @@
.value_or(0);
}
-void BrowserDataMigrator::ClearMigrationAttemptCountForUser(
+// static
+void BrowserDataMigratorImpl::ClearMigrationAttemptCountForUser(
PrefService* local_state,
const std::string& user_id_hash) {
DictionaryPrefUpdate update(local_state, kMigrationAttemptCountPref);
@@ -846,7 +855,7 @@
}
// static
-void BrowserDataMigrator::DryRunToCollectUMA(
+void BrowserDataMigratorImpl::DryRunToCollectUMA(
const base::FilePath& profile_data_dir) {
TargetInfo target_info = GetTargetInfo(profile_data_dir);
@@ -885,7 +894,7 @@
}
// staic
-void BrowserDataMigrator::RecordTargetItemSizes(
+void BrowserDataMigratorImpl::RecordTargetItemSizes(
const std::vector<TargetItem>& items) {
for (auto& item : items)
browser_data_migrator_util::RecordUserDataSize(item.path, item.size);
diff --git a/chrome/browser/ash/crosapi/browser_data_migrator.h b/chrome/browser/ash/crosapi/browser_data_migrator.h
index 1b8cc16e..ddb2fff 100644
--- a/chrome/browser/ash/crosapi/browser_data_migrator.h
+++ b/chrome/browser/ash/crosapi/browser_data_migrator.h
@@ -12,6 +12,7 @@
#include "base/callback.h"
#include "base/files/file_path.h"
#include "base/gtest_prod_util.h"
+#include "base/sequence_checker.h"
#include "base/strings/string_piece.h"
#include "base/synchronization/atomic_flag.h"
#include "base/timer/elapsed_timer.h"
@@ -51,7 +52,7 @@
"Ash.BrowserDataMigrator.CreateDirectoryFailure";
// The following UMAs are recorded from
-// `BrowserDataMigrator::DryRunToCollectUMA()`.
+// `BrowserDataMigratorImpl::DryRunToCollectUMA()`.
constexpr char kDryRunNoCopyDataSize[] =
"Ash.BrowserDataMigrator.DryRunNoCopyDataSizeMB";
constexpr char kDryRunAshDataSize[] =
@@ -105,10 +106,20 @@
std::atomic_bool cancelled_;
};
-// BrowserDataMigrator is responsible for one time browser data migration from
-// ash-chrome to lacros-chrome.
+// The interface is exposed to be inherited by fakes in tests.
class BrowserDataMigrator {
public:
+ virtual ~BrowserDataMigrator() = default;
+ // Carries out the migration. It needs to be called on UI thread.
+ virtual void Migrate() = 0;
+ // Cancels the migration.
+ virtual void Cancel() = 0;
+};
+
+// BrowserDataMigratorImpl is responsible for one time browser data migration
+// from ash-chrome to lacros-chrome.
+class BrowserDataMigratorImpl : public BrowserDataMigrator {
+ public:
// Used to describe a file/dir that has to be migrated.
struct TargetItem {
enum class ItemType { kFile, kDirectory };
@@ -204,6 +215,20 @@
// TargetInfo::no_copy_items to make extra space.
};
+ // `BrowserDataMigratorImpl` migrates browser data from `original_profile_dir`
+ // to a new profile location for lacros chrome. `progress_callback` is called
+ // to update the progress bar on the screen. `completion_callback` passed as
+ // an argument will be called on the UI thread where `Migrate()` is called
+ // once migration has completed or failed.
+ BrowserDataMigratorImpl(const base::FilePath& original_profile_dir,
+ const std::string& user_id_hash,
+ const ProgressCallback& progress_callback,
+ base::OnceClosure completion_callback,
+ PrefService* local_state);
+ BrowserDataMigratorImpl(const BrowserDataMigratorImpl&) = delete;
+ BrowserDataMigratorImpl& operator=(const BrowserDataMigratorImpl&) = delete;
+ ~BrowserDataMigratorImpl() override;
+
// Checks if migration is required for the user identified by `user_id_hash`
// and if it is required, calls a D-Bus method to session_manager and
// terminates ash-chrome. It returns true if the D-Bus call to the
@@ -212,16 +237,9 @@
static bool MaybeRestartToMigrate(const AccountId& account_id,
const std::string& user_id_hash);
- // The method needs to be called on UI thread. It posts `MigrateInternal()` on
- // a worker thread and returns a callback which can be used to cancel
- // migration mid way. `progress_callback` is called to update the progress bar
- // on the screen.
- // `completion_callbackchrome/browser/ash/crosapi/browser_data_migrator.cc`
- // passed as an argument will be called on the original thread once migration
- // has completed or failed.
- static base::OnceClosure Migrate(const std::string& user_id_hash,
- const ProgressCallback& progress_callback,
- base::OnceClosure completion_callback);
+ // `BrowserDataMigrator` methods.
+ void Migrate() override;
+ void Cancel() override;
// Registers boolean pref `kCheckForMigrationOnRestart` with default as false.
static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
@@ -229,6 +247,8 @@
// Clears the value of `kMigrationStep` in Local State.
static void ClearMigrationStep(PrefService* local_state);
+ ResultValue GetFinalStatus();
+
// Collects migration specific UMAs without actually running the migration. It
// does not check if lacros is enabled.
static void DryRunToCollectUMA(const base::FilePath& profile_data_dir);
@@ -241,14 +261,16 @@
FRIEND_TEST_ALL_PREFIXES(BrowserDataMigratorTest, SetupTmpDir);
FRIEND_TEST_ALL_PREFIXES(BrowserDataMigratorTest, CancelSetupTmpDir);
FRIEND_TEST_ALL_PREFIXES(BrowserDataMigratorTest, RecordStatus);
+ FRIEND_TEST_ALL_PREFIXES(BrowserDataMigratorTest, MigrateInternal);
FRIEND_TEST_ALL_PREFIXES(BrowserDataMigratorTest, Migrate);
-
- // Gets the value of `kMigrationStep` in Local State.
- static MigrationStep GetMigrationStep(PrefService* local_state);
+ FRIEND_TEST_ALL_PREFIXES(BrowserDataMigratorTest, MigrateCancelled);
// Sets the value of `kMigrationStep` in Local State.
static void SetMigrationStep(PrefService* local_state, MigrationStep step);
+ // Gets the value of `kMigrationStep` in Local State.
+ static MigrationStep GetMigrationStep(PrefService* local_state);
+
// Increments the migration attempt count stored in
// `kMigrationAttemptCountPref` by 1 for the user identified by
// `user_id_hash`.
@@ -281,9 +303,7 @@
const std::string& user_id_hash);
// Called on UI thread once migration is finished.
- static void MigrateInternalFinishedUIThread(base::OnceClosure callback,
- const std::string& user_id_hash,
- MigrationResult result);
+ void MigrateInternalFinishedUIThread(MigrationResult result);
// Records to UMA histograms. Note that if `target_info` is nullptr, timer
// will be ignored.
@@ -319,7 +339,7 @@
// Copies `item` to location pointed by `dest`. Returns true on success and
// false on failure.
- static bool CopyTargetItem(const BrowserDataMigrator::TargetItem& item,
+ static bool CopyTargetItem(const BrowserDataMigratorImpl::TargetItem& item,
const base::FilePath& dest,
CancelFlag* cancel_flag,
MigrationProgressTracker* progress_tracker);
@@ -333,6 +353,28 @@
// Records the sizes of `TargetItem`s.
static void RecordTargetItemSizes(const std::vector<TargetItem>& items);
+
+ // Path to the original profile data directory, which is directly under the
+ // user data directory.
+ const base::FilePath original_profile_dir_;
+ // A hash string of the profile user ID.
+ const std::string user_id_hash_;
+ // `progress_tracker_` is used to report progress status to the screen.
+ std::unique_ptr<MigrationProgressTracker> progress_tracker_;
+ // Callback to be called once migration is done. It is called regardless of
+ // whether migration succeeded or not.
+ base::OnceClosure completion_callback_;
+ // `cancel_flag_` gets set by `Cancel()` and tasks posted to worker threads
+ // can check if migration is cancelled or not.
+ scoped_refptr<CancelFlag> cancel_flag_;
+ // Local state prefs, not owned.
+ PrefService* local_state_ = nullptr;
+ // Final status of the migration.
+ ResultValue final_status_;
+
+ SEQUENCE_CHECKER(sequence_checker_);
+
+ base::WeakPtrFactory<BrowserDataMigratorImpl> weak_factory_{this};
};
} // namespace ash
diff --git a/chrome/browser/ash/crosapi/browser_data_migrator_unittest.cc b/chrome/browser/ash/crosapi/browser_data_migrator_unittest.cc
index 89c1c076..26c2591f 100644
--- a/chrome/browser/ash/crosapi/browser_data_migrator_unittest.cc
+++ b/chrome/browser/ash/crosapi/browser_data_migrator_unittest.cc
@@ -9,14 +9,17 @@
#include "ash/constants/ash_features.h"
#include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h"
+#include "base/run_loop.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
+#include "base/test/task_environment.h"
#include "chrome/browser/ash/crosapi/browser_data_migrator_util.h"
#include "chrome/browser/ash/crosapi/browser_util.h"
#include "chrome/browser/ash/crosapi/fake_migration_progress_tracker.h"
#include "chrome/browser/ash/login/users/fake_chrome_user_manager.h"
#include "chrome/test/base/scoped_testing_local_state.h"
#include "components/prefs/testing_pref_service.h"
+#include "components/version_info/version_info.h"
#include "testing/gtest/include/gtest/gtest.h"
using user_manager::User;
@@ -36,8 +39,8 @@
constexpr char kAffiliationDatabase[] = "Affiliation Database";
struct TargetItemComparator {
- bool operator()(const BrowserDataMigrator::TargetItem& t1,
- const BrowserDataMigrator::TargetItem& t2) const {
+ bool operator()(const BrowserDataMigratorImpl::TargetItem& t1,
+ const BrowserDataMigratorImpl::TargetItem& t2) const {
return t1.path < t2.path;
}
};
@@ -100,7 +103,7 @@
kDataFile) /* .../user/Affiliation Database/Downloads/data */,
kDataContent, kFileSize));
- BrowserDataMigrator::RegisterLocalStatePrefs(pref_service_.registry());
+ BrowserDataMigratorImpl::RegisterLocalStatePrefs(pref_service_.registry());
crosapi::browser_util::RegisterLocalStatePrefs(pref_service_.registry());
}
@@ -114,31 +117,31 @@
TEST_F(BrowserDataMigratorTest, ManipulateMigrationAttemptCount) {
const std::string user_id_hash = "user";
- EXPECT_EQ(BrowserDataMigrator::GetMigrationAttemptCountForUser(&pref_service_,
- user_id_hash),
+ EXPECT_EQ(BrowserDataMigratorImpl::GetMigrationAttemptCountForUser(
+ &pref_service_, user_id_hash),
0);
- BrowserDataMigrator::UpdateMigrationAttemptCountForUser(&pref_service_,
- user_id_hash);
- EXPECT_EQ(BrowserDataMigrator::GetMigrationAttemptCountForUser(&pref_service_,
- user_id_hash),
+ BrowserDataMigratorImpl::UpdateMigrationAttemptCountForUser(&pref_service_,
+ user_id_hash);
+ EXPECT_EQ(BrowserDataMigratorImpl::GetMigrationAttemptCountForUser(
+ &pref_service_, user_id_hash),
1);
- BrowserDataMigrator::UpdateMigrationAttemptCountForUser(&pref_service_,
- user_id_hash);
- EXPECT_EQ(BrowserDataMigrator::GetMigrationAttemptCountForUser(&pref_service_,
- user_id_hash),
+ BrowserDataMigratorImpl::UpdateMigrationAttemptCountForUser(&pref_service_,
+ user_id_hash);
+ EXPECT_EQ(BrowserDataMigratorImpl::GetMigrationAttemptCountForUser(
+ &pref_service_, user_id_hash),
2);
- BrowserDataMigrator::ClearMigrationAttemptCountForUser(&pref_service_,
- user_id_hash);
- EXPECT_EQ(BrowserDataMigrator::GetMigrationAttemptCountForUser(&pref_service_,
- user_id_hash),
+ BrowserDataMigratorImpl::ClearMigrationAttemptCountForUser(&pref_service_,
+ user_id_hash);
+ EXPECT_EQ(BrowserDataMigratorImpl::GetMigrationAttemptCountForUser(
+ &pref_service_, user_id_hash),
0);
}
TEST_F(BrowserDataMigratorTest, GetTargetInfo) {
- BrowserDataMigrator::TargetInfo target_info =
- BrowserDataMigrator::GetTargetInfo(from_dir_);
+ BrowserDataMigratorImpl::TargetInfo target_info =
+ BrowserDataMigratorImpl::GetTargetInfo(from_dir_);
EXPECT_EQ(target_info.ash_data_size, kFileSize * 2 /* expect two files */);
EXPECT_EQ(target_info.no_copy_data_size, kFileSize /* expect one file */);
@@ -146,11 +149,11 @@
EXPECT_EQ(target_info.common_data_size, kFileSize * 2 /* expect two file */);
// Check for ash data.
- std::vector<BrowserDataMigrator::TargetItem> expected_ash_data_items = {
+ std::vector<BrowserDataMigratorImpl::TargetItem> expected_ash_data_items = {
{from_dir_.Append(kDownloads), kFileSize,
- BrowserDataMigrator::TargetItem::ItemType::kDirectory},
+ BrowserDataMigratorImpl::TargetItem::ItemType::kDirectory},
{from_dir_.Append(kFullRestoreData), kFileSize,
- BrowserDataMigrator::TargetItem::ItemType::kFile}};
+ BrowserDataMigratorImpl::TargetItem::ItemType::kFile}};
std::sort(target_info.ash_data_items.begin(),
target_info.ash_data_items.end(), TargetItemComparator());
ASSERT_EQ(target_info.ash_data_items.size(), expected_ash_data_items.size());
@@ -160,12 +163,13 @@
}
// Check for lacros data.
- std::vector<BrowserDataMigrator::TargetItem> expected_lacros_data_items = {
- {from_dir_.Append(kBookmarks), kFileSize,
- BrowserDataMigrator::TargetItem::ItemType::kFile},
- {from_dir_.Append(kCookies), kFileSize,
- BrowserDataMigrator::TargetItem::ItemType::kFile},
- };
+ std::vector<BrowserDataMigratorImpl::TargetItem> expected_lacros_data_items =
+ {
+ {from_dir_.Append(kBookmarks), kFileSize,
+ BrowserDataMigratorImpl::TargetItem::ItemType::kFile},
+ {from_dir_.Append(kCookies), kFileSize,
+ BrowserDataMigratorImpl::TargetItem::ItemType::kFile},
+ };
ASSERT_EQ(target_info.lacros_data_items.size(),
expected_lacros_data_items.size());
std::sort(target_info.lacros_data_items.begin(),
@@ -176,9 +180,9 @@
}
// Check for common data.
- std::vector<BrowserDataMigrator::TargetItem> expected_common_data_items = {
- {from_dir_.Append(kAffiliationDatabase), kFileSize * 2,
- BrowserDataMigrator::TargetItem::ItemType::kDirectory}};
+ std::vector<BrowserDataMigratorImpl::TargetItem> expected_common_data_items =
+ {{from_dir_.Append(kAffiliationDatabase), kFileSize * 2,
+ BrowserDataMigratorImpl::TargetItem::ItemType::kDirectory}};
ASSERT_EQ(target_info.common_data_items.size(),
expected_common_data_items.size());
std::sort(target_info.common_data_items.begin(),
@@ -210,7 +214,7 @@
scoped_refptr<CancelFlag> cancelled = base::MakeRefCounted<CancelFlag>();
FakeMigrationProgressTracker progress_tracker;
- ASSERT_TRUE(BrowserDataMigrator::CopyDirectory(
+ ASSERT_TRUE(BrowserDataMigratorImpl::CopyDirectory(
copy_from, copy_to, cancelled.get(), &progress_tracker));
// Setup `copy_from` as below.
@@ -242,7 +246,7 @@
ASSERT_TRUE(base::WriteFile(from_dir_.Append(FILE_PATH_LITERAL("abcd")),
kDataContent, kFileSize));
- BrowserDataMigrator::DryRunToCollectUMA(from_dir_);
+ BrowserDataMigratorImpl::DryRunToCollectUMA(from_dir_);
std::string uma_name_cache =
std::string(browser_data_migrator_util::kUserDataStatsRecorderDataSize) +
@@ -312,15 +316,15 @@
// copied data size or total time.
base::HistogramTester histogram_tester;
- BrowserDataMigrator::RecordStatus(
- BrowserDataMigrator::FinalStatus::kSkipped);
+ BrowserDataMigratorImpl::RecordStatus(
+ BrowserDataMigratorImpl::FinalStatus::kSkipped);
histogram_tester.ExpectTotalCount(kFinalStatus, 1);
histogram_tester.ExpectTotalCount(kCopiedDataSize, 0);
histogram_tester.ExpectTotalCount(kTotalTime, 0);
histogram_tester.ExpectBucketCount(
- kFinalStatus, BrowserDataMigrator::FinalStatus::kSkipped, 1);
+ kFinalStatus, BrowserDataMigratorImpl::FinalStatus::kSkipped, 1);
}
{
@@ -328,7 +332,7 @@
// `kCopiedDataSize`, `kTotalTime` should be recorded.
base::HistogramTester histogram_tester;
- BrowserDataMigrator::TargetInfo target_info;
+ BrowserDataMigratorImpl::TargetInfo target_info;
target_info.ash_data_size = /* 300 MBs */ 300 * 1024 * 1024;
target_info.lacros_data_size = /* 400 MBs */ 400 * 1024 * 1024;
target_info.common_data_size = /* 500 MBs */ 500 * 1024 * 1024;
@@ -336,8 +340,8 @@
base::ElapsedTimer timer;
- BrowserDataMigrator::RecordStatus(
- BrowserDataMigrator::FinalStatus::kSuccess, &target_info, &timer);
+ BrowserDataMigratorImpl::RecordStatus(
+ BrowserDataMigratorImpl::FinalStatus::kSuccess, &target_info, &timer);
histogram_tester.ExpectTotalCount(kFinalStatus, 1);
histogram_tester.ExpectTotalCount(kCopiedDataSize, 1);
@@ -347,7 +351,7 @@
histogram_tester.ExpectTotalCount(kTotalTime, 1);
histogram_tester.ExpectBucketCount(
- kFinalStatus, BrowserDataMigrator::FinalStatus::kSuccess, 1);
+ kFinalStatus, BrowserDataMigratorImpl::FinalStatus::kSuccess, 1);
histogram_tester.ExpectBucketCount(
kCopiedDataSize, target_info.TotalCopySize() / (1024 * 1024), 1);
histogram_tester.ExpectBucketCount(
@@ -364,10 +368,10 @@
TEST_F(BrowserDataMigratorTest, SetupTmpDir) {
base::FilePath tmp_dir = from_dir_.Append(kTmpDir);
scoped_refptr<CancelFlag> cancel_flag = base::MakeRefCounted<CancelFlag>();
- BrowserDataMigrator::TargetInfo target_info =
- BrowserDataMigrator::GetTargetInfo(from_dir_);
+ BrowserDataMigratorImpl::TargetInfo target_info =
+ BrowserDataMigratorImpl::GetTargetInfo(from_dir_);
FakeMigrationProgressTracker progress_tracker;
- EXPECT_TRUE(BrowserDataMigrator::SetupTmpDir(
+ EXPECT_TRUE(BrowserDataMigratorImpl::SetupTmpDir(
target_info, from_dir_, tmp_dir, cancel_flag.get(), &progress_tracker));
EXPECT_TRUE(base::PathExists(tmp_dir));
@@ -392,12 +396,12 @@
base::FilePath tmp_dir = from_dir_.Append(kTmpDir);
scoped_refptr<CancelFlag> cancel_flag = base::MakeRefCounted<CancelFlag>();
FakeMigrationProgressTracker progress_tracker;
- BrowserDataMigrator::TargetInfo target_info =
- BrowserDataMigrator::GetTargetInfo(from_dir_);
+ BrowserDataMigratorImpl::TargetInfo target_info =
+ BrowserDataMigratorImpl::GetTargetInfo(from_dir_);
// Set cancel_flag to cancel migrationl.
cancel_flag->Set();
- EXPECT_FALSE(BrowserDataMigrator::SetupTmpDir(
+ EXPECT_FALSE(BrowserDataMigratorImpl::SetupTmpDir(
target_info, user_data_dir_.GetPath(), tmp_dir, cancel_flag.get(),
&progress_tracker));
@@ -409,15 +413,15 @@
base::PathExists(tmp_dir.Append(kLacrosProfilePath).Append(kCookies)));
}
-TEST_F(BrowserDataMigratorTest, Migrate) {
+TEST_F(BrowserDataMigratorTest, MigrateInternal) {
base::HistogramTester histogram_tester;
{
scoped_refptr<CancelFlag> cancelled = base::MakeRefCounted<CancelFlag>();
std::unique_ptr<MigrationProgressTracker> progress_tracker =
std::make_unique<FakeMigrationProgressTracker>();
- BrowserDataMigrator::MigrateInternal(from_dir_, std::move(progress_tracker),
- cancelled);
+ BrowserDataMigratorImpl::MigrateInternal(
+ from_dir_, std::move(progress_tracker), cancelled);
// Expected dir structure after migration.
// ./ /* user_data_dir_ */
@@ -467,8 +471,91 @@
histogram_tester.ExpectTotalCount(kCreateDirectoryFail, 0);
histogram_tester.ExpectBucketCount(
- kFinalStatus, BrowserDataMigrator::FinalStatus::kSuccess, 1);
+ kFinalStatus, BrowserDataMigratorImpl::FinalStatus::kSuccess, 1);
histogram_tester.ExpectBucketCount(kCopiedDataSize,
kFileSize * 4 / (1024 * 1024), 1);
}
+
+TEST_F(BrowserDataMigratorTest, Migrate) {
+ base::test::TaskEnvironment task_environment;
+ scoped_refptr<CancelFlag> cancelled = base::MakeRefCounted<CancelFlag>();
+ std::unique_ptr<MigrationProgressTracker> progress_tracker =
+ std::make_unique<FakeMigrationProgressTracker>();
+ const std::string user_id_hash = "abcd";
+ BrowserDataMigratorImpl::SetMigrationStep(
+ &pref_service_, BrowserDataMigratorImpl::MigrationStep::kRestartCalled);
+ // Set migration attempt count to 1.
+ BrowserDataMigratorImpl::UpdateMigrationAttemptCountForUser(&pref_service_,
+ user_id_hash);
+
+ base::RunLoop run_loop;
+ std::unique_ptr<BrowserDataMigratorImpl> migrator =
+ std::make_unique<BrowserDataMigratorImpl>(
+ from_dir_, user_id_hash, base::DoNothing(), run_loop.QuitClosure(),
+ &pref_service_);
+ migrator->Migrate();
+ run_loop.Run();
+
+ const base::FilePath new_user_data_dir = from_dir_.Append(kLacrosDir);
+ const base::FilePath new_profile_data_dir =
+ new_user_data_dir.Append("Default");
+ // Check that `First Run` file is created inside the new data directory.
+ EXPECT_TRUE(base::PathExists(new_user_data_dir.Append(kFirstRun)));
+ // Check that migration is marked as completed for the user.
+ EXPECT_TRUE(crosapi::browser_util::IsProfileMigrationCompletedForUser(
+ &pref_service_, user_id_hash));
+ EXPECT_EQ(migrator->GetFinalStatus(),
+ BrowserDataMigratorImpl::ResultValue::kSucceeded);
+ EXPECT_EQ(BrowserDataMigratorImpl::GetMigrationStep(&pref_service_),
+ BrowserDataMigratorImpl::MigrationStep::kEnded);
+ // Successful migration should clear the migration attempt count.
+ EXPECT_EQ(BrowserDataMigratorImpl::GetMigrationAttemptCountForUser(
+ &pref_service_, user_id_hash),
+ 0);
+ // Data version should be updated to the current version after a migration.
+ EXPECT_EQ(crosapi::browser_util::GetDataVer(&pref_service_, user_id_hash),
+ version_info::GetVersion());
+}
+
+TEST_F(BrowserDataMigratorTest, MigrateCancelled) {
+ base::test::TaskEnvironment task_environment;
+ scoped_refptr<CancelFlag> cancelled = base::MakeRefCounted<CancelFlag>();
+ std::unique_ptr<MigrationProgressTracker> progress_tracker =
+ std::make_unique<FakeMigrationProgressTracker>();
+ const std::string user_id_hash = "abcd";
+ BrowserDataMigratorImpl::SetMigrationStep(
+ &pref_service_, BrowserDataMigratorImpl::MigrationStep::kRestartCalled);
+ // Set migration attempt count to 1.
+ BrowserDataMigratorImpl::UpdateMigrationAttemptCountForUser(&pref_service_,
+ user_id_hash);
+
+ base::RunLoop run_loop;
+ std::unique_ptr<BrowserDataMigratorImpl> migrator =
+ std::make_unique<BrowserDataMigratorImpl>(
+ from_dir_, user_id_hash, base::DoNothing(), run_loop.QuitClosure(),
+ &pref_service_);
+ migrator->Migrate();
+ migrator->Cancel();
+ run_loop.Run();
+
+ const base::FilePath new_user_data_dir = from_dir_.Append(kLacrosDir);
+ const base::FilePath new_profile_data_dir =
+ new_user_data_dir.Append("Default");
+ EXPECT_FALSE(base::PathExists(new_user_data_dir.Append(kFirstRun)));
+ EXPECT_FALSE(crosapi::browser_util::IsProfileMigrationCompletedForUser(
+ &pref_service_, user_id_hash));
+ EXPECT_EQ(migrator->GetFinalStatus(),
+ BrowserDataMigratorImpl::ResultValue::kCancelled);
+ EXPECT_EQ(BrowserDataMigratorImpl::GetMigrationStep(&pref_service_),
+ BrowserDataMigratorImpl::MigrationStep::kEnded);
+ // If migration fails, migration attempt count should not be cleared thus
+ // should remain as 1.
+ EXPECT_EQ(BrowserDataMigratorImpl::GetMigrationAttemptCountForUser(
+ &pref_service_, user_id_hash),
+ 1);
+ // Even if migration is cancelled, lacros data dir is cleared and thus data
+ // version should be updated.
+ EXPECT_EQ(crosapi::browser_util::GetDataVer(&pref_service_, user_id_hash),
+ version_info::GetVersion());
+}
} // namespace ash
diff --git a/chrome/browser/ash/crosapi/browser_manager.cc b/chrome/browser/ash/crosapi/browser_manager.cc
index aa2dd997..52441c8 100644
--- a/chrome/browser/ash/crosapi/browser_manager.cc
+++ b/chrome/browser/ash/crosapi/browser_manager.cc
@@ -530,7 +530,7 @@
// inside the profile data directory.
base::ThreadPool::PostTask(
FROM_HERE, {base::MayBlock()},
- base::BindOnce(&ash::BrowserDataMigrator::DryRunToCollectUMA,
+ base::BindOnce(&ash::BrowserDataMigratorImpl::DryRunToCollectUMA,
ProfileManager::GetPrimaryUserProfile()->GetPath()));
}
diff --git a/chrome/browser/ash/login/screens/lacros_data_migration_screen.cc b/chrome/browser/ash/login/screens/lacros_data_migration_screen.cc
index ef8314e..5e97f48 100644
--- a/chrome/browser/ash/login/screens/lacros_data_migration_screen.cc
+++ b/chrome/browser/ash/login/screens/lacros_data_migration_screen.cc
@@ -6,12 +6,16 @@
#include "ash/constants/ash_switches.h"
#include "base/command_line.h"
+#include "base/path_service.h"
#include "base/task/bind_post_task.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "chrome/browser/ash/crosapi/browser_data_migrator.h"
+#include "chrome/browser/ash/profiles/profile_helper.h"
+#include "chrome/browser/browser_process.h"
#include "chrome/browser/lifetime/application_lifetime.h"
+#include "chrome/common/chrome_paths.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/device_service.h"
#include "services/device/public/mojom/wake_lock_provider.mojom.h"
@@ -20,26 +24,13 @@
namespace {
constexpr char kUserActionCancel[] = "cancel";
constexpr base::TimeDelta kShowSkipButtonDuration = base::Seconds(20);
-
-class MigratorDelegateImpl
- : public LacrosDataMigrationScreen::MigratorDelegate {
- base::OnceClosure Migrate(
- const std::string& user_id_hash,
- const base::RepeatingCallback<void(int)>& progress_callback) override {
- return BrowserDataMigrator::Migrate(
- user_id_hash, progress_callback,
- base::BindOnce(&chrome::AttemptRestart));
- }
-};
-
} // namespace
LacrosDataMigrationScreen::LacrosDataMigrationScreen(
LacrosDataMigrationScreenView* view)
: BaseScreen(LacrosDataMigrationScreenView::kScreenId,
OobeScreenPriority::SCREEN_DEVICE_DEVELOPER_MODIFICATION),
- view_(view),
- migrator_delegate_(std::make_unique<MigratorDelegateImpl>()) {
+ view_(view) {
DCHECK(view_);
if (view_)
view_->Bind(this);
@@ -60,22 +51,44 @@
if (!view_)
return;
- // user_id_hash_ is not empty if it is already set by
- // `SetUserIdHashForTesting()`.
- if (user_id_hash_.empty()) {
- user_id_hash_ = base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
- switches::kBrowserDataMigrationForUser);
+ if (!migrator_) {
+ const std::string user_id_hash =
+ base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+ switches::kBrowserDataMigrationForUser);
+
+ if (user_id_hash.empty()) {
+ LOG(ERROR) << "Colud not retrieve user_id_hash from switch "
+ << switches::kBrowserDataMigrationForUser
+ << ". Aborting migration.";
+
+ chrome::AttemptRestart();
+ }
+ DCHECK(!user_id_hash.empty()) << "user_id_hash should not be empty.";
+
+ base::FilePath user_data_dir;
+ if (!base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)) {
+ LOG(ERROR) << "Could not get the original user data dir path. Aborting "
+ "migration.";
+ chrome::AttemptRestart();
+ return;
+ }
+
+ const base::FilePath profile_data_dir =
+ user_data_dir.Append(ProfileHelper::GetUserProfileDir(user_id_hash));
+
+ base::RepeatingCallback<void(int)> progress_callback = base::BindPostTask(
+ base::SequencedTaskRunnerHandle::Get(),
+ base::BindRepeating(&LacrosDataMigrationScreen::OnProgressUpdate,
+ weak_factory_.GetWeakPtr()),
+ FROM_HERE);
+
+ migrator_ = std::make_unique<BrowserDataMigratorImpl>(
+ profile_data_dir, user_id_hash, progress_callback,
+ base::BindOnce(&chrome::AttemptRestart),
+ g_browser_process->local_state());
+
+ migrator_->Migrate();
}
- DCHECK(!user_id_hash_.empty()) << "user_id_hash_ should not be empty.";
-
- base::RepeatingCallback<void(int)> progress_callback = base::BindPostTask(
- base::SequencedTaskRunnerHandle::Get(),
- base::BindRepeating(&LacrosDataMigrationScreen::OnProgressUpdate,
- weak_factory_.GetWeakPtr()),
- FROM_HERE);
-
- cancel_callback_ =
- migrator_delegate_->Migrate(user_id_hash_, progress_callback);
// Show the screen.
view_->Show();
@@ -105,9 +118,9 @@
void LacrosDataMigrationScreen::OnUserAction(const std::string& action_id) {
if (action_id == kUserActionCancel) {
- if (cancel_callback_) {
+ if (migrator_) {
LOG(WARNING) << "User has cancelled the migration.";
- std::move(cancel_callback_).Run();
+ migrator_->Cancel();
}
} else {
BaseScreen::OnUserAction(action_id);
@@ -138,18 +151,13 @@
return wake_lock_.get();
}
-void LacrosDataMigrationScreen::SetMigratorDelegateForTesting(
- std::unique_ptr<MigratorDelegate> migrator_delegate) {
- migrator_delegate_ = std::move(migrator_delegate);
-}
-
-void LacrosDataMigrationScreen::SetUserIdHashForTesting(
- const std::string& user_id_hash) {
- user_id_hash_ = user_id_hash;
-}
-
void LacrosDataMigrationScreen::SetSkipPostShowButtonForTesting(bool value) {
skip_post_show_button_for_testing_ = value;
}
+void LacrosDataMigrationScreen::SetMigratorForTesting(
+ std::unique_ptr<BrowserDataMigrator> migrator) {
+ migrator_ = std::move(migrator);
+}
+
} // namespace ash
diff --git a/chrome/browser/ash/login/screens/lacros_data_migration_screen.h b/chrome/browser/ash/login/screens/lacros_data_migration_screen.h
index a3f063c0..4793699 100644
--- a/chrome/browser/ash/login/screens/lacros_data_migration_screen.h
+++ b/chrome/browser/ash/login/screens/lacros_data_migration_screen.h
@@ -6,6 +6,7 @@
#define CHROME_BROWSER_ASH_LOGIN_SCREENS_LACROS_DATA_MIGRATION_SCREEN_H_
#include "base/callback_forward.h"
+#include "chrome/browser/ash/crosapi/browser_data_migrator.h"
#include "chrome/browser/ash/login/screens/base_screen.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "services/device/public/mojom/wake_lock.mojom.h"
@@ -18,17 +19,6 @@
// directory. The screen is shown during login.
class LacrosDataMigrationScreen : public BaseScreen {
public:
- // MigratorDelegate initiates the migration. A fake migrator delegate can be
- // set for testing.
- class MigratorDelegate {
- public:
- // Calls the actual migrator method `ash::BrowserDataMigrator::Migrate()`.
- virtual base::OnceClosure Migrate(
- const std::string& user_id_hash,
- const base::RepeatingCallback<void(int)>& progress_callback) = 0;
- virtual ~MigratorDelegate() = default;
- };
-
explicit LacrosDataMigrationScreen(LacrosDataMigrationScreenView* view);
~LacrosDataMigrationScreen() override;
LacrosDataMigrationScreen(const LacrosDataMigrationScreen&) = delete;
@@ -51,17 +41,13 @@
// name on `LacrosDataMigrationScreenView`.
void ShowSkipButton();
+ // Set `migrator_` for testing.
+ void SetMigratorForTesting(std::unique_ptr<BrowserDataMigrator> migrator);
+
// Sets `skip_post_show_button_for_testing_` for testing. Setting this to true
// prevents `ShowSkipButton()` from being posted.
void SetSkipPostShowButtonForTesting(bool value);
- // Set `migrator_delegate_` for testing.
- void SetMigratorDelegateForTesting(
- std::unique_ptr<MigratorDelegate> migrator_delegate);
-
- // Set `user_id_hash_` for testing.
- void SetUserIdHashForTesting(const std::string& user_id_hash);
-
private:
// BaseScreen:
void ShowImpl() override;
@@ -73,11 +59,10 @@
mojo::Remote<device::mojom::WakeLock> wake_lock_;
LacrosDataMigrationScreenView* view_;
+ std::unique_ptr<BrowserDataMigrator> migrator_;
// Callback to cancel migration. Stores the return value from
// `migrator_delegate->Migrate()`.
base::OnceClosure cancel_callback_;
- std::unique_ptr<MigratorDelegate> migrator_delegate_;
- std::string user_id_hash_;
bool skip_post_show_button_for_testing_ = false;
base::WeakPtrFactory<LacrosDataMigrationScreen> weak_factory_{this};
};
diff --git a/chrome/browser/ash/login/screens/lacros_data_migration_screen_browsertest.cc b/chrome/browser/ash/login/screens/lacros_data_migration_screen_browsertest.cc
index 7b8f55b..abd45b4f 100644
--- a/chrome/browser/ash/login/screens/lacros_data_migration_screen_browsertest.cc
+++ b/chrome/browser/ash/login/screens/lacros_data_migration_screen_browsertest.cc
@@ -27,16 +27,12 @@
constexpr char kLacrosDataMigrationId[] = "lacros-data-migration";
const test::UIPath kSkipButton = {kLacrosDataMigrationId, "cancelButton"};
-class FakeMigrator : public LacrosDataMigrationScreen::MigratorDelegate {
+class FakeMigrator : public BrowserDataMigrator {
public:
- base::OnceClosure Migrate(
- const std::string& user_id_hash,
- const base::RepeatingCallback<void(int)>& progress_callback) override {
- return base::BindOnce([](bool* cancel_called) { *cancel_called = true; },
- &cancel_called_);
- }
+ // BrowserDataMigrator overrides.
+ void Migrate() override {}
+ void Cancel() override { cancel_called_ = true; }
- // Checks if the returned function from `Migrate()` is called.
bool IsCancelCalled() { return cancel_called_; }
private:
@@ -69,9 +65,8 @@
WizardController::default_controller()->GetScreen(
LacrosDataMigrationScreenView::kScreenId));
fake_migrator_ = new FakeMigrator();
- lacros_data_migration_screen->SetMigratorDelegateForTesting(
+ lacros_data_migration_screen->SetMigratorForTesting(
base::WrapUnique(fake_migrator_));
- lacros_data_migration_screen->SetUserIdHashForTesting("user");
lacros_data_migration_screen->SetSkipPostShowButtonForTesting(true);
OobeBaseTest::SetUpOnMainThread();
}
diff --git a/chrome/browser/ash/login/session/user_session_manager.cc b/chrome/browser/ash/login/session/user_session_manager.cc
index b48058d..2459977 100644
--- a/chrome/browser/ash/login/session/user_session_manager.cc
+++ b/chrome/browser/ash/login/session/user_session_manager.cc
@@ -2205,7 +2205,7 @@
// Call this before `RestartToApplyPerSessionFlagsIfNeed()` in the login
// process.
- ash::BrowserDataMigrator::ClearMigrationStep(
+ ash::BrowserDataMigratorImpl::ClearMigrationStep(
g_browser_process->local_state());
if (RestartToApplyPerSessionFlagsIfNeed(profile, false))
@@ -2213,8 +2213,8 @@
const user_manager::User* user =
chromeos::ProfileHelper::Get()->GetUserByProfile(profile);
- if (ash::BrowserDataMigrator::MaybeRestartToMigrate(user->GetAccountId(),
- user->username_hash())) {
+ if (ash::BrowserDataMigratorImpl::MaybeRestartToMigrate(
+ user->GetAccountId(), user->username_hash())) {
LOG(WARNING) << "Restarting chrome to run profile migration.";
return;
}
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 3aa33b0..043a224 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -1016,7 +1016,7 @@
ash::ChromeUserManagerImpl::RegisterPrefs(registry);
crosapi::browser_util::RegisterLocalStatePrefs(registry);
chromeos::CupsPrintersManager::RegisterLocalStatePrefs(registry);
- ash::BrowserDataMigrator::RegisterLocalStatePrefs(registry);
+ ash::BrowserDataMigratorImpl::RegisterLocalStatePrefs(registry);
chromeos::bluetooth_config::BluetoothPowerControllerImpl::
RegisterLocalStatePrefs(registry);
chromeos::bluetooth_config::DeviceNameManagerImpl::RegisterLocalStatePrefs(
diff --git a/chrome/browser/ui/webui/flags/flags_ui_handler.cc b/chrome/browser/ui/webui/flags/flags_ui_handler.cc
index 043f66a..2ea1a7b 100644
--- a/chrome/browser/ui/webui/flags/flags_ui_handler.cc
+++ b/chrome/browser/ui/webui/flags/flags_ui_handler.cc
@@ -190,7 +190,7 @@
// 2. User logs in and migration is completed.
// 3. User disabled lacros in session.
// 4. User re-enables lacros in session.
- ash::BrowserDataMigrator::ClearMigrationStep(
+ ash::BrowserDataMigratorImpl::ClearMigrationStep(
g_browser_process->local_state());
#endif
chrome::AttemptRestart();
