CacheStorage: Reduce number of CacheManager() calls.

Previously there were many different places where CacheManager() was
being called like this:

  if (CacheManager())
    CacheManager()->DoSomething();

This is problematic for two reasons.  First, CacheManager() can begin
returning nullptr at any time if shutdown is initiated on a separate
thread.  Second, CacheManager() will return a new object every time its
called from a separate sequence which is wasteful in this pattern.

This CL corrects these call sites to only invoke CacheManager() once
and to use the returned reference instead.

Bug: 1033251
Change-Id: I61c24fd9bbf20c8ad7791fdc5bb1698ea8a4c5bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1973998Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726383}

content/browser/cache_storage/cache_storage_context_impl.cc

@@ -180,29 +180,32 @@ void CacheStorageContextImpl::GetAllOriginsInfo(
       base::BindOnce(
           [](scoped_refptr<CacheStorageContextImpl> context,
              GetUsageInfoCallback callback) {
-            if (!context->CacheManager()) {
+            scoped_refptr<CacheStorageManager> manager =
+                context->CacheManager();
+            if (!manager) {
               std::move(callback).Run(std::vector<StorageUsageInfo>());
               return;
             }
-            context->CacheManager()->GetAllOriginsUsage(
-                CacheStorageOwner::kCacheAPI, std::move(callback));
+            manager->GetAllOriginsUsage(CacheStorageOwner::kCacheAPI,
+                                        std::move(callback));
           },
           base::RetainedRef(this), std::move(callback)));
 }
 
 void CacheStorageContextImpl::DeleteForOrigin(const GURL& origin) {
   // Can be called on any sequence.
-  task_runner_->PostTask(FROM_HERE,
-                         base::BindOnce(
-                             [](scoped_refptr<CacheStorageContextImpl> context,
-                                const GURL& origin) {
-                               if (!context->CacheManager())
-                                 return;
-                               context->CacheManager()->DeleteOriginData(
-                                   url::Origin::Create(origin),
-                                   CacheStorageOwner::kCacheAPI);
-                             },
-                             base::RetainedRef(this), origin));
+  task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(
+                     [](scoped_refptr<CacheStorageContextImpl> context,
+                        const GURL& origin) {
+                       scoped_refptr<CacheStorageManager> manager =
+                           context->CacheManager();
+                       if (!manager)
+                         return;
+                       manager->DeleteOriginData(url::Origin::Create(origin),
+                                                 CacheStorageOwner::kCacheAPI);
+                     },
+                     base::RetainedRef(this), origin));
 }
 
 void CacheStorageContextImpl::AddObserver(
@@ -298,12 +301,15 @@ void CacheStorageContextImpl::CreateQuotaClientsOnIOThread(
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!quota_manager_proxy.get())
     return;
+  scoped_refptr<CacheStorageManager> manager = CacheManager();
+  if (!manager)
+    return;
   quota_manager_proxy->RegisterClient(
       base::MakeRefCounted<CacheStorageQuotaClient>(
-          CacheManager(), CacheStorageOwner::kCacheAPI));
+          manager, CacheStorageOwner::kCacheAPI));
   quota_manager_proxy->RegisterClient(
       base::MakeRefCounted<CacheStorageQuotaClient>(
-          CacheManager(), CacheStorageOwner::kBackgroundFetch));
+          manager, CacheStorageOwner::kBackgroundFetch));
 }
 
 }  // namespace content

content/browser/cache_storage/cache_storage_context_impl.h

@@ -91,6 +91,10 @@ class CONTENT_EXPORT CacheStorageContextImpl
   // If called on the cache_storage target sequence the real manager will be
   // returned directly.  If called on any other sequence then a cross-sequence
   // wrapper object will be created and returned instead.
+  //
+  // Note, this may begun returning nullptr at any time if shutdown is initiated
+  // on a separate thread.  Prefer to call CacheManager() once and hold a
+  // reference to the returned object.
   scoped_refptr<CacheStorageManager> CacheManager() override;
 
   bool is_incognito() const { return is_incognito_; }

content/browser/cache_storage/cache_storage_dispatcher_host.cc

@@ -727,12 +727,14 @@ void CacheStorageDispatcherHost::AddCacheReceiver(
 CacheStorageHandle CacheStorageDispatcherHost::OpenCacheStorage(
     const url::Origin& origin) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (!context_ || !context_->CacheManager() ||
-      !OriginCanAccessCacheStorage(origin))
+  if (!context_ || !OriginCanAccessCacheStorage(origin))
     return CacheStorageHandle();
 
-  return context_->CacheManager()->OpenCacheStorage(
-      origin, CacheStorageOwner::kCacheAPI);
+  scoped_refptr<CacheStorageManager> manager = context_->CacheManager();
+  if (!manager)
+    return CacheStorageHandle();
+
+  return manager->OpenCacheStorage(origin, CacheStorageOwner::kCacheAPI);
 }
 
 }  // namespace content

content/browser/renderer_host/code_cache_host_impl.cc

@@ -207,7 +207,9 @@ void CodeCacheHostImpl::DidGenerateCacheableMetadataInCacheStorage(
       "CodeCacheHostImpl::DidGenerateCacheableMetadataInCacheStorage",
       TRACE_ID_GLOBAL(trace_id), TRACE_EVENT_FLAG_FLOW_OUT, "url", url.spec());
 
-  if (!cache_storage_context_->CacheManager())
+  scoped_refptr<CacheStorageManager> manager =
+      cache_storage_context_->CacheManager();
+  if (!manager)
     return;
 
   scoped_refptr<net::IOBuffer> buf =
@@ -215,9 +217,8 @@ void CodeCacheHostImpl::DidGenerateCacheableMetadataInCacheStorage(
   if (data.size())
     memcpy(buf->data(), data.data(), data.size());
 
-  CacheStorageHandle cache_storage =
-      cache_storage_context_->CacheManager()->OpenCacheStorage(
-          cache_storage_origin, CacheStorageOwner::kCacheAPI);
+  CacheStorageHandle cache_storage = manager->OpenCacheStorage(
+      cache_storage_origin, CacheStorageOwner::kCacheAPI);
   cache_storage.value()->OpenCache(
       cache_storage_cache_name, trace_id,
       base::BindOnce(&CodeCacheHostImpl::OnCacheStorageOpenCallback,