Bug 1329669 Gracefully handle nullptr Cache actor in CacheStorage::Open() result. r=asuth
authorBen Kelly <ben@wanderview.com>
Tue, 10 Jan 2017 08:20:15 -0800
changeset 328778 ba05b37f1dd902d5d8119a0f9d54014adfe9939e
parent 328777 bc19f506f9fcc6cd8441ee708e916e7d3abb860c
child 328779 325c95d5ccf9b71167f78346f8b60c7fab9c7bea
push id31187
push userkwierso@gmail.com
push dateWed, 11 Jan 2017 01:56:54 +0000
treeherdermozilla-central@b079c9833e3e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1329669
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1329669 Gracefully handle nullptr Cache actor in CacheStorage::Open() result. r=asuth
dom/bindings/Errors.msg
dom/cache/CacheOpChild.cpp
dom/cache/Manager.cpp
--- a/dom/bindings/Errors.msg
+++ b/dom/bindings/Errors.msg
@@ -95,8 +95,9 @@ MSG_DEF(MSG_INVALID_EASING_ERROR, 1, JSE
 MSG_DEF(MSG_INVALID_SPACING_MODE_ERROR, 1, JSEXN_TYPEERR, "Invalid spacing '{0}'.")
 MSG_DEF(MSG_USELESS_SETTIMEOUT, 1, JSEXN_TYPEERR, "Useless {0} call (missing quotes around argument?)")
 MSG_DEF(MSG_TOKENLIST_NO_SUPPORTED_TOKENS, 2, JSEXN_TYPEERR, "{0} attribute of <{1}> does not define any supported tokens")
 MSG_DEF(MSG_CACHE_STREAM_CLOSED, 0, JSEXN_TYPEERR, "Response body is a cache file stream that has already been closed.")
 MSG_DEF(MSG_TIME_VALUE_OUT_OF_RANGE, 1, JSEXN_TYPEERR, "{0} is outside the supported range for time values.")
 MSG_DEF(MSG_ONLY_IF_CACHED_WITHOUT_SAME_ORIGIN, 1, JSEXN_TYPEERR, "Request mode '{0}' was used, but request cache mode 'only-if-cached' can only be used with request mode 'same-origin'.")
 MSG_DEF(MSG_THRESHOLD_RANGE_ERROR, 0, JSEXN_RANGEERR, "Threshold values must all be in the range [0, 1].")
 MSG_DEF(MSG_WORKER_THREAD_SHUTTING_DOWN, 0, JSEXN_TYPEERR, "The Worker thread is shutting down.")
+MSG_DEF(MSG_CACHE_OPEN_FAILED, 0, JSEXN_TYPEERR, "CacheStorage.open() failed to access the storage system.")
--- a/dom/cache/CacheOpChild.cpp
+++ b/dom/cache/CacheOpChild.cpp
@@ -150,16 +150,27 @@ CacheOpChild::Recv__delete__(const Error
     {
       mPromise->MaybeResolve(aResult.get_StorageHasResult().success());
       break;
     }
     case CacheOpResult::TStorageOpenResult:
     {
       auto actor = static_cast<CacheChild*>(
         aResult.get_StorageOpenResult().actorChild());
+
+      // If we have a success status then we should have an actor.  Gracefully
+      // reject instead of crashing, though, if we get a nullptr here.
+      MOZ_DIAGNOSTIC_ASSERT(actor);
+      if (!actor) {
+        ErrorResult status;
+        status.ThrowTypeError<MSG_CACHE_OPEN_FAILED>();
+        mPromise->MaybeReject(status);
+        break;
+      }
+
       actor->SetWorkerHolder(GetWorkerHolder());
       RefPtr<Cache> cache = new Cache(mGlobal, actor);
       mPromise->MaybeResolve(cache);
       break;
     }
     case CacheOpResult::TStorageDeleteResult:
     {
       mPromise->MaybeResolve(aResult.get_StorageDeleteResult().success());
--- a/dom/cache/Manager.cpp
+++ b/dom/cache/Manager.cpp
@@ -1210,34 +1210,37 @@ public:
                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
     // Look for existing cache
     bool cacheFound;
     nsresult rv = db::StorageGetCacheId(aConn, mNamespace, mArgs.key(),
                                         &cacheFound, &mCacheId);
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
     if (cacheFound) {
+      MOZ_DIAGNOSTIC_ASSERT(mCacheId != INVALID_CACHE_ID);
       return rv;
     }
 
     rv = db::CreateCacheId(aConn, &mCacheId);
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
     rv = db::StoragePutCache(aConn, mNamespace, mArgs.key(), mCacheId);
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
     rv = trans.Commit();
     if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }
 
+    MOZ_DIAGNOSTIC_ASSERT(mCacheId != INVALID_CACHE_ID);
     return rv;
   }
 
   virtual void
   Complete(Listener* aListener, ErrorResult&& aRv) override
   {
+    MOZ_DIAGNOSTIC_ASSERT(aRv.Failed() || mCacheId != INVALID_CACHE_ID);
     aListener->OnOpComplete(Move(aRv), StorageOpenResult(), mCacheId);
   }
 
 private:
   const Namespace mNamespace;
   const StorageOpenArgs mArgs;
   CacheId mCacheId;
 };