[bfcache] Fix crash in ServiceWorkerVersion::RestoreControlleeFromBfcacheMap

This CL intends to fix crash happening in ServiceWorkerVersion::
RestoreControlleeFromBackForwardCacheMap.
The crash hit the check !base::Contains(controllee_map_, client_uuid),
so it seems that the client exists in controlle_map_ when it is
supposed to be in bfcached_controllee_map_.

This could happen when a controllee was not controlled by a version
and started to be controlled in bfcache.
This change adds code to make sure that UpdateController checks
bfcache status of the client.

Bug: 1021718, 1034141
Change-Id: I1e054cfb43b4ce0e6df33fbbf0e1ec239faf1803
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1959238
Commit-Queue: Yuzu Saijo <yuzus@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726265}

content/browser/back_forward_cache_browsertest.cc

@@ -2798,6 +2798,59 @@ IN_PROC_BROWSER_TEST_P(BackForwardCacheBrowserTestWithServiceWorkerEnabled,
       FROM_HERE);
 }
 
+IN_PROC_BROWSER_TEST_P(BackForwardCacheBrowserTestWithServiceWorkerEnabled,
+                       CachedClientBecomesControlledByServiceWorker) {
+  net::EmbeddedTestServer https_server(net::EmbeddedTestServer::TYPE_HTTPS);
+  https_server.RegisterRequestHandler(
+      base::BindRepeating(&RequestHandlerForUpdateWorker));
+  https_server.AddDefaultHandlers(GetTestDataFilePath());
+  https_server.SetSSLConfig(net::EmbeddedTestServer::CERT_OK);
+  SetupCrossSiteRedirector(&https_server);
+  ASSERT_TRUE(https_server.Start());
+
+  Shell* tab_to_be_bfcached = shell();
+  Shell* tab_to_execute_service_worker = CreateBrowser();
+
+  // 1) Navigate to A in |tab_to_be_bfcached|.
+  EXPECT_TRUE(NavigateToURL(
+      tab_to_be_bfcached,
+      https_server.GetURL(
+          "a.com", "/back_forward_cache/service_worker_registration.html")));
+  RenderFrameHostImpl* rfh_a = current_frame_host();
+  RenderFrameDeletedObserver deleted(rfh_a);
+
+  // 2) Navigate away to B in |tab_to_be_bfcached|.
+  EXPECT_TRUE(NavigateToURL(tab_to_be_bfcached,
+                            https_server.GetURL("b.com", "/title1.html")));
+  EXPECT_FALSE(deleted.deleted());
+  EXPECT_TRUE(rfh_a->is_in_back_forward_cache());
+  RenderFrameHostImpl* rfh_b = current_frame_host();
+
+  // 3) Navigate to A in |tab_to_execute_service_worker|.
+  EXPECT_TRUE(NavigateToURL(
+      tab_to_execute_service_worker,
+      https_server.GetURL(
+          "a.com", "/back_forward_cache/service_worker_registration.html")));
+
+  // 4) Register a service worker for |tab_to_execute_service_worker|.
+  //    Here, rfh_a also becomes controlled by ServiceWorker by clients.claim().
+  //    TODO(yuzus): Instead of waiting for ready, this should wait for claim()
+  //    to resolve.
+  EXPECT_EQ("DONE", EvalJs(tab_to_execute_service_worker,
+                           "register('service_worker_registration.js')"));
+
+  // 5) Navigate to A in |tab_to_be_bfcached|.
+  tab_to_be_bfcached->web_contents()->GetController().GoBack();
+  EXPECT_TRUE(WaitForLoadStop(tab_to_be_bfcached->web_contents()));
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  EXPECT_FALSE(deleted.deleted());
+  EXPECT_EQ(rfh_a, current_frame_host());
+  EXPECT_FALSE(rfh_a->is_in_back_forward_cache());
+  EXPECT_TRUE(rfh_b->is_in_back_forward_cache());
+  ExpectOutcome(BackForwardCacheMetrics::HistoryNavigationOutcome::kRestored,
+                FROM_HERE);
+}
+
 INSTANTIATE_TEST_SUITE_P(All,
                          BackForwardCacheBrowserTestWithServiceWorkerEnabled,
                          testing::Bool());

content/browser/service_worker/service_worker_container_host.cc

@@ -1113,9 +1113,14 @@ void ServiceWorkerContainerHost::UpdateController(
 
   scoped_refptr<ServiceWorkerVersion> previous_version = controller_;
   controller_ = version;
-
-  if (version)
+  if (version) {
     version->AddControllee(this);
+    if (IsBackForwardCacheEnabled() && IsInBackForwardCache()) {
+      // |this| was not |version|'s controllee when |OnEnterBackForwardCache|
+      // was called.
+      version->MoveControlleeToBackForwardCacheMap(client_uuid());
+    }
+  }
   if (previous_version)
     previous_version->RemoveControllee(client_uuid());
 

content/test/data/back_forward_cache/service_worker_registration.html

+<html>
+<title>service worker registration</title>
+<script>
+let clients;
+
+async function register(worker_url, scope) {
+  try {
+    const init = scope ? {scope} : {};
+    await navigator.serviceWorker.register(worker_url, init);
+    await navigator.serviceWorker.ready;
+    return 'DONE';
+  } catch (error) {
+    return `${error}`;
+  }
+}
+</script>
+</html>

content/test/data/back_forward_cache/service_worker_registration.js

+// Copyright 2019 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.
+
+self.addEventListener('activate', async e => {
+  e.waitUntil(self.clients.claim());
+});