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..1d11003038db3ab55f4285199854f2531cdae7f7 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"
@@ -2581,23 +2582,55 @@ 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).

[michaeln, 2015/10/07 01:16:44]

please keep this comment about when/why a provider would already exist, its the
sort of hidden behavior that should be documented

[Fabrice (no longer in Chrome), 2015/10/07 12:59:43]

Done.

-  if (!ServiceWorkerNetworkProvider::FromDocumentState(
-          DocumentState::FromDataSource(datasource))) {
-    ServiceWorkerProviderType provider_type =
-        SERVICE_WORKER_PROVIDER_FOR_WINDOW;
+  bool browser_side_navigation =
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation);
+
+  if (ServiceWorkerNetworkProvider::FromDocumentState(
+          DocumentState::FromDataSource(datasource)))
+    return;
+
+  // PlzNavigate
+  // The RequestNavigationParams are required to initialize the
+  // ServiceWorkerNetworkProvider.
+  if (browser_side_navigation && content_initiated)

[michaeln, 2015/10/07 01:16:44]

There are a serveral callsites where a non-null provider is expected for a
frame's datasource. I think we should maintain that invariant instead of fixing
the callsites to allow otherwise.

You mentioned elsewhere that this is called twice for plznav and the first time
thru we don't have the RequestNavigationParams. If i understand properly, those
two calls are for two different WebDataSource instances. The first instance is
the provisional data source that's created (and deleted) prior to
FrameHostMsg_BeginNavigation being sent to the browser process. The second
instance is created much later, when the renderer is processing the
FrameMsg_CommitNavigation message.

If thats right, for the 'first' instances, lets create and attach an empty
provider, new ServiceWorkerNetworkProvider() so its reliably non-null.

[Fabrice (no longer in Chrome), 2015/10/07 12:59:43]

Done.

+    return;
+
+  ServiceWorkerProviderType provider_type = SERVICE_WORKER_PROVIDER_FOR_WINDOW;
+  scoped_ptr<ServiceWorkerNetworkProvider> network_provider;
+
+  if (browser_side_navigation) {
+    // PlzNavigate
+    // Retrieve the service_worker_provider_id from the RequestNavigationParams.
+    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) {
+      provider_type = SERVICE_WORKER_PROVIDER_FOR_SANDBOXED_FRAME;

[michaeln, 2015/10/07 01:16:44]

we could use the empty ctor here too

[Fabrice (no longer in Chrome), 2015/10/07 12:59:43]

Done.

+    } else {
+      DCHECK(ServiceWorkerUtils::IsBrowserAssignedProviderId(
+          service_worker_provider_id));
+    }
+
+    // Initalize a new ServiceWorkerNetworkProvider.
+    network_provider = scoped_ptr<ServiceWorkerNetworkProvider>(
+        new ServiceWorkerNetworkProvider(routing_id_, provider_type,
+                                         service_worker_provider_id));
+  } else {
     if ((frame->effectiveSandboxFlags() & blink::WebSandboxFlags::Origin) ==
         blink::WebSandboxFlags::Origin) {
       provider_type = SERVICE_WORKER_PROVIDER_FOR_SANDBOXED_FRAME;
     }
-    scoped_ptr<ServiceWorkerNetworkProvider> network_provider(
+    network_provider = scoped_ptr<ServiceWorkerNetworkProvider>(
         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,17 +3378,20 @@ void RenderFrameImpl::willSendRequest(
       ServiceWorkerNetworkProvider* provider =
           ServiceWorkerNetworkProvider::FromDocumentState(
               DocumentState::FromDataSource(frame->provisionalDataSource()));
-      provider_id = provider->provider_id();
+      if (provider)

[michaeln, 2015/10/07 01:16:44]

provided the existing invariant is maintained, this change isn't needed

[Fabrice (no longer in Chrome), 2015/10/07 12:59:43]

I DCHECK'd it instead. SIGSEGV crashes tend to not be very verbose.

+        provider_id = provider->provider_id();
     }
   } else if (frame->dataSource()) {
     ServiceWorkerNetworkProvider* provider =
         ServiceWorkerNetworkProvider::FromDocumentState(
             DocumentState::FromDataSource(frame->dataSource()));
-    provider_id = provider->provider_id();
-    // Explicitly set the SkipServiceWorker flag here if the renderer process
-    // hasn't received SetControllerServiceWorker message.
-    if (!provider->IsControlledByServiceWorker())
-      request.setSkipServiceWorker(true);
+    if (provider) {

[michaeln, 2015/10/07 01:16:44]

ditto

[Fabrice (no longer in Chrome), 2015/10/07 12:59:43]

Done.

+      provider_id = provider->provider_id();
+      // Explicitly set the SkipServiceWorker flag here if the renderer process
+      // hasn't received SetControllerServiceWorker message.
+      if (!provider->IsControlledByServiceWorker())
+        request.setSkipServiceWorker(true);
+    }
   }
 
   WebFrame* parent = frame->parent();