Skip to content

Commit

Permalink
system-extensions: Implement uninstalling a System Extension
Browse files Browse the repository at this point in the history
Implements basic uninstalling of system extensions i.e.
removing the WebUIConfig, deleting assets, and removing
the extension from the registry.

Bug: b/221121088
Change-Id: I0a1c4f23f0d3192999f68302f5b131eef906bd5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3809194
Reviewed-by: Jiewei Qian <qjw@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1033301}
  • Loading branch information
Giovanni Ortuño Urquidi authored and Chromium LUCI CQ committed Aug 10, 2022
1 parent 36edc7e commit f9bbf5e
Show file tree
Hide file tree
Showing 11 changed files with 337 additions and 43 deletions.
219 changes: 195 additions & 24 deletions chrome/browser/ash/system_extensions/system_extensions_browsertest.cc
Expand Up @@ -6,11 +6,14 @@
#include "ash/constants/ash_switches.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/memory/weak_ptr.h"
#include "base/path_service.h"
#include "base/scoped_observation.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread_restrictions.h"
#include "chrome/browser/ash/system_extensions/system_extensions_install_manager.h"
#include "chrome/browser/ash/system_extensions/system_extensions_profile_utils.h"
#include "chrome/browser/ash/system_extensions/system_extensions_provider.h"
#include "chrome/browser/ash/system_extensions/system_extensions_provider_factory.h"
#include "chrome/browser/profiles/profile.h"
Expand All @@ -19,6 +22,8 @@
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/service_worker_context.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/common/content_features.h"
#include "content/public/common/page_type.h"
#include "content/public/test/browser_test.h"
Expand All @@ -44,48 +49,146 @@ base::FilePath GetBasicSystemExtensionDir() {
return test_dir.Append("system_extensions").Append("basic_system_extension");
}

// Class that returns the result of the first System Extension service worker it
// sees.
class ServiceWorkerRegistrationObserver
// Wrapper around base::OneShotEvent that allows callers to signal with
// arguments.
template <typename... Args>
class OneShotEventWrapper {
public:
OneShotEventWrapper() = default;

~OneShotEventWrapper() = default;

void Signal(Args... args) {
run_with_args_ = base::BindRepeating(&OneShotEventWrapper::RunWithArgs,
weak_ptr_factory_.GetWeakPtr(),
std::forward<Args>(args)...);
one_shot_event_.Signal();
}

void Post(const base::Location& from_here,
base::OnceCallback<void(Args...)> task) {
one_shot_event_.Post(
from_here,
base::BindOnce(&OneShotEventWrapper::RunTask,
weak_ptr_factory_.GetWeakPtr(), std::move(task)));
}

private:
void RunTask(base::OnceCallback<void(Args...)> task) {
run_with_args_.Run(std::move(task));
}

void RunWithArgs(Args... args, base::OnceCallback<void(Args...)> task) {
std::move(task).Run(std::forward<Args>(args)...);
}

base::OneShotEvent one_shot_event_;
base::RepeatingCallback<void(base::OnceCallback<void(Args...)>)>
run_with_args_;
base::WeakPtrFactory<OneShotEventWrapper> weak_ptr_factory_{this};
};

// Class that can be used to wait for SystemExtensionsInstallManager events.
// If an event is triggered more than once, this class will CHECK.
class TestInstallationEventsWaiter
: public SystemExtensionsInstallManager::Observer {
public:
explicit ServiceWorkerRegistrationObserver(SystemExtensionsProvider& provider)
explicit TestInstallationEventsWaiter(SystemExtensionsProvider& provider)
: install_manager_(provider.install_manager()) {
install_manager_.AddObserver(this);
observation_.Observe(&install_manager_);
}
~ServiceWorkerRegistrationObserver() override {}

// Returns the saved result or waits to get a result and returns it.
~TestInstallationEventsWaiter() override = default;

// Returns the result of a Service Worker registration. Waits if there hasn't
// been one yet.
std::pair<SystemExtensionId, blink::ServiceWorkerStatusCode>
GetIdAndStatusCode() {
if (id_.has_value())
return {id_.value(), status_code_.value()};
WaitForServiceWorkerRegistered() {
absl::optional<SystemExtensionId> id;
absl::optional<blink::ServiceWorkerStatusCode> status_code;

base::RunLoop run_loop;
on_service_worker_registered_.Post(
FROM_HERE,
base::BindLambdaForTesting(
[&](SystemExtensionId returned_id,
blink::ServiceWorkerStatusCode returned_status_code) {
id = returned_id;
status_code = returned_status_code;
run_loop.Quit();
}));
run_loop.Run();

return {id.value(), status_code.value()};
}

run_loop_.Run();
return {id_.value(), status_code_.value()};
// Returns the result of a Service Worker unregistration. Waits if there
// hasn't been one yet.
std::pair<SystemExtensionId, bool> WaitForServiceWorkerUnregistered() {
absl::optional<SystemExtensionId> id;
absl::optional<bool> succeeded;

base::RunLoop run_loop;
on_service_worker_unregistered_.Post(
FROM_HERE, base::BindLambdaForTesting([&](SystemExtensionId returned_id,
bool returned_succeeded) {
id = returned_id;
succeeded = returned_succeeded;
run_loop.Quit();
}));
run_loop.Run();

return {id.value(), succeeded.value()};
}

// Returns the result of a asset deletion operations. Waits if there
// hasn't been one yet.
std::pair<SystemExtensionId, bool> WaitForAssetsDeleted() {
absl::optional<SystemExtensionId> id;
absl::optional<bool> succeeded;

base::RunLoop run_loop;
on_assets_deleted_.Post(
FROM_HERE, base::BindLambdaForTesting([&](SystemExtensionId returned_id,
bool returned_succeeded) {
id = returned_id;
succeeded = returned_succeeded;
run_loop.Quit();
}));
run_loop.Run();

return {id.value(), succeeded.value()};
}

// SystemExtensionsInstallManager::Observer
void OnServiceWorkerRegistered(
const SystemExtensionId& id,
blink::ServiceWorkerStatusCode status_code) override {
install_manager_.RemoveObserver(this);
on_service_worker_registered_.Signal(id, status_code);
}

// Should happen because we unregistered as observers.
DCHECK(!id_.has_value());
void OnServiceWorkerUnregistered(const SystemExtensionId& id,
bool succeeded) override {
on_service_worker_unregistered_.Signal(id, succeeded);
}

id_ = id;
status_code_ = status_code;
run_loop_.Quit();
void OnSystemExtensionAssetsDeleted(const SystemExtensionId& id,
bool succeeded) override {
on_assets_deleted_.Signal(id, succeeded);
}

private:
OneShotEventWrapper<SystemExtensionId, blink::ServiceWorkerStatusCode>
on_service_worker_registered_;
OneShotEventWrapper<SystemExtensionId, bool> on_service_worker_unregistered_;
OneShotEventWrapper<SystemExtensionId, bool> on_assets_deleted_;

base::ScopedObservation<SystemExtensionsInstallManager,
SystemExtensionsInstallManager::Observer>
observation_{this};

// Should be present for the duration of the test.
SystemExtensionsInstallManager& install_manager_;

base::RunLoop run_loop_;
absl::optional<SystemExtensionId> id_;
absl::optional<blink::ServiceWorkerStatusCode> status_code_;
};

class SystemExtensionsBrowserTest : public InProcessBrowserTest {
Expand Down Expand Up @@ -155,7 +258,7 @@ IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest, InstallFromDir_Success) {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& install_manager = provider.install_manager();

ServiceWorkerRegistrationObserver sw_registration_observer(provider);
TestInstallationEventsWaiter waiter(provider);
base::RunLoop run_loop;
install_manager.InstallUnpackedExtensionFromDir(
GetBasicSystemExtensionDir(),
Expand All @@ -166,13 +269,81 @@ IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest, InstallFromDir_Success) {
}));
run_loop.Run();

const auto [id, status_code] = sw_registration_observer.GetIdAndStatusCode();
const auto [id, status_code] = waiter.WaitForServiceWorkerRegistered();
EXPECT_EQ(kTestSystemExtensionId, id);
EXPECT_EQ(blink::ServiceWorkerStatusCode::kOk, status_code);

TestInstalledTestExtensionWorks();
}

IN_PROC_BROWSER_TEST_F(SystemExtensionsBrowserTest, Uninstall_Success) {
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();
}

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

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

// 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);

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();
}
}

IN_PROC_BROWSER_TEST_F(SystemExtensionsSwitchBrowserTest, ExtensionInstalled) {
auto& provider = SystemExtensionsProvider::Get(browser()->profile());
auto& install_manager = provider.install_manager();
Expand Down

0 comments on commit f9bbf5e

Please sign in to comment.