Keep a pending buffer for update checking until it's read

Previously ServiceWorkerSingleScriptUpdateChecker reads the body from the
network and use a MojoToNetPendingBuffer to pass the buffer to a cache writer,
which compares the body. When a diff is found, the body that has a diff is kept
in the cache writer, and ServiceWorkerUpdatedScriptLoader gets the portion from
the cache writer when the script is loaded.

However, ServiceWorkerSingleScriptUpdateChecker calls CompleteRead() after
finding a diff, so that the contents of the buffer might be updated to the
succeeding contents when the buffer is re-used. (Precisely, the internal buffer
in Mojo data pipe is a ring buffer, so this happens when the writer overtake the
reader.)

This CL fixes the issue by keeping the pending buffer until resuming finishes.
Now we can make sure that the portion of the body which has a diff is kept until
the body is served to the renderer process.

Bug: 1032517
Change-Id: I39376dd37a00ce15d6e016ba787dafe50746b765
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1981247
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727407}

content/browser/service_worker/service_worker_script_loader_factory_unittest.cc

@@ -193,22 +193,15 @@ TEST_F(ServiceWorkerScriptLoaderFactoryCopyResumeTest,
       "HTTP/1.0 200 OK\0Content-Type: text/javascript\0Content-Length: 0\0\0";
   const std::string kNewData = "";
 
-  mojo::ScopedDataPipeProducerHandle network_producer;
-  mojo::ScopedDataPipeConsumerHandle network_consumer;
-
-  ASSERT_EQ(MOJO_RESULT_OK, mojo::CreateDataPipe(nullptr, &network_producer,
-                                                 &network_consumer));
   ServiceWorkerUpdateCheckTestUtils::CreateAndSetComparedScriptInfoForVersion(
       script_url_, 0, kNewHeaders, kNewData, kOldResourceId, kNewResourceId,
       helper_.get(), ServiceWorkerUpdatedScriptLoader::LoaderState::kCompleted,
       ServiceWorkerUpdatedScriptLoader::WriterState::kCompleted,
-      std::move(network_consumer),
       ServiceWorkerSingleScriptUpdateChecker::Result::kDifferent,
-      version_.get());
+      version_.get(), nullptr);
 
   mojo::PendingRemote<network::mojom::URLLoader> loader =
       CreateTestLoaderAndStart(&client_);
-  network_producer.reset();
   client_.RunUntilComplete();
 
   EXPECT_EQ(net::OK, client_.completion_status().error_code);

content/browser/service_worker/service_worker_single_script_update_checker.cc

@@ -624,21 +624,26 @@ void ServiceWorkerSingleScriptUpdateChecker::OnCompareDataComplete(
       "bytes_written", bytes_written);
 
   DCHECK(pending_buffer || bytes_written == 0);
-  if (pending_buffer) {
-    // We consumed |bytes_written| bytes of data from the network so call
-    // CompleteRead(), regardless of what |error| is.
-    pending_buffer->CompleteRead(bytes_written);
-    network_consumer_ = pending_buffer->ReleaseHandle();
-  }
 
   if (cache_writer_->is_pausing()) {
     // |cache_writer_| can be pausing only when it finds difference between
     // stored body and network body.
     DCHECK_EQ(error, net::ERR_IO_PENDING);
-    Succeed(Result::kDifferent);
+    auto paused_state = std::make_unique<PausedState>(
+        std::move(cache_writer_), std::move(network_loader_),
+        network_client_receiver_.Unbind(), std::move(pending_buffer),
+        bytes_written, network_loader_state_, body_writer_state_);
+    Succeed(Result::kDifferent, std::move(paused_state));
     return;
   }
 
+  if (pending_buffer) {
+    // We consumed |bytes_written| bytes of data from the network so call
+    // CompleteRead(), regardless of what |error| is.
+    pending_buffer->CompleteRead(bytes_written);
+    network_consumer_ = pending_buffer->ReleaseHandle();
+  }
+
   if (error != net::OK) {
     // Something went wrong reading from the disk cache.
     Fail(blink::ServiceWorkerStatusCode::kErrorDiskCache,
@@ -650,7 +655,7 @@ void ServiceWorkerSingleScriptUpdateChecker::OnCompareDataComplete(
   if (bytes_written == 0) {
     // All data has been read. If we reach here without any error, the script
     // from the network was identical to the one in the disk cache.
-    Succeed(Result::kIdentical);
+    Succeed(Result::kIdentical, /*paused_state=*/nullptr);
     return;
   }
 
@@ -668,28 +673,29 @@ void ServiceWorkerSingleScriptUpdateChecker::Fail(
                          "error_message", error_message);
 
   Finish(Result::kFailed,
+         /*paused_state=*/nullptr,
          std::make_unique<FailureInfo>(status, error_message,
                                        std::move(network_status)));
 }
 
-void ServiceWorkerSingleScriptUpdateChecker::Succeed(Result result) {
+void ServiceWorkerSingleScriptUpdateChecker::Succeed(
+    Result result,
+    std::unique_ptr<PausedState> paused_state) {
   TRACE_EVENT_WITH_FLOW1(
       "ServiceWorker", "ServiceWorkerSingleScriptUpdateChecker::Succeed", this,
       TRACE_EVENT_FLAG_FLOW_IN, "result", ResultToString(result));
 
   DCHECK_NE(result, Result::kFailed);
-  Finish(result, nullptr);
+  Finish(result, std::move(paused_state), /*failure_info=*/nullptr);
 }
 
 void ServiceWorkerSingleScriptUpdateChecker::Finish(
     Result result,
+    std::unique_ptr<PausedState> paused_state,
     std::unique_ptr<FailureInfo> failure_info) {
   network_watcher_.Cancel();
   if (Result::kDifferent == result) {
-    auto paused_state = std::make_unique<PausedState>(
-        std::move(cache_writer_), std::move(network_loader_),
-        network_client_receiver_.Unbind(), std::move(network_consumer_),
-        network_loader_state_, body_writer_state_);
+    DCHECK(paused_state);
     std::move(callback_).Run(script_url_, result, nullptr,
                              std::move(paused_state));
     return;
@@ -709,13 +715,15 @@ ServiceWorkerSingleScriptUpdateChecker::PausedState::PausedState(
         network_loader,
     mojo::PendingReceiver<network::mojom::URLLoaderClient>
         network_client_receiver,
-    mojo::ScopedDataPipeConsumerHandle network_consumer,
+    scoped_refptr<network::MojoToNetPendingBuffer> pending_network_buffer,
+    uint32_t consumed_bytes,
     ServiceWorkerUpdatedScriptLoader::LoaderState network_loader_state,
     ServiceWorkerUpdatedScriptLoader::WriterState body_writer_state)
     : cache_writer(std::move(cache_writer)),
       network_loader(std::move(network_loader)),
       network_client_receiver(std::move(network_client_receiver)),
-      network_consumer(std::move(network_consumer)),
+      pending_network_buffer(std::move(pending_network_buffer)),
+      consumed_bytes(consumed_bytes),
       network_loader_state(network_loader_state),
       body_writer_state(body_writer_state) {}
 

content/browser/service_worker/service_worker_single_script_update_checker.h

@@ -64,7 +64,8 @@ class CONTENT_EXPORT ServiceWorkerSingleScriptUpdateChecker
             network_loader,
         mojo::PendingReceiver<network::mojom::URLLoaderClient>
             network_client_receiver,
-        mojo::ScopedDataPipeConsumerHandle network_consumer,
+        scoped_refptr<network::MojoToNetPendingBuffer> pending_network_buffer,
+        uint32_t consumed_bytes,
         ServiceWorkerUpdatedScriptLoader::LoaderState network_loader_state,
         ServiceWorkerUpdatedScriptLoader::WriterState body_writer_state);
     PausedState(const PausedState& other) = delete;
@@ -77,7 +78,15 @@ class CONTENT_EXPORT ServiceWorkerSingleScriptUpdateChecker
         network_loader;
     mojo::PendingReceiver<network::mojom::URLLoaderClient>
         network_client_receiver;
-    mojo::ScopedDataPipeConsumerHandle network_consumer;
+
+    // The buffer which has a part of the body from the network which had a
+    // diff. This could be nullptr when the data from the network is smaller
+    // than the data in the disk.
+    scoped_refptr<network::MojoToNetPendingBuffer> pending_network_buffer;
+    // The number of bytes in |pending_network_buffer| that have already been
+    // processed by the cache writer.
+    uint32_t consumed_bytes;
+
     ServiceWorkerUpdatedScriptLoader::LoaderState network_loader_state;
     ServiceWorkerUpdatedScriptLoader::WriterState body_writer_state;
   };
@@ -161,8 +170,11 @@ class CONTENT_EXPORT ServiceWorkerSingleScriptUpdateChecker
             network::URLLoaderCompletionStatus network_status);
 
   // Called when the update check for this script succeeded. It calls Finish().
-  void Succeed(Result result);
-  void Finish(Result result, std::unique_ptr<FailureInfo> failure_info);
+  // |paused_state| should be set when result is kDifferent.
+  void Succeed(Result result, std::unique_ptr<PausedState> paused_state);
+  void Finish(Result result,
+              std::unique_ptr<PausedState> paused_state,
+              std::unique_ptr<FailureInfo> failure_info);
 
   const GURL script_url_;
   const bool is_main_script_;

content/browser/service_worker/service_worker_test_utils.cc

@@ -625,7 +625,8 @@ ServiceWorkerUpdateCheckTestUtils::CreatePausedCacheWriter(
     EmbeddedWorkerTestHelper* worker_test_helper,
     size_t bytes_compared,
     const std::string& new_headers,
-    const std::string& diff_data_block,
+    scoped_refptr<network::MojoToNetPendingBuffer> pending_network_buffer,
+    uint32_t consumed_size,
     int64_t old_resource_id,
     int64_t new_resource_id) {
   auto cache_writer = ServiceWorkerCacheWriter::CreateForComparison(
@@ -644,9 +645,9 @@ ServiceWorkerUpdateCheckTestUtils::CreatePausedCacheWriter(
   cache_writer->headers_to_write_ =
       base::MakeRefCounted<HttpResponseInfoIOBuffer>(std::move(info));
   cache_writer->bytes_compared_ = bytes_compared;
-  cache_writer->data_to_write_ =
-      base::MakeRefCounted<net::WrappedIOBuffer>(diff_data_block.data());
-  cache_writer->len_to_write_ = diff_data_block.length();
+  cache_writer->data_to_write_ = base::MakeRefCounted<net::WrappedIOBuffer>(
+      pending_network_buffer ? pending_network_buffer->buffer() : nullptr);
+  cache_writer->len_to_write_ = consumed_size;
   cache_writer->bytes_written_ = 0;
   cache_writer->io_pending_ = true;
   cache_writer->state_ = ServiceWorkerCacheWriter::State::STATE_PAUSING;
@@ -658,15 +659,17 @@ ServiceWorkerUpdateCheckTestUtils::CreateUpdateCheckerPausedState(
     std::unique_ptr<ServiceWorkerCacheWriter> cache_writer,
     ServiceWorkerUpdatedScriptLoader::LoaderState network_loader_state,
     ServiceWorkerUpdatedScriptLoader::WriterState body_writer_state,
-    mojo::ScopedDataPipeConsumerHandle network_consumer) {
+    scoped_refptr<network::MojoToNetPendingBuffer> pending_network_buffer,
+    uint32_t consumed_size) {
   mojo::PendingRemote<network::mojom::URLLoaderClient> network_loader_client;
   mojo::PendingReceiver<network::mojom::URLLoaderClient>
       network_loader_client_receiver =
           network_loader_client.InitWithNewPipeAndPassReceiver();
   return std::make_unique<ServiceWorkerSingleScriptUpdateChecker::PausedState>(
       std::move(cache_writer), /*network_loader=*/nullptr,
-      std::move(network_loader_client_receiver), std::move(network_consumer),
-      network_loader_state, body_writer_state);
+      std::move(network_loader_client_receiver),
+      std::move(pending_network_buffer), consumed_size, network_loader_state,
+      body_writer_state);
 }
 
 void ServiceWorkerUpdateCheckTestUtils::SetComparedScriptInfoForVersion(
@@ -700,15 +703,37 @@ void ServiceWorkerUpdateCheckTestUtils::
         EmbeddedWorkerTestHelper* worker_test_helper,
         ServiceWorkerUpdatedScriptLoader::LoaderState network_loader_state,
         ServiceWorkerUpdatedScriptLoader::WriterState body_writer_state,
-        mojo::ScopedDataPipeConsumerHandle network_consumer,
         ServiceWorkerSingleScriptUpdateChecker::Result compare_result,
-        ServiceWorkerVersion* version) {
+        ServiceWorkerVersion* version,
+        mojo::ScopedDataPipeProducerHandle* out_body_handle) {
+  scoped_refptr<network::MojoToNetPendingBuffer> pending_buffer;
+  uint32_t bytes_available = 0;
+  if (!diff_data_block.empty()) {
+    mojo::ScopedDataPipeConsumerHandle network_consumer;
+    // Create a data pipe which has the new block sent from the network.
+    ASSERT_EQ(MOJO_RESULT_OK, mojo::CreateDataPipe(nullptr, out_body_handle,
+                                                   &network_consumer));
+    uint32_t written_size = diff_data_block.size();
+    ASSERT_EQ(MOJO_RESULT_OK,
+              (*out_body_handle)
+                  ->WriteData(diff_data_block.c_str(), &written_size,
+                              MOJO_WRITE_DATA_FLAG_ALL_OR_NONE));
+    ASSERT_EQ(diff_data_block.size(), written_size);
+    base::RunLoop().RunUntilIdle();
+
+    // Read the data to make a pending buffer.
+    ASSERT_EQ(MOJO_RESULT_OK,
+              network::MojoToNetPendingBuffer::BeginRead(
+                  &network_consumer, &pending_buffer, &bytes_available));
+    ASSERT_EQ(diff_data_block.size(), bytes_available);
+  }
+
   auto cache_writer = CreatePausedCacheWriter(
-      worker_test_helper, bytes_compared, new_headers, diff_data_block,
-      old_resource_id, new_resource_id);
+      worker_test_helper, bytes_compared, new_headers, pending_buffer,
+      bytes_available, old_resource_id, new_resource_id);
   auto paused_state = CreateUpdateCheckerPausedState(
       std::move(cache_writer), network_loader_state, body_writer_state,
-      std::move(network_consumer));
+      pending_buffer, bytes_available);
   SetComparedScriptInfoForVersion(script_url, old_resource_id, compare_result,
                                   std::move(paused_state), version);
 }

content/browser/service_worker/service_worker_test_utils.h

@@ -362,13 +362,16 @@ class ServiceWorkerUpdateCheckTestUtils {
   // Creates a cache writer in the paused state (a difference was found between
   // the old and new script data). |bytes_compared| is the length compared
   // until the difference was found. |new_headers| is the new script's headers.
-  // |diff_data_block| is the first block of new script data that differs from
-  // the old data.
+  // |pending_network_buffer| is a buffer that has the first block of new script
+  // data that differs from the old data. |concumsed_size| is the number of
+  // bytes of the data consumed from the Mojo data pipe kept in
+  // |pending_network_buffer|.
   static std::unique_ptr<ServiceWorkerCacheWriter> CreatePausedCacheWriter(
       EmbeddedWorkerTestHelper* worker_test_helper,
       size_t bytes_compared,
       const std::string& new_headers,
-      const std::string& diff_data_block,
+      scoped_refptr<network::MojoToNetPendingBuffer> pending_network_buffer,
+      uint32_t consumed_size,
       int64_t old_resource_id,
       int64_t new_resource_id);
 
@@ -377,7 +380,8 @@ class ServiceWorkerUpdateCheckTestUtils {
       std::unique_ptr<ServiceWorkerCacheWriter> cache_writer,
       ServiceWorkerUpdatedScriptLoader::LoaderState network_loader_state,
       ServiceWorkerUpdatedScriptLoader::WriterState body_writer_state,
-      mojo::ScopedDataPipeConsumerHandle network_consumer);
+      scoped_refptr<network::MojoToNetPendingBuffer> pending_network_buffer,
+      uint32_t consumed_size);
 
   static void SetComparedScriptInfoForVersion(
       const GURL& script_url,
@@ -399,9 +403,9 @@ class ServiceWorkerUpdateCheckTestUtils {
       EmbeddedWorkerTestHelper* worker_test_helper,
       ServiceWorkerUpdatedScriptLoader::LoaderState network_loader_state,
       ServiceWorkerUpdatedScriptLoader::WriterState body_writer_state,
-      mojo::ScopedDataPipeConsumerHandle network_consumer,
       ServiceWorkerSingleScriptUpdateChecker::Result compare_result,
-      ServiceWorkerVersion* version);
+      ServiceWorkerVersion* version,
+      mojo::ScopedDataPipeProducerHandle* out_body_handle);
 
   // Returns false if the entry for |resource_id| doesn't exist in the storage.
   // Returns true when response status is "OK" and response body is same as

content/browser/service_worker/service_worker_updated_script_loader.cc

@@ -213,7 +213,6 @@ ServiceWorkerUpdatedScriptLoader::ServiceWorkerUpdatedScriptLoader(
   network_loader_ = std::move(info.paused_state->network_loader);
   pending_network_client_receiver_ =
       std::move(info.paused_state->network_client_receiver);
-  network_consumer_ = std::move(info.paused_state->network_consumer);
 
   network_loader_state_ = info.paused_state->network_loader_state;
   DCHECK(network_loader_state_ == LoaderState::kLoadingBody ||
@@ -229,12 +228,14 @@ ServiceWorkerUpdatedScriptLoader::ServiceWorkerUpdatedScriptLoader(
   // Resume the cache writer and observe its writes, so all data written
   // is sent to |client_|.
   cache_writer_->set_write_observer(this);
-  net::Error error = cache_writer_->Resume(
-      base::BindOnce(&ServiceWorkerUpdatedScriptLoader::OnCacheWriterResumed,
-                     weak_factory_.GetWeakPtr()));
+  net::Error error = cache_writer_->Resume(base::BindOnce(
+      &ServiceWorkerUpdatedScriptLoader::OnCacheWriterResumed,
+      weak_factory_.GetWeakPtr(), info.paused_state->pending_network_buffer,
+      info.paused_state->consumed_bytes));
 
   if (error != net::ERR_IO_PENDING) {
-    OnCacheWriterResumed(error);
+    OnCacheWriterResumed(info.paused_state->pending_network_buffer,
+                         info.paused_state->consumed_bytes, error);
   }
 }
 
@@ -425,7 +426,10 @@ int ServiceWorkerUpdatedScriptLoader::WillWriteData(
   return net::ERR_IO_PENDING;
 }
 
-void ServiceWorkerUpdatedScriptLoader::OnCacheWriterResumed(net::Error error) {
+void ServiceWorkerUpdatedScriptLoader::OnCacheWriterResumed(
+    scoped_refptr<network::MojoToNetPendingBuffer> pending_network_buffer,
+    uint32_t consumed_bytes,
+    net::Error error) {
   DCHECK_NE(error, net::ERR_IO_PENDING);
   // Stop observing write operations in cache writer as further data are
   // from network which would be processed by OnNetworkDataAvailable().
@@ -443,6 +447,13 @@ void ServiceWorkerUpdatedScriptLoader::OnCacheWriterResumed(net::Error error) {
     return;
   }
 
+  // The data in the pending buffer has been processed during resuming. At this
+  // point, this completes the pending read and releases the Mojo handle to
+  // continue with reading the rest of the body.
+  DCHECK(pending_network_buffer);
+  pending_network_buffer->CompleteRead(consumed_bytes);
+  network_consumer_ = pending_network_buffer->ReleaseHandle();
+
   // Continue to load the rest of the body from the network.
   DCHECK_EQ(body_writer_state_, WriterState::kWriting);
   DCHECK(network_consumer_);

content/browser/service_worker/service_worker_updated_script_loader.h

@@ -208,7 +208,10 @@ class CONTENT_EXPORT ServiceWorkerUpdatedScriptLoader final
 
   // Called when ServiceWorkerCacheWriter::Resume() completes its work.
   // If not all data are received, it continues to download from network.
-  void OnCacheWriterResumed(net::Error error);
+  void OnCacheWriterResumed(
+      scoped_refptr<network::MojoToNetPendingBuffer> pending_network_buffer,
+      uint32_t consumed_bytes,
+      net::Error error);
 
 #if DCHECK_IS_ON()
   void CheckVersionStatusBeforeLoad();

content/browser/service_worker/service_worker_updated_script_loader_unittest.cc

@@ -149,15 +149,12 @@ class ServiceWorkerUpdatedScriptLoaderTest : public testing::Test {
       const std::string& diff_data_block,
       ServiceWorkerUpdatedScriptLoader::LoaderState network_loader_state,
       ServiceWorkerUpdatedScriptLoader::WriterState body_writer_state) {
-    // Create a data pipe which has the new block sent from the network.
-    ASSERT_EQ(MOJO_RESULT_OK, mojo::CreateDataPipe(nullptr, &network_producer_,
-                                                   &network_consumer_));
     ServiceWorkerUpdateCheckTestUtils::CreateAndSetComparedScriptInfoForVersion(
         kScriptURL, bytes_compared, new_headers, diff_data_block,
         kOldResourceId, kNewResourceId, helper_.get(), network_loader_state,
-        body_writer_state, std::move(network_consumer_),
+        body_writer_state,
         ServiceWorkerSingleScriptUpdateChecker::Result::kDifferent,
-        version_.get());
+        version_.get(), &network_producer_);
   }
 
   void NotifyLoaderCompletion(net::Error error) {
@@ -200,7 +197,6 @@ class ServiceWorkerUpdatedScriptLoaderTest : public testing::Test {
   const int64_t kOldResourceId = 1;
   const int64_t kNewResourceId = 2;
   mojo::ScopedDataPipeProducerHandle network_producer_;
-  mojo::ScopedDataPipeConsumerHandle network_consumer_;
 };
 
 // Tests the loader when the first script data block is different.

content/browser/service_worker/service_worker_version_browsertest.cc

@@ -262,6 +262,24 @@ std::unique_ptr<net::test_server::HttpResponse> RequestHandlerForUpdateWorker(
   return http_response;
 }
 
+// Returns a unique script for each request, to test service worker update.
+std::unique_ptr<net::test_server::HttpResponse>
+RequestHandlerForBigWorkerScript(const net::test_server::HttpRequest& request) {
+  static int counter = 0;
+
+  if (request.relative_url != "/service_worker/update_worker.js")
+    return std::unique_ptr<net::test_server::HttpResponse>();
+
+  auto http_response = std::make_unique<net::test_server::BasicHttpResponse>();
+  http_response->set_code(net::HTTP_OK);
+  std::string script =
+      base::StringPrintf("// empty script. counter = %d\n// ", counter++);
+  script.resize(1E6, 'x');
+  http_response->set_content(std::move(script));
+  http_response->set_content_type("text/javascript");
+  return http_response;
+}
+
 }  // namespace
 
 class ConsoleListener : public EmbeddedWorkerInstance::Listener {
@@ -1439,6 +1457,48 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
   run_loop.Run();
 }
 
+// Regression test for https://crbug.com/1032517.
+IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
+                       UpdateWithScriptLargerThanMojoDataPipeBuffer) {
+  const char kScope[] = "/service_worker/handle_fetch.html";
+  const char kWorkerUrl[] = "/service_worker/update_worker.js";
+
+  // Tell the server to return a new script for each `update_worker.js` request.
+  embedded_test_server()->RegisterRequestHandler(
+      base::BindRepeating(&RequestHandlerForBigWorkerScript));
+  StartServerAndNavigateToSetup();
+
+  // Register a service worker and wait for activation.
+  blink::mojom::ServiceWorkerRegistrationOptions options(
+      embedded_test_server()->GetURL(kScope),
+      blink::mojom::ScriptType::kClassic,
+      blink::mojom::ServiceWorkerUpdateViaCache::kNone);
+  auto observer = base::MakeRefCounted<WorkerActivatedObserver>(wrapper());
+  observer->Init();
+  public_context()->RegisterServiceWorker(
+      embedded_test_server()->GetURL(kWorkerUrl), options,
+      base::BindOnce(&ExpectResultAndRun, true, base::DoNothing()));
+  observer->Wait();
+  int64_t registration_id = observer->registration_id();
+
+  // Try to update. Update should have succeeded.
+  {
+    blink::ServiceWorkerStatusCode status =
+        blink::ServiceWorkerStatusCode::kErrorFailed;
+    bool update_found = true;
+    UpdateRegistration(registration_id, &status, &update_found);
+    EXPECT_EQ(blink::ServiceWorkerStatusCode::kOk, status);
+    EXPECT_TRUE(update_found);
+  }
+
+  // Tidy up.
+  base::RunLoop run_loop;
+  public_context()->UnregisterServiceWorker(
+      embedded_test_server()->GetURL(kScope),
+      base::BindOnce(&ExpectResultAndRun, true, run_loop.QuitClosure()));
+  run_loop.Run();
+}
+
 IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, FetchWithSaveData) {
   embedded_test_server()->RegisterRequestHandler(
       base::BindRepeating(&VerifySaveDataHeaderInRequest));