Bug 1110485 P8 Correctly set the Feature on the stream control child …

…actor.

dom/cache/ActorChild.cpp

@@ -7,6 +7,7 @@
 #include "mozilla/dom/cache/ActorChild.h"
 
 #include "mozilla/dom/cache/Feature.h"
+#include "nsThreadUtils.h"
 
 namespace mozilla {
 namespace dom {
@@ -25,6 +26,7 @@ ActorChild::SetFeature(Feature* aFeature)
 void
 ActorChild::RemoveFeature()
 {
+  MOZ_ASSERT_IF(!NS_IsMainThread(), mFeature);
   if (mFeature) {
     mFeature->RemoveActor(this);
     mFeature = nullptr;

dom/cache/CacheChild.cpp

@@ -11,7 +11,6 @@
 #include "mozilla/dom/cache/Cache.h"
 #include "mozilla/dom/cache/CacheOpChild.h"
 #include "mozilla/dom/cache/CachePushStreamChild.h"
-#include "mozilla/dom/cache/StreamUtils.h"
 
 namespace mozilla {
 namespace dom {

dom/cache/CacheOpChild.cpp

@@ -11,11 +11,51 @@
 #include "mozilla/dom/Response.h"
 #include "mozilla/dom/cache/Cache.h"
 #include "mozilla/dom/cache/CacheChild.h"
+#include "mozilla/dom/cache/CacheStreamControlChild.h"
 
 namespace mozilla {
 namespace dom {
 namespace cache {
 
+namespace {
+
+void
+AddFeatureToStreamChild(const CacheReadStream& aReadStream, Feature* aFeature)
+{
+  MOZ_ASSERT_IF(!NS_IsMainThread(), aFeature);
+  CacheStreamControlChild* cacheControl =
+    static_cast<CacheStreamControlChild*>(aReadStream.controlChild());
+  if (cacheControl) {
+    cacheControl->SetFeature(aFeature);
+  }
+}
+
+void
+AddFeatureToStreamChild(const CacheResponse& aResponse, Feature* aFeature)
+{
+  MOZ_ASSERT_IF(!NS_IsMainThread(), aFeature);
+
+  if (aResponse.body().type() == CacheReadStreamOrVoid::Tvoid_t) {
+    return;
+  }
+
+  AddFeatureToStreamChild(aResponse.body().get_CacheReadStream(), aFeature);
+}
+
+void
+AddFeatureToStreamChild(const CacheRequest& aRequest, Feature* aFeature)
+{
+  MOZ_ASSERT_IF(!NS_IsMainThread(), aFeature);
+
+  if (aRequest.body().type() == CacheReadStreamOrVoid::Tvoid_t) {
+    return;
+  }
+
+  AddFeatureToStreamChild(aRequest.body().get_CacheReadStream(), aFeature);
+}
+
+} // anonymous namespace
+
 CacheOpChild::CacheOpChild(Feature* aFeature, nsIGlobalObject* aGlobal,
                            Promise* aPromise)
   : mGlobal(aGlobal)
@@ -56,6 +96,7 @@ CacheOpChild::Recv__delete__(const ErrorResult& aRv,
   NS_ASSERT_OWNINGTHREAD(CacheOpChild);
 
   if (aRv.Failed()) {
+    MOZ_ASSERT(aResult.type() == CacheOpResult::Tvoid_t);
     // TODO: Remove this const_cast (bug 1152078).
     // It is safe for now since this ErrorResult is handed off to us by IPDL
     // and is thrown into the trash afterwards.
@@ -161,16 +202,16 @@ CacheOpChild::CreatePushStream(nsIAsyncInputStream* aStream)
 void
 CacheOpChild::HandleResponse(const CacheResponseOrVoid& aResponseOrVoid)
 {
-  nsRefPtr<Response> response;
-  if (aResponseOrVoid.type() == CacheResponseOrVoid::TCacheResponse) {
-    response = ToResponse(aResponseOrVoid);
-  }
-
-  if (!response) {
+  if (aResponseOrVoid.type() == CacheResponseOrVoid::Tvoid_t) {
     mPromise->MaybeResolve(JS::UndefinedHandleValue);
     return;
   }
 
+  const CacheResponse& cacheResponse = aResponseOrVoid.get_CacheResponse();
+
+  AddFeatureToStreamChild(cacheResponse, GetFeature());
+  nsRefPtr<Response> response = ToResponse(cacheResponse);
+
   mPromise->MaybeResolve(response);
 }
 
@@ -181,6 +222,7 @@ CacheOpChild::HandleResponseList(const nsTArray<CacheResponse>& aResponseList)
   responses.SetCapacity(aResponseList.Length());
 
   for (uint32_t i = 0; i < aResponseList.Length(); ++i) {
+    AddFeatureToStreamChild(aResponseList[i], GetFeature());
     responses.AppendElement(ToResponse(aResponseList[i]));
   }
 
@@ -194,6 +236,7 @@ CacheOpChild::HandleRequestList(const nsTArray<CacheRequest>& aRequestList)
   requests.SetCapacity(aRequestList.Length());
 
   for (uint32_t i = 0; i < aRequestList.Length(); ++i) {
+    AddFeatureToStreamChild(aRequestList[i], GetFeature());
     requests.AppendElement(ToRequest(aRequestList[i]));
   }
 

dom/cache/CacheOpParent.cpp

@@ -191,6 +191,14 @@ CacheOpParent::OnOpComplete(ErrorResult&& aRv, const CacheOpResult& aResult,
   MOZ_ASSERT(mIpcManager);
   MOZ_ASSERT(mManager);
 
+  // Never send an op-specific result if we have an error.  Instead, send
+  // void_t() to ensure that we don't leak actors on the child side.
+  if (aRv.Failed()) {
+    unused << Send__delete__(this, aRv, void_t());
+    aRv.ClearMessage(); // This may contain a TypeError.
+    return;
+  }
+
   // The result must contain the appropriate type at this point.  It may
   // or may not contain the additional result data yet.  For types that
   // do not need special processing, it should already be set.  If the
@@ -199,13 +207,6 @@ CacheOpParent::OnOpComplete(ErrorResult&& aRv, const CacheOpResult& aResult,
   // assertion in AutoParentOpResult::Add().
   AutoParentOpResult result(mIpcManager, aResult);
 
-  if (aRv.Failed()) {
-    // TODO: send full ErrorCode
-    unused << Send__delete__(this, aRv, result.SendAsOpResult());
-    aRv.ClearMessage(); // This may contain a TypeError.
-    return;
-  }
-
   if (aOpenedCacheId != INVALID_CACHE_ID) {
     result.Add(aOpenedCacheId, mManager);
   }
@@ -218,7 +219,6 @@ CacheOpParent::OnOpComplete(ErrorResult&& aRv, const CacheOpResult& aResult,
     result.Add(aSavedRequestList[i], aStreamList);
   }
 
-  // TODO: send full ErrorCode
   unused << Send__delete__(this, aRv, result.SendAsOpResult());
 }
 

dom/cache/CacheStorageChild.cpp

@@ -10,7 +10,6 @@
 #include "mozilla/dom/cache/CacheChild.h"
 #include "mozilla/dom/cache/CacheOpChild.h"
 #include "mozilla/dom/cache/CacheStorage.h"
-#include "mozilla/dom/cache/StreamUtils.h"
 
 namespace mozilla {
 namespace dom {

dom/cache/moz.build

@@ -37,7 +37,6 @@ EXPORTS.mozilla.dom.cache += [
     'SavedTypes.h',
     'StreamControl.h',
     'StreamList.h',
-    'StreamUtils.h',
     'Types.h',
     'TypeUtils.h',
 ]
@@ -72,7 +71,6 @@ UNIFIED_SOURCES += [
     'ReadStream.cpp',
     'StreamControl.cpp',
     'StreamList.cpp',
-    'StreamUtils.cpp',
     'TypeUtils.cpp',
 ]