PlzNavigate: Make ServiceWorker work with PlzNavigate.

content/browser/frame_host/navigation_request.cc

Index: content/browser/frame_host/navigation_request.cc
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
index 94f061ad1f3236a509801b56c99981317f673394..a2674a34105a4ddd1ce224cea25dd2bb5a8d1bc8 100644
--- a/content/browser/frame_host/navigation_request.cc
+++ b/content/browser/frame_host/navigation_request.cc
@@ -11,6 +11,7 @@
 #include "content/browser/frame_host/navigation_request_info.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/loader/navigation_url_loader.h"
+#include "content/browser/service_worker/service_worker_navigation_handle.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/common/resource_request_body.h"
 #include "content/public/browser/navigation_controller.h"
@@ -20,6 +21,7 @@
 #include "net/base/load_flags.h"
 #include "net/http/http_request_headers.h"
 #include "net/url_request/redirect_info.h"
+#include "third_party/WebKit/public/web/WebSandboxFlags.h"
 
 namespace content {
 
@@ -211,7 +213,7 @@ bool NavigationRequest::BeginNavigation() {
 
     loader_ = NavigationURLLoader::Create(
         frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
-        info_.Pass(), this);
+        info_.Pass(), navigation_handle_->service_worker_handle(), this);
     return true;
   }
 
@@ -269,6 +271,17 @@ void NavigationRequest::OnResponseStarted(
     scoped_ptr<StreamHandle> body) {
   DCHECK(state_ == STARTED);
   state_ = RESPONSE_STARTED;
+
+  // Update the service worker params of the request params.
+  request_params_.should_create_service_worker =
+      (frame_tree_node_->current_replication_state().sandbox_flags &

[michaeln, 2015/10/20 22:12:53]

i'm wondering about potential errors stemming from the sandbox_flags in the
renderer and browser not matching or the flag changing between when we're
sampleing it (here vs in the ctor)

[nasko, 2015/10/20 23:07:42]

There is important behavior here. Sandbox flags updating works by updating
sandbox_flags, but keeping effective_sandbox_flags around. Security checks
usually need to be performed against the effective_sandbox_flags. That said, I
can't find where any spec talks about the interaction of the allow-same-origin
flag and ServiceWorker. Can you point me to that?

[clamy, 2015/10/21 16:33:48]

So should I change to checking against effective_sandbox_flags?

For the effects of it changing between when we're sampleing it:
- allowed to not allowed: we pre-created a SWProviderHost, but will not create
the corresponding renderer object. The SWProviderHost will be destroyed when the
navigation commits.
- not to allowed to allowed: we did not pre-create a SWProviderHost for the
navigation. However, we will create a SWNetworkProvider in the renderer when the
navigation commits.

Is that acceptable behavior for ServiceWorker?

[nasko, 2015/10/21 22:28:08]

I can't say for certain which one should be used, since I don't know what the
spec requires. michaeln@/kinuko@, can you help point us to the spec that
explains how sandbox flags and SW interact?

[kinuko, 2015/10/22 12:16:02]

In the latter case the first main resource load may not be controlled but
subsequent resource loads could be controlled?  That might feel slightly weird,
though I don't strongly feel we should make it behave differently. (The spec
text for dynamically changing sandbox flag seems to be a little unclear too)

It'd be still probably worth noting this behavior in a comment?

+       blink::WebSandboxFlags::Origin) != blink::WebSandboxFlags::Origin;
+  if (navigation_handle_->service_worker_handle()) {
+    request_params_.service_worker_provider_id =
+        navigation_handle_->service_worker_handle()
+            ->service_worker_provider_host_id();
+  }
+
   frame_tree_node_->navigator()->CommitNavigation(frame_tree_node_,
                                                   response.get(), body.Pass());
 }