From baa43f42e8ca7518efb0cc427252676d9ec0c63b Mon Sep 17 00:00:00 2001
From: "Carsten \"Tomcat\" Book" <cbook@mozilla.com>
Date: Wed, 15 Apr 2015 09:54:39 +0200
Subject: [PATCH] Backed out changeset f37dc22f4c4f (bug 1110485)

---
 dom/cache/Cache.cpp                   | 11 +++++--
 dom/cache/CacheChild.cpp              | 46 +++------------------------
 dom/cache/CacheChild.h                | 20 ------------
 dom/cache/CacheOpChild.cpp            |  4 ++-
 dom/cache/CacheOpChild.h              | 12 +++----
 dom/cache/CachePushStreamChild.h      | 11 +++----
 dom/cache/CacheStorage.cpp            |  5 ++-
 dom/cache/CacheStorageChild.cpp       | 33 ++-----------------
 dom/cache/CacheStorageChild.h         | 15 ---------
 dom/cache/CacheStreamControlChild.cpp | 25 ---------------
 dom/cache/CacheStreamControlChild.h   |  1 -
 dom/cache/ReadStream.cpp              | 25 ---------------
 dom/cache/ReadStream.h                |  3 --
 dom/cache/StreamControl.cpp           | 12 -------
 dom/cache/StreamControl.h             |  3 --
 15 files changed, 32 insertions(+), 194 deletions(-)

diff --git a/dom/cache/Cache.cpp b/dom/cache/Cache.cpp
index 7a69510bb43..c2bff652928 100644
--- a/dom/cache/Cache.cpp
+++ b/dom/cache/Cache.cpp
@@ -14,6 +14,7 @@
 #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"
@@ -349,7 +350,10 @@ Cache::CreatePushStream(nsIAsyncInputStream* aStream)
   NS_ASSERT_OWNINGTHREAD(Cache);
   MOZ_ASSERT(mActor);
   MOZ_ASSERT(aStream);
-  return mActor->CreatePushStream(aStream);
+  auto actor = mActor->SendPCachePushStreamConstructor(
+    new CachePushStreamChild(mActor->GetFeature(), aStream));
+  MOZ_ASSERT(actor);
+  return static_cast<CachePushStreamChild*>(actor);
 }
 
 Cache::~Cache()
@@ -371,7 +375,10 @@ Cache::ExecuteOp(AutoChildOpArgs& aOpArgs, ErrorResult& aRv)
     return nullptr;
   }
 
-  mActor->ExecuteOp(mGlobal, promise, aOpArgs.SendAsOpArgs());
+  unused << mActor->SendPCacheOpConstructor(
+    new CacheOpChild(mActor->GetFeature(), mGlobal, this, promise),
+    aOpArgs.SendAsOpArgs());
+
   return promise.forget();
 }
 
diff --git a/dom/cache/CacheChild.cpp b/dom/cache/CacheChild.cpp
index f814e242001..c283d01e240 100644
--- a/dom/cache/CacheChild.cpp
+++ b/dom/cache/CacheChild.cpp
@@ -9,8 +9,8 @@
 #include "mozilla/unused.h"
 #include "mozilla/dom/cache/ActorUtils.h"
 #include "mozilla/dom/cache/Cache.h"
-#include "mozilla/dom/cache/CacheOpChild.h"
-#include "mozilla/dom/cache/CachePushStreamChild.h"
+#include "mozilla/dom/cache/PCacheOpChild.h"
+#include "mozilla/dom/cache/PCachePushStreamChild.h"
 #include "mozilla/dom/cache/StreamUtils.h"
 
 namespace mozilla {
@@ -33,7 +33,6 @@ DeallocPCacheChild(PCacheChild* aActor)
 
 CacheChild::CacheChild()
   : mListener(nullptr)
-  , mNumChildActors(0)
 {
   MOZ_COUNT_CTOR(cache::CacheChild);
 }
@@ -43,7 +42,6 @@ CacheChild::~CacheChild()
   MOZ_COUNT_DTOR(cache::CacheChild);
   NS_ASSERT_OWNINGTHREAD(CacheChild);
   MOZ_ASSERT(!mListener);
-  MOZ_ASSERT(!mNumChildActors);
 }
 
 void
@@ -63,25 +61,6 @@ CacheChild::ClearListener()
   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()
 {
@@ -94,19 +73,13 @@ CacheChild::StartDestroy()
     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();
 }
@@ -136,7 +109,6 @@ bool
 CacheChild::DeallocPCacheOpChild(PCacheOpChild* aActor)
 {
   delete aActor;
-  NoteDeletedActor();
   return true;
 }
 
@@ -151,19 +123,9 @@ 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
diff --git a/dom/cache/CacheChild.h b/dom/cache/CacheChild.h
index 9c27aaf0f3c..54250877f45 100644
--- a/dom/cache/CacheChild.h
+++ b/dom/cache/CacheChild.h
@@ -10,19 +10,11 @@
 #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
@@ -38,13 +30,6 @@ public:
   // 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
@@ -68,15 +53,10 @@ private:
   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
 };
diff --git a/dom/cache/CacheOpChild.cpp b/dom/cache/CacheOpChild.cpp
index de667688b71..797e41af522 100644
--- a/dom/cache/CacheOpChild.cpp
+++ b/dom/cache/CacheOpChild.cpp
@@ -17,11 +17,13 @@ namespace dom {
 namespace cache {
 
 CacheOpChild::CacheOpChild(Feature* aFeature, nsIGlobalObject* aGlobal,
-                           Promise* aPromise)
+                           nsISupports* aParent, Promise* aPromise)
   : mGlobal(aGlobal)
+  , mParent(aParent)
   , mPromise(aPromise)
 {
   MOZ_ASSERT(mGlobal);
+  MOZ_ASSERT(mParent);
   MOZ_ASSERT(mPromise);
 
   MOZ_ASSERT_IF(!NS_IsMainThread(), aFeature);
diff --git a/dom/cache/CacheOpChild.h b/dom/cache/CacheOpChild.h
index a2395b9614a..4794bebad35 100644
--- a/dom/cache/CacheOpChild.h
+++ b/dom/cache/CacheOpChild.h
@@ -25,15 +25,12 @@ class CacheOpChild final : public PCacheOpChild
                          , public ActorChild
                          , public TypeUtils
 {
-  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);
+public:
+  CacheOpChild(Feature* aFeature, nsIGlobalObject* aGlobal,
+               nsISupports* aParent, Promise* aPromise);
   ~CacheOpChild();
 
+private:
   // PCacheOpChild methods
   virtual void
   ActorDestroy(ActorDestroyReason aReason) override;
@@ -68,6 +65,7 @@ private:
   HandleRequestList(const nsTArray<CacheRequest>& aRequestList);
 
   nsCOMPtr<nsIGlobalObject> mGlobal;
+  nsCOMPtr<nsISupports> mParent;
   nsRefPtr<Promise> mPromise;
 
   NS_DECL_OWNINGTHREAD
diff --git a/dom/cache/CachePushStreamChild.h b/dom/cache/CachePushStreamChild.h
index 03b1d3ed4f1..cfbf1759181 100644
--- a/dom/cache/CachePushStreamChild.h
+++ b/dom/cache/CachePushStreamChild.h
@@ -20,20 +20,17 @@ namespace cache {
 class CachePushStreamChild final : public PCachePushStreamChild
                                  , public ActorChild
 {
-  friend class CacheChild;
-
 public:
-  void Start();
+  CachePushStreamChild(Feature* aFeature, nsIAsyncInputStream* aStream);
+  ~CachePushStreamChild();
 
   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;
diff --git a/dom/cache/CacheStorage.cpp b/dom/cache/CacheStorage.cpp
index a4ff49ed7a8..c5ed9f74796 100644
--- a/dom/cache/CacheStorage.cpp
+++ b/dom/cache/CacheStorage.cpp
@@ -13,6 +13,7 @@
 #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"
@@ -439,7 +440,9 @@ CacheStorage::MaybeRunPendingRequests()
       entry->mPromise->MaybeReject(rv);
       continue;
     }
-    mActor->ExecuteOp(mGlobal, entry->mPromise, args.SendAsOpArgs());
+    unused << mActor->SendPCacheOpConstructor(
+      new CacheOpChild(mActor->GetFeature(), mGlobal, this, entry->mPromise),
+      args.SendAsOpArgs());
   }
   mPendingRequests.Clear();
 }
diff --git a/dom/cache/CacheStorageChild.cpp b/dom/cache/CacheStorageChild.cpp
index 15c1af168c6..eb1e69a4369 100644
--- a/dom/cache/CacheStorageChild.cpp
+++ b/dom/cache/CacheStorageChild.cpp
@@ -8,8 +8,8 @@
 
 #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 {
@@ -25,7 +25,6 @@ DeallocPCacheStorageChild(PCacheStorageChild* aActor)
 
 CacheStorageChild::CacheStorageChild(CacheStorage* aListener, Feature* aFeature)
   : mListener(aListener)
-  , mNumChildActors(0)
 {
   MOZ_COUNT_CTOR(cache::CacheStorageChild);
   MOZ_ASSERT(mListener);
@@ -48,15 +47,6 @@ CacheStorageChild::ClearListener()
   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()
 {
@@ -71,19 +61,13 @@ CacheStorageChild::StartDestroy()
     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();
 }
@@ -113,20 +97,9 @@ 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
diff --git a/dom/cache/CacheStorageChild.h b/dom/cache/CacheStorageChild.h
index 1d0fa194a44..d904cf9e118 100644
--- a/dom/cache/CacheStorageChild.h
+++ b/dom/cache/CacheStorageChild.h
@@ -11,16 +11,10 @@
 #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;
@@ -38,10 +32,6 @@ public:
   // 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
@@ -58,15 +48,10 @@ private:
   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
 };
diff --git a/dom/cache/CacheStreamControlChild.cpp b/dom/cache/CacheStreamControlChild.cpp
index 78b12221dd0..920b3050f4e 100644
--- a/dom/cache/CacheStreamControlChild.cpp
+++ b/dom/cache/CacheStreamControlChild.cpp
@@ -41,7 +41,6 @@ DeallocPCacheStreamControlChild(PCacheStreamControlChild* aActor)
 
 CacheStreamControlChild::CacheStreamControlChild()
   : mDestroyStarted(false)
-  , mDestroyDelayed(false)
 {
   MOZ_COUNT_CTOR(cache::CacheStreamControlChild);
 }
@@ -64,21 +63,6 @@ CacheStreamControlChild::StartDestroy()
   }
   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();
@@ -136,15 +120,6 @@ 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
diff --git a/dom/cache/CacheStreamControlChild.h b/dom/cache/CacheStreamControlChild.h
index ff0c6c6e715..50b956809a1 100644
--- a/dom/cache/CacheStreamControlChild.h
+++ b/dom/cache/CacheStreamControlChild.h
@@ -56,7 +56,6 @@ private:
   virtual bool RecvCloseAll() override;
 
   bool mDestroyStarted;
-  bool mDestroyDelayed;
 
   NS_DECL_OWNINGTHREAD
 };
diff --git a/dom/cache/ReadStream.cpp b/dom/cache/ReadStream.cpp
index 68fa862742a..22aa95cfed9 100644
--- a/dom/cache/ReadStream.cpp
+++ b/dom/cache/ReadStream.cpp
@@ -50,9 +50,6 @@ public:
   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();
@@ -106,7 +103,6 @@ private:
     NumStates
   };
   Atomic<State> mState;
-  Atomic<bool> mHasEverBeenRead;
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(cache::ReadStream::Inner, override)
 };
@@ -253,13 +249,6 @@ ReadStream::Inner::MatchId(const nsID& aId) const
   return mId.Equals(aId);
 }
 
-bool
-ReadStream::Inner::HasEverBeenRead() const
-{
-  MOZ_ASSERT(NS_GetCurrentThread() == mOwningThread);
-  return mHasEverBeenRead;
-}
-
 NS_IMETHODIMP
 ReadStream::Inner::Close()
 {
@@ -295,8 +284,6 @@ ReadStream::Inner::Read(char* aBuf, uint32_t aCount, uint32_t* aNumReadOut)
     Close();
   }
 
-  mHasEverBeenRead = true;
-
   return rv;
 }
 
@@ -307,10 +294,6 @@ ReadStream::Inner::ReadSegments(nsWriteSegmentFun aWriter, void* aClosure,
   // stream ops can happen on any thread
   MOZ_ASSERT(aNumReadOut);
 
-  if (aCount) {
-    mHasEverBeenRead = true;
-  }
-
   nsresult rv = mSnappyStream->ReadSegments(aWriter, aClosure, aCount,
                                             aNumReadOut);
 
@@ -319,14 +302,6 @@ ReadStream::Inner::ReadSegments(nsWriteSegmentFun aWriter, void* aClosure,
     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;
 }
 
diff --git a/dom/cache/ReadStream.h b/dom/cache/ReadStream.h
index eaec9bfd1a9..6c1381b3c0b 100644
--- a/dom/cache/ReadStream.h
+++ b/dom/cache/ReadStream.h
@@ -63,9 +63,6 @@ public:
     virtual bool
     MatchId(const nsID& aId) const = 0;
 
-    virtual bool
-    HasEverBeenRead() const = 0;
-
     NS_IMETHOD_(MozExternalRefCountType)
     AddRef(void) = 0;
 
diff --git a/dom/cache/StreamControl.cpp b/dom/cache/StreamControl.cpp
index 72655d2408f..8be74bab728 100644
--- a/dom/cache/StreamControl.cpp
+++ b/dom/cache/StreamControl.cpp
@@ -84,18 +84,6 @@ StreamControl::CloseAllReadStreamsWithoutReporting()
   }
 }
 
-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
diff --git a/dom/cache/StreamControl.h b/dom/cache/StreamControl.h
index 9dbe50b3f1c..a1e5518186c 100644
--- a/dom/cache/StreamControl.h
+++ b/dom/cache/StreamControl.h
@@ -68,9 +68,6 @@ protected:
   void
   CloseAllReadStreamsWithoutReporting();
 
-  bool
-  HasEverBeenRead() const;
-
   // protected parts of the abstract interface
   virtual void
   NoteClosedAfterForget(const nsID& aId) = 0;