Move ContainerId pref logic to guest_os
Bug: 1336460
Change-Id: If26d0f4639c9664dc8fe984187772f9917e0259e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3702992
Reviewed-by: Nicholas Verne <[email protected]>
Reviewed-by: Lei Zhang <[email protected]>
Commit-Queue: Joel Hockey <[email protected]>
Reviewed-by: Xiaohui Chen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1015205}
diff --git a/chrome/browser/ash/crostini/crostini_installer.cc b/chrome/browser/ash/crostini/crostini_installer.cc
index 5224430b..c948f4e 100644
--- a/chrome/browser/ash/crostini/crostini_installer.cc
+++ b/chrome/browser/ash/crostini/crostini_installer.cc
@@ -229,13 +229,6 @@
base::BindOnce(&CrostiniInstaller::OnAvailableDiskSpace,
weak_ptr_factory_.GetWeakPtr()));
- // The default value of kCrostiniContainers is set to migrate existing
- // crostini users who don't have the pref set. If crostini is being installed,
- // then we know the user must not actually have any containers yet, so we set
- // this pref to the empty list.
- profile_->GetPrefs()->Set(crostini::prefs::kCrostiniContainers,
- base::Value(base::Value::Type::LIST));
-
// Reset mic permissions, we don't want it to persist across
// re-installation.
profile_->GetPrefs()->SetBoolean(prefs::kCrostiniMicAllowed, false);
diff --git a/chrome/browser/ash/crostini/crostini_manager.cc b/chrome/browser/ash/crostini/crostini_manager.cc
index a85028b..1d260eb0 100644
--- a/chrome/browser/ash/crostini/crostini_manager.cc
+++ b/chrome/browser/ash/crostini/crostini_manager.cc
@@ -43,6 +43,7 @@
#include "chrome/browser/ash/crostini/throttle/crostini_throttle.h"
#include "chrome/browser/ash/drive/drive_integration_service.h"
#include "chrome/browser/ash/file_manager/path_util.h"
+#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
#include "chrome/browser/ash/guest_os/guest_os_share_path.h"
#include "chrome/browser/ash/guest_os/guest_os_stability_monitor.h"
#include "chrome/browser/ash/guest_os/public/guest_os_service.h"
@@ -1061,9 +1062,11 @@
ContainerOsVersion version = VersionFromOsRelease(os_release);
// Store the os release version in prefs. We can use this value to decide if
// an upgrade can be offered.
- UpdateContainerPref(profile_, container_id, prefs::kContainerOsVersionKey,
+ UpdateContainerPref(profile_, container_id,
+ guest_os::prefs::kContainerOsVersionKey,
base::Value(static_cast<int>(version)));
- UpdateContainerPref(profile_, container_id, prefs::kContainerOsPrettyNameKey,
+ UpdateContainerPref(profile_, container_id,
+ guest_os::prefs::kContainerOsPrettyNameKey,
base::Value(os_release.pretty_name()));
absl::optional<ContainerOsVersion> old_version;
@@ -1151,7 +1154,7 @@
} else {
// Check prefs instead.
const base::Value* value = GetContainerPrefValue(
- profile_, container_id, prefs::kContainerOsVersionKey);
+ profile_, container_id, guest_os::prefs::kContainerOsVersionKey);
if (value) {
version = static_cast<ContainerOsVersion>(value->GetInt());
}
@@ -1250,7 +1253,7 @@
// which is fine, since e.g. force-quitting a running VM because policy
// changed isn't something we're going to do.
if (crostini::CrostiniFeatures::Get()->IsEnabled(profile_)) {
- for (const auto& container : GetContainers(profile_)) {
+ for (const auto& container : guest_os::GetContainers(profile_)) {
RegisterContainer(container);
}
}
diff --git a/chrome/browser/ash/crostini/crostini_manager_unittest.cc b/chrome/browser/ash/crostini/crostini_manager_unittest.cc
index 3d9211e..779149da 100644
--- a/chrome/browser/ash/crostini/crostini_manager_unittest.cc
+++ b/chrome/browser/ash/crostini/crostini_manager_unittest.cc
@@ -22,6 +22,7 @@
#include "chrome/browser/ash/crostini/crostini_types.mojom-shared.h"
#include "chrome/browser/ash/crostini/crostini_util.h"
#include "chrome/browser/ash/crostini/fake_crostini_features.h"
+#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
#include "chrome/browser/ash/guest_os/public/guest_os_service.h"
#include "chrome/browser/ash/guest_os/public/guest_os_wayland_server.h"
#include "chrome/browser/ash/login/users/fake_chrome_user_manager.h"
@@ -711,20 +712,20 @@
TEST_F(CrostiniManagerTest, RegisterContainerPrefWhenContainerCreated) {
const base::Value* pref =
- profile_->GetPrefs()->GetList(crostini::prefs::kCrostiniContainers);
+ profile_->GetPrefs()->GetList(guest_os::prefs::kGuestOsContainers);
EXPECT_EQ(pref->GetList().size(), 0);
crostini_manager()->CreateLxdContainer(
container_id(), absl::nullopt, absl::nullopt,
base::BindOnce(&ExpectCrostiniResult, run_loop()->QuitClosure(),
CrostiniResult::SUCCESS));
run_loop()->Run();
- pref = profile_->GetPrefs()->GetList(crostini::prefs::kCrostiniContainers);
+ pref = profile_->GetPrefs()->GetList(guest_os::prefs::kGuestOsContainers);
EXPECT_EQ(pref->GetList().size(), 1);
}
TEST_F(CrostiniManagerTest, RegisterContainerPrefWhenContainerExists) {
const base::Value* pref =
- profile_->GetPrefs()->GetList(crostini::prefs::kCrostiniContainers);
+ profile_->GetPrefs()->GetList(guest_os::prefs::kGuestOsContainers);
EXPECT_EQ(pref->GetList().size(), 0);
vm_tools::cicerone::CreateLxdContainerResponse response;
response.set_status(vm_tools::cicerone::CreateLxdContainerResponse::EXISTS);
@@ -734,7 +735,7 @@
base::BindOnce(&ExpectCrostiniResult, run_loop()->QuitClosure(),
CrostiniResult::SUCCESS));
run_loop()->Run();
- pref = profile_->GetPrefs()->GetList(crostini::prefs::kCrostiniContainers);
+ pref = profile_->GetPrefs()->GetList(guest_os::prefs::kGuestOsContainers);
EXPECT_EQ(pref->GetList().size(), 1);
}
@@ -1561,7 +1562,7 @@
// The data for this container should also be stored in prefs.
const base::Value* os_release_pref_value = GetContainerPrefValue(
- profile(), container_id(), prefs::kContainerOsVersionKey);
+ profile(), container_id(), guest_os::prefs::kContainerOsVersionKey);
EXPECT_NE(os_release_pref_value, nullptr);
EXPECT_EQ(os_release_pref_value->GetInt(),
static_cast<int>(ContainerOsVersion::kDebianBuster));
diff --git a/chrome/browser/ash/crostini/crostini_port_forwarder.cc b/chrome/browser/ash/crostini/crostini_port_forwarder.cc
index 939e8fd..0da041d 100644
--- a/chrome/browser/ash/crostini/crostini_port_forwarder.cc
+++ b/chrome/browser/ash/crostini/crostini_port_forwarder.cc
@@ -12,6 +12,7 @@
#include "chrome/browser/ash/crostini/crostini_manager.h"
#include "chrome/browser/ash/crostini/crostini_pref_names.h"
#include "chrome/browser/ash/crostini/crostini_util.h"
+#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
#include "chrome/browser/profiles/profile.h"
#include "chromeos/dbus/permission_broker/permission_broker_client.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
@@ -100,8 +101,9 @@
new_port_metadata.SetIntKey(kPortProtocolKey,
static_cast<int>(key.protocol_type));
new_port_metadata.SetStringKey(kPortLabelKey, label);
- new_port_metadata.SetStringKey(prefs::kVmKey, key.container_id.vm_name);
- new_port_metadata.SetStringKey(prefs::kContainerKey,
+ new_port_metadata.SetStringKey(guest_os::prefs::kVmKey,
+ key.container_id.vm_name);
+ new_port_metadata.SetStringKey(guest_os::prefs::kContainerKey,
key.container_id.container_name);
all_ports->Append(std::move(new_port_metadata));
}
diff --git a/chrome/browser/ash/crostini/crostini_pref_names.cc b/chrome/browser/ash/crostini/crostini_pref_names.cc
index 48d2b52..336190bf4 100644
--- a/chrome/browser/ash/crostini/crostini_pref_names.cc
+++ b/chrome/browser/ash/crostini/crostini_pref_names.cc
@@ -22,15 +22,8 @@
// List of USB devices with their system guid, a name/description and their
// enabled state for use with Crostini.
const char kCrostiniSharedUsbDevices[] = "crostini.shared_usb_devices";
-const char kCrostiniContainers[] = "crostini.containers";
// Dictionary of terminal UI settings such as font style, colors, etc.
const char kCrostiniTerminalSettings[] = "crostini.terminal_settings";
-const char kVmKey[] = "vm_name";
-const char kContainerKey[] = "container_name";
-const char kContainerOsVersionKey[] = "container_os_version";
-const char kContainerOsPrettyNameKey[] = "container_os_pretty_name";
-// SkColor used to assign badges to apps associated with this container.
-const char kContainerColorKey[] = "badge_color";
// Boolean preferences indicating whether Crostini is allowed to use mic.
const char kCrostiniMicAllowed[] = "crostini.mic_allowed";
@@ -102,7 +95,6 @@
registry->RegisterListPref(kCrostiniSharedUsbDevices);
registry->RegisterBooleanPref(kCrostiniMicAllowed, false);
registry->RegisterBooleanPref(kTerminalSshAllowedByPolicy, true);
- registry->RegisterListPref(kCrostiniContainers);
registry->RegisterBooleanPref(crostini::prefs::kReportCrostiniUsageEnabled,
false);
registry->RegisterStringPref(kCrostiniLastLaunchTerminaComponentVersion,
diff --git a/chrome/browser/ash/crostini/crostini_pref_names.h b/chrome/browser/ash/crostini/crostini_pref_names.h
index 43371724..1dea525 100644
--- a/chrome/browser/ash/crostini/crostini_pref_names.h
+++ b/chrome/browser/ash/crostini/crostini_pref_names.h
@@ -19,13 +19,7 @@
extern const char kCrostiniEnabled[];
extern const char kCrostiniSharedUsbDevices[];
-extern const char kCrostiniContainers[];
extern const char kCrostiniTerminalSettings[];
-extern const char kVmKey[];
-extern const char kContainerKey[];
-extern const char kContainerOsVersionKey[];
-extern const char kContainerOsPrettyNameKey[];
-extern const char kContainerColorKey[];
extern const char kCrostiniMicAllowed[];
extern const char kUserCrostiniAllowedByPolicy[];
diff --git a/chrome/browser/ash/crostini/crostini_remover.cc b/chrome/browser/ash/crostini/crostini_remover.cc
index 784a1355..bb576ce 100644
--- a/chrome/browser/ash/crostini/crostini_remover.cc
+++ b/chrome/browser/ash/crostini/crostini_remover.cc
@@ -76,8 +76,7 @@
profile_->GetPrefs()->SetBoolean(prefs::kCrostiniEnabled, false);
profile_->GetPrefs()->ClearPref(prefs::kCrostiniLastDiskSize);
- profile_->GetPrefs()->Set(prefs::kCrostiniContainers,
- base::Value(base::Value::Type::LIST));
+ guest_os::RemoveVmFromPrefs(profile_, kCrostiniDefaultVmName);
profile_->GetPrefs()->ClearPref(prefs::kCrostiniDefaultContainerConfigured);
std::move(callback_).Run(CrostiniResult::SUCCESS);
}
diff --git a/chrome/browser/ash/crostini/crostini_util.cc b/chrome/browser/ash/crostini/crostini_util.cc
index bf0a67d..f37ac6f 100644
--- a/chrome/browser/ash/crostini/crostini_util.cc
+++ b/chrome/browser/ash/crostini/crostini_util.cc
@@ -22,6 +22,7 @@
#include "chrome/browser/ash/file_manager/path_util.h"
#include "chrome/browser/ash/guest_os/guest_os_mime_types_service.h"
#include "chrome/browser/ash/guest_os/guest_os_mime_types_service_factory.h"
+#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
#include "chrome/browser/ash/guest_os/guest_os_registry_service.h"
#include "chrome/browser/ash/guest_os/guest_os_registry_service_factory.h"
#include "chrome/browser/ash/guest_os/guest_os_share_path.h"
@@ -359,80 +360,18 @@
return base::FilePath("/mnt/chromeos");
}
-namespace {
-
-bool MatchContainerDict(const base::Value& dict,
- const guest_os::GuestId& container_id) {
- const std::string* vm_name = dict.FindStringKey(prefs::kVmKey);
- const std::string* container_name = dict.FindStringKey(prefs::kContainerKey);
- return (vm_name && *vm_name == container_id.vm_name) &&
- (container_name && *container_name == container_id.container_name);
-}
-
-} // namespace
-
-void RemoveDuplicateContainerEntries(PrefService* prefs) {
- ListPrefUpdate updater(prefs, crostini::prefs::kCrostiniContainers);
-
- std::set<guest_os::GuestId> seen_containers;
- auto& containers = updater->GetList();
- for (auto it = containers.begin(); it != containers.end();) {
- guest_os::GuestId containerId(*it);
- if (seen_containers.find(containerId) == seen_containers.end()) {
- seen_containers.insert(containerId);
- it++;
- } else {
- it = containers.erase(it);
- }
- }
-}
-
-std::vector<guest_os::GuestId> GetContainers(Profile* profile) {
- std::vector<guest_os::GuestId> result;
- const base::Value::List& container_list =
- profile->GetPrefs()
- ->GetList(crostini::prefs::kCrostiniContainers)
- ->GetList();
- for (const auto& container : container_list) {
- guest_os::GuestId id(container);
- if (!id.vm_name.empty() && !id.container_name.empty()) {
- result.push_back(std::move(id));
- }
- }
- return result;
-}
-
void AddNewLxdContainerToPrefs(Profile* profile,
const guest_os::GuestId& container_id) {
- ListPrefUpdate updater(profile->GetPrefs(),
- crostini::prefs::kCrostiniContainers);
- auto it = std::find_if(
- updater->GetListDeprecated().begin(), updater->GetListDeprecated().end(),
- [&](const auto& dict) { return MatchContainerDict(dict, container_id); });
- if (it != updater->GetListDeprecated().end()) {
- return;
- }
-
- base::Value new_container(base::Value::Type::DICTIONARY);
- new_container.SetKey(prefs::kVmKey, base::Value(container_id.vm_name));
- new_container.SetKey(prefs::kContainerKey,
- base::Value(container_id.container_name));
- new_container.SetIntKey(prefs::kContainerOsVersionKey,
- static_cast<int>(ContainerOsVersion::kUnknown));
- new_container.SetStringKey(prefs::kContainerOsPrettyNameKey, "");
- updater->Append(std::move(new_container));
+ base::Value::Dict properties;
+ properties.Set(guest_os::prefs::kContainerOsVersionKey,
+ static_cast<int>(ContainerOsVersion::kUnknown));
+ properties.Set(guest_os::prefs::kContainerOsPrettyNameKey, "");
+ guest_os::AddContainerToPrefs(profile, container_id, std::move(properties));
}
void RemoveLxdContainerFromPrefs(Profile* profile,
const guest_os::GuestId& container_id) {
- auto* pref_service = profile->GetPrefs();
- ListPrefUpdate updater(pref_service, crostini::prefs::kCrostiniContainers);
- updater->EraseListIter(
- std::find_if(updater->GetListDeprecated().begin(),
- updater->GetListDeprecated().end(), [&](const auto& dict) {
- return MatchContainerDict(dict, container_id);
- }));
-
+ guest_os::RemoveContainerFromPrefs(profile, container_id);
guest_os::GuestOsRegistryServiceFactory::GetForProfile(profile)
->ClearApplicationList(guest_os::VmType::TERMINA, container_id.vm_name,
container_id.container_name);
@@ -440,39 +379,10 @@
->ClearMimeTypes(container_id.vm_name, container_id.container_name);
}
-const base::Value* GetContainerPrefValue(Profile* profile,
- const guest_os::GuestId& container_id,
- const std::string& key) {
- const base::Value* containers =
- profile->GetPrefs()->GetList(crostini::prefs::kCrostiniContainers);
- if (!containers) {
- return nullptr;
- }
- for (const auto& dict : containers->GetListDeprecated()) {
- if (MatchContainerDict(dict, container_id))
- return dict.FindKey(key);
- }
- return nullptr;
-}
-
-void UpdateContainerPref(Profile* profile,
- const guest_os::GuestId& container_id,
- const std::string& key,
- base::Value value) {
- ListPrefUpdate updater(profile->GetPrefs(),
- crostini::prefs::kCrostiniContainers);
- auto it = std::find_if(
- updater->GetListDeprecated().begin(), updater->GetListDeprecated().end(),
- [&](const auto& dict) { return MatchContainerDict(dict, container_id); });
- if (it != updater->GetListDeprecated().end()) {
- it->SetKey(key, std::move(value));
- }
-}
-
SkColor GetContainerBadgeColor(Profile* profile,
const guest_os::GuestId& container_id) {
- const base::Value* badge_color_value =
- GetContainerPrefValue(profile, container_id, prefs::kContainerColorKey);
+ const base::Value* badge_color_value = GetContainerPrefValue(
+ profile, container_id, guest_os::prefs::kContainerColorKey);
if (badge_color_value) {
return badge_color_value->GetIfInt().value_or(SK_ColorTRANSPARENT);
} else {
@@ -483,8 +393,9 @@
void SetContainerBadgeColor(Profile* profile,
const guest_os::GuestId& container_id,
SkColor badge_color) {
- UpdateContainerPref(profile, container_id, prefs::kContainerColorKey,
- base::Value(static_cast<int>(badge_color)));
+ guest_os::UpdateContainerPref(profile, container_id,
+ guest_os::prefs::kContainerColorKey,
+ base::Value(static_cast<int>(badge_color)));
guest_os::GuestOsRegistryServiceFactory::GetForProfile(profile)
->ContainerBadgeColorChanged(container_id);
@@ -493,7 +404,7 @@
bool IsContainerVersionExpired(Profile* profile,
const guest_os::GuestId& container_id) {
auto* value = GetContainerPrefValue(profile, container_id,
- prefs::kContainerOsVersionKey);
+ guest_os::prefs::kContainerOsVersionKey);
if (!value)
return false;
@@ -581,23 +492,16 @@
}
bool ShouldStopVm(Profile* profile, const guest_os::GuestId& container_id) {
- bool is_last_container = true;
- base::Value::ConstListView containers =
- profile->GetPrefs()
- ->GetList(prefs::kCrostiniContainers)
- ->GetListDeprecated();
- for (const auto& dict : containers) {
- guest_os::GuestId container(dict);
+ for (const auto& container : guest_os::GetContainers(profile)) {
if (container.container_name != container_id.container_name &&
container.vm_name == container_id.vm_name) {
if (CrostiniManager::GetForProfile(profile)->GetContainerInfo(
container)) {
- is_last_container = false;
- break;
+ return false;
}
}
}
- return is_last_container;
+ return true;
}
} // namespace crostini
diff --git a/chrome/browser/ash/crostini/crostini_util.h b/chrome/browser/ash/crostini/crostini_util.h
index 8cbfcb1..9673bba 100644
--- a/chrome/browser/ash/crostini/crostini_util.h
+++ b/chrome/browser/ash/crostini/crostini_util.h
@@ -33,7 +33,6 @@
class Widget;
} // namespace views
-class PrefService;
class Profile;
namespace crostini {
@@ -162,12 +161,6 @@
const std::vector<LaunchArg>& args,
CrostiniSuccessCallback callback);
-// Remove duplicate containers in the existing kCrostiniContainers pref.
-void RemoveDuplicateContainerEntries(PrefService* prefs);
-
-// Returns a list of all containers in prefs.
-std::vector<guest_os::GuestId> GetContainers(Profile* profile);
-
// Add a newly created LXD container to the kCrostiniContainers pref
void AddNewLxdContainerToPrefs(Profile* profile,
const guest_os::GuestId& container_id);
@@ -182,17 +175,6 @@
// at |percent| way through.
std::u16string GetTimeRemainingMessage(base::TimeTicks start, int percent);
-// Returns a pref value stored for a specific container.
-const base::Value* GetContainerPrefValue(Profile* profile,
- const guest_os::GuestId& container_id,
- const std::string& key);
-
-// Sets a pref value for a specific container.
-void UpdateContainerPref(Profile* profile,
- const guest_os::GuestId& container_id,
- const std::string& key,
- base::Value value);
-
SkColor GetContainerBadgeColor(Profile* profile,
const guest_os::GuestId& container_id);
diff --git a/chrome/browser/ash/crostini/crostini_util_unittest.cc b/chrome/browser/ash/crostini/crostini_util_unittest.cc
index ce6856c..c85aa6b 100644
--- a/chrome/browser/ash/crostini/crostini_util_unittest.cc
+++ b/chrome/browser/ash/crostini/crostini_util_unittest.cc
@@ -5,11 +5,11 @@
#include "chrome/browser/ash/crostini/crostini_util.h"
#include "base/bind.h"
-#include "base/json/json_reader.h"
#include "base/run_loop.h"
#include "chrome/browser/ash/crostini/crostini_manager.h"
#include "chrome/browser/ash/crostini/crostini_pref_names.h"
#include "chrome/browser/ash/crostini/crostini_test_helper.h"
+#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
#include "chrome/browser/component_updater/fake_cros_component_manager.h"
#include "chrome/test/base/browser_process_platform_part_test_api_chromeos.h"
#include "chrome/test/base/scoped_testing_local_state.h"
@@ -125,43 +125,6 @@
run_loop_->Run();
}
-TEST_F(CrostiniUtilTest, DuplicateContainerNamesInPrefsAreRemoved) {
- guest_os::GuestId container1("test1", "test1");
- base::Value::Dict dictionary1 = container1.ToDictValue();
- dictionary1.Set(prefs::kContainerOsPrettyNameKey, "Test OS Name 1");
- dictionary1.Set(prefs::kContainerOsVersionKey, 1);
-
- guest_os::GuestId container2("test1", "test2");
- base::Value::Dict dictionary2 = container2.ToDictValue();
- dictionary2.Set(prefs::kContainerOsPrettyNameKey, "Test OS Name 2");
- dictionary2.Set(prefs::kContainerOsVersionKey, 2);
-
- guest_os::GuestId container3("test2", "test1");
- base::Value::Dict dictionary3 = container3.ToDictValue();
- dictionary3.Set(prefs::kContainerOsPrettyNameKey, "Test OS Name 3");
- dictionary3.Set(prefs::kContainerOsVersionKey, 3);
-
- base::Value::List containers;
- containers.Append(dictionary1.Clone());
- containers.Append(dictionary2.Clone());
- containers.Append(dictionary1.Clone());
- containers.Append(dictionary2.Clone());
- containers.Append(dictionary3.Clone());
-
- PrefService* prefs = profile_->GetPrefs();
- prefs->SetList(prefs::kCrostiniContainers, std::move(containers));
-
- RemoveDuplicateContainerEntries(prefs);
-
- const base::Value::List& result =
- prefs->Get(prefs::kCrostiniContainers)->GetList();
-
- ASSERT_EQ(result.size(), 3);
- EXPECT_EQ(result[0].GetDict(), dictionary1);
- EXPECT_EQ(result[1].GetDict(), dictionary2);
- EXPECT_EQ(result[2].GetDict(), dictionary3);
-}
-
TEST_F(CrostiniUtilTest, ShouldStopVm) {
CrostiniManager* manager = CrostiniManager::GetForProfile(profile_.get());
guest_os::GuestId containera("apple", "banana");
@@ -180,7 +143,7 @@
ASSERT_TRUE(manager->IsVmRunning("apple"));
ASSERT_TRUE(manager->IsVmRunning("potato"));
- profile_->GetPrefs()->SetList(prefs::kCrostiniContainers,
+ profile_->GetPrefs()->SetList(guest_os::prefs::kGuestOsContainers,
std::move(containers));
EXPECT_TRUE(ShouldStopVm(profile_.get(), containera));
@@ -202,22 +165,10 @@
ASSERT_TRUE(manager->IsVmRunning("apple"));
- profile_->GetPrefs()->SetList(prefs::kCrostiniContainers,
+ profile_->GetPrefs()->SetList(guest_os::prefs::kGuestOsContainers,
std::move(containers));
EXPECT_FALSE(ShouldStopVm(profile_.get(), containera));
}
-TEST_F(CrostiniUtilTest, GetContainers) {
- auto pref = base::JSONReader::Read(R"([
- {"vm_name": "vm1", "container_name": "c1"},
- {"vm_name": "vm2", "container_name": "c2"},
- {"vm_name": "vm3"}
- ])");
- ASSERT_TRUE(pref.has_value());
- profile_->GetPrefs()->Set(prefs::kCrostiniContainers, std::move(*pref));
- std::vector<guest_os::GuestId> expected = {guest_os::GuestId("vm1", "c1"),
- guest_os::GuestId("vm2", "c2")};
- EXPECT_EQ(GetContainers(profile_.get()), expected);
-}
} // namespace crostini
diff --git a/chrome/browser/ash/guest_os/guest_id.cc b/chrome/browser/ash/guest_os/guest_id.cc
index 4c981149..cc435d66 100644
--- a/chrome/browser/ash/guest_os/guest_id.cc
+++ b/chrome/browser/ash/guest_os/guest_id.cc
@@ -4,9 +4,33 @@
#include "chrome/browser/ash/guest_os/guest_id.h"
-#include "chrome/browser/ash/crostini/crostini_pref_names.h"
+#include <memory>
+#include <vector>
+
+#include "base/containers/contains.h"
+#include "base/no_destructor.h"
+#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/scoped_user_pref_update.h"
namespace guest_os {
+namespace {
+
+bool MatchContainerDict(const base::Value& dict, const GuestId& container_id) {
+ const std::string* vm_name = dict.FindStringKey(prefs::kVmKey);
+ const std::string* container_name = dict.FindStringKey(prefs::kContainerKey);
+ return (vm_name && *vm_name == container_id.vm_name) &&
+ (container_name && *container_name == container_id.container_name);
+}
+
+static const base::NoDestructor<std::vector<std::string>> kPropertiesAllowList{{
+ prefs::kContainerOsVersionKey,
+ prefs::kContainerOsPrettyNameKey,
+ prefs::kContainerColorKey,
+}};
+
+} // namespace
GuestId::GuestId(std::string vm_name, std::string container_name) noexcept
: vm_name(std::move(vm_name)), container_name(std::move(container_name)) {}
@@ -16,8 +40,8 @@
const std::string* vm = nullptr;
const std::string* container = nullptr;
if (dict != nullptr) {
- vm = dict->FindString(crostini::prefs::kVmKey);
- container = dict->FindString(crostini::prefs::kContainerKey);
+ vm = dict->FindString(prefs::kVmKey);
+ container = dict->FindString(prefs::kContainerKey);
}
vm_name = vm ? *vm : "";
container_name = container ? *container : "";
@@ -25,15 +49,15 @@
base::flat_map<std::string, std::string> GuestId::ToMap() const {
base::flat_map<std::string, std::string> extras;
- extras[crostini::prefs::kVmKey] = vm_name;
- extras[crostini::prefs::kContainerKey] = container_name;
+ extras[prefs::kVmKey] = vm_name;
+ extras[prefs::kContainerKey] = container_name;
return extras;
}
base::Value::Dict GuestId::ToDictValue() const {
base::Value::Dict dict;
- dict.Set(crostini::prefs::kVmKey, vm_name);
- dict.Set(crostini::prefs::kContainerKey, container_name);
+ dict.Set(prefs::kVmKey, vm_name);
+ dict.Set(prefs::kContainerKey, container_name);
return dict;
}
@@ -51,4 +75,107 @@
<< container_id.container_name << "\")";
}
+void RemoveDuplicateContainerEntries(PrefService* prefs) {
+ ListPrefUpdate updater(prefs, prefs::kGuestOsContainers);
+
+ std::set<GuestId> seen_containers;
+ auto& containers = updater->GetList();
+ for (auto it = containers.begin(); it != containers.end();) {
+ GuestId id(*it);
+ if (seen_containers.find(id) == seen_containers.end()) {
+ seen_containers.insert(id);
+ it++;
+ } else {
+ it = containers.erase(it);
+ }
+ }
+}
+
+std::vector<GuestId> GetContainers(Profile* profile) {
+ std::vector<GuestId> result;
+ const base::Value::List& container_list =
+ profile->GetPrefs()->GetList(prefs::kGuestOsContainers)->GetList();
+ for (const auto& container : container_list) {
+ guest_os::GuestId id(container);
+ if (!id.vm_name.empty() && !id.container_name.empty()) {
+ result.push_back(std::move(id));
+ }
+ }
+ return result;
+}
+
+void AddContainerToPrefs(Profile* profile,
+ const GuestId& container_id,
+ base::Value::Dict properties) {
+ ListPrefUpdate updater(profile->GetPrefs(), prefs::kGuestOsContainers);
+ auto it = std::find_if(
+ updater->GetListDeprecated().begin(), updater->GetListDeprecated().end(),
+ [&](const auto& dict) { return MatchContainerDict(dict, container_id); });
+ if (it != updater->GetListDeprecated().end()) {
+ return;
+ }
+
+ base::Value new_container(base::Value::Type::DICTIONARY);
+ new_container.SetKey(prefs::kVmKey, base::Value(container_id.vm_name));
+ new_container.SetKey(prefs::kContainerKey,
+ base::Value(container_id.container_name));
+ for (const auto item : properties) {
+ if (base::Contains(*kPropertiesAllowList, item.first)) {
+ new_container.SetKey(std::move(item.first), std::move(item.second));
+ }
+ }
+ updater->Append(std::move(new_container));
+}
+
+void RemoveContainerFromPrefs(Profile* profile, const GuestId& container_id) {
+ auto* pref_service = profile->GetPrefs();
+ ListPrefUpdate updater(pref_service, prefs::kGuestOsContainers);
+ updater->EraseListIter(
+ std::find_if(updater->GetListDeprecated().begin(),
+ updater->GetListDeprecated().end(), [&](const auto& dict) {
+ return MatchContainerDict(dict, container_id);
+ }));
+}
+
+void RemoveVmFromPrefs(Profile* profile, const std::string& vm_name) {
+ auto* pref_service = profile->GetPrefs();
+ ListPrefUpdate updater(pref_service, prefs::kGuestOsContainers);
+ updater->EraseListIter(std::find_if(
+ updater->GetListDeprecated().begin(), updater->GetListDeprecated().end(),
+ [&](const auto& dict) {
+ const std::string* dict_vm_name = dict.FindStringKey(prefs::kVmKey);
+ return dict_vm_name && *dict_vm_name == vm_name;
+ }));
+}
+
+const base::Value* GetContainerPrefValue(Profile* profile,
+ const GuestId& container_id,
+ const std::string& key) {
+ const base::Value* containers =
+ profile->GetPrefs()->GetList(prefs::kGuestOsContainers);
+ if (!containers) {
+ return nullptr;
+ }
+ for (const auto& dict : containers->GetListDeprecated()) {
+ if (MatchContainerDict(dict, container_id))
+ return dict.FindKey(key);
+ }
+ return nullptr;
+}
+
+void UpdateContainerPref(Profile* profile,
+ const GuestId& container_id,
+ const std::string& key,
+ base::Value value) {
+ ListPrefUpdate updater(profile->GetPrefs(), prefs::kGuestOsContainers);
+ auto it = std::find_if(
+ updater->GetListDeprecated().begin(), updater->GetListDeprecated().end(),
+ [&](const auto& dict) { return MatchContainerDict(dict, container_id); });
+ if (it != updater->GetListDeprecated().end()) {
+ if (base::Contains(*kPropertiesAllowList, key)) {
+ it->SetKey(key, std::move(value));
+ }
+ }
+}
+
} // namespace guest_os
diff --git a/chrome/browser/ash/guest_os/guest_id.h b/chrome/browser/ash/guest_os/guest_id.h
index 4fff43a..16a1f48 100644
--- a/chrome/browser/ash/guest_os/guest_id.h
+++ b/chrome/browser/ash/guest_os/guest_id.h
@@ -11,6 +11,9 @@
#include "base/containers/flat_map.h"
#include "base/values.h"
+class PrefService;
+class Profile;
+
namespace guest_os {
// A unique identifier for our guests.
@@ -33,6 +36,34 @@
std::ostream& operator<<(std::ostream& ostream, const GuestId& container_id);
+// Returns a list of all containers in prefs.
+std::vector<GuestId> GetContainers(Profile* profile);
+
+// Remove duplicate containers in the existing kGuestOsContainers pref.
+void RemoveDuplicateContainerEntries(PrefService* prefs);
+
+// Add a new container to the kGuestOsContainers pref
+void AddContainerToPrefs(Profile* profile,
+ const GuestId& container_id,
+ base::Value::Dict properties);
+
+// Remove a deleted container from the kGuestOsContainers pref.
+void RemoveContainerFromPrefs(Profile* profile, const GuestId& container_id);
+
+// Remove a deleted container from the kGuestOsContainers pref.
+void RemoveVmFromPrefs(Profile* profile, const std::string& vm_name);
+
+// Returns a pref value stored for a specific container.
+const base::Value* GetContainerPrefValue(Profile* profile,
+ const GuestId& container_id,
+ const std::string& key);
+
+// Sets a pref value for a specific container.
+void UpdateContainerPref(Profile* profile,
+ const GuestId& container_id,
+ const std::string& key,
+ base::Value value);
+
} // namespace guest_os
#endif // CHROME_BROWSER_ASH_GUEST_OS_GUEST_ID_H_
diff --git a/chrome/browser/ash/guest_os/guest_id_unittest.cc b/chrome/browser/ash/guest_os/guest_id_unittest.cc
index 3a45f29..9a7323c4 100644
--- a/chrome/browser/ash/guest_os/guest_id_unittest.cc
+++ b/chrome/browser/ash/guest_os/guest_id_unittest.cc
@@ -4,13 +4,21 @@
#include "chrome/browser/ash/guest_os/guest_id.h"
+#include "base/json/json_reader.h"
#include "base/values.h"
-#include "chrome/browser/ash/crostini/crostini_pref_names.h"
+#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/prefs/pref_service.h"
+#include "content/public/test/browser_task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace guest_os {
-using GuestIdTest = testing::Test;
+class GuestIdTest : public testing::Test {
+ protected:
+ content::BrowserTaskEnvironment task_environment_;
+ TestingProfile profile_;
+};
TEST_F(GuestIdTest, GuestIdEquality) {
auto container1 = GuestId{"test1", "test2"};
@@ -24,8 +32,8 @@
TEST_F(GuestIdTest, GuestIdFromDictValue) {
base::Value dict(base::Value::Type::DICT);
- dict.SetStringKey(crostini::prefs::kVmKey, "foo");
- dict.SetStringKey(crostini::prefs::kContainerKey, "bar");
+ dict.SetStringKey(prefs::kVmKey, "foo");
+ dict.SetStringKey(prefs::kContainerKey, "bar");
EXPECT_TRUE(GuestId(dict) == GuestId("foo", "bar"));
}
@@ -34,4 +42,53 @@
EXPECT_TRUE(GuestId(non_dict) == GuestId("", ""));
}
+TEST_F(GuestIdTest, DuplicateContainerNamesInPrefsAreRemoved) {
+ GuestId container1("test1", "test1");
+ base::Value::Dict dictionary1 = container1.ToDictValue();
+ dictionary1.Set(prefs::kContainerOsPrettyNameKey, "Test OS Name 1");
+ dictionary1.Set(prefs::kContainerOsVersionKey, 1);
+
+ GuestId container2("test1", "test2");
+ base::Value::Dict dictionary2 = container2.ToDictValue();
+ dictionary2.Set(prefs::kContainerOsPrettyNameKey, "Test OS Name 2");
+ dictionary2.Set(prefs::kContainerOsVersionKey, 2);
+
+ GuestId container3("test2", "test1");
+ base::Value::Dict dictionary3 = container3.ToDictValue();
+ dictionary3.Set(prefs::kContainerOsPrettyNameKey, "Test OS Name 3");
+ dictionary3.Set(prefs::kContainerOsVersionKey, 3);
+
+ base::Value::List containers;
+ containers.Append(dictionary1.Clone());
+ containers.Append(dictionary2.Clone());
+ containers.Append(dictionary1.Clone());
+ containers.Append(dictionary2.Clone());
+ containers.Append(dictionary3.Clone());
+
+ PrefService* prefs = profile_.GetPrefs();
+ prefs->SetList(prefs::kGuestOsContainers, std::move(containers));
+
+ RemoveDuplicateContainerEntries(prefs);
+
+ const base::Value::List& result =
+ prefs->Get(prefs::kGuestOsContainers)->GetList();
+
+ ASSERT_EQ(result.size(), 3);
+ EXPECT_EQ(result[0].GetDict(), dictionary1);
+ EXPECT_EQ(result[1].GetDict(), dictionary2);
+ EXPECT_EQ(result[2].GetDict(), dictionary3);
+}
+
+TEST_F(GuestIdTest, GetContainers) {
+ auto pref = base::JSONReader::Read(R"([
+ {"vm_name": "vm1", "container_name": "c1"},
+ {"vm_name": "vm2", "container_name": "c2"},
+ {"vm_name": "vm3"}
+ ])");
+ ASSERT_TRUE(pref.has_value());
+ profile_.GetPrefs()->Set(prefs::kGuestOsContainers, std::move(*pref));
+ std::vector<GuestId> expected = {GuestId("vm1", "c1"), GuestId("vm2", "c2")};
+ EXPECT_EQ(GetContainers(&profile_), expected);
+}
+
} // namespace guest_os
diff --git a/chrome/browser/ash/guest_os/guest_os_pref_names.cc b/chrome/browser/ash/guest_os/guest_os_pref_names.cc
index 5788f633..4db8b23 100644
--- a/chrome/browser/ash/guest_os/guest_os_pref_names.cc
+++ b/chrome/browser/ash/guest_os/guest_os_pref_names.cc
@@ -39,10 +39,20 @@
const char kAppInstallTimeKey[] = "install_time";
const char kAppLastLaunchTimeKey[] = "last_launch_time";
+// GuestOsContainerId
+const char kGuestOsContainers[] = "crostini.containers";
+const char kVmKey[] = "vm_name";
+const char kContainerKey[] = "container_name";
+const char kContainerOsVersionKey[] = "container_os_version";
+const char kContainerOsPrettyNameKey[] = "container_os_pretty_name";
+// SkColor used to assign badges to apps associated with this container.
+const char kContainerColorKey[] = "badge_color";
+
void RegisterProfilePrefs(PrefRegistrySimple* registry) {
registry->RegisterDictionaryPref(kGuestOSPathsSharedToVms);
registry->RegisterDictionaryPref(kGuestOsMimeTypes);
registry->RegisterDictionaryPref(kGuestOsRegistry);
+ registry->RegisterListPref(kGuestOsContainers);
}
} // namespace prefs
diff --git a/chrome/browser/ash/guest_os/guest_os_pref_names.h b/chrome/browser/ash/guest_os/guest_os_pref_names.h
index 731ea82..9716d1db 100644
--- a/chrome/browser/ash/guest_os/guest_os_pref_names.h
+++ b/chrome/browser/ash/guest_os/guest_os_pref_names.h
@@ -10,9 +10,13 @@
namespace guest_os {
namespace prefs {
+// GuestOsSharedPath
extern const char kGuestOSPathsSharedToVms[];
+// GuestOsMimeTypes
extern const char kGuestOsMimeTypes[];
+
+// GuestOsRegistry
extern const char kGuestOsRegistry[];
extern const char kAppDesktopFileIdKey[];
extern const char kAppVmTypeKey[];
@@ -33,6 +37,14 @@
extern const char kAppInstallTimeKey[];
extern const char kAppLastLaunchTimeKey[];
+// GuestOsContainerId
+extern const char kGuestOsContainers[];
+extern const char kVmKey[];
+extern const char kContainerKey[];
+extern const char kContainerOsVersionKey[];
+extern const char kContainerOsPrettyNameKey[];
+extern const char kContainerColorKey[];
+
void RegisterProfilePrefs(PrefRegistrySimple* registry);
} // namespace prefs
diff --git a/chrome/browser/extensions/api/settings_private/prefs_util.cc b/chrome/browser/extensions/api/settings_private/prefs_util.cc
index 9d57820..06ed6f3 100644
--- a/chrome/browser/extensions/api/settings_private/prefs_util.cc
+++ b/chrome/browser/extensions/api/settings_private/prefs_util.cc
@@ -554,7 +554,7 @@
settings_api::PrefType::PREF_TYPE_BOOLEAN;
(*s_allowlist)[crostini::prefs::kCrostiniSharedUsbDevices] =
settings_api::PrefType::PREF_TYPE_LIST;
- (*s_allowlist)[crostini::prefs::kCrostiniContainers] =
+ (*s_allowlist)[guest_os::prefs::kGuestOsContainers] =
settings_api::PrefType::PREF_TYPE_LIST;
(*s_allowlist)[crostini::prefs::kCrostiniPortForwarding] =
settings_api::PrefType::PREF_TYPE_LIST;
diff --git a/chrome/browser/extensions/api/terminal/terminal_private_api.cc b/chrome/browser/extensions/api/terminal/terminal_private_api.cc
index b4b9a59..4c44cf12 100644
--- a/chrome/browser/extensions/api/terminal/terminal_private_api.cc
+++ b/chrome/browser/extensions/api/terminal/terminal_private_api.cc
@@ -34,6 +34,7 @@
#include "chrome/browser/ash/crostini/crostini_terminal.h"
#include "chrome/browser/ash/crostini/crostini_util.h"
#include "chrome/browser/ash/guest_os/guest_id.h"
+#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/extensions/api/terminal/crostini_startup_status.h"
#include "chrome/browser/extensions/extension_service.h"
@@ -97,10 +98,10 @@
// Prefs that we read and observe.
static const base::NoDestructor<std::vector<std::string>> kPrefsReadAllowList{{
ash::prefs::kAccessibilitySpokenFeedbackEnabled,
- crostini::prefs::kCrostiniContainers,
crostini::prefs::kCrostiniEnabled,
crostini::prefs::kCrostiniTerminalSettings,
crostini::prefs::kTerminalSshAllowedByPolicy,
+ guest_os::prefs::kGuestOsContainers,
}};
void CloseTerminal(const std::string& terminal_id,
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 92a3788a..3132a0b 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -324,13 +324,13 @@
#include "chrome/browser/ash/crosapi/network_settings_service_ash.h"
#include "chrome/browser/ash/crostini/crostini_pref_names.h"
#include "chrome/browser/ash/crostini/crostini_terminal.h"
-#include "chrome/browser/ash/crostini/crostini_util.h"
#include "chrome/browser/ash/cryptauth/client_app_metadata_provider_service.h"
#include "chrome/browser/ash/cryptauth/cryptauth_device_id_provider_impl.h"
#include "chrome/browser/ash/customization/customization_document.h"
#include "chrome/browser/ash/file_system_provider/registry.h"
#include "chrome/browser/ash/first_run/first_run.h"
#include "chrome/browser/ash/floating_workspace/floating_workspace_util.h"
+#include "chrome/browser/ash/guest_os/guest_id.h"
#include "chrome/browser/ash/guest_os/guest_os_mime_types_service.h"
#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
#include "chrome/browser/ash/lock_screen_apps/state_controller.h"
@@ -1874,7 +1874,7 @@
#if BUILDFLAG(IS_CHROMEOS_ASH)
// Added 02/2022.
// TODO(crbug.com/1298250): Remove after M107.
- crostini::RemoveDuplicateContainerEntries(profile_prefs);
+ guest_os::RemoveDuplicateContainerEntries(profile_prefs);
#endif
// Added 03/2022
diff --git a/chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc b/chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc
index 5e1a6d7..dde8a0b 100644
--- a/chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc
+++ b/chrome/browser/ui/webui/settings/chromeos/crostini_handler.cc
@@ -19,6 +19,7 @@
#include "chrome/browser/ash/crostini/crostini_types.mojom.h"
#include "chrome/browser/ash/crostini/crostini_util.h"
#include "chrome/browser/ash/file_manager/path_util.h"
+#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/lifetime/application_lifetime.h"
#include "chrome/browser/profiles/profile.h"
@@ -205,7 +206,7 @@
// Observe changes to containers in general
pref_change_registrar_.Add(
- crostini::prefs::kCrostiniContainers,
+ guest_os::prefs::kGuestOsContainers,
base::BindRepeating(&CrostiniHandler::HandleRequestContainerInfo,
handler_weak_ptr_factory_.GetWeakPtr(),
base::Value::List()));
@@ -758,13 +759,7 @@
base::Value::List container_info_list;
- const base::Value::List& containers =
- profile_->GetPrefs()
- ->Get(crostini::prefs::kCrostiniContainers)
- ->GetList();
-
- for (const auto& dict : containers) {
- guest_os::GuestId container_id(dict);
+ for (const auto& container_id : guest_os::GetContainers(profile_)) {
base::Value::Dict container_info_value;
container_info_value.Set(kIdKey, container_id.ToDictValue());
auto info =
diff --git a/chrome/browser/ui/webui/settings/chromeos/crostini_section.cc b/chrome/browser/ui/webui/settings/chromeos/crostini_section.cc
index 7220d3f7..3308eaa 100644
--- a/chrome/browser/ui/webui/settings/chromeos/crostini_section.cc
+++ b/chrome/browser/ui/webui/settings/chromeos/crostini_section.cc
@@ -14,6 +14,7 @@
#include "chrome/browser/ash/crostini/crostini_features.h"
#include "chrome/browser/ash/crostini/crostini_pref_names.h"
#include "chrome/browser/ash/crostini/crostini_util.h"
+#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
#include "chrome/browser/ash/profiles/profile_helper.h"
#include "chrome/browser/policy/profile_policy_connector.h"
#include "chrome/browser/profiles/profile.h"
@@ -397,9 +398,9 @@
IDS_SETTINGS_CROSTINI_CONTAINER_UPGRADE_MESSAGE));
}
- if (auto* pretty_name_value = crostini::GetContainerPrefValue(
+ if (auto* pretty_name_value = guest_os::GetContainerPrefValue(
profile_, crostini::DefaultContainerId(),
- crostini::prefs::kContainerOsPrettyNameKey)) {
+ guest_os::prefs::kContainerOsPrettyNameKey)) {
std::string pretty_name = pretty_name_value->GetString();
html_source->AddString("crostiniContainerUpgradeSubtext",
l10n_util::GetStringFUTF16(
