PlzNavigate: Make ServiceWorker work with PlzNavigate.

content/browser/service_worker/service_worker_provider_host.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/service_worker/service_worker_provider_host.h"
 
+#include "base/command_line.h"
 #include "base/guid.h"
 #include "base/stl_util.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/message_port_message_filter.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_request_handler.h"
 #include "content/browser/service_worker/service_worker_controllee_request_handler.h"
 #include "content/browser/service_worker/service_worker_dispatcher_host.h"
 #include "content/browser/service_worker/service_worker_handle.h"
 #include "content/browser/service_worker/service_worker_registration_handle.h"
 #include "content/browser/service_worker/service_worker_version.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/resource_request_body.h"
 #include "content/common/service_worker/service_worker_messages.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/child_process_host.h"
+#include "content/public/common/content_switches.h"
 
 namespace content {
 
 namespace {
 
 ServiceWorkerClientInfo FocusOnUIThread(int render_process_id,
                                         int render_frame_id) {
   RenderFrameHostImpl* render_frame_host =
       RenderFrameHostImpl::FromID(render_process_id, render_frame_id);
   WebContentsImpl* web_contents = static_cast<WebContentsImpl*>(
@@ skipping 12 matching lines @@
 
   // Move the web contents to the foreground.
   web_contents->Activate();
 
   return ServiceWorkerProviderHost::GetWindowClientInfoOnUI(render_process_id,
                                                             render_frame_id);
 }
 
 }  // anonymous namespace
 
+// PlzNavigate
+int ServiceWorkerProviderHost::kVirtualProcessIDForBrowserRequest = -2;
+
+// PlzNavigate
+int ServiceWorkerProviderHost::next_navigation_provider_id_ = -2;
+
+// PlzNavigate
+int ServiceWorkerProviderHost::GetNextNavigationProviderId() {

[michaeln, 2015/10/01 01:42:16]

please DCHECK_CURRENTLY_ON(BrowserThread::UI), since its unusual for sw code to
run on the UI thread i'd like to call attention to that

[Fabrice (no longer in Chrome), 2015/10/01 18:29:55]

I moved it so there should no longer be any confusion here.

+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(

[michaeln, 2015/10/01 01:42:16]

Does this need to be a runtime check? I don't think this method should be
getting called unless this switch is set. If that's right, can you change this
to a DCHECK instead.

[Fabrice (no longer in Chrome), 2015/10/01 18:29:55]

It's called in both cases actually, when initializing RequestNavigationParams.

+          switches::kEnableBrowserSideNavigation)) {
+    return next_navigation_provider_id_--;
+  }
+  return kInvalidServiceWorkerProviderId;
+}
+
 ServiceWorkerProviderHost::OneShotGetReadyCallback::OneShotGetReadyCallback(
     const GetRegistrationForReadyCallback& callback)
     : callback(callback),
       called(false) {
 }
 
 ServiceWorkerProviderHost::OneShotGetReadyCallback::~OneShotGetReadyCallback() {
 }
 
 ServiceWorkerProviderHost::ServiceWorkerProviderHost(
     int render_process_id,
     int route_id,
     int provider_id,
     ServiceWorkerProviderType provider_type,
     base::WeakPtr<ServiceWorkerContextCore> context,
     ServiceWorkerDispatcherHost* dispatcher_host)
     : client_uuid_(base::GenerateGUID()),
       render_process_id_(render_process_id),
       route_id_(route_id),
       render_thread_id_(kDocumentMainThreadId),
       provider_id_(provider_id),
       provider_type_(provider_type),
       context_(context),
       dispatcher_host_(dispatcher_host),
       allow_association_(true) {
   DCHECK_NE(ChildProcessHost::kInvalidUniqueID, render_process_id_);
   DCHECK_NE(SERVICE_WORKER_PROVIDER_UNKNOWN, provider_type_);
   DCHECK_NE(SERVICE_WORKER_PROVIDER_FOR_SANDBOXED_FRAME, provider_type_);
+
+  // PlzNavigate
+  CHECK_IMPLIES(render_process_id == kVirtualProcessIDForBrowserRequest,
+                base::CommandLine::ForCurrentProcess()->HasSwitch(
+                    switches::kEnableBrowserSideNavigation));
+
   if (provider_type_ == SERVICE_WORKER_PROVIDER_FOR_CONTROLLER) {
     // Actual thread id is set when the service worker context gets started.
     render_thread_id_ = kInvalidEmbeddedWorkerThreadId;
   }
   context_->RegisterProviderHostByClientID(client_uuid_, this);
 }
 
 ServiceWorkerProviderHost::~ServiceWorkerProviderHost() {
   if (context_)
     context_->UnregisterProviderHostByClientID(client_uuid_);
@@ skipping 405 matching lines @@
   provider_type_ = SERVICE_WORKER_PROVIDER_UNKNOWN;
   dispatcher_host_ = nullptr;
 }
 
 void ServiceWorkerProviderHost::CompleteCrossSiteTransfer(
     int new_process_id,
     int new_frame_id,
     int new_provider_id,
     ServiceWorkerProviderType new_provider_type,
     ServiceWorkerDispatcherHost* new_dispatcher_host) {
+  CHECK(!base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation));

[michaeln, 2015/10/01 01:42:16]

ah, i didn't notice the ! char!

Ok, thanx for pointing out the CrossSiteXferring and PlzNavigating are mutually
exclusive. Is there reason to CHECK this or would a DCHECK to document it and
catch programmer errors be enuf?

[Fabrice (no longer in Chrome), 2015/10/01 18:29:55]

I think we should leave a CHECK here as stated above.

[michaeln, 2015/10/01 23:11:12]

If this check is to defend against bad ipc inputs, please hoist the test higher
up in the stack as mentioned below.

[Fabrice (no longer in Chrome), 2015/10/02 16:37:34]

Looking at the calling stack, this is actually taken care of in
ResourceDispatcherHostImpl::BeginRequest. Removing it here.

   DCHECK_EQ(ChildProcessHost::kInvalidUniqueID, render_process_id_);
   DCHECK_NE(ChildProcessHost::kInvalidUniqueID, new_process_id);
   DCHECK_NE(MSG_ROUTING_NONE, new_frame_id);
 
   render_process_id_ = new_process_id;
   route_id_ = new_frame_id;
   render_thread_id_ = kDocumentMainThreadId;
   provider_id_ = new_provider_id;
   provider_type_ = new_provider_type;
   dispatcher_host_ = new_dispatcher_host;
 
   for (const GURL& pattern : associated_patterns_)
     IncreaseProcessReference(pattern);
 
   for (auto& key_registration : matching_registrations_)
     IncreaseProcessReference(key_registration.second->pattern());
 
   if (associated_registration_.get()) {
     SendAssociateRegistrationMessage();
     if (dispatcher_host_ && associated_registration_->active_version()) {
       Send(new ServiceWorkerMsg_SetControllerServiceWorker(
           render_thread_id_, provider_id(),
           GetOrCreateServiceWorkerHandle(
               associated_registration_->active_version()),
           false /* shouldNotifyControllerChange */));
     }
   }
 }
 
+// PlzNavigate
+void ServiceWorkerProviderHost::CompleteNavigationInitialized(
+    int process_id,
+    int frame_id,
+    ServiceWorkerDispatcherHost* dispatcher_host) {
+  CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(

[michaeln, 2015/10/01 01:42:16]

same question about CHECK vs DCHECK?

[Fabrice (no longer in Chrome), 2015/10/01 18:29:55]

If we get here, it means the browser received a negative provider ID from the
renderer (could have been compromised) and was able to retrieve a SWProviderHost
with that ID from the map that should only be used with PlzNavigate.

[michaeln, 2015/10/01 23:11:11]

Please use BadMessageRecieved for that and put the test closer to where messages
are first received.

It's good to put checks of that nature where the inputs from the renderer are
first received. Also we don't want to kill the browser process in cases like
that. We want to kill the borken renderer. See usages of BadMessageRecieved for
examples of that. Notice that all (modulo one special case) uses of that
function under service_worker/ are in the ServiceWorkerDispatcherHost class.
That is the ipc message dispatching class which first handles a message from
potentially busted renderer.

If your other usages of CHECK are to defend against bad ipc inputs, please
shuffle them around to where messages are initially received and convert them to
BadMessageReceived.

+      switches::kEnableBrowserSideNavigation));
+  DCHECK_EQ(kVirtualProcessIDForBrowserRequest, render_process_id_);
+  DCHECK_EQ(SERVICE_WORKER_PROVIDER_FOR_WINDOW, provider_type_);
+  DCHECK_EQ(kDocumentMainThreadId, render_thread_id_);
+
+  DCHECK_NE(ChildProcessHost::kInvalidUniqueID, process_id);
+  DCHECK_NE(MSG_ROUTING_NONE, frame_id);
+
+  render_process_id_ = process_id;
+  route_id_ = frame_id;
+  dispatcher_host_ = dispatcher_host;
+
+  for (const GURL& pattern : associated_patterns_)

[michaeln, 2015/10/01 01:42:16]

CompleteCrossSiteTransfer and CompleteNavigationInitialized contain a lot of the
same code, can you factor out the common bits and reuse it

[Fabrice (no longer in Chrome), 2015/10/01 18:29:55]

Done.

+    IncreaseProcessReference(pattern);
+
+  for (auto& key_registration : matching_registrations_)
+    IncreaseProcessReference(key_registration.second->pattern());
+
+  if (associated_registration_.get()) {
+    SendAssociateRegistrationMessage();
+    if (dispatcher_host_ && associated_registration_->active_version()) {
+      Send(new ServiceWorkerMsg_SetControllerServiceWorker(
+          render_thread_id_, provider_id(),
+          GetOrCreateServiceWorkerHandle(
+              associated_registration_->active_version()),
+          false /* shouldNotifyControllerChange */));
+    }
+  }
+}
+
 void ServiceWorkerProviderHost::SendUpdateFoundMessage(
     int registration_handle_id) {
   if (!dispatcher_host_)
     return;  // Could be nullptr in some tests.
 
   if (!IsReadyToSendMessages()) {
     queued_events_.push_back(
         base::Bind(&ServiceWorkerProviderHost::SendUpdateFoundMessage,
                    AsWeakPtr(), registration_handle_id));
     return;
@@ skipping 124 matching lines @@
   return context_ != NULL;
 }
 
 void ServiceWorkerProviderHost::Send(IPC::Message* message) const {
   DCHECK(dispatcher_host_);
   DCHECK(IsReadyToSendMessages());
   dispatcher_host_->Send(message);
 }
 
 }  // namespace content