PlzNavigate: Make ServiceWorker work with PlzNavigate.

content/browser/service_worker/service_worker_dispatcher_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_dispatcher_host.h"
 
+#include "base/command_line.h"
 #include "base/logging.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/message_port_message_filter.h"
 #include "content/browser/message_port_service.h"
 #include "content/browser/service_worker/embedded_worker_registry.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_handle.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_registration_handle.h"
 #include "content/common/service_worker/embedded_worker_messages.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/content_browser_client.h"
 #include "content/public/common/content_client.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/origin_util.h"
 #include "ipc/ipc_message_macros.h"
 #include "net/base/net_util.h"
 #include "third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerError.h"
 #include "url/gurl.h"
 
 using blink::WebServiceWorkerError;
 
 namespace content {
 
@@ skipping 682 matching lines @@
           "477117 ServiceWorkerDispatcherHost::OnProviderCreated"));
   TRACE_EVENT0("ServiceWorker",
                "ServiceWorkerDispatcherHost::OnProviderCreated");
   if (!GetContext())
     return;
   if (GetContext()->GetProviderHost(render_process_id_, provider_id)) {
     bad_message::ReceivedBadMessage(this,
                                     bad_message::SWDH_PROVIDER_CREATED_NO_HOST);
     return;
   }
-  scoped_ptr<ServiceWorkerProviderHost> provider_host(
-      new ServiceWorkerProviderHost(render_process_id_, route_id, provider_id,
-                                    provider_type, GetContext()->AsWeakPtr(),
-                                    this));
+
+  scoped_ptr<ServiceWorkerProviderHost> provider_host;
+  if (ServiceWorkerUtils::IsBrowserAssignedProviderId(provider_id)) {
+    // PlzNavigate
+    CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(

[michaeln, 2015/09/01 01:15:23]

Should this be treated as a bad_message instead of a CHECK? A buggy or
compromised renderer could provide garbage for this id and I don't think the
browser should crash as a result.

[Fabrice (no longer in Chrome), 2015/09/07 12:09:48]

That's a good point. How about changing it to a DCHECK here instead? We'd still
have a way to catch it on tests.

[nasko, 2015/09/14 21:28:32]

If we know that this can be hit from compromised renderer, then using CHECK is
the wrong thing to do. It is indeed a great fit for bad_message.

[clamy, 2015/09/14 21:34:59]

Note that historically, we have put CHECKs in PlzNavigate specific IPC handlers
in RFH. I think those can be triggered by a compromised renderer as well.

[Fabrice (no longer in Chrome), 2015/09/15 15:38:54]

I do not know if having a test at all is relevant then. Essentially, we would be
testing that if the provider_id received is negative, then PlzNavigate must be
enabled. This would only work during the transition period. Either way, the
second test on line 737 already takes care of that, indirectly.
I will remove this CHECK in a future patch.

+        switches::kEnableBrowserSideNavigation));
+    provider_host = GetContext()->TakeBrowserProviderHost(provider_id);
+    if (provider_host == nullptr) {
+      bad_message::ReceivedBadMessage(
+          this, bad_message::SWDH_PROVIDER_CREATED_NO_HOST);
+      return;
+    }

[michaeln, 2015/09/01 01:15:23]

That CHECK(CommandLine) might make more sense on this line, but it's at the top
of CompleteBrowserIntialized() so maybe its not needed in here at all?

[Fabrice (no longer in Chrome), 2015/09/07 12:09:48]

Acknowledged.

+    DCHECK_EQ(SERVICE_WORKER_PROVIDER_FOR_WINDOW, provider_type);
+    provider_host->CompleteBrowserInitialized(render_process_id_, route_id);
+  } else {
+    provider_host =
+        scoped_ptr<ServiceWorkerProviderHost>(new ServiceWorkerProviderHost(
+            render_process_id_, route_id, provider_id, provider_type,
+            GetContext()->AsWeakPtr(), this));
+  }
   GetContext()->AddProviderHost(provider_host.Pass());
 }
 
 void ServiceWorkerDispatcherHost::OnProviderDestroyed(int provider_id) {
   TRACE_EVENT0("ServiceWorker",
                "ServiceWorkerDispatcherHost::OnProviderDestroyed");
   if (!GetContext())
     return;
   if (!GetContext()->GetProviderHost(render_process_id_, provider_id)) {
     bad_message::ReceivedBadMessage(
@@ skipping 519 matching lines @@
   if (!handle) {
     bad_message::ReceivedBadMessage(this,
                                     bad_message::SWDH_TERMINATE_BAD_HANDLE);
     return;
   }
   handle->version()->StopWorker(
       base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));
 }
 
 }  // namespace content