Skip to content

Commit

Permalink
ServiceWorker: Mojofication of ServiceWorkerDispatcherHost
Browse files Browse the repository at this point in the history
This patch is to mojofy ServiceWorkerDispatcherHost which is a browser-side
MessageFilter handling chromium IPC. Most of IPCs from the renderer to the
browser related to the service workers are going through this object, so I can
mojofy all of the IPCs using this way. In this patch, the ordering issue is
avoided by using AssociatedInterface bound to MessageChannel. As the first
touch, I mojofied ProviderCreated.

BUG=629701

Review-Url: https://codereview.chromium.org/2396273002
Cr-Commit-Position: refs/heads/master@{#425639}
  • Loading branch information
makotoshimazu authored and Commit bot committed Oct 17, 2016
1 parent 70aeb57 commit 804b3e4
Show file tree
Hide file tree
Showing 16 changed files with 185 additions and 18 deletions.
14 changes: 13 additions & 1 deletion content/browser/service_worker/service_worker_dispatcher_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,12 @@ ServiceWorkerDispatcherHost::ServiceWorkerDispatcherHost(
render_process_id_(render_process_id),
message_port_message_filter_(message_port_message_filter),
resource_context_(resource_context),
channel_ready_(false) {
channel_ready_(false),
weak_factory_(this) {
AddAssociatedInterface(
mojom::ServiceWorkerDispatcherHost::Name_,
base::Bind(&ServiceWorkerDispatcherHost::AddMojoBinding,
base::Unretained(this)));
}

ServiceWorkerDispatcherHost::~ServiceWorkerDispatcherHost() {
Expand Down Expand Up @@ -219,6 +224,13 @@ bool ServiceWorkerDispatcherHost::OnMessageReceived(
return handled;
}

void ServiceWorkerDispatcherHost::AddMojoBinding(
mojo::ScopedInterfaceEndpointHandle handle) {
bindings_.AddBinding(
this, mojo::MakeAssociatedRequest<mojom::ServiceWorkerDispatcherHost>(
std::move(handle)));
}

bool ServiceWorkerDispatcherHost::Send(IPC::Message* message) {
if (channel_ready_) {
BrowserMessageFilter::Send(message);
Expand Down
24 changes: 19 additions & 5 deletions content/browser/service_worker/service_worker_dispatcher_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
#include "base/memory/weak_ptr.h"
#include "base/strings/string16.h"
#include "content/browser/service_worker/service_worker_registration_status.h"
#include "content/common/service_worker/service_worker.mojom.h"
#include "content/common/service_worker/service_worker_types.h"
#include "content/public/browser/browser_message_filter.h"
#include "mojo/public/cpp/bindings/associated_binding_set.h"

class GURL;
struct EmbeddedWorkerHostMsg_ReportConsoleMessage_Params;
Expand All @@ -41,7 +43,9 @@ struct ServiceWorkerRegistrationInfo;
struct ServiceWorkerRegistrationObjectInfo;
struct ServiceWorkerVersionAttributes;

class CONTENT_EXPORT ServiceWorkerDispatcherHost : public BrowserMessageFilter {
class CONTENT_EXPORT ServiceWorkerDispatcherHost
: public mojom::ServiceWorkerDispatcherHost,
public BrowserMessageFilter {
public:
ServiceWorkerDispatcherHost(
int render_process_id,
Expand Down Expand Up @@ -92,6 +96,16 @@ class CONTENT_EXPORT ServiceWorkerDispatcherHost : public BrowserMessageFilter {

using StatusCallback = base::Callback<void(ServiceWorkerStatusCode status)>;

// Called when mojom::ServiceWorkerDispatcherHostPtr is created on the
// renderer-side.
void AddMojoBinding(mojo::ScopedInterfaceEndpointHandle handle);

// mojom::ServiceWorkerDispatcherHost implementation
void OnProviderCreated(int provider_id,
int route_id,
ServiceWorkerProviderType provider_type,
bool is_parent_frame_secure) override;

// IPC Message handlers
void OnRegisterServiceWorker(int thread_id,
int request_id,
Expand All @@ -114,10 +128,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcherHost : public BrowserMessageFilter {
void OnGetRegistrationForReady(int thread_id,
int request_id,
int provider_id);
void OnProviderCreated(int provider_id,
int route_id,
ServiceWorkerProviderType provider_type,
bool is_parent_frame_secure);
void OnProviderDestroyed(int provider_id);
void OnSetHostedVersionId(int provider_id,
int64_t version_id,
Expand Down Expand Up @@ -243,6 +253,10 @@ class CONTENT_EXPORT ServiceWorkerDispatcherHost : public BrowserMessageFilter {
bool channel_ready_; // True after BrowserMessageFilter::sender_ != NULL.
std::vector<std::unique_ptr<IPC::Message>> pending_messages_;

mojo::AssociatedBindingSet<mojom::ServiceWorkerDispatcherHost> bindings_;

base::WeakPtrFactory<ServiceWorkerDispatcherHost> weak_factory_;

DISALLOW_COPY_AND_ASSIGN(ServiceWorkerDispatcherHost);
};

Expand Down
4 changes: 2 additions & 2 deletions content/browser/service_worker/service_worker_provider_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
void NotifyControllerLost();

private:
friend class ServiceWorkerProviderHostTest;
friend class ServiceWorkerProviderHostTestP;
friend class ServiceWorkerWriteToCacheJobTest;
FRIEND_TEST_ALL_PREFIXES(ServiceWorkerWriteToCacheJobTest, Update_SameScript);
FRIEND_TEST_ALL_PREFIXES(ServiceWorkerWriteToCacheJobTest,
Expand All @@ -285,7 +285,7 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
UpdateForceBypassCache);
FRIEND_TEST_ALL_PREFIXES(ServiceWorkerContextRequestHandlerTest,
ServiceWorkerDataRequestAnnotation);
FRIEND_TEST_ALL_PREFIXES(ServiceWorkerProviderHostTest, ContextSecurity);
FRIEND_TEST_ALL_PREFIXES(ServiceWorkerProviderHostTestP, ContextSecurity);

struct OneShotGetReadyCallback {
GetRegistrationForReadyCallback callback;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "content/browser/service_worker/service_worker_context_core.h"
#include "content/browser/service_worker/service_worker_register_job.h"
#include "content/browser/service_worker/service_worker_registration.h"
#include "content/browser/service_worker/service_worker_test_utils.h"
#include "content/browser/service_worker/service_worker_version.h"
#include "content/public/common/origin_util.h"
#include "content/public/test/test_browser_thread_bundle.h"
Expand Down Expand Up @@ -101,7 +102,10 @@ class ServiceWorkerProviderHostTest : public testing::Test {
DISALLOW_COPY_AND_ASSIGN(ServiceWorkerProviderHostTest);
};

TEST_F(ServiceWorkerProviderHostTest, PotentialRegistration_ProcessStatus) {
class ServiceWorkerProviderHostTestP
: public MojoServiceWorkerTestP<ServiceWorkerProviderHostTest> {};

TEST_P(ServiceWorkerProviderHostTestP, PotentialRegistration_ProcessStatus) {
// Matching registrations have already been set by SetDocumentUrl.
ASSERT_TRUE(PatternHasProcessToRun(registration1_->pattern()));

Expand Down Expand Up @@ -130,7 +134,7 @@ TEST_F(ServiceWorkerProviderHostTest, PotentialRegistration_ProcessStatus) {
ASSERT_TRUE(PatternHasProcessToRun(registration3_->pattern())); // host1,2
}

TEST_F(ServiceWorkerProviderHostTest, AssociatedRegistration_ProcessStatus) {
TEST_P(ServiceWorkerProviderHostTestP, AssociatedRegistration_ProcessStatus) {
// Associating the registration will also increase the process refs for
// the registration's pattern.
provider_host1_->AssociateRegistration(registration1_.get(),
Expand All @@ -143,7 +147,7 @@ TEST_F(ServiceWorkerProviderHostTest, AssociatedRegistration_ProcessStatus) {
ASSERT_TRUE(PatternHasProcessToRun(registration1_->pattern()));
}

TEST_F(ServiceWorkerProviderHostTest, MatchRegistration) {
TEST_P(ServiceWorkerProviderHostTestP, MatchRegistration) {
// Match registration should return the longest matching one.
ASSERT_EQ(registration2_, provider_host1_->MatchRegistration());
provider_host1_->RemoveMatchingRegistration(registration2_.get());
Expand All @@ -166,7 +170,7 @@ TEST_F(ServiceWorkerProviderHostTest, MatchRegistration) {
ASSERT_EQ(nullptr, provider_host1_->MatchRegistration());
}

TEST_F(ServiceWorkerProviderHostTest, ContextSecurity) {
TEST_P(ServiceWorkerProviderHostTestP, ContextSecurity) {
using FrameSecurityLevel = ServiceWorkerProviderHost::FrameSecurityLevel;
content::ResetSchemesAndOriginsWhitelistForTesting();

Expand Down Expand Up @@ -199,4 +203,8 @@ TEST_F(ServiceWorkerProviderHostTest, ContextSecurity) {
EXPECT_FALSE(provider_host1_->IsContextSecureForServiceWorker());
}

INSTANTIATE_TEST_CASE_P(ServiceWorkerProviderHostTest,
ServiceWorkerProviderHostTestP,
::testing::Values(false, true));

} // namespace content
22 changes: 22 additions & 0 deletions content/browser/service_worker/service_worker_test_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,32 @@

#include "base/bind.h"
#include "base/callback.h"
#include "base/command_line.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/common/content_switches.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace content {

template <class TestClass>
class MojoServiceWorkerTestP : public TestClass,
public testing::WithParamInterface<bool> {
protected:
void SetUp() override {
is_mojo_enabled_ = GetParam();
if (is_mojo_enabled()) {
base::CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kMojoServiceWorker);
}
TestClass::SetUp();
}

bool is_mojo_enabled() const { return is_mojo_enabled_; }

private:
bool is_mojo_enabled_ = false;
};

template <typename Arg>
void ReceiveResult(BrowserThread::ID run_quit_thread,
const base::Closure& quit,
Expand Down
12 changes: 10 additions & 2 deletions content/child/service_worker/service_worker_network_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "content/common/service_worker/service_worker_messages.h"
#include "content/common/service_worker/service_worker_utils.h"
#include "content/public/common/browser_side_navigation_policy.h"
#include "ipc/ipc_sync_channel.h"
#include "third_party/WebKit/public/platform/WebSecurityOrigin.h"
#include "third_party/WebKit/public/web/WebLocalFrame.h"
#include "third_party/WebKit/public/web/WebSandboxFlags.h"
Expand Down Expand Up @@ -128,8 +129,15 @@ ServiceWorkerNetworkProvider::ServiceWorkerNetworkProvider(
context_ = new ServiceWorkerProviderContext(
provider_id_, provider_type,
ChildThreadImpl::current()->thread_safe_sender());
ChildThreadImpl::current()->Send(new ServiceWorkerHostMsg_ProviderCreated(
provider_id_, route_id, provider_type, is_parent_frame_secure));
if (ServiceWorkerUtils::IsMojoForServiceWorkerEnabled()) {
ChildThreadImpl::current()->channel()->GetRemoteAssociatedInterface(
&dispatcher_host_);
dispatcher_host_->OnProviderCreated(provider_id_, route_id, provider_type,
is_parent_frame_secure);
} else {
ChildThreadImpl::current()->Send(new ServiceWorkerHostMsg_ProviderCreated(
provider_id_, route_id, provider_type, is_parent_frame_secure));
}
}

ServiceWorkerNetworkProvider::ServiceWorkerNetworkProvider(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/memory/ref_counted.h"
#include "base/supports_user_data.h"
#include "content/common/content_export.h"
#include "content/common/service_worker/service_worker.mojom.h"
#include "content/common/service_worker/service_worker_types.h"

namespace blink {
Expand Down Expand Up @@ -78,6 +79,7 @@ class CONTENT_EXPORT ServiceWorkerNetworkProvider
private:
const int provider_id_;
scoped_refptr<ServiceWorkerProviderContext> context_;
mojom::ServiceWorkerDispatcherHostAssociatedPtr dispatcher_host_;
DISALLOW_COPY_AND_ASSIGN(ServiceWorkerNetworkProvider);
};

Expand Down
2 changes: 2 additions & 0 deletions content/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,8 @@ mojom("mojo_bindings") {
"service_worker/embedded_worker.mojom",
"service_worker/embedded_worker_setup.mojom",
"service_worker/fetch_event_dispatcher.mojom",
"service_worker/service_worker.mojom",
"service_worker/service_worker_types.mojom",
"storage_partition_service.mojom",
"url_loader.mojom",
"url_loader_factory.mojom",
Expand Down
17 changes: 17 additions & 0 deletions content/common/service_worker/service_worker.mojom
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

module content.mojom;

import "content/common/service_worker/service_worker_types.mojom";

// Per-process browser-side interface bound to ServiceWorkerDispatcherHost.
// Each InterfacePtrs on the same render process will be bound to the same
// ServiceWorkerDispatcherHost.
interface ServiceWorkerDispatcherHost {
OnProviderCreated(int32 provider_id,
int32 route_id,
ServiceWorkerProviderType provider_type,
bool is_parent_frame_secure);
};
20 changes: 20 additions & 0 deletions content/common/service_worker/service_worker_types.mojom
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

module content.mojom;

enum ServiceWorkerProviderType {
SERVICE_WORKER_PROVIDER_UNKNOWN,

// For ServiceWorker clients.
SERVICE_WORKER_PROVIDER_FOR_WINDOW,
SERVICE_WORKER_PROVIDER_FOR_WORKER,
SERVICE_WORKER_PROVIDER_FOR_SHARED_WORKER,

// For ServiceWorkers.
SERVICE_WORKER_PROVIDER_FOR_CONTROLLER,

SERVICE_WORKER_PROVIDER_TYPE_LAST =
SERVICE_WORKER_PROVIDER_FOR_CONTROLLER
};
12 changes: 12 additions & 0 deletions content/common/service_worker/service_worker_types.typemap
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright 2016 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

mojom = "//content/common/service_worker/service_worker_types.mojom"
public_headers = [ "//content/common/service_worker/service_worker_types.h" ]
traits_headers =
[ "//content/common/service_worker/service_worker_types_traits.h" ]
sources = [
"//content/common/service_worker/service_worker_types_traits.cc",
]
type_mappings = [ "content.mojom.ServiceWorkerProviderType=::content::ServiceWorkerProviderType" ]
24 changes: 24 additions & 0 deletions content/common/service_worker/service_worker_types_traits.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "content/common/service_worker/service_worker_types_traits.h"

namespace mojo {

content::mojom::ServiceWorkerProviderType EnumTraits<
content::mojom::ServiceWorkerProviderType,
content::ServiceWorkerProviderType>::ToMojom(
content::ServiceWorkerProviderType input) {
return static_cast<content::mojom::ServiceWorkerProviderType>(input);
}

bool EnumTraits<content::mojom::ServiceWorkerProviderType,
content::ServiceWorkerProviderType>::FromMojom(
content::mojom::ServiceWorkerProviderType input,
content::ServiceWorkerProviderType* out) {
*out = static_cast<content::ServiceWorkerProviderType>(input);
return true;
}

} // namespace mojo
24 changes: 24 additions & 0 deletions content/common/service_worker/service_worker_types_traits.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CONTENT_COMMON_SERVICE_WORKER_SERVICE_WORKER_TYPES_TRAITS_H_
#define CONTENT_COMMON_SERVICE_WORKER_SERVICE_WORKER_TYPES_TRAITS_H_

#include "content/common/service_worker/service_worker_types.mojom.h"

namespace mojo {

template <>
struct EnumTraits<content::mojom::ServiceWorkerProviderType,
content::ServiceWorkerProviderType> {
static content::mojom::ServiceWorkerProviderType ToMojom(
content::ServiceWorkerProviderType input);

static bool FromMojom(content::mojom::ServiceWorkerProviderType input,
content::ServiceWorkerProviderType* out);
};

} // namespace mojo

#endif // CONTENT_COMMON_SERVICE_WORKER_SERVICE_WORKER_TYPES_TRAITS_H_
1 change: 1 addition & 0 deletions content/common/typemaps.gni
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ typemaps = [
"//content/common/media/media_devices.typemap",
"//content/common/service_worker/embedded_worker.typemap",
"//content/common/service_worker/fetch_event_dispatcher.typemap",
"//content/common/service_worker/service_worker_types.typemap",
"//content/common/url_loader_status.typemap",
"//content/common/url_request.typemap",
"//content/common/url_response_head.typemap",
Expand Down
1 change: 1 addition & 0 deletions content/public/app/mojo/content_browser_manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"blink::mojom::ShapeDetection",
"blink::mojom::WebSocket",
"content::mojom::MemoryCoordinatorHandle",
"content::mojom::ServiceWorkerDispatcherHost",
"content::mojom::StoragePartitionService",
"content::mojom::URLLoaderFactory",
"content::mojom::VideoCaptureHost",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -685,10 +685,10 @@ ServiceWorkerContextClient::createServiceWorkerNetworkProvider(

// Create a content::ServiceWorkerNetworkProvider for this data source so
// we can observe its requests.
std::unique_ptr<ServiceWorkerNetworkProvider> provider(
new ServiceWorkerNetworkProvider(MSG_ROUTING_NONE,
SERVICE_WORKER_PROVIDER_FOR_CONTROLLER,
true /* is_parent_frame_secure */));
std::unique_ptr<ServiceWorkerNetworkProvider> provider =
base::MakeUnique<ServiceWorkerNetworkProvider>(
MSG_ROUTING_NONE, SERVICE_WORKER_PROVIDER_FOR_CONTROLLER,
true /* is_parent_frame_secure */);
provider_context_ = provider->context();

// Tell the network provider about which version to load.
Expand Down

0 comments on commit 804b3e4

Please sign in to comment.