PlzNavigate: Make ServiceWorker work with PlzNavigate.

content/browser/service_worker/service_worker_request_handler.cc

Index: content/browser/service_worker/service_worker_request_handler.cc
diff --git a/content/browser/service_worker/service_worker_request_handler.cc b/content/browser/service_worker/service_worker_request_handler.cc
index 6d7d04b2e0a1b99a9101916faee3f4dd1612a9fb..dfe44ef94d0fb11b46cad1111c41674e43095627 100644
--- a/content/browser/service_worker/service_worker_request_handler.cc
+++ b/content/browser/service_worker/service_worker_request_handler.cc
@@ -6,6 +6,8 @@
 
 #include <string>
 
+#include "base/command_line.h"
+#include "content/browser/loader/navigation_url_loader_impl_core.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_provider_host.h"
@@ -15,6 +17,7 @@
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/resource_context.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/origin_util.h"
 #include "net/base/net_util.h"
 #include "net/url_request/url_request.h"
@@ -64,7 +67,8 @@ void ServiceWorkerRequestHandler::InitializeHandler(
     ResourceType resource_type,
     RequestContextType request_context_type,
     RequestContextFrameType frame_type,
-    scoped_refptr<ResourceRequestBody> body) {
+    scoped_refptr<ResourceRequestBody> body,
+    NavigationURLLoaderImplCore* loader) {

[michaeln, 2015/09/16 00:56:42]

it'd be nice to not have this dependency

[Fabrice (no longer in Chrome), 2015/09/30 17:32:07]

Done.

   // Create the handler even for insecure HTTP since it's used in the
   // case of redirect to HTTPS.
   if (!request->url().SchemeIsHTTPOrHTTPS() &&
@@ -77,13 +81,27 @@ void ServiceWorkerRequestHandler::InitializeHandler(
     return;
   }
 
-  ServiceWorkerProviderHost* provider_host =
-      context_wrapper->context()->GetProviderHost(process_id, provider_id);
+  ServiceWorkerProviderHost* provider_host = nullptr;
+  if (process_id == -1) {
+    // PlzNavigate
+    // Initialize a dummy SWProviderHost for browser-initiated navigations.

[michaeln, 2015/09/16 00:56:42]

"Dummy" might be misleading. It's a real 'host'. It gets constructed when a
navigation is initiated. That happens differently for brower vs renderer
intiated navs.

[Fabrice (no longer in Chrome), 2015/09/30 17:32:07]

Done.

+    CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
+        switches::kEnableBrowserSideNavigation));
+    DCHECK(loader);
+    DCHECK(ServiceWorkerUtils::IsBrowserAssignedProviderId(provider_id));
+    provider_host =
+        context_wrapper->context()->CreateBrowserProviderHost(provider_id);
+  } else {
+    provider_host =
+        context_wrapper->context()->GetProviderHost(process_id, provider_id);
+  }
+
   if (!provider_host || !provider_host->IsContextAlive())
     return;
 
   if (skip_service_worker) {
     if (ServiceWorkerUtils::IsMainResourceType(resource_type)) {
+      // TODO(horo): Does this work properly for PlzNavigate?
       provider_host->SetDocumentUrl(net::SimplifyUrlForRequest(request->url()));
       provider_host->SetTopmostFrameUrl(request->first_party_for_cookies());
       // A page load with skip_service_worker should be triggered by
@@ -102,6 +120,8 @@ void ServiceWorkerRequestHandler::InitializeHandler(
     return;
 
   request->SetUserData(&kUserDataKey, handler.release());
+  if (process_id == -1)
+    loader->SetServiceWorkerHandler(handler.get());

[michaeln, 2015/09/16 00:56:42]

This method has quirky artifacts now depending on how its called. I'd like to
make the differences in behavior more obvious. The differences in behavior are
pretty significant...

* sometimes this method creates a SWProviderHost vs sometimes not
* sometimes this method assigns ownership of the host to the 'handler' vs
sometimes not

Making these hidden behaviors more explicit could help down the road when its
time to delete code for the non plznavigate way of doing navigations and also
help readers understand what's going on and why.

[michaeln, 2015/09/19 00:14:40]

bah, handler.get() will always be null here given the handler.release() on the
previous line

[Fabrice (no longer in Chrome), 2015/09/30 17:32:07]

I added a new method with different semantics so this is no longer relevant.

 }
 
 ServiceWorkerRequestHandler* ServiceWorkerRequestHandler::GetHandler(
@@ -128,37 +148,54 @@ bool ServiceWorkerRequestHandler::IsControlledByServiceWorker(
 
 void ServiceWorkerRequestHandler::PrepareForCrossSiteTransfer(
     int old_process_id) {
+  CHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));

[michaeln, 2015/09/16 00:56:42]

just checking, but are these pre-existing methods only reachable with the
plznavigate flag is set?

[Fabrice (no longer in Chrome), 2015/09/30 17:32:07]

These methods are only reachable when the flag is not set (aka current
navigation).
There is no cross-site transfer semantics in the PlzNavigate world.

   if (!provider_host_ || !context_)
     return;
   old_process_id_ = old_process_id;
   old_provider_id_ = provider_host_->provider_id();
-  host_for_cross_site_transfer_ =
-      context_->TransferProviderHostOut(old_process_id,
-                                        provider_host_->provider_id());
-  DCHECK_EQ(provider_host_.get(), host_for_cross_site_transfer_.get());
+  temporary_host_ = context_->TransferProviderHostOut(
+      old_process_id, provider_host_->provider_id());
+  DCHECK_EQ(provider_host_.get(), temporary_host_.get());
 }
 
 void ServiceWorkerRequestHandler::CompleteCrossSiteTransfer(
     int new_process_id, int new_provider_id) {
-  if (!host_for_cross_site_transfer_.get() || !context_)
+  CHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  if (!temporary_host_.get() || !context_)
     return;
-  DCHECK_EQ(provider_host_.get(), host_for_cross_site_transfer_.get());
-  context_->TransferProviderHostIn(
-      new_process_id,
-      new_provider_id,
-      host_for_cross_site_transfer_.Pass());
+  DCHECK_EQ(provider_host_.get(), temporary_host_.get());
+  context_->TransferProviderHostIn(new_process_id, new_provider_id,
+                                   temporary_host_.Pass());
   DCHECK_EQ(provider_host_->provider_id(), new_provider_id);
 }
 
 void ServiceWorkerRequestHandler::MaybeCompleteCrossSiteTransferInOldProcess(
     int old_process_id) {
-  if (!host_for_cross_site_transfer_.get() || !context_ ||
+  CHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  if (!temporary_host_.get() || !context_ ||
       old_process_id_ != old_process_id) {
     return;
   }
   CompleteCrossSiteTransfer(old_process_id_, old_provider_id_);
 }
 
+scoped_ptr<ServiceWorkerProviderHost>
+ServiceWorkerRequestHandler::TakeBrowserProviderHost() {
+  CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  return temporary_host_.Pass();
+}
+
+void ServiceWorkerRequestHandler::SetBrowserProviderHost(
+    scoped_ptr<ServiceWorkerProviderHost> provider_host) {
+  CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));
+  temporary_host_ = provider_host.Pass();
+}
+
 ServiceWorkerRequestHandler::~ServiceWorkerRequestHandler() {
 }