Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(746)

Unified Diff: content/browser/service_worker/service_worker_provider_host.cc

Issue 2653493009: Add two interfaces for ServiceWorkerProviderContext/ProviderHost (Closed)
Patch Set: Addressed comments from falken Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/service_worker/service_worker_provider_host.cc
diff --git a/content/browser/service_worker/service_worker_provider_host.cc b/content/browser/service_worker/service_worker_provider_host.cc
index a616574a951dc13897a7a22b5a5657e960e11cde..7e1390bd0abfd89abe1b6c38c4a3b121ced45090 100644
--- a/content/browser/service_worker/service_worker_provider_host.cc
+++ b/content/browser/service_worker/service_worker_provider_host.cc
@@ -61,8 +61,8 @@ class ServiceWorkerURLTrackingRequestHandler
// Called via custom URLRequestJobFactory.
net::URLRequestJob* MaybeCreateJob(
net::URLRequest* request,
- net::NetworkDelegate* network_delegate,
- ResourceContext* resource_context) override {
+ net::NetworkDelegate* /* network_delegate */,
+ ResourceContext* /* resource_context */) override {
// |provider_host_| may have been deleted when the request is resumed.
if (!provider_host_)
return nullptr;
@@ -76,6 +76,25 @@ class ServiceWorkerURLTrackingRequestHandler
DISALLOW_COPY_AND_ASSIGN(ServiceWorkerURLTrackingRequestHandler);
};
+void RemoveProviderHost(base::WeakPtr<ServiceWorkerContextCore> context,
+ int process_id,
+ int provider_id) {
+ TRACE_EVENT0("ServiceWorker",
+ "ServiceWorkerProviderHost::RemoveProviderHost");
+ if (!context)
+ return;
+ if (!context->GetProviderHost(process_id, provider_id)) {
+ // PlzNavigate: in some cancellation of navigation cases, it is possible
+ // for the pre-created host to have been destroyed before being claimed by
+ // the renderer. The provider is then destroyed in the renderer, and no
+ // matching host will be found.
falken 2017/05/22 08:28:12 "pre-created host" means ServiceWorkerUtils::IsBro
shimazu 2017/05/23 06:29:34 Done.
+ CHECK(IsBrowserSideNavigationEnabled() &&
+ ServiceWorkerUtils::IsBrowserAssignedProviderId(provider_id));
falken 2017/05/22 08:28:12 Why a CHECK here? From the style guide: "Use CHECK
shimazu 2017/05/23 06:29:34 Thanks, it's just inherited from the previous impl
+ return;
+ }
+ context->RemoveProviderHost(process_id, provider_id);
+}
+
} // anonymous namespace
ServiceWorkerProviderHost::OneShotGetReadyCallback::OneShotGetReadyCallback(
@@ -152,7 +171,8 @@ ServiceWorkerProviderHost::ServiceWorkerProviderHost(
info_(std::move(info)),
context_(context),
dispatcher_host_(dispatcher_host),
- allow_association_(true) {
+ allow_association_(true),
+ binding_(this) {
DCHECK_NE(SERVICE_WORKER_PROVIDER_UNKNOWN, info_.type);
// PlzNavigate
@@ -164,6 +184,18 @@ ServiceWorkerProviderHost::ServiceWorkerProviderHost(
render_thread_id_ = kInvalidEmbeddedWorkerThreadId;
}
context_->RegisterProviderHostByClientID(client_uuid_, this);
+
+ // PlzNavigate
+ // |provider_| and |binding_| will be bound on CompleteNavigationInitialized.
+ if (IsBrowserSideNavigationEnabled()) {
+ DCHECK(!info.client_ptr_info.is_valid() && !info.host_request.is_pending());
+ return;
+ }
+
+ provider_.Bind(std::move(info_.client_ptr_info));
+ binding_.Bind(std::move(info_.host_request));
+ binding_.set_connection_error_handler(base::Bind(
+ &RemoveProviderHost, context_, render_process_id, info_.provider_id));
}
ServiceWorkerProviderHost::~ServiceWorkerProviderHost() {
@@ -210,7 +242,7 @@ bool ServiceWorkerProviderHost::IsContextSecureForServiceWorker() const {
void ServiceWorkerProviderHost::OnVersionAttributesChanged(
ServiceWorkerRegistration* registration,
ChangedVersionAttributesMask changed_mask,
- const ServiceWorkerRegistrationInfo& info) {
+ const ServiceWorkerRegistrationInfo& /* info */) {
if (!get_ready_callback_ || get_ready_callback_->called)
return;
if (changed_mask.active_changed() && registration->active_version()) {
@@ -538,7 +570,10 @@ ServiceWorkerProviderHost::PrepareForCrossSiteTransfer() {
std::unique_ptr<ServiceWorkerProviderHost> provisional_host =
base::WrapUnique(new ServiceWorkerProviderHost(
- process_id(), std::move(info_), context_, dispatcher_host()));
+ process_id(),
+ ServiceWorkerProviderHostInfo(std::move(info_), binding_.Unbind(),
+ provider_.PassInterface()),
+ context_, dispatcher_host()));
for (const GURL& pattern : associated_patterns_)
DecreaseProcessReference(pattern);
@@ -569,6 +604,15 @@ void ServiceWorkerProviderHost::CompleteCrossSiteTransfer(
info_.provider_id = provisional_host->provider_id();
info_.type = provisional_host->provider_type();
+ // Take the connection over from the provisional host.
+ DCHECK(!provider_.is_bound());
+ DCHECK(!binding_.is_bound());
+ provider_.Bind(provisional_host->provider_.PassInterface());
+ binding_.Bind(provisional_host->binding_.Unbind());
+ binding_.set_connection_error_handler(
+ base::Bind(&RemoveProviderHost, context_, provisional_host->process_id(),
+ provisional_host->provider_id()));
+
FinalizeInitialization(provisional_host->process_id(),
provisional_host->frame_id(),
provisional_host->dispatcher_host());
@@ -577,7 +621,7 @@ void ServiceWorkerProviderHost::CompleteCrossSiteTransfer(
// PlzNavigate
void ServiceWorkerProviderHost::CompleteNavigationInitialized(
int process_id,
- int frame_routing_id,
+ ServiceWorkerProviderHostInfo info,
ServiceWorkerDispatcherHost* dispatcher_host) {
CHECK(IsBrowserSideNavigationEnabled());
DCHECK_EQ(ChildProcessHost::kInvalidUniqueID, render_process_id_);
@@ -585,9 +629,18 @@ void ServiceWorkerProviderHost::CompleteNavigationInitialized(
DCHECK_EQ(kDocumentMainThreadId, render_thread_id_);
DCHECK_NE(ChildProcessHost::kInvalidUniqueID, process_id);
- DCHECK_NE(MSG_ROUTING_NONE, frame_routing_id);
-
- FinalizeInitialization(process_id, frame_routing_id, dispatcher_host);
+ DCHECK_EQ(info_.provider_id, info.provider_id);
+ DCHECK_NE(MSG_ROUTING_NONE, info.route_id);
+
+ // Connect with the provider on the renderer.
+ DCHECK(!provider_.is_bound());
+ DCHECK(!binding_.is_bound());
+ provider_.Bind(std::move(info.client_ptr_info));
+ binding_.Bind(std::move(info.host_request));
+ binding_.set_connection_error_handler(
+ base::Bind(&RemoveProviderHost, context_, process_id, provider_id()));
+
+ FinalizeInitialization(process_id, info.route_id, dispatcher_host);
}
void ServiceWorkerProviderHost::SendUpdateFoundMessage(

Powered by Google App Engine
This is Rietveld 408576698