Skip to content

Commit

Permalink
[M102] Fix tast lacros.Migrate.copy.
Browse files Browse the repository at this point in the history
crrev.com/c/3556846 targeting MoveMigrator removed "Preferences"
from the list of files to be copied thus CopyMigrator stopped copying
"Preferences" to Lacros causing a regression. This patch
makes CopyMigrator copy "Preferences" to fix the issue.

(cherry picked from commit 4747ba5)

Test: tast run lacros.Migrate.*
Bug: 1316590
Change-Id: I932366d4ded08b7bbd054a8effabf34575f2d96a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3589435
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Yuta Hijikata <ythjkt@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#994023}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3601889
Cr-Commit-Position: refs/branch-heads/5005@{#100}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
  • Loading branch information
Yuta Hijikata authored and Chromium LUCI CQ committed Apr 22, 2022
1 parent ca89d8e commit 3cd792e
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 14 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/ash/crosapi/browser_data_migrator.cc
Expand Up @@ -49,7 +49,7 @@ uint64_t DiskCheck(const base::FilePath& profile_data_dir) {
TargetItems lacros_items =
GetTargetItems(profile_data_dir, ItemType::kLacros);
TargetItems need_copy_items =
GetTargetItems(profile_data_dir, ItemType::kNeedCopy);
GetTargetItems(profile_data_dir, ItemType::kNeedCopyForMove);
TargetItems deletable_items =
GetTargetItems(profile_data_dir, ItemType::kDeletable);

Expand Down
9 changes: 6 additions & 3 deletions chrome/browser/ash/crosapi/browser_data_migrator_util.cc
Expand Up @@ -236,8 +236,11 @@ TargetItems GetTargetItems(const base::FilePath& original_profile_dir,
case ItemType::kDeletable:
target_paths = base::span<const char* const>(kDeletablePaths);
break;
case ItemType::kNeedCopy:
target_paths = base::span<const char* const>(kNeedCopyDataPaths);
case ItemType::kNeedCopyForMove:
target_paths = base::span<const char* const>(kNeedCopyForMoveDataPaths);
break;
case ItemType::kNeedCopyForCopy:
target_paths = base::span<const char* const>(kNeedCopyForCopyDataPaths);
break;
default:
NOTREACHED();
Expand Down Expand Up @@ -501,7 +504,7 @@ void DryRunToCollectUMA(const base::FilePath& profile_data_dir) {
TargetItems lacros_items =
GetTargetItems(profile_data_dir, ItemType::kLacros);
TargetItems need_copy_items =
GetTargetItems(profile_data_dir, ItemType::kNeedCopy);
GetTargetItems(profile_data_dir, ItemType::kNeedCopyForMove);
TargetItems remain_in_ash_items =
GetTargetItems(profile_data_dir, ItemType::kRemainInAsh);
TargetItems deletable_items =
Expand Down
14 changes: 11 additions & 3 deletions chrome/browser/ash/crosapi/browser_data_migrator_util.h
Expand Up @@ -183,9 +183,14 @@ constexpr const char* const kLacrosDataPaths[]{
// The base names of files/dirs that are required by both ash and lacros and
// thus should be copied to lacros while keeping the original files/dirs in ash
// data dir.
constexpr const char* const kNeedCopyDataPaths[]{
constexpr const char* const kNeedCopyForMoveDataPaths[]{
"DNR Extension Rules", "Extension Cookies", "Policy", "shared_proto_db"};

// The same as `kNeedCopyDataPathsForMove` + "Preferences".
constexpr const char* const kNeedCopyForCopyDataPaths[]{
"DNR Extension Rules", "Extension Cookies", "Policy", "Preferences",
"shared_proto_db"};

// List of extension ids to be kept in Ash.
// TODO(crbug.com/1302613): make sure this is the complete list.
constexpr const char* const kExtensionsAshOnly[] = {
Expand Down Expand Up @@ -337,8 +342,11 @@ struct TargetItems {
enum class ItemType {
kLacros = 0, // Item that should be moved to lacros profile directory.
kRemainInAsh = 1, // Item that should remain in ash.
kNeedCopy = 2, // Item that should be copied to lacros.
kDeletable = 3, // Item that can be deleted to free up space i.e. cache.
kNeedCopyForMove =
2, // Item that should be copied to lacros during move migration.
kNeedCopyForCopy = 3, // Item that should be copied to lacros during copy
// migration. This is kNeedCopyForMove + "Preferences".
kDeletable = 4, // Item that can be deleted to free up space i.e. cache.
};

// It enumerates the file/dirs in the given directory and returns items of
Expand Down
Expand Up @@ -119,7 +119,7 @@ TEST(BrowserDataMigratorUtilTest, NoPathOverlaps) {
base::span<const char* const> deletable_paths =
base::make_span(kDeletablePaths);
base::span<const char* const> common_data_paths =
base::make_span(kNeedCopyDataPaths);
base::make_span(kNeedCopyForMoveDataPaths);

std::vector<base::span<const char* const>> paths_groups{
remain_in_ash_paths, lacros_data_paths, deletable_paths,
Expand Down Expand Up @@ -569,7 +569,7 @@ TEST_F(BrowserDataMigratorUtilWithTargetsTest, GetTargetItems) {
{profile_data_dir_.Append(kPolicyDataPath), kTextFileSize,
TargetItem::ItemType::kFile}};
TargetItems need_copy_items =
GetTargetItems(profile_data_dir_, ItemType::kNeedCopy);
GetTargetItems(profile_data_dir_, ItemType::kNeedCopyForMove);
EXPECT_EQ(need_copy_items.total_size, kTextFileSize);
ASSERT_EQ(need_copy_items.items.size(), expected_need_copy_items.size());
EXPECT_EQ(need_copy_items.items[0], expected_need_copy_items[0]);
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ash/crosapi/copy_migrator.cc
Expand Up @@ -83,7 +83,7 @@ BrowserDataMigratorImpl::MigrationResult CopyMigrator::MigrateInternal(
browser_data_migrator_util::TargetItems need_copy_items =
browser_data_migrator_util::GetTargetItems(
original_profile_dir,
browser_data_migrator_util::ItemType::kNeedCopy);
browser_data_migrator_util::ItemType::kNeedCopyForCopy);
browser_data_migrator_util::TargetItems lacros_items =
browser_data_migrator_util::GetTargetItems(
original_profile_dir, browser_data_migrator_util::ItemType::kLacros);
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ash/crosapi/copy_migrator_unittest.cc
Expand Up @@ -101,7 +101,7 @@ TEST_F(CopyMigratorTest, SetupTmpDir) {
from_dir_, browser_data_migrator_util::ItemType::kLacros);
browser_data_migrator_util::TargetItems need_copy_items =
browser_data_migrator_util::GetTargetItems(
from_dir_, browser_data_migrator_util::ItemType::kNeedCopy);
from_dir_, browser_data_migrator_util::ItemType::kNeedCopyForCopy);
FakeMigrationProgressTracker progress_tracker;
EXPECT_TRUE(CopyMigrator::SetupTmpDir(lacros_items, need_copy_items, tmp_dir,
cancel_flag.get(), &progress_tracker));
Expand Down Expand Up @@ -141,7 +141,7 @@ TEST_F(CopyMigratorTest, CancelSetupTmpDir) {
from_dir_, browser_data_migrator_util::ItemType::kLacros);
browser_data_migrator_util::TargetItems need_copy_items =
browser_data_migrator_util::GetTargetItems(
from_dir_, browser_data_migrator_util::ItemType::kNeedCopy);
from_dir_, browser_data_migrator_util::ItemType::kNeedCopyForCopy);

// Set cancel_flag to cancel migrationl.
cancel_flag->Set();
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ash/crosapi/move_migrator.cc
Expand Up @@ -274,7 +274,7 @@ MoveMigrator::TaskResult MoveMigrator::PreMigrationCleanUp(
browser_data_migrator_util::TargetItems need_copy_items =
browser_data_migrator_util::GetTargetItems(
original_profile_dir,
browser_data_migrator_util::ItemType::kNeedCopy);
browser_data_migrator_util::ItemType::kNeedCopyForMove);

const int64_t extra_bytes_required_to_be_freed =
browser_data_migrator_util::ExtraBytesRequiredToBeFreed(
Expand Down Expand Up @@ -343,7 +343,7 @@ MoveMigrator::TaskResult MoveMigrator::SetupLacrosDir(
browser_data_migrator_util::TargetItems need_copy_items =
browser_data_migrator_util::GetTargetItems(
original_profile_dir,
browser_data_migrator_util::ItemType::kNeedCopy);
browser_data_migrator_util::ItemType::kNeedCopyForMove);

progress_tracker->SetTotalSizeToCopy(need_copy_items.total_size);

Expand Down

0 comments on commit 3cd792e

Please sign in to comment.