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..62196e2fa858e25d894e7e14a94a44957b383043 100644
--- a/content/browser/frame_host/navigation_request.cc
+++ b/content/browser/frame_host/navigation_request.cc
@@ -13,6 +13,7 @@
 #include "content/browser/loader/navigation_url_loader.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/common/resource_request_body.h"
+#include "content/common/service_worker/service_worker_types.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/stream_handle.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 {
 
@@ -50,6 +52,19 @@ int LoadFlagFromNavigationType(FrameMsg_Navigate_Type::Value navigation_type) {
   return load_flags;
 }
 
+// Next ServiceWorkerProviderHost ID for navigations, starts at -2 and keeps
+// going down.
+int g_next_navigation_provider_id = -2;
+
+// Returns the next ServiceWorkerProviderHost ID for navigations,
+// kInvalidServiceWorkerProviderId if the frame is sandboxed.
+int GetNextNavigationProviderId(blink::WebSandboxFlags sandbox_flags) {
+  if ((sandbox_flags & blink::WebSandboxFlags::Origin) ==
+          blink::WebSandboxFlags::Origin)
+    return kInvalidServiceWorkerProviderId;
+  return g_next_navigation_provider_id--;
+}
+
 }  // namespace
 
 // static
@@ -209,6 +224,16 @@ bool NavigationRequest::BeginNavigation() {
       return false;
     }
 
+    // Initialize the service_worker_provider_id here.
+    blink::WebSandboxFlags sandbox_flags =
+        frame_tree_node_->current_replication_state().sandbox_flags;
+    int service_worker_provider_id = GetNextNavigationProviderId(sandbox_flags);
+
+    info_->service_worker_provider_id = service_worker_provider_id;
+    request_params_.service_worker_provider_id = service_worker_provider_id;
+    navigation_handle_->set_service_worker_provider_id(

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

Nice, deferring this swspecific init work until this point is definitely an
improvement.

This could be a great place to create a wrapper class that encapsulates the
serviceworker specific bits of what happens on the browser-side and to take
ownership of it.

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

One of the issues is that we are currently on the IO thread and the dtor for
this new class will be called on the UI thread. We'd be passing a whole lot of
information to a new class while adding little in the way of clarification since
the dtor for the SWPH would end being called from another file anyway.
However, I can add comments to clarify the lifetime of the SWPH for navigations
at creation and destruction time and who will be responsible for that.

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

Hmmm, but thats not right? First, this method runs on the UI thread (right?).
And the only input needed for the wrapper class would be the
ServiceWorkerContextWrapper and maybe the sandbox_flags. Here's a public
interface that I think covers it. And the NavigationHandle having a scoped_ptr<>
to one of these would actually clarify ownership at a glance.

class ServiceWorkerNavigationHandle {
 public:
  ServiceWorkerNavigationHandle(context_wrapper, sandbox_flags);
  ~ServiceWorkerNavigationHandle();
  int provider_host_id();
  ServiceWorkerContextWrapper* context_wrapper();
};

Internally that class would
* produce the magic id value
* construct the host object and initialize it properly on the io thread
* conditionally destruct the host if needed

It might be more little complicated to put that class together than it is to
have the logic be diffuse as in the existing CL. I understand you want to be
done with the CL.

+        service_worker_provider_id);
+
     loader_ = NavigationURLLoader::Create(
         frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
         info_.Pass(), this);