Skip to content

Commit

Permalink
system-extensions: Register previously persisted extensions
Browse files Browse the repository at this point in the history
Loads previously persisted extensions and registers them.

Fixed: b/221121174
Change-Id: Ibf35b131d564cc37f161e72971be2d3e4dfdf870
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3823703
Reviewed-by: Dominic Schulz <dominicschulz@google.com>
Commit-Queue: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1034970}
  • Loading branch information
Giovanni Ortuño Urquidi authored and Chromium LUCI CQ committed Aug 15, 2022
1 parent a2de1a7 commit d045279
Show file tree
Hide file tree
Showing 8 changed files with 416 additions and 99 deletions.
197 changes: 163 additions & 34 deletions chrome/browser/ash/system_extensions/system_extensions_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,55 @@ class SystemExtensionsBrowserTest : public InProcessBrowserTest {
}
}

void TestExtensionUninstalled() {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& registry = provider.registry();

EXPECT_TRUE(registry.GetIds().empty());
EXPECT_FALSE(registry.GetById(kTestSystemExtensionId));

// Tests that the System Extension is no longer in persistent storage.
absl::optional<SystemExtensionPersistenceInfo> persistence_info =
provider.persistence_manager().Get(kTestSystemExtensionId);
EXPECT_FALSE(persistence_info);

// Test that navigating to the System Extension's resources fails.
auto* tab = browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(ui_test_utils::NavigateToURL(
browser(), GURL(kTestSystemExtensionIndexURL)));
content::NavigationEntry* entry = tab->GetController().GetVisibleEntry();
EXPECT_EQ(content::PAGE_TYPE_ERROR, entry->GetPageType());

{
// Test that the resources have been deleted.
base::ScopedAllowBlockingForTesting allow_blocking;
const base::FilePath system_extension_dir =
GetDirectoryForSystemExtension(*browser()->profile(),
kTestSystemExtensionId);
EXPECT_FALSE(base::PathExists(system_extension_dir));
}

{
// Test that the service worker has been unregistered.
const GURL scope(kTestSystemExtensionEmptyPathURL);
auto* worker_context = browser()
->profile()
->GetDefaultStoragePartition()
->GetServiceWorkerContext();

base::RunLoop run_loop;
worker_context->CheckHasServiceWorker(
scope, blink::StorageKey(url::Origin::Create(scope)),
base::BindLambdaForTesting(
[&](content::ServiceWorkerCapability capability) {
EXPECT_EQ(capability,
content::ServiceWorkerCapability::NO_SERVICE_WORKER);
run_loop.Quit();
}));
run_loop.Run();
}
}

private:
base::test::ScopedFeatureList feature_list_;
};
Expand Down Expand Up @@ -314,59 +363,139 @@ IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest, Uninstall_Success) {
// Uninstall the extension.
install_manager.Uninstall(kTestSystemExtensionId);

auto& registry = provider.registry();
EXPECT_TRUE(registry.GetIds().empty());
EXPECT_FALSE(registry.GetById(kTestSystemExtensionId));

// Tests that the System Extension is no longer in persistent storage.
absl::optional<SystemExtensionPersistenceInfo> persistence_info =
provider.persistence_manager().Get(kTestSystemExtensionId);
EXPECT_FALSE(persistence_info);

// Test that navigating to the System Extension's resources fails.
auto* tab = browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(),
GURL(kTestSystemExtensionIndexURL)));
content::NavigationEntry* entry = tab->GetController().GetVisibleEntry();
EXPECT_EQ(content::PAGE_TYPE_ERROR, entry->GetPageType());

{
// Test that the resources have been deleted.
const auto [id, deletion_succeeded] = waiter.WaitForAssetsDeleted();
EXPECT_EQ(id, kTestSystemExtensionId);
EXPECT_TRUE(deletion_succeeded);
base::ScopedAllowBlockingForTesting allow_blocking;
const base::FilePath system_extension_dir = GetDirectoryForSystemExtension(
*browser()->profile(), kTestSystemExtensionId);
EXPECT_FALSE(base::PathExists(system_extension_dir));
}

{
// Test that the service worker has been unregistered.
const auto [id, unregistration_succeeded] =
waiter.WaitForServiceWorkerUnregistered();
EXPECT_EQ(id, kTestSystemExtensionId);
EXPECT_TRUE(unregistration_succeeded);
}

TestExtensionUninstalled();
}

const GURL scope(kTestSystemExtensionEmptyPathURL);
auto* worker_context = browser()
->profile()
->GetDefaultStoragePartition()
->GetServiceWorkerContext();
// Tests that extensions are persisted across restarts.
IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest,
PRE_PersistedAcrossRestart) {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& install_manager = provider.install_manager();

TestInstallationEventsWaiter waiter(provider);

{
// Install and wait for the service worker to be registered.
base::RunLoop run_loop;
worker_context->CheckHasServiceWorker(
scope, blink::StorageKey(url::Origin::Create(scope)),
install_manager.InstallUnpackedExtensionFromDir(
GetBasicSystemExtensionDir(),
base::BindLambdaForTesting(
[&](content::ServiceWorkerCapability capability) {
EXPECT_EQ(capability,
content::ServiceWorkerCapability::NO_SERVICE_WORKER);
run_loop.Quit();
}));
[&](InstallStatusOrSystemExtensionId result) { run_loop.Quit(); }));
run_loop.Run();
waiter.WaitForServiceWorkerRegistered();
}
}

IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest, PersistedAcrossRestart) {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& install_manager = provider.install_manager();

// Wait for previously persisted System Extensions to be registered.
base::RunLoop run_loop;
install_manager.on_register_previously_persisted_finished().Post(
FROM_HERE, run_loop.QuitClosure());
run_loop.Run();

TestInstalledTestExtensionWorks();
}

// Tests that an extension can be uninstalled after restart.
IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest, PRE_UninstallAfterRestart) {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& install_manager = provider.install_manager();

TestInstallationEventsWaiter waiter(provider);

{
// Install and wait for the service worker to be registered.
base::RunLoop run_loop;
install_manager.InstallUnpackedExtensionFromDir(
GetBasicSystemExtensionDir(),
base::BindLambdaForTesting(
[&](InstallStatusOrSystemExtensionId result) { run_loop.Quit(); }));
run_loop.Run();
waiter.WaitForServiceWorkerRegistered();
}
}

IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest, UninstallAfterRestart) {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& install_manager = provider.install_manager();

// Wait for previously persisted System Extensions to be registered.
base::RunLoop run_loop;
install_manager.on_register_previously_persisted_finished().Post(
FROM_HERE, run_loop.QuitClosure());
run_loop.Run();

// Uninstall the extension.
install_manager.Uninstall(kTestSystemExtensionId);
TestExtensionUninstalled();
}

// Tests that if an extension is uninstalled, it stays uninstalled.
IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest,
PRE_PRE_PermanentlyUninstalled) {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& install_manager = provider.install_manager();

TestInstallationEventsWaiter waiter(provider);

{
// Install and wait for the service worker to be registered.
base::RunLoop run_loop;
install_manager.InstallUnpackedExtensionFromDir(
GetBasicSystemExtensionDir(),
base::BindLambdaForTesting(
[&](InstallStatusOrSystemExtensionId result) { run_loop.Quit(); }));
run_loop.Run();
waiter.WaitForServiceWorkerRegistered();
}
}

IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest,
PRE_PermanentlyUninstalled) {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& install_manager = provider.install_manager();

// Wait for previously persisted System Extensions to be registered.
base::RunLoop run_loop;
install_manager.on_register_previously_persisted_finished().Post(
FROM_HERE, run_loop.QuitClosure());
run_loop.Run();

// Uninstall the extension.
install_manager.Uninstall(kTestSystemExtensionId);

TestExtensionUninstalled();
}

IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest, PermanentlyUninstalled) {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& install_manager = provider.install_manager();

// Wait for previously persisted System Extensions to be registered.
base::RunLoop run_loop;
install_manager.on_register_previously_persisted_finished().Post(
FROM_HERE, run_loop.QuitClosure());
run_loop.Run();

TestExtensionUninstalled();
}

IN_PROC_BROWSER_TEST_F(SystemExtensionsSwitchBrowserTest, ExtensionInstalled) {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& install_manager = provider.install_manager();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,44 @@ SystemExtensionsInstallManager::SystemExtensionsInstallManager(
registry_manager_(registry_manager),
registry_(registry),
persistence_manager_(persistence_manager) {
RegisterPreviouslyPersistedSystemExtensions();
InstallFromCommandLineIfNecessary();
}

SystemExtensionsInstallManager::~SystemExtensionsInstallManager() = default;

void SystemExtensionsInstallManager::
RegisterPreviouslyPersistedSystemExtensions() {
const std::vector<SystemExtensionPersistenceInfo> persisted_infos =
persistence_manager_->GetAll();
for (const auto& persisted_info : persisted_infos) {
InstallStatusOrSystemExtension status_or_extension =
sandboxed_unpacker_.GetSystemExtensionFromValue(
persisted_info.manifest);

if (!status_or_extension.ok()) {
LOG(ERROR) << "Failed to register System Extension from Persistence "
<< "Manager.";
continue;
}

RegisterSystemExtension(std::move(status_or_extension).value());
}

on_register_previously_persisted_finished_.Signal();
}

void SystemExtensionsInstallManager::InstallUnpackedExtensionFromDir(
const base::FilePath& unpacked_system_extension_dir,
OnceInstallCallback final_callback) {
DCHECK(on_register_previously_persisted_finished_.is_signaled());

StartInstallation(std::move(final_callback), unpacked_system_extension_dir);
}

void SystemExtensionsInstallManager::InstallFromCommandLineIfNecessary() {
DCHECK(on_register_previously_persisted_finished_.is_signaled());

base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
if (!command_line->HasSwitch(ash::switches::kInstallSystemExtension)) {
return;
Expand Down Expand Up @@ -128,19 +154,26 @@ void SystemExtensionsInstallManager::OnAssetsCopiedToProfileDir(
return;
}

// Installation Step #3: Create a WebUIConfig so that resources are served.
// Installation Step #3: Persist the System Extension across restarts.
persistence_manager_->Persist(system_extension);

SystemExtensionId id = system_extension.id;
RegisterSystemExtension(std::move(system_extension));

std::move(final_callback).Run(std::move(id));
}

void SystemExtensionsInstallManager::RegisterSystemExtension(
SystemExtension system_extension) {
// Installation Step #4: Create a WebUIConfig so that resources are served.
auto config = std::make_unique<SystemExtensionsWebUIConfig>(system_extension);
content::WebUIConfigMap::GetInstance().AddUntrustedWebUIConfig(
std::move(config));

// Installation Step #4: Persist the System Extension across restarts.
persistence_manager_->Persist(system_extension);

// Installation Step #5: Add the System Extension to the registry.
SystemExtensionId id = system_extension.id;
registry_manager_->AddSystemExtension(std::move(system_extension));

std::move(final_callback).Run(std::move(id));
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(&SystemExtensionsInstallManager::RegisterServiceWorker,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,19 @@ class SystemExtensionsInstallManager {
const base::FilePath& unpacked_system_extension_dir,
OnceInstallCallback final_callback);

// Event that signals when a System Extension is installed from the command
// line. Signals even if the System Extension failed to be installed, but
// doesn't if there were no command line arguments to install.
const base::OneShotEvent& on_command_line_install_finished() {
return on_command_line_install_finished_;
}

// Event that signals when all System Extensions that were persisted in a
// previous session are registered.
const base::OneShotEvent& on_register_previously_persisted_finished() {
return on_register_previously_persisted_finished_;
}

// Uninstallation always succeeds.
//
// Currently only two operations can fail, unregistering the Service Worker,
Expand All @@ -96,6 +105,7 @@ class SystemExtensionsInstallManager {
bool RemoveExtensionAssets(const base::FilePath& system_extension_dir);
};

void RegisterPreviouslyPersistedSystemExtensions();
void InstallFromCommandLineIfNecessary();
void OnInstallFromCommandLineFinished(
InstallStatusOrSystemExtensionId result);
Expand All @@ -109,6 +119,7 @@ class SystemExtensionsInstallManager {
void OnAssetsCopiedToProfileDir(OnceInstallCallback final_callback,
SystemExtension system_extension,
bool did_succeed);
void RegisterSystemExtension(SystemExtension system_extension);
void RegisterServiceWorker(const SystemExtensionId& id);
void DispatchWindowManagerStartEvent(const SystemExtensionId& id,
int64_t version_id,
Expand Down Expand Up @@ -136,6 +147,7 @@ class SystemExtensionsInstallManager {
std::map<SystemExtensionId, SystemExtension> system_extensions_;

base::OneShotEvent on_command_line_install_finished_;
base::OneShotEvent on_register_previously_persisted_finished_;

SystemExtensionsSandboxedUnpacker sandboxed_unpacker_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,26 @@ SystemExtensionsPersistenceManager::Get(
return info;
}

std::vector<SystemExtensionPersistenceInfo>
SystemExtensionsPersistenceManager::GetAll() {
std::vector<SystemExtensionPersistenceInfo> infos;

auto* prefs = profile_->GetPrefs();
const base::Value::Dict& persisted_system_extensions_map =
prefs->GetValueDict(prefs::kPersistedSystemExtensions);
for (const auto [id_str, _] : persisted_system_extensions_map) {
absl::optional<SystemExtensionId> id = SystemExtension::StringToId(id_str);
if (!id)
continue;

absl::optional<SystemExtensionPersistenceInfo> info = Get(id.value());
if (!info)
continue;

infos.emplace_back(std::move(info.value()));
}

return infos;
}

} // namespace ash
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class SystemExtensionsPersistenceManager {
// persistent storage, or nullopt if it's not.
absl::optional<SystemExtensionPersistenceInfo> Get(
const SystemExtensionId& system_extension_id);
std::vector<SystemExtensionPersistenceInfo> GetAll();

private:
// Safe to hold a pointer because the parent class is owned by Profile.
Expand Down

0 comments on commit d045279

Please sign in to comment.