Skip to content

Commit

Permalink
Define an extensions.mojom.ServiceWorker interface
Browse files Browse the repository at this point in the history
This is a reland of 14dccee but
with support for conditional compilation.

Push the permissions from the browser to each service worker so
that we avoid racing with the main thread when permissions
are updated.

Currently if a permission update is sent it is sent via the
extensions.mojom.Renderer interface and all these events are propagated
to the WorkerThreads. In order to unblock the ResponseWorkerAck message
from not hitting the main thread and directly going to the worker
thread (as a callback on extensions.ServiceWorkerHost.RequestWorker)
we proactively push the updated permissions on the associated channel
with the ServiceWorker. This avoids the race that will happen.

This is tested by ExtensionApiTest.EventAfterPermissionRemoved

Bug: 1364183
Change-Id: I8ce2af8a9c3c2647bc45fb2d07692070e9d1d22d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4930736
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1208834}
  • Loading branch information
dtapuska authored and Chromium LUCI CQ committed Oct 12, 2023
1 parent 0203cb9 commit 1a61207
Show file tree
Hide file tree
Showing 14 changed files with 193 additions and 12 deletions.
1 change: 1 addition & 0 deletions components/performance_manager/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ include_rules = [
"+services/metrics/public/cpp",
"+services/resource_coordinator/public/cpp/memory_instrumentation",
"+services/service_manager/public/cpp",
"+third_party/blink/public/common/associated_interfaces/associated_interface_provider.h",
"+third_party/blink/public/common/features.h",
"+third_party/blink/public/common/storage_key/storage_key.h",
"+third_party/blink/public/common/tokens",
Expand Down
12 changes: 8 additions & 4 deletions components/performance_manager/service_worker_context_adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "base/task/single_thread_task_runner.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_process_host_observer.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"

namespace performance_manager {

Expand Down Expand Up @@ -221,10 +222,13 @@ bool ServiceWorkerContextAdapter::IsLiveRunningServiceWorker(
service_manager::InterfaceProvider&
ServiceWorkerContextAdapter::GetRemoteInterfaces(
int64_t service_worker_version_id) {
NOTIMPLEMENTED();
static base::NoDestructor<service_manager::InterfaceProvider>
interface_provider(base::SingleThreadTaskRunner::GetCurrentDefault());
return *interface_provider;
NOTREACHED_NORETURN();
}

blink::AssociatedInterfaceProvider&
ServiceWorkerContextAdapter::GetRemoteAssociatedInterfaces(
int64_t service_worker_version_id) {
NOTREACHED_NORETURN();
}

void ServiceWorkerContextAdapter::StartServiceWorkerAndDispatchMessage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ class ServiceWorkerContextAdapter
bool IsLiveRunningServiceWorker(int64_t service_worker_version_id) override;
service_manager::InterfaceProvider& GetRemoteInterfaces(
int64_t service_worker_version_id) override;
blink::AssociatedInterfaceProvider& GetRemoteAssociatedInterfaces(
int64_t service_worker_version_id) override;

// content::ServiceWorkerContextObserver:
void OnRegistrationCompleted(const GURL& scope) override;
Expand Down
13 changes: 13 additions & 0 deletions content/browser/service_worker/service_worker_context_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,19 @@ ServiceWorkerContextWrapper::GetRemoteInterfaces(
return version.worker_host()->remote_interfaces();
}

blink::AssociatedInterfaceProvider&
ServiceWorkerContextWrapper::GetRemoteAssociatedInterfaces(
int64_t service_worker_version_id) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
CHECK(IsLiveRunningServiceWorker(service_worker_version_id));

// This function should only be called on live running service workers
// so it should be safe to dereference the returned pointer without
// checking it first.
auto& version = *context()->GetLiveVersion(service_worker_version_id);
return *version.associated_interface_provider();
}

scoped_refptr<ServiceWorkerRegistration>
ServiceWorkerContextWrapper::GetLiveRegistration(int64_t registration_id) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
bool IsLiveRunningServiceWorker(int64_t service_worker_version_id) override;
service_manager::InterfaceProvider& GetRemoteInterfaces(
int64_t service_worker_version_id) override;
blink::AssociatedInterfaceProvider& GetRemoteAssociatedInterfaces(
int64_t service_worker_version_id) override;

scoped_refptr<ServiceWorkerRegistration> GetLiveRegistration(
int64_t registration_id);
Expand Down
8 changes: 8 additions & 0 deletions content/public/browser/service_worker_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class Uuid;
}

namespace blink {
class AssociatedInterfaceProvider;
class StorageKey;
} // namespace blink

Expand Down Expand Up @@ -287,6 +288,13 @@ class CONTENT_EXPORT ServiceWorkerContext {
virtual service_manager::InterfaceProvider& GetRemoteInterfaces(
int64_t service_worker_version_id) = 0;

// Returns the AssociatedInterfaceProvider for the worker specified by
// `service_worker_version_id`. The caller can use InterfaceProvider to bind
// interfaces exposed by the Service Worker. CHECKs if
// `IsLiveRunningServiceWorker()` returns false.
virtual blink::AssociatedInterfaceProvider& GetRemoteAssociatedInterfaces(
int64_t service_worker_version_id) = 0;

protected:
ServiceWorkerContext() {}
virtual ~ServiceWorkerContext() {}
Expand Down
12 changes: 8 additions & 4 deletions content/public/test/fake_service_worker_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/notreached.h"
#include "base/task/single_thread_task_runner.h"
#include "content/public/browser/service_worker_context_observer.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
#include "third_party/blink/public/common/messaging/transferable_message.h"
#include "third_party/blink/public/common/storage_key/storage_key.h"

Expand Down Expand Up @@ -119,10 +120,13 @@ bool FakeServiceWorkerContext::IsLiveRunningServiceWorker(
service_manager::InterfaceProvider&
FakeServiceWorkerContext::GetRemoteInterfaces(
int64_t service_worker_version_id) {
NOTREACHED();
static service_manager::InterfaceProvider interface_provider(
base::SingleThreadTaskRunner::GetCurrentDefault());
return interface_provider;
NOTREACHED_NORETURN();
}

blink::AssociatedInterfaceProvider&
FakeServiceWorkerContext::GetRemoteAssociatedInterfaces(
int64_t service_worker_version_id) {
NOTREACHED_NORETURN();
}

void FakeServiceWorkerContext::StartServiceWorkerForNavigationHint(
Expand Down
2 changes: 2 additions & 0 deletions content/public/test/fake_service_worker_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ class FakeServiceWorkerContext : public ServiceWorkerContext {
bool IsLiveRunningServiceWorker(int64_t service_worker_version_id) override;
service_manager::InterfaceProvider& GetRemoteInterfaces(
int64_t service_worker_version_id) override;
blink::AssociatedInterfaceProvider& GetRemoteAssociatedInterfaces(
int64_t service_worker_version_id) override;
void StartServiceWorkerAndDispatchMessage(
const GURL& scope,
const blink::StorageKey& key,
Expand Down
58 changes: 56 additions & 2 deletions extensions/browser/service_worker/service_worker_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@
#include "extensions/browser/event_router.h"
#include "extensions/browser/extension_function_dispatcher.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_util.h"
#include "extensions/browser/process_map.h"
#include "extensions/browser/service_worker_task_queue.h"
#include "extensions/common/constants.h"
#include "extensions/common/mojom/frame.mojom.h"
#include "extensions/common/permissions/permissions_data.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"

namespace extensions {

Expand Down Expand Up @@ -54,7 +57,16 @@ void ServiceWorkerHost::BindReceiver(
}

service_worker_host->receiver_.Bind(std::move(receiver));
service_worker_host->receiver_.reset_on_disconnect();
service_worker_host->receiver_.set_disconnect_handler(
base::BindOnce(&ServiceWorkerHost::RemoteDisconnected,
base::Unretained(service_worker_host)));
}

void ServiceWorkerHost::RemoteDisconnected() {
receiver_.reset();
#if !BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
permissions_observer_.Reset();
#endif
}

void ServiceWorkerHost::DidInitializeServiceWorkerContext(
Expand Down Expand Up @@ -85,6 +97,11 @@ void ServiceWorkerHost::DidInitializeServiceWorkerContext(
// running in a given process.
return;
}
#if !BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
service_worker_version_id_ = service_worker_version_id;
extension_id_ = extension_id;
permissions_observer_.Observe(PermissionsManager::Get(browser_context));
#endif

ServiceWorkerTaskQueue::Get(browser_context)
->DidInitializeServiceWorkerContext(render_process_id, extension_id,
Expand Down Expand Up @@ -144,7 +161,9 @@ void ServiceWorkerHost::DidStopServiceWorkerContext(
}
CHECK(service_worker_scope.SchemeIs(kExtensionScheme) &&
extension_id == service_worker_scope.host_piece());

CHECK(!extension_id.empty());
CHECK_NE(blink::mojom::kInvalidServiceWorkerVersionId,
service_worker_version_id);
ServiceWorkerTaskQueue::Get(browser_context)
->DidStopServiceWorkerContext(
render_process_id, extension_id, activation_token,
Expand Down Expand Up @@ -174,4 +193,39 @@ content::BrowserContext* ServiceWorkerHost::GetBrowserContext() {
return render_process_host_->GetBrowserContext();
}

#if !BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
mojom::ServiceWorker* ServiceWorkerHost::GetServiceWorker() {
if (!remote_.is_bound()) {
content::ServiceWorkerContext* context =
util::GetServiceWorkerContextForExtensionId(extension_id_,
GetBrowserContext());
CHECK(context);
context->GetRemoteAssociatedInterfaces(service_worker_version_id_)
.GetInterface(&remote_);
}
return remote_.get();
}

void ServiceWorkerHost::OnExtensionPermissionsUpdated(
const Extension& extension,
const PermissionSet& permissions,
PermissionsManager::UpdateReason reason) {
if (extension.id() != extension_id_) {
return;
}
content::ServiceWorkerContext* context =
util::GetServiceWorkerContextForExtensionId(extension_id_,
GetBrowserContext());
CHECK(context);
if (!context->IsLiveRunningServiceWorker(service_worker_version_id_)) {
return;
}

const PermissionsData* permissions_data = extension.permissions_data();
GetServiceWorker()->UpdatePermissions(
std::move(*permissions_data->active_permissions().Clone()),
std::move(*permissions_data->withheld_permissions().Clone()));
}
#endif

} // namespace extensions
32 changes: 32 additions & 0 deletions extensions/browser/service_worker/service_worker_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,17 @@
#include <unordered_set>

#include "base/memory/raw_ptr.h"
#include "base/scoped_observation.h"
#include "base/supports_user_data.h"
#include "content/public/browser/browser_thread.h"
#include "extensions/browser/permissions_manager.h"
#include "extensions/buildflags/buildflags.h"
#include "extensions/common/extension_id.h"
#include "extensions/common/mojom/service_worker.mojom.h"
#include "extensions/common/mojom/service_worker_host.mojom.h"
#include "mojo/public/cpp/bindings/associated_receiver.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "third_party/blink/public/mojom/service_worker/service_worker_database.mojom.h"

class GURL;

Expand All @@ -33,6 +39,9 @@ class ExtensionFunctionDispatcher;
// This class is the host of service worker execution context for extension
// in the renderer process. Lives on the UI thread.
class ServiceWorkerHost : public base::SupportsUserData::Data,
#if !BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
public PermissionsManager::Observer,
#endif
public mojom::ServiceWorkerHost {
public:
explicit ServiceWorkerHost(content::RenderProcessHost* render_process_host);
Expand Down Expand Up @@ -66,18 +75,41 @@ class ServiceWorkerHost : public base::SupportsUserData::Data,
void RequestWorker(mojom::RequestParamsPtr params) override;
void WorkerResponseAck(const base::Uuid& request_uuid) override;

#if !BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
// PermissionManager::Observer overrides.
void OnExtensionPermissionsUpdated(
const Extension& extension,
const PermissionSet& permissions,
PermissionsManager::UpdateReason reason) override;
#endif

private:
// Returns the browser context associated with the render process this
// `ServiceWorkerHost` belongs to.
content::BrowserContext* GetBrowserContext();

#if !BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
mojom::ServiceWorker* GetServiceWorker();
#endif

void RemoteDisconnected();

// This is safe because ServiceWorkerHost is tied to the life time of
// RenderProcessHost.
const raw_ptr<content::RenderProcessHost> render_process_host_;

std::unique_ptr<ExtensionFunctionDispatcher> dispatcher_;

mojo::AssociatedReceiver<mojom::ServiceWorkerHost> receiver_{this};
#if !BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
mojo::AssociatedRemote<mojom::ServiceWorker> remote_;
int64_t service_worker_version_id_ =
blink::mojom::kInvalidServiceWorkerVersionId;
ExtensionId extension_id_;

base::ScopedObservation<PermissionsManager, PermissionsManager::Observer>
permissions_observer_{this};
#endif
};

} // namespace extensions
Expand Down
1 change: 1 addition & 0 deletions extensions/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ mojom("mojom") {
"mojom/renderer.mojom",
"mojom/renderer_host.mojom",
"mojom/run_location.mojom",
"mojom/service_worker.mojom",
"mojom/service_worker_host.mojom",
"mojom/stack_frame.mojom",
"mojom/url_pattern_set.mojom",
Expand Down
15 changes: 15 additions & 0 deletions extensions/common/mojom/service_worker.mojom
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

module extensions.mojom;

import "extensions/common/mojom/permission_set.mojom";

// An interface for an extension Service Worker. Implemented
// in the renderer.
interface ServiceWorker {
// Tells the worker to update an extension's permission set.
UpdatePermissions(PermissionSet active_permissions,
PermissionSet withheld_permissions);
};
32 changes: 31 additions & 1 deletion extensions/renderer/service_worker_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,41 @@ ServiceWorkerData::ServiceWorkerData(
activation_sequence_(std::move(activation_sequence)),
context_(context),
v8_schema_registry_(new V8SchemaRegistry),
bindings_system_(std::move(bindings_system)) {}
bindings_system_(std::move(bindings_system)) {
#if !BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
proxy_->GetAssociatedInterfaceRegistry().AddInterface<mojom::ServiceWorker>(
base::BindRepeating(&ServiceWorkerData::OnServiceWorkerRequest,
weak_ptr_factory_.GetWeakPtr()));
#endif
}

ServiceWorkerData::~ServiceWorkerData() = default;

#if !BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
void ServiceWorkerData::OnServiceWorkerRequest(
mojo::PendingAssociatedReceiver<mojom::ServiceWorker> receiver) {
receiver_.reset();
receiver_.Bind(std::move(receiver));
}

void ServiceWorkerData::UpdatePermissions(PermissionSet active_permissions,
PermissionSet withheld_permissions) {
DCHECK(worker_thread_util::IsWorkerThread());

const ExtensionId& extension_id = context_->GetExtensionID();
const Extension* extension =
RendererExtensionRegistry::Get()->GetByID(extension_id);
if (!extension) {
return;
}
extension->permissions_data()->SetPermissions(
std::make_unique<const PermissionSet>(std::move(active_permissions)),
std::make_unique<const PermissionSet>(std::move(withheld_permissions)));

bindings_system_->UpdateBindings(extension_id, /*permissions_changed=*/true,
Dispatcher::GetWorkerScriptContextSet());
}

mojom::ServiceWorkerHost* ServiceWorkerData::GetServiceWorkerHost() {
if (!service_worker_host_.is_bound()) {
proxy_->GetRemoteAssociatedInterface(
Expand Down

0 comments on commit 1a61207

Please sign in to comment.