CacheStorage: Only create AbortController for addAll() with multiple requests.

There was a minor memory regression when we landed the cache.addAll()
abort logic.  To minimize this avoid creating the AbortController when
there is only one request.

Bug: 1130781,1133989
Change-Id: I19a324c0ce14dd100b5dc350a17a60f7172709a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461410Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817077}

third_party/blink/renderer/modules/cache_storage/cache.cc

@@ -185,8 +185,6 @@ class Cache::BarrierCallbackForPutResponse final
                                 const ExceptionState& exception_state,
                                 int64_t trace_id)
       : resolver_(MakeGarbageCollected<ScriptPromiseResolver>(script_state)),
-        abort_controller_(
-            cache->CreateAbortController(ExecutionContext::From(script_state))),
         cache_(cache),
         method_name_(method_name),
         request_list_(request_list),
@@ -195,12 +193,19 @@ class Cache::BarrierCallbackForPutResponse final
         interface_name_(exception_state.InterfaceName()),
         trace_id_(trace_id),
         response_list_(request_list_.size()),
-        blob_list_(request_list_.size()) {}
+        blob_list_(request_list_.size()) {
+    if (request_list.size() > 1) {
+      abort_controller_ =
+          cache_->CreateAbortController(ExecutionContext::From(script_state));
+    }
+  }
 
   // Must be called prior to starting the load of any response.
   ScriptPromise Promise() const { return resolver_->Promise(); }
 
-  AbortSignal* Signal() const { return abort_controller_->signal(); }
+  AbortSignal* Signal() const {
+    return abort_controller_ ? abort_controller_->signal() : nullptr;
+  }
 
   void CompletedResponse(int index,
                          Response* response,
@@ -267,7 +272,8 @@ class Cache::BarrierCallbackForPutResponse final
   void Stop() {
     if (stopped_)
       return;
-    abort_controller_->abort();
+    if (abort_controller_)
+      abort_controller_->abort();
     blob_list_.clear();
     stopped_ = true;
   }
@@ -1071,7 +1077,8 @@ ScriptPromise Cache::AddAllImpl(ScriptState* script_state,
   for (wtf_size_t i = 0; i < request_list.size(); ++i) {
     // Chain the AbortSignal objects together so the requests will abort if
     // the |barrier_callback| encounters an error.
-    request_list[i]->signal()->Follow(barrier_callback->Signal());
+    if (barrier_callback->Signal())
+      request_list[i]->signal()->Follow(barrier_callback->Signal());
 
     RequestInfo info;
     info.SetRequest(request_list[i]);

third_party/blink/renderer/modules/cache_storage/cache_test.cc

@@ -783,9 +783,38 @@ TEST_F(CacheStorageTest, Add) {
             test_cache()->GetAndClearLastErrorWebCacheMethodCalled());
 }
 
+// Verify we don't create and trigger the AbortController when a single request
+// to add() addAll() fails.
+TEST_F(CacheStorageTest, AddAllAbortOne) {
+  ScriptState::Scope scope(GetScriptState());
+  DummyExceptionStateForTesting exception_state;
+  auto* fetcher = MakeGarbageCollected<ScopedFetcherForTests>();
+  const String url = "http://www.cacheadd.test/";
+  const String content_type = "text/plain";
+  const String content = "hello cache";
+
+  TestCache* cache =
+      CreateCache(fetcher, std::make_unique<NotImplementedErrorCache>());
+
+  Request* request = NewRequestFromUrl(url);
+  fetcher->SetExpectedFetchUrl(&url);
+
+  Response* response = Response::error(GetScriptState());
+  fetcher->SetResponse(response);
+
+  HeapVector<RequestInfo> info_list;
+  info_list.push_back(RequestToRequestInfo(request));
+
+  ScriptPromise promise =
+      cache->addAll(GetScriptState(), info_list, exception_state);
+
+  EXPECT_EQ("TypeError: Request failed", GetRejectString(promise));
+  EXPECT_FALSE(cache->IsAborted());
+}
+
 // Verify an error response causes Cache::addAll() to trigger its associated
 // AbortController to cancel outstanding requests.
-TEST_F(CacheStorageTest, AddAllAbort) {
+TEST_F(CacheStorageTest, AddAllAbortMany) {
   ScriptState::Scope scope(GetScriptState());
   DummyExceptionStateForTesting exception_state;
   auto* fetcher = MakeGarbageCollected<ScopedFetcherForTests>();
@@ -804,6 +833,7 @@ TEST_F(CacheStorageTest, AddAllAbort) {
 
   HeapVector<RequestInfo> info_list;
   info_list.push_back(RequestToRequestInfo(request));
+  info_list.push_back(RequestToRequestInfo(request));
 
   ScriptPromise promise =
       cache->addAll(GetScriptState(), info_list, exception_state);