Bug 1110485 P4 Keep Cache Actors alive during async operations. r=baku
authorBen Kelly <ben@wanderview.com>
Thu, 16 Apr 2015 12:00:15 -0700
changeset 239647 eb26492727eaadaccdb1e582381a827465ad9cc3
parent 239646 35ce6f6ef1f2ce514a18082871aab86e3e6cad82
child 239648 9013c5c61f3468e122da7f406683313b4ad3b61f
push id12444
push userryanvm@gmail.com
push dateFri, 17 Apr 2015 20:04:42 +0000
treeherderfx-team@560a202db924 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1110485
milestone40.0a1
Bug 1110485 P4 Keep Cache Actors alive during async operations. r=baku
dom/cache/Cache.cpp
dom/cache/CacheChild.cpp
dom/cache/CacheChild.h
dom/cache/CacheOpChild.cpp
dom/cache/CacheOpChild.h
dom/cache/CachePushStreamChild.h
dom/cache/CacheStorage.cpp
dom/cache/CacheStorageChild.cpp
dom/cache/CacheStorageChild.h
dom/cache/CacheStreamControlChild.cpp
dom/cache/CacheStreamControlChild.h
dom/cache/ReadStream.cpp
dom/cache/ReadStream.h
dom/cache/StreamControl.cpp
dom/cache/StreamControl.h
--- a/dom/cache/Cache.cpp
+++ b/dom/cache/Cache.cpp
@@ -9,17 +9,16 @@
 #include "mozilla/dom/Headers.h"
 #include "mozilla/dom/InternalResponse.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/Response.h"
 #include "mozilla/dom/WorkerPrivate.h"
 #include "mozilla/dom/CacheBinding.h"
 #include "mozilla/dom/cache/AutoUtils.h"
 #include "mozilla/dom/cache/CacheChild.h"
-#include "mozilla/dom/cache/CacheOpChild.h"
 #include "mozilla/dom/cache/CachePushStreamChild.h"
 #include "mozilla/dom/cache/ReadStream.h"
 #include "mozilla/ErrorResult.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/unused.h"
 #include "nsIGlobalObject.h"
 #include "nsNetUtil.h"
 
@@ -345,20 +344,17 @@ Cache::AssertOwningThread() const
 #endif
 
 CachePushStreamChild*
 Cache::CreatePushStream(nsIAsyncInputStream* aStream)
 {
   NS_ASSERT_OWNINGTHREAD(Cache);
   MOZ_ASSERT(mActor);
   MOZ_ASSERT(aStream);
-  auto actor = mActor->SendPCachePushStreamConstructor(
-    new CachePushStreamChild(mActor->GetFeature(), aStream));
-  MOZ_ASSERT(actor);
-  return static_cast<CachePushStreamChild*>(actor);
+  return mActor->CreatePushStream(aStream);
 }
 
 Cache::~Cache()
 {
   NS_ASSERT_OWNINGTHREAD(Cache);
   if (mActor) {
     mActor->StartDestroy();
     // DestroyInternal() is called synchronously by StartDestroy().  So we
@@ -370,18 +366,15 @@ Cache::~Cache()
 already_AddRefed<Promise>
 Cache::ExecuteOp(AutoChildOpArgs& aOpArgs, ErrorResult& aRv)
 {
   nsRefPtr<Promise> promise = Promise::Create(mGlobal, aRv);
   if (!promise) {
     return nullptr;
   }
 
-  unused << mActor->SendPCacheOpConstructor(
-    new CacheOpChild(mActor->GetFeature(), mGlobal, this, promise),
-    aOpArgs.SendAsOpArgs());
-
+  mActor->ExecuteOp(mGlobal, promise, aOpArgs.SendAsOpArgs());
   return promise.forget();
 }
 
 } // namespace cache
 } // namespace dom
 } // namespace mozilla
--- a/dom/cache/CacheChild.cpp
+++ b/dom/cache/CacheChild.cpp
@@ -4,18 +4,18 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/dom/cache/CacheChild.h"
 
 #include "mozilla/unused.h"
 #include "mozilla/dom/cache/ActorUtils.h"
 #include "mozilla/dom/cache/Cache.h"
-#include "mozilla/dom/cache/PCacheOpChild.h"
-#include "mozilla/dom/cache/PCachePushStreamChild.h"
+#include "mozilla/dom/cache/CacheOpChild.h"
+#include "mozilla/dom/cache/CachePushStreamChild.h"
 #include "mozilla/dom/cache/StreamUtils.h"
 
 namespace mozilla {
 namespace dom {
 namespace cache {
 
 // Declared in ActorUtils.h
 PCacheChild*
@@ -28,25 +28,27 @@ AllocPCacheChild()
 void
 DeallocPCacheChild(PCacheChild* aActor)
 {
   delete aActor;
 }
 
 CacheChild::CacheChild()
   : mListener(nullptr)
+  , mNumChildActors(0)
 {
   MOZ_COUNT_CTOR(cache::CacheChild);
 }
 
 CacheChild::~CacheChild()
 {
   MOZ_COUNT_DTOR(cache::CacheChild);
   NS_ASSERT_OWNINGTHREAD(CacheChild);
   MOZ_ASSERT(!mListener);
+  MOZ_ASSERT(!mNumChildActors);
 }
 
 void
 CacheChild::SetListener(Cache* aListener)
 {
   NS_ASSERT_OWNINGTHREAD(CacheChild);
   MOZ_ASSERT(!mListener);
   mListener = aListener;
@@ -57,34 +59,59 @@ void
 CacheChild::ClearListener()
 {
   NS_ASSERT_OWNINGTHREAD(CacheChild);
   MOZ_ASSERT(mListener);
   mListener = nullptr;
 }
 
 void
+CacheChild::ExecuteOp(nsIGlobalObject* aGlobal, Promise* aPromise,
+                      const CacheOpArgs& aArgs)
+{
+  mNumChildActors += 1;
+  MOZ_ALWAYS_TRUE(SendPCacheOpConstructor(
+    new CacheOpChild(GetFeature(), aGlobal, aPromise), aArgs));
+}
+
+CachePushStreamChild*
+CacheChild::CreatePushStream(nsIAsyncInputStream* aStream)
+{
+  mNumChildActors += 1;
+  auto actor = SendPCachePushStreamConstructor(
+    new CachePushStreamChild(GetFeature(), aStream));
+  MOZ_ASSERT(actor);
+  return static_cast<CachePushStreamChild*>(actor);
+}
+
+void
 CacheChild::StartDestroy()
 {
   nsRefPtr<Cache> listener = mListener;
 
   // StartDestroy() can get called from either Cache or the Feature.
   // Theoretically we can get double called if the right race happens.  Handle
   // that by just ignoring the second StartDestroy() call.
   if (!listener) {
     return;
   }
 
-  // TODO: only destroy if there are no ops or push streams still running
-
   listener->DestroyInternal(this);
 
   // Cache listener should call ClearListener() in DestroyInternal()
   MOZ_ASSERT(!mListener);
 
+  // If we have outstanding child actors, then don't destroy ourself yet.
+  // The child actors should be short lived and we should allow them to complete
+  // if possible.  SendTeardown() will be called when the count drops to zero
+  // in NoteDeletedActor().
+  if (mNumChildActors) {
+    return;
+  }
+
   // Start actor destruction from parent process
   unused << SendTeardown();
 }
 
 void
 CacheChild::ActorDestroy(ActorDestroyReason aReason)
 {
   NS_ASSERT_OWNINGTHREAD(CacheChild);
@@ -104,28 +131,39 @@ CacheChild::AllocPCacheOpChild(const Cac
   MOZ_CRASH("CacheOpChild should be manually constructed.");
   return nullptr;
 }
 
 bool
 CacheChild::DeallocPCacheOpChild(PCacheOpChild* aActor)
 {
   delete aActor;
+  NoteDeletedActor();
   return true;
 }
 
 PCachePushStreamChild*
 CacheChild::AllocPCachePushStreamChild()
 {
   MOZ_CRASH("CachePushStreamChild should be manually constructed.");
   return nullptr;
 }
 
 bool
 CacheChild::DeallocPCachePushStreamChild(PCachePushStreamChild* aActor)
 {
   delete aActor;
+  NoteDeletedActor();
   return true;
 }
 
+void
+CacheChild::NoteDeletedActor()
+{
+  mNumChildActors -= 1;
+  if (!mNumChildActors && !mListener) {
+    unused << SendTeardown();
+  }
+}
+
 } // namespace cache
 } // namespace dom
 } // namesapce mozilla
--- a/dom/cache/CacheChild.h
+++ b/dom/cache/CacheChild.h
@@ -5,36 +5,51 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_cache_CacheChild_h
 #define mozilla_dom_cache_CacheChild_h
 
 #include "mozilla/dom/cache/ActorChild.h"
 #include "mozilla/dom/cache/PCacheChild.h"
 
+class nsIAsyncInputStream;
+class nsIGlobalObject;
+
 namespace mozilla {
 namespace dom {
+
+class Promise;
+
 namespace cache {
 
 class Cache;
+class CacheOpArgs;
+class CachePushStreamChild;
 
 class CacheChild final : public PCacheChild
                        , public ActorChild
 {
 public:
   CacheChild();
   ~CacheChild();
 
   void SetListener(Cache* aListener);
 
   // Must be called by the associated Cache listener in its ActorDestroy()
   // method.  Also, Cache must Send__delete__() the actor in its destructor to
   // trigger ActorDestroy() if it has not been called yet.
   void ClearListener();
 
+  void
+  ExecuteOp(nsIGlobalObject* aGlobal, Promise* aPromise,
+            const CacheOpArgs& aArgs);
+
+  CachePushStreamChild*
+  CreatePushStream(nsIAsyncInputStream* aStream);
+
   // ActorChild methods
 
   // Synchronously call ActorDestroy on our Cache listener and then start the
   // actor destruction asynchronously from the parent-side.
   virtual void StartDestroy() override;
 
 private:
   // PCacheChild methods
@@ -48,20 +63,25 @@ private:
   DeallocPCacheOpChild(PCacheOpChild* aActor) override;
 
   virtual PCachePushStreamChild*
   AllocPCachePushStreamChild() override;
 
   virtual bool
   DeallocPCachePushStreamChild(PCachePushStreamChild* aActor) override;
 
+  // utility methods
+  void
+  NoteDeletedActor();
+
   // Use a weak ref so actor does not hold DOM object alive past content use.
   // The Cache object must call ClearListener() to null this before its
   // destroyed.
   Cache* MOZ_NON_OWNING_REF mListener;
+  uint32_t mNumChildActors;
 
   NS_DECL_OWNINGTHREAD
 };
 
 } // namespace cache
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/cache/CacheOpChild.cpp
+++ b/dom/cache/CacheOpChild.cpp
@@ -12,23 +12,21 @@
 #include "mozilla/dom/cache/Cache.h"
 #include "mozilla/dom/cache/CacheChild.h"
 
 namespace mozilla {
 namespace dom {
 namespace cache {
 
 CacheOpChild::CacheOpChild(Feature* aFeature, nsIGlobalObject* aGlobal,
-                           nsISupports* aParent, Promise* aPromise)
+                           Promise* aPromise)
   : mGlobal(aGlobal)
-  , mParent(aParent)
   , mPromise(aPromise)
 {
   MOZ_ASSERT(mGlobal);
-  MOZ_ASSERT(mParent);
   MOZ_ASSERT(mPromise);
 
   MOZ_ASSERT_IF(!NS_IsMainThread(), aFeature);
   SetFeature(aFeature);
 }
 
 CacheOpChild::~CacheOpChild()
 {
--- a/dom/cache/CacheOpChild.h
+++ b/dom/cache/CacheOpChild.h
@@ -20,22 +20,25 @@ namespace dom {
 class Promise;
 
 namespace cache {
 
 class CacheOpChild final : public PCacheOpChild
                          , public ActorChild
                          , public TypeUtils
 {
-public:
-  CacheOpChild(Feature* aFeature, nsIGlobalObject* aGlobal,
-               nsISupports* aParent, Promise* aPromise);
+  friend class CacheChild;
+  friend class CacheStorageChild;
+
+private:
+  // This class must be constructed by CacheChild or CacheStorageChild using
+  // their ExecuteOp() factory method.
+  CacheOpChild(Feature* aFeature, nsIGlobalObject* aGlobal, Promise* aPromise);
   ~CacheOpChild();
 
-private:
   // PCacheOpChild methods
   virtual void
   ActorDestroy(ActorDestroyReason aReason) override;
 
   virtual bool
   Recv__delete__(const ErrorResult& aRv, const CacheOpResult& aResult) override;
 
   // ActorChild methods
@@ -60,17 +63,16 @@ private:
 
   void
   HandleResponseList(const nsTArray<CacheResponse>& aResponseList);
 
   void
   HandleRequestList(const nsTArray<CacheRequest>& aRequestList);
 
   nsCOMPtr<nsIGlobalObject> mGlobal;
-  nsCOMPtr<nsISupports> mParent;
   nsRefPtr<Promise> mPromise;
 
   NS_DECL_OWNINGTHREAD
 };
 
 } // namespace cache
 } // namespace dom
 } // namespace mozilla
--- a/dom/cache/CachePushStreamChild.h
+++ b/dom/cache/CachePushStreamChild.h
@@ -15,27 +15,30 @@ class nsIAsyncInputStream;
 
 namespace mozilla {
 namespace dom {
 namespace cache {
 
 class CachePushStreamChild final : public PCachePushStreamChild
                                  , public ActorChild
 {
+  friend class CacheChild;
+
 public:
-  CachePushStreamChild(Feature* aFeature, nsIAsyncInputStream* aStream);
-  ~CachePushStreamChild();
+  void Start();
 
   virtual void StartDestroy() override;
 
-  void Start();
-
 private:
   class Callback;
 
+  // This class must be constructed using CacheChild::CreatePushStream()
+  CachePushStreamChild(Feature* aFeature, nsIAsyncInputStream* aStream);
+  ~CachePushStreamChild();
+
   // PCachePushStreamChild methods
   virtual void
   ActorDestroy(ActorDestroyReason aReason) override;
 
   void DoRead();
 
   void Wait();
 
--- a/dom/cache/CacheStorage.cpp
+++ b/dom/cache/CacheStorage.cpp
@@ -8,17 +8,16 @@
 
 #include "mozilla/unused.h"
 #include "mozilla/dom/CacheStorageBinding.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/Response.h"
 #include "mozilla/dom/cache/AutoUtils.h"
 #include "mozilla/dom/cache/Cache.h"
 #include "mozilla/dom/cache/CacheChild.h"
-#include "mozilla/dom/cache/CacheOpChild.h"
 #include "mozilla/dom/cache/CacheStorageChild.h"
 #include "mozilla/dom/cache/Feature.h"
 #include "mozilla/dom/cache/PCacheChild.h"
 #include "mozilla/dom/cache/ReadStream.h"
 #include "mozilla/dom/cache/TypeUtils.h"
 #include "mozilla/ipc/BackgroundChild.h"
 #include "mozilla/ipc/BackgroundUtils.h"
 #include "mozilla/ipc/PBackgroundChild.h"
@@ -435,18 +434,16 @@ CacheStorage::MaybeRunPendingRequests()
     if (entry->mRequest) {
       args.Add(entry->mRequest, IgnoreBody, PassThroughReferrer,
                IgnoreInvalidScheme, rv);
     }
     if (rv.Failed()) {
       entry->mPromise->MaybeReject(rv);
       continue;
     }
-    unused << mActor->SendPCacheOpConstructor(
-      new CacheOpChild(mActor->GetFeature(), mGlobal, this, entry->mPromise),
-      args.SendAsOpArgs());
+    mActor->ExecuteOp(mGlobal, entry->mPromise, args.SendAsOpArgs());
   }
   mPendingRequests.Clear();
 }
 
 } // namespace cache
 } // namespace dom
 } // namespace mozilla
--- a/dom/cache/CacheStorageChild.cpp
+++ b/dom/cache/CacheStorageChild.cpp
@@ -3,33 +3,34 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/dom/cache/CacheStorageChild.h"
 
 #include "mozilla/unused.h"
 #include "mozilla/dom/cache/CacheChild.h"
+#include "mozilla/dom/cache/CacheOpChild.h"
 #include "mozilla/dom/cache/CacheStorage.h"
-#include "mozilla/dom/cache/PCacheOpChild.h"
 #include "mozilla/dom/cache/StreamUtils.h"
 
 namespace mozilla {
 namespace dom {
 namespace cache {
 
 // declared in ActorUtils.h
 void
 DeallocPCacheStorageChild(PCacheStorageChild* aActor)
 {
   delete aActor;
 }
 
 CacheStorageChild::CacheStorageChild(CacheStorage* aListener, Feature* aFeature)
   : mListener(aListener)
+  , mNumChildActors(0)
 {
   MOZ_COUNT_CTOR(cache::CacheStorageChild);
   MOZ_ASSERT(mListener);
 
   SetFeature(aFeature);
 }
 
 CacheStorageChild::~CacheStorageChild()
@@ -43,36 +44,51 @@ void
 CacheStorageChild::ClearListener()
 {
   NS_ASSERT_OWNINGTHREAD(CacheStorageChild);
   MOZ_ASSERT(mListener);
   mListener = nullptr;
 }
 
 void
+CacheStorageChild::ExecuteOp(nsIGlobalObject* aGlobal, Promise* aPromise,
+                             const CacheOpArgs& aArgs)
+{
+  mNumChildActors += 1;
+  unused << SendPCacheOpConstructor(
+    new CacheOpChild(GetFeature(), aGlobal, aPromise), aArgs);
+}
+
+void
 CacheStorageChild::StartDestroy()
 {
   NS_ASSERT_OWNINGTHREAD(CacheStorageChild);
 
   nsRefPtr<CacheStorage> listener = mListener;
 
   // StartDestroy() can get called from either CacheStorage or the Feature.
   // Theoretically we can get double called if the right race happens.  Handle
   // that by just ignoring the second StartDestroy() call.
   if (!listener) {
     return;
   }
 
-  // TODO: don't destroy if we have outstanding ops
-
   listener->DestroyInternal(this);
 
   // CacheStorage listener should call ClearListener() in DestroyInternal()
   MOZ_ASSERT(!mListener);
 
+  // If we have outstanding child actors, then don't destroy ourself yet.
+  // The child actors should be short lived and we should allow them to complete
+  // if possible.  SendTeardown() will be called when the count drops to zero
+  // in NoteDeletedActor().
+  if (mNumChildActors) {
+    return;
+  }
+
   // Start actor destruction from parent process
   unused << SendTeardown();
 }
 
 void
 CacheStorageChild::ActorDestroy(ActorDestroyReason aReason)
 {
   NS_ASSERT_OWNINGTHREAD(CacheStorageChild);
@@ -92,14 +108,25 @@ CacheStorageChild::AllocPCacheOpChild(co
   MOZ_CRASH("CacheOpChild should be manually constructed.");
   return nullptr;
 }
 
 bool
 CacheStorageChild::DeallocPCacheOpChild(PCacheOpChild* aActor)
 {
   delete aActor;
+  NoteDeletedActor();
   return true;
 }
 
+void
+CacheStorageChild::NoteDeletedActor()
+{
+  MOZ_ASSERT(mNumChildActors);
+  mNumChildActors -= 1;
+  if (!mNumChildActors && !mListener) {
+    unused << SendTeardown();
+  }
+}
+
 } // namespace cache
 } // namespace dom
 } // namespace mozilla
--- a/dom/cache/CacheStorageChild.h
+++ b/dom/cache/CacheStorageChild.h
@@ -6,20 +6,26 @@
 
 #ifndef mozilla_dom_cache_CacheStorageChild_h
 #define mozilla_dom_cache_CacheStorageChild_h
 
 #include "mozilla/dom/cache/ActorChild.h"
 #include "mozilla/dom/cache/Types.h"
 #include "mozilla/dom/cache/PCacheStorageChild.h"
 
+class nsIGlobalObject;
+
 namespace mozilla {
 namespace dom {
+
+class Promise;
+
 namespace cache {
 
+class CacheOpArgs;
 class CacheStorage;
 class PCacheChild;
 class Feature;
 
 class CacheStorageChild final : public PCacheStorageChild
                               , public ActorChild
 {
 public:
@@ -27,36 +33,45 @@ public:
   ~CacheStorageChild();
 
   // Must be called by the associated CacheStorage listener in its
   // ActorDestroy() method.  Also, CacheStorage must call SendDestroy() on the
   // actor in its destructor to trigger ActorDestroy() if it has not been
   // called yet.
   void ClearListener();
 
+  void
+  ExecuteOp(nsIGlobalObject* aGlobal, Promise* aPromise,
+            const CacheOpArgs& aArgs);
+
   // ActorChild methods
 
   // Synchronously call ActorDestroy on our CacheStorage listener and then start
   // the actor destruction asynchronously from the parent-side.
   virtual void StartDestroy() override;
 
 private:
   // PCacheStorageChild methods
   virtual void ActorDestroy(ActorDestroyReason aReason) override;
 
   virtual PCacheOpChild*
   AllocPCacheOpChild(const CacheOpArgs& aOpArgs) override;
 
   virtual bool
   DeallocPCacheOpChild(PCacheOpChild* aActor) override;
 
+  // utility methods
+  void
+  NoteDeletedActor();
+
   // Use a weak ref so actor does not hold DOM object alive past content use.
   // The CacheStorage object must call ClearListener() to null this before its
   // destroyed.
   CacheStorage* MOZ_NON_OWNING_REF mListener;
+  uint32_t mNumChildActors;
 
   NS_DECL_OWNINGTHREAD
 };
 
 } // namespace cache
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/cache/CacheStreamControlChild.cpp
+++ b/dom/cache/CacheStreamControlChild.cpp
@@ -36,16 +36,17 @@ AllocPCacheStreamControlChild()
 void
 DeallocPCacheStreamControlChild(PCacheStreamControlChild* aActor)
 {
   delete aActor;
 }
 
 CacheStreamControlChild::CacheStreamControlChild()
   : mDestroyStarted(false)
+  , mDestroyDelayed(false)
 {
   MOZ_COUNT_CTOR(cache::CacheStreamControlChild);
 }
 
 CacheStreamControlChild::~CacheStreamControlChild()
 {
   NS_ASSERT_OWNINGTHREAD(CacheStreamControlChild);
   MOZ_COUNT_DTOR(cache::CacheStreamControlChild);
@@ -58,16 +59,31 @@ CacheStreamControlChild::StartDestroy()
   // This can get called twice under some circumstances.  For example, if the
   // actor is added to a Feature that has already been notified and the Cache
   // actor has no mListener.
   if (mDestroyStarted) {
     return;
   }
   mDestroyStarted = true;
 
+  // If any of the streams have started to be read, then wait for them to close
+  // naturally.
+  if (HasEverBeenRead()) {
+    // Note that we are delaying so that we can re-check for active streams
+    // in NoteClosedAfterForget().
+    mDestroyDelayed = true;
+    return;
+  }
+
+  // Otherwise, if the streams have not been touched then just pre-emptively
+  // close them now.  This handles the case where someone retrieves a Response
+  // from the Cache, but never accesses the body.  We should not keep the
+  // Worker alive until that Response is GC'd just because of its ignored
+  // body stream.
+
   // Begin shutting down all streams.  This is the same as if the parent had
   // asked us to shutdown.  So simulate the CloseAll IPC message.
   RecvCloseAll();
 }
 
 void
 CacheStreamControlChild::SerializeControl(CacheReadStream* aReadStreamOut)
 {
@@ -115,16 +131,25 @@ CacheStreamControlChild::DeserializeFds(
   unused << fdSetActor->Send__delete__(fdSetActor);
 }
 
 void
 CacheStreamControlChild::NoteClosedAfterForget(const nsID& aId)
 {
   NS_ASSERT_OWNINGTHREAD(CacheStreamControlChild);
   unused << SendNoteClosed(aId);
+
+  // A stream has closed.  If we delayed StartDestry() due to this stream
+  // being read, then we should check to see if any of the remaining streams
+  // are active.  If none of our other streams have been read, then we can
+  // proceed with the shutdown now.
+  if (mDestroyDelayed && !HasEverBeenRead()) {
+    mDestroyDelayed = false;
+    RecvCloseAll();
+  }
 }
 
 #ifdef DEBUG
 void
 CacheStreamControlChild::AssertOwningThread()
 {
   NS_ASSERT_OWNINGTHREAD(CacheStreamControlChild);
 }
--- a/dom/cache/CacheStreamControlChild.h
+++ b/dom/cache/CacheStreamControlChild.h
@@ -51,16 +51,17 @@ private:
 #endif
 
   // PCacheStreamControlChild methods
   virtual void ActorDestroy(ActorDestroyReason aReason) override;
   virtual bool RecvClose(const nsID& aId) override;
   virtual bool RecvCloseAll() override;
 
   bool mDestroyStarted;
+  bool mDestroyDelayed;
 
   NS_DECL_OWNINGTHREAD
 };
 
 } // namespace cache
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/cache/ReadStream.cpp
+++ b/dom/cache/ReadStream.cpp
@@ -45,16 +45,19 @@ public:
   CloseStream() override;
 
   virtual void
   CloseStreamWithoutReporting() override;
 
   virtual bool
   MatchId(const nsID& aId) const override;
 
+  virtual bool
+  HasEverBeenRead() const override;
+
   // Simulate nsIInputStream methods, but we don't actually inherit from it
   NS_METHOD
   Close();
 
   NS_METHOD
   Available(uint64_t *aNumAvailableOut);
 
   NS_METHOD
@@ -98,16 +101,17 @@ private:
 
   enum State
   {
     Open,
     Closed,
     NumStates
   };
   Atomic<State> mState;
+  Atomic<bool> mHasEverBeenRead;
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(cache::ReadStream::Inner, override)
 };
 
 // ----------------------------------------------------------------------------
 
 // Runnable to notify actors that the ReadStream has closed.  This must
 // be done on the thread associated with the PBackground actor.  Must be
@@ -244,16 +248,23 @@ ReadStream::Inner::CloseStreamWithoutRep
 
 bool
 ReadStream::Inner::MatchId(const nsID& aId) const
 {
   MOZ_ASSERT(NS_GetCurrentThread() == mOwningThread);
   return mId.Equals(aId);
 }
 
+bool
+ReadStream::Inner::HasEverBeenRead() const
+{
+  MOZ_ASSERT(NS_GetCurrentThread() == mOwningThread);
+  return mHasEverBeenRead;
+}
+
 NS_IMETHODIMP
 ReadStream::Inner::Close()
 {
   // stream ops can happen on any thread
   nsresult rv = mStream->Close();
   NoteClosed();
   return rv;
 }
@@ -279,34 +290,48 @@ ReadStream::Inner::Read(char* aBuf, uint
 
   nsresult rv = mSnappyStream->Read(aBuf, aCount, aNumReadOut);
 
   if ((NS_FAILED(rv) && rv != NS_BASE_STREAM_WOULD_BLOCK) ||
       *aNumReadOut == 0) {
     Close();
   }
 
+  mHasEverBeenRead = true;
+
   return rv;
 }
 
 NS_IMETHODIMP
 ReadStream::Inner::ReadSegments(nsWriteSegmentFun aWriter, void* aClosure,
                                 uint32_t aCount, uint32_t* aNumReadOut)
 {
   // stream ops can happen on any thread
   MOZ_ASSERT(aNumReadOut);
 
+  if (aCount) {
+    mHasEverBeenRead = true;
+  }
+
   nsresult rv = mSnappyStream->ReadSegments(aWriter, aClosure, aCount,
                                             aNumReadOut);
 
   if ((NS_FAILED(rv) && rv != NS_BASE_STREAM_WOULD_BLOCK &&
                         rv != NS_ERROR_NOT_IMPLEMENTED) || *aNumReadOut == 0) {
     Close();
   }
 
+  // Verify bytes were actually read before marking as being ever read.  For
+  // example, code can test if the stream supports ReadSegments() by calling
+  // this method with a dummy callback which doesn't read anything.  We don't
+  // want to trigger on that.
+  if (*aNumReadOut) {
+    mHasEverBeenRead = true;
+  }
+
   return rv;
 }
 
 NS_IMETHODIMP
 ReadStream::Inner::IsNonBlocking(bool* aNonBlockingOut)
 {
   // stream ops can happen on any thread
   return mSnappyStream->IsNonBlocking(aNonBlockingOut);
--- a/dom/cache/ReadStream.h
+++ b/dom/cache/ReadStream.h
@@ -58,16 +58,19 @@ public:
     // Closes the stream and then forgets the stream control.  Does not
     // notify.
     virtual void
     CloseStreamWithoutReporting() = 0;
 
     virtual bool
     MatchId(const nsID& aId) const = 0;
 
+    virtual bool
+    HasEverBeenRead() const = 0;
+
     NS_IMETHOD_(MozExternalRefCountType)
     AddRef(void) = 0;
 
     NS_IMETHOD_(MozExternalRefCountType)
     Release(void) = 0;
   };
 
   static already_AddRefed<ReadStream>
--- a/dom/cache/StreamControl.cpp
+++ b/dom/cache/StreamControl.cpp
@@ -79,11 +79,23 @@ StreamControl::CloseAllReadStreamsWithou
   while (iter.HasMore()) {
     nsRefPtr<ReadStream::Controllable> stream = iter.GetNext();
     // Note, we cannot trigger IPC traffic here.  So use
     // CloseStreamWithoutReporting().
     stream->CloseStreamWithoutReporting();
   }
 }
 
+bool
+StreamControl::HasEverBeenRead() const
+{
+  ReadStreamList::ForwardIterator iter(mReadStreamList);
+  while (iter.HasMore()) {
+    if (iter.GetNext()->HasEverBeenRead()) {
+      return true;
+    }
+  }
+  return false;
+}
+
 } // namespace cache
 } // namespace dom
 } // namespace mozilla
--- a/dom/cache/StreamControl.h
+++ b/dom/cache/StreamControl.h
@@ -63,16 +63,19 @@ protected:
   CloseReadStreams(const nsID& aId);
 
   void
   CloseAllReadStreams();
 
   void
   CloseAllReadStreamsWithoutReporting();
 
+  bool
+  HasEverBeenRead() const;
+
   // protected parts of the abstract interface
   virtual void
   NoteClosedAfterForget(const nsID& aId) = 0;
 
 #ifdef DEBUG
   virtual void
   AssertOwningThread() = 0;
 #else