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 045540defef435cabb1f3e97d3223776c42107e0..c795f3cffa1b3ca540dc0104493133cb3d6cb9d6 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,49 @@ 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;
+  bool browser_side_navigation =
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation);
+
+  if (ServiceWorkerNetworkProvider::FromDocumentState(
+          DocumentState::FromDataSource(datasource)))
+    return;

[nasko, 2015/10/02 22:09:46]

nit: Empty line after the return and before the next if statement.

[Fabrice (no longer in Chrome), 2015/10/06 17:21:38]

Done.

+  if (browser_side_navigation && content_initiated)

[nasko, 2015/10/02 22:09:46]

Can you include a comment why we are returning here. It isn't completely
intuitive.

[Fabrice (no longer in Chrome), 2015/10/06 17:21:38]

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 -1 for sandboxed frames.

[nasko, 2015/10/02 22:09:46]

Let's use the symbolic name, not the constant -1.

[Fabrice (no longer in Chrome), 2015/10/06 17:21:38]

Done.

+    if (service_worker_provider_id == kInvalidServiceWorkerProviderId)
+      provider_type = SERVICE_WORKER_PROVIDER_FOR_SANDBOXED_FRAME;
+    else
+      DCHECK(ServiceWorkerUtils::IsBrowserAssignedProviderId(

[nasko, 2015/10/02 22:09:45]

More than one line body requires {}.

[Fabrice (no longer in Chrome), 2015/10/06 17:21:38]

Done.

+          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,
@@ -3337,17 +3364,20 @@ void RenderFrameImpl::willSendRequest(
       ServiceWorkerNetworkProvider* provider =
           ServiceWorkerNetworkProvider::FromDocumentState(
               DocumentState::FromDataSource(frame->provisionalDataSource()));
-      provider_id = provider->provider_id();
+      if (provider)
+        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) {
+      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();
@@ -4908,12 +4938,28 @@ void RenderFrameImpl::BeginNavigation(blink::WebURLRequest* request) {
   if (data_source && render_view_->history_list_length_ > 0) {
     should_replace_current_entry = data_source->replacesCurrentHistoryItem();
   }
+
+  DCHECK_EQ(FETCH_REQUEST_MODE_SAME_ORIGIN,

[nasko, 2015/10/02 22:09:45]

Let's put a comment above all of the DCHECKs to explain why we are performing
them - aka we assume those values on the browser side, so they better match
here.

[Fabrice (no longer in Chrome), 2015/10/06 17:21:38]

Done.

+            GetFetchRequestModeForWebURLRequest(*request));
+  DCHECK_EQ(FETCH_CREDENTIALS_MODE_INCLUDE,
+            GetFetchCredentialsModeForWebURLRequest(*request));
+  DCHECK(GetFetchRedirectModeForWebURLRequest(*request) ==
+         FetchRedirectMode::MANUAL_MODE);
+  DCHECK_IMPLIES(!frame_->parent(),
+                 GetRequestContextFrameTypeForWebURLRequest(*request) ==
+                     REQUEST_CONTEXT_FRAME_TYPE_TOP_LEVEL);
+  DCHECK_IMPLIES(frame_->parent(),
+                 GetRequestContextFrameTypeForWebURLRequest(*request) ==
+                     REQUEST_CONTEXT_FRAME_TYPE_NESTED);
+
   Send(new FrameHostMsg_BeginNavigation(
       routing_id_,
       MakeCommonNavigationParams(request, should_replace_current_entry),
       BeginNavigationParams(
           request->httpMethod().latin1(), GetWebURLRequestHeaders(*request),
-          GetLoadFlagsForWebURLRequest(*request), request->hasUserGesture()),
+          GetLoadFlagsForWebURLRequest(*request), request->hasUserGesture(),
+          request->skipServiceWorker(),
+          GetRequestContextTypeForWebURLRequest(*request)),
       GetRequestBodyForWebURLRequest(*request)));
 }