PlzNavigate: Make ServiceWorker work with PlzNavigate.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 551545eb266cc147cb0d83ef246234a110e9dd27..4b86e8f70953478383e380533c2d07b24393e18b 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -42,6 +42,7 @@
 #include "content/common/input_messages.h"
 #include "content/common/navigation_params.h"
 #include "content/common/service_worker/service_worker_types.h"
+#include "content/common/service_worker/service_worker_utils.h"
 #include "content/common/site_isolation_policy.h"
 #include "content/common/swapped_out_messages.h"
 #include "content/common/view_messages.h"
@@ -2584,20 +2585,56 @@ void RenderFrameImpl::didCreateDataSource(blink::WebLocalFrame* frame,
   // Create the serviceworker's per-document network observing object if it
   // does not exist (When navigation happens within a page, the provider already
   // exists).
-  if (!ServiceWorkerNetworkProvider::FromDocumentState(
-          DocumentState::FromDataSource(datasource))) {
-    ServiceWorkerProviderType provider_type =
-        SERVICE_WORKER_PROVIDER_FOR_WINDOW;
+  if (ServiceWorkerNetworkProvider::FromDocumentState(
+          DocumentState::FromDataSource(datasource)))
+    return;
+
+  bool browser_side_navigation =
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation);
+  scoped_ptr<ServiceWorkerNetworkProvider> network_provider;
+
+  if (browser_side_navigation && content_initiated) {

[michaeln, 2015/10/08 02:18:51]

thnx for restructuring this code, the different cases are a lot easier to follow
now

+    // PlzNavigate
+    // Initialize an empty ServiceWorkerNetworkProvider.

[michaeln, 2015/10/08 02:18:51]

nit: this comment isn't needed, it states the obvious

A comment that explains how there are two calls to this methods for two
different objects, one prior to sending the message requesting the browser to
navigate, and another after when committing the navigation, would help.

+    network_provider = scoped_ptr<ServiceWorkerNetworkProvider>(
+        new ServiceWorkerNetworkProvider());
+  } else if (browser_side_navigation && !content_initiated) {
+    // PlzNavigate
+    // Retrieve the service_worker_provider_id from the RequestNavigationParams.

[michaeln, 2015/10/08 02:18:51]

nit: also stating the obvious

+    NavigationStateImpl* navigation_state = static_cast<NavigationStateImpl*>(
+        DocumentState::FromDataSource(datasource)->navigation_state());
+    int service_worker_provider_id =
+        navigation_state->request_params().service_worker_provider_id;
+
+    // The browser sets service_worker_provider_id to
+    // kInvalidServiceWorkerProviderId for sandboxed frames.
+    if (service_worker_provider_id == kInvalidServiceWorkerProviderId) {
+      network_provider = scoped_ptr<ServiceWorkerNetworkProvider>(
+          new ServiceWorkerNetworkProvider());
+    } else {
+      DCHECK(ServiceWorkerUtils::IsBrowserAssignedProviderId(
+          service_worker_provider_id));
+      network_provider = scoped_ptr<ServiceWorkerNetworkProvider>(
+          new ServiceWorkerNetworkProvider(
+              routing_id_, SERVICE_WORKER_PROVIDER_FOR_WINDOW,
+              service_worker_provider_id));
+    }
+  } else {
     if ((frame->effectiveSandboxFlags() & blink::WebSandboxFlags::Origin) ==
         blink::WebSandboxFlags::Origin) {
-      provider_type = SERVICE_WORKER_PROVIDER_FOR_SANDBOXED_FRAME;
+      network_provider = scoped_ptr<ServiceWorkerNetworkProvider>(
+          new ServiceWorkerNetworkProvider(
+              routing_id_, SERVICE_WORKER_PROVIDER_FOR_SANDBOXED_FRAME));

[michaeln, 2015/10/08 02:18:51]

nit: could use the empty ctor here too so its clear that what you get on line
2164 matches what you get here

+    } else {
+      network_provider = scoped_ptr<ServiceWorkerNetworkProvider>(
+          new ServiceWorkerNetworkProvider(
+              routing_id_, SERVICE_WORKER_PROVIDER_FOR_WINDOW));
     }
-    scoped_ptr<ServiceWorkerNetworkProvider> network_provider(
-        new ServiceWorkerNetworkProvider(routing_id_, provider_type));
-    ServiceWorkerNetworkProvider::AttachToDocumentState(
-        DocumentState::FromDataSource(datasource),
-        network_provider.Pass());
   }
+
+  ServiceWorkerNetworkProvider::AttachToDocumentState(
+      DocumentState::FromDataSource(datasource), network_provider.Pass());
 }
 
 void RenderFrameImpl::didStartProvisionalLoad(blink::WebLocalFrame* frame,
@@ -3345,12 +3382,14 @@ void RenderFrameImpl::willSendRequest(
       ServiceWorkerNetworkProvider* provider =
           ServiceWorkerNetworkProvider::FromDocumentState(
               DocumentState::FromDataSource(frame->provisionalDataSource()));
+      DCHECK(provider);
       provider_id = provider->provider_id();
     }
   } else if (frame->dataSource()) {
     ServiceWorkerNetworkProvider* provider =
         ServiceWorkerNetworkProvider::FromDocumentState(
             DocumentState::FromDataSource(frame->dataSource()));
+    DCHECK(provider);
     provider_id = provider->provider_id();
     // Explicitly set the SkipServiceWorker flag here if the renderer process
     // hasn't received SetControllerServiceWorker message.