service worker: Rely completely on SiteInstance when PlzNavigate is enabled

This is a step toward removing the process tracking code, which we
shouldn't need to do in the PlzNavigate case. In PlzNavigate mode,
we can rely on SiteInstance to track renderer process.

I also somewhat suspect the IPC_FAILED errors are coming from using a
dead process, which can happen when we don't rely on SiteInstance.
See: https://bugs.chromium.org/p/chromium/issues/detail?id=732729#c23

Bug: 732729,  765526
Change-Id: I56d54f3c0b7200bf3e38693436619dd56c2811c2
Reviewed-on: https://chromium-review.googlesource.com/668354Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505261}

content/browser/service_worker/service_worker_process_manager.cc

@@ -14,6 +14,7 @@
 #include "content/browser/site_instance_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/site_instance.h"
+#include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/child_process_host.h"
 #include "url/gurl.h"
 
@@ -182,7 +183,10 @@ ServiceWorkerStatusCode ServiceWorkerProcessManager::AllocateWorkerProcess(
   DCHECK(!base::ContainsKey(instance_info_, embedded_worker_id))
       << embedded_worker_id << " already has a process allocated";
 
-  if (can_use_existing_process) {
+  // In non-PlzNavigate, we must manually track renderer processes in order to
+  // use an existing process. In PlzNavigate, we can depend on SiteInstance to
+  // return an existing process, so just skip this part.
+  if (!content::IsBrowserSideNavigationEnabled() && can_use_existing_process) {
     int process_id = FindAvailableProcess(pattern);
     if (process_id != ChildProcessHost::kInvalidUniqueID) {
       RenderProcessHost::FromID(process_id)->IncrementKeepAliveRefCount();

content/browser/service_worker/service_worker_process_manager.h

@@ -114,8 +114,9 @@ class CONTENT_EXPORT ServiceWorkerProcessManager {
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerProcessManagerTest, SortProcess);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerProcessManagerTest,
                            FindAvailableProcess);
-  FRIEND_TEST_ALL_PREFIXES(ServiceWorkerProcessManagerTest,
-                           AllocateWorkerProcess_FindAvailableProcess);
+  FRIEND_TEST_ALL_PREFIXES(
+      ServiceWorkerProcessManagerTest,
+      AllocateWorkerProcess_FindAvailableProcess_NonPlzNavigate);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerProcessManagerTest,
                            AllocateWorkerProcess_WithProcessReuse);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerProcessManagerTest,

content/browser/service_worker/service_worker_process_manager_unittest.cc

@@ -6,9 +6,11 @@
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/test/scoped_feature_list.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/common/service_worker/embedded_worker_settings.h"
 #include "content/public/common/child_process_host.h"
+#include "content/public/common/content_features.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
@@ -108,8 +110,13 @@ TEST_F(ServiceWorkerProcessManagerTest, FindAvailableProcess) {
   EXPECT_EQ(host3->GetID(), process_manager_->FindAvailableProcess(pattern_));
 }
 
+// This tests the process host tracking by ServiceWorkerProcessManager
+// which only is done when PlzNavigate is disabled.
 TEST_F(ServiceWorkerProcessManagerTest,
-       AllocateWorkerProcess_FindAvailableProcess) {
+       AllocateWorkerProcess_FindAvailableProcess_NonPlzNavigate) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndDisableFeature(features::kBrowserSideNavigation);
+
   const int kEmbeddedWorkerId1 = 100;
   const int kEmbeddedWorkerId2 = 200;
   const int kEmbeddedWorkerId3 = 300;