CacheStorage: Check for null manager in cross-sequence wrappers.

A nullptr manager can be encountered if a cross-sequence operation
is begun during the shutdown of the storage partition.

Bug: 1033251
Change-Id: I98c77a714dc7eb04a31a103c95012a7a989b9580
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1964931Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726032}

content/browser/background_fetch/background_fetch_data_manager.cc

@@ -61,8 +61,6 @@ void BackgroundFetchDataManager::InitializeOnCoreThread() {
 
   // Delete inactive registrations still in the DB.
   Cleanup();
-
-  DCHECK(cache_manager_);
 }
 
 void BackgroundFetchDataManager::AddObserver(

content/browser/cache_storage/cache_storage_context_impl.cc

@@ -42,7 +42,8 @@ scoped_refptr<base::SequencedTaskRunner> CreateSchedulerTaskRunner() {
 CacheStorageContextImpl::CacheStorageContextImpl(
     BrowserContext* browser_context)
     : task_runner_(CreateSchedulerTaskRunner()),
-      observers_(base::MakeRefCounted<ObserverList>()) {
+      observers_(base::MakeRefCounted<ObserverList>()),
+      shutdown_(false) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 }
 
@@ -87,6 +88,12 @@ void CacheStorageContextImpl::Init(
 
 void CacheStorageContextImpl::Shutdown() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(!shutdown_);
+
+  // Set an atomic flag indicating shutdown has been entered.  This allows us to
+  // avoid creating CrossSequenceCacheStorageManager objects when there will
+  // no longer be an underlying manager.
+  shutdown_ = true;
 
   task_runner_->PostTask(
       FROM_HERE,
@@ -108,6 +115,12 @@ void CacheStorageContextImpl::AddReceiver(
 }
 
 scoped_refptr<CacheStorageManager> CacheStorageContextImpl::CacheManager() {
+  // Always return nullptr once shutdown has begun regardless of which
+  // sequence we are called on.  This check is necessary to avoid creating
+  // CrossSequenceCacheStorageManager wrappers when there will no longer be an
+  // underlying manager.
+  if (shutdown_)
+    return nullptr;
   // If we're already on the target sequence, then just return the real manager.
   //
   // Note, we can't check for nullptr cache_manager_ here because it is not
@@ -218,6 +231,7 @@ void CacheStorageContextImpl::CreateCacheStorageManagerOnTaskRunner(
 
 void CacheStorageContextImpl::ShutdownOnTaskRunner() {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
+  DCHECK(shutdown_);
 
   // Delete session-only ("clear on exit") origins.
   if (special_storage_policy_ &&

content/browser/cache_storage/cache_storage_context_impl.h

@@ -138,6 +138,9 @@ class CONTENT_EXPORT CacheStorageContextImpl
   // Initialized in Init(); true if the user data directory is empty.
   bool is_incognito_ = false;
 
+  // True once Shutdown() has been called on the UI thread.
+  std::atomic<bool> shutdown_;
+
   // Initialized in Init().
   scoped_refptr<storage::SpecialStoragePolicy> special_storage_policy_;
 

content/browser/cache_storage/cross_sequence/cross_sequence_cache_storage.cc

@@ -24,8 +24,11 @@ class CrossSequenceCacheStorage::Inner {
 
   Inner(const url::Origin& origin,
         CacheStorageOwner owner,
-        scoped_refptr<CacheStorageContextWithManager> context)
-      : handle_(context->CacheManager()->OpenCacheStorage(origin, owner)) {}
+        scoped_refptr<CacheStorageContextWithManager> context) {
+    scoped_refptr<CacheStorageManager> manager = context->CacheManager();
+    if (manager)
+      handle_ = manager->OpenCacheStorage(origin, owner);
+  }
 
   void OpenCache(scoped_refptr<CrossSequenceCacheStorageCache> cache_wrapper,
                  const std::string& cache_name,
@@ -143,7 +146,7 @@ class CrossSequenceCacheStorage::Inner {
   }
 
  private:
-  const CacheStorageHandle handle_;
+  CacheStorageHandle handle_;
   SEQUENCE_CHECKER(sequence_checker_);
 };
 

content/browser/cache_storage/cross_sequence/cross_sequence_cache_storage_manager.cc

@@ -8,6 +8,7 @@
 #include "content/browser/cache_storage/cache_storage_context_impl.h"
 #include "content/browser/cache_storage/cross_sequence/cross_sequence_cache_storage.h"
 #include "content/browser/cache_storage/cross_sequence/cross_sequence_utils.h"
+#include "third_party/blink/public/mojom/quota/quota_types.mojom.h"
 
 namespace content {
 
@@ -19,7 +20,9 @@ namespace content {
 class CrossSequenceCacheStorageManager::Inner {
  public:
   explicit Inner(scoped_refptr<CacheStorageContextWithManager> context)
-      : target_manager_(context->CacheManager()) {}
+      : target_manager_(context->CacheManager()) {
+    DCHECK(target_manager_);
+  }
 
   void GetAllOriginsUsage(CacheStorageOwner owner,
                           CacheStorageContext::GetUsageInfoCallback callback) {