Bug 1110485 P8 Correctly set the Feature on the stream control child actor. r=baku
authorBen Kelly <ben@wanderview.com>
Thu, 16 Apr 2015 12:00:16 -0700
changeset 239651 563ec088fb14c1b3de38c94946ac2329846ed5b5
parent 239650 12bd6399af4c57a92cdd4966728d863aa6c2a3b8
child 239652 d4988bb6de2f05601e0c7d0c9f31bd7183f12a92
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 P8 Correctly set the Feature on the stream control child actor. r=baku
dom/cache/ActorChild.cpp
dom/cache/CacheChild.cpp
dom/cache/CacheOpChild.cpp
dom/cache/CacheOpParent.cpp
dom/cache/CacheStorageChild.cpp
dom/cache/StreamUtils.cpp
dom/cache/StreamUtils.h
dom/cache/moz.build
--- a/dom/cache/ActorChild.cpp
+++ b/dom/cache/ActorChild.cpp
@@ -2,16 +2,17 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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/ActorChild.h"
 
 #include "mozilla/dom/cache/Feature.h"
+#include "nsThreadUtils.h"
 
 namespace mozilla {
 namespace dom {
 namespace cache {
 
 void
 ActorChild::SetFeature(Feature* aFeature)
 {
@@ -27,16 +28,17 @@ ActorChild::SetFeature(Feature* aFeature
   if (mFeature) {
     mFeature->AddActor(this);
   }
 }
 
 void
 ActorChild::RemoveFeature()
 {
+  MOZ_ASSERT_IF(!NS_IsMainThread(), mFeature);
   if (mFeature) {
     mFeature->RemoveActor(this);
     mFeature = nullptr;
   }
 }
 
 Feature*
 ActorChild::GetFeature() const
--- a/dom/cache/CacheChild.cpp
+++ b/dom/cache/CacheChild.cpp
@@ -6,17 +6,16 @@
 
 #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/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*
 AllocPCacheChild()
--- a/dom/cache/CacheOpChild.cpp
+++ b/dom/cache/CacheOpChild.cpp
@@ -6,21 +6,61 @@
 
 #include "mozilla/dom/cache/CacheOpChild.h"
 
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/Request.h"
 #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)
   , mPromise(aPromise)
 {
   MOZ_ASSERT(mGlobal);
   MOZ_ASSERT(mPromise);
 
@@ -51,16 +91,17 @@ CacheOpChild::ActorDestroy(ActorDestroyR
 
 bool
 CacheOpChild::Recv__delete__(const ErrorResult& aRv,
                              const CacheOpResult& aResult)
 {
   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.
     mPromise->MaybeReject(const_cast<ErrorResult&>(aRv));
     mPromise = nullptr;
     return true;
   }
 
@@ -156,49 +197,51 @@ CachePushStreamChild*
 CacheOpChild::CreatePushStream(nsIAsyncInputStream* aStream)
 {
   MOZ_CRASH("CacheOpChild should never create a push stream actor!");
 }
 
 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);
 }
 
 void
 CacheOpChild::HandleResponseList(const nsTArray<CacheResponse>& aResponseList)
 {
   nsAutoTArray<nsRefPtr<Response>, 256> responses;
   responses.SetCapacity(aResponseList.Length());
 
   for (uint32_t i = 0; i < aResponseList.Length(); ++i) {
+    AddFeatureToStreamChild(aResponseList[i], GetFeature());
     responses.AppendElement(ToResponse(aResponseList[i]));
   }
 
   mPromise->MaybeResolve(responses);
 }
 
 void
 CacheOpChild::HandleRequestList(const nsTArray<CacheRequest>& aRequestList)
 {
   nsAutoTArray<nsRefPtr<Request>, 256> requests;
   requests.SetCapacity(aRequestList.Length());
 
   for (uint32_t i = 0; i < aRequestList.Length(); ++i) {
+    AddFeatureToStreamChild(aRequestList[i], GetFeature());
     requests.AppendElement(ToRequest(aRequestList[i]));
   }
 
   mPromise->MaybeResolve(requests);
 }
 
 } // namespace cache
 } // namespace dom
--- a/dom/cache/CacheOpParent.cpp
+++ b/dom/cache/CacheOpParent.cpp
@@ -186,44 +186,44 @@ CacheOpParent::OnOpComplete(ErrorResult&
                             const nsTArray<SavedResponse>& aSavedResponseList,
                             const nsTArray<SavedRequest>& aSavedRequestList,
                             StreamList* aStreamList)
 {
   NS_ASSERT_OWNINGTHREAD(CacheOpParent);
   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
   // result requires actor-specific operations, then we do that below.
   // If the type and data types don't match, then we will trigger an
   // 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);
   }
 
   for (uint32_t i = 0; i < aSavedResponseList.Length(); ++i) {
     result.Add(aSavedResponseList[i], aStreamList);
   }
 
   for (uint32_t i = 0; i < aSavedRequestList.Length(); ++i) {
     result.Add(aSavedRequestList[i], aStreamList);
   }
 
-  // TODO: send full ErrorCode
   unused << Send__delete__(this, aRv, result.SendAsOpResult());
 }
 
 void
 CacheOpParent::OnFetchPut(FetchPut* aFetchPut, ErrorResult&& aRv)
 {
   NS_ASSERT_OWNINGTHREAD(CacheOpParent);
   MOZ_ASSERT(aFetchPut);
--- a/dom/cache/CacheStorageChild.cpp
+++ b/dom/cache/CacheStorageChild.cpp
@@ -5,17 +5,16 @@
  * 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/StreamUtils.h"
 
 namespace mozilla {
 namespace dom {
 namespace cache {
 
 // declared in ActorUtils.h
 void
 DeallocPCacheStorageChild(PCacheStorageChild* aActor)
deleted file mode 100644
--- a/dom/cache/StreamUtils.cpp
+++ /dev/null
@@ -1,151 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* 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/StreamUtils.h"
-
-#include "mozilla/unused.h"
-#include "mozilla/dom/cache/CacheStreamControlChild.h"
-#include "mozilla/dom/cache/CacheTypes.h"
-#include "mozilla/ipc/FileDescriptor.h"
-#include "mozilla/ipc/FileDescriptorSetChild.h"
-
-namespace mozilla {
-namespace dom {
-namespace cache {
-
-namespace {
-
-using mozilla::unused;
-using mozilla::void_t;
-using mozilla::dom::cache::CacheStreamControlChild;
-using mozilla::dom::cache::Feature;
-using mozilla::dom::cache::CacheReadStream;
-using mozilla::ipc::FileDescriptor;
-using mozilla::ipc::FileDescriptorSetChild;
-using mozilla::ipc::OptionalFileDescriptorSet;
-
-void
-StartDestroyStreamChild(const CacheReadStream& aReadStream)
-{
-  CacheStreamControlChild* cacheControl =
-    static_cast<CacheStreamControlChild*>(aReadStream.controlChild());
-  if (cacheControl) {
-    cacheControl->StartDestroy();
-  }
-
-  if (aReadStream.fds().type() ==
-      OptionalFileDescriptorSet::TPFileDescriptorSetChild) {
-    nsAutoTArray<FileDescriptor, 4> fds;
-
-    FileDescriptorSetChild* fdSetActor =
-      static_cast<FileDescriptorSetChild*>(aReadStream.fds().get_PFileDescriptorSetChild());
-    MOZ_ASSERT(fdSetActor);
-
-    fdSetActor->ForgetFileDescriptors(fds);
-    MOZ_ASSERT(!fds.IsEmpty());
-
-    unused << fdSetActor->Send__delete__(fdSetActor);
-  }
-}
-
-void
-AddFeatureToStreamChild(const CacheReadStream& aReadStream, Feature* aFeature)
-{
-  CacheStreamControlChild* cacheControl =
-    static_cast<CacheStreamControlChild*>(aReadStream.controlChild());
-  if (cacheControl) {
-    cacheControl->SetFeature(aFeature);
-  }
-}
-
-} // anonymous namespace
-
-void
-StartDestroyStreamChild(const CacheResponseOrVoid& aResponseOrVoid)
-{
-  if (aResponseOrVoid.type() == CacheResponseOrVoid::Tvoid_t) {
-    return;
-  }
-
-  StartDestroyStreamChild(aResponseOrVoid.get_CacheResponse());
-}
-
-void
-StartDestroyStreamChild(const CacheResponse& aResponse)
-{
-  if (aResponse.body().type() == CacheReadStreamOrVoid::Tvoid_t) {
-    return;
-  }
-
-  StartDestroyStreamChild(aResponse.body().get_CacheReadStream());
-}
-
-void
-StartDestroyStreamChild(const nsTArray<CacheResponse>& aResponses)
-{
-  for (uint32_t i = 0; i < aResponses.Length(); ++i) {
-    StartDestroyStreamChild(aResponses[i]);
-  }
-}
-
-void
-StartDestroyStreamChild(const nsTArray<CacheRequest>& aRequests)
-{
-  for (uint32_t i = 0; i < aRequests.Length(); ++i) {
-    if (aRequests[i].body().type() == CacheReadStreamOrVoid::Tvoid_t) {
-      continue;
-    }
-    StartDestroyStreamChild(aRequests[i].body().get_CacheReadStream());
-  }
-}
-
-void
-AddFeatureToStreamChild(const CacheResponseOrVoid& aResponseOrVoid,
-                        Feature* aFeature)
-{
-  if (aResponseOrVoid.type() == CacheResponseOrVoid::Tvoid_t) {
-    return;
-  }
-
-  AddFeatureToStreamChild(aResponseOrVoid.get_CacheResponse(), aFeature);
-}
-
-void
-AddFeatureToStreamChild(const CacheResponse& aResponse,
-                        Feature* aFeature)
-{
-  if (aResponse.body().type() == CacheReadStreamOrVoid::Tvoid_t) {
-    return;
-  }
-
-  AddFeatureToStreamChild(aResponse.body().get_CacheReadStream(), aFeature);
-}
-
-void
-AddFeatureToStreamChild(const nsTArray<CacheResponse>& aResponses,
-                         Feature* aFeature)
-{
-  for (uint32_t i = 0; i < aResponses.Length(); ++i) {
-    AddFeatureToStreamChild(aResponses[i], aFeature);
-  }
-}
-
-void
-AddFeatureToStreamChild(const nsTArray<CacheRequest>& aRequests,
-                         Feature* aFeature)
-{
-  for (uint32_t i = 0; i < aRequests.Length(); ++i) {
-    if (aRequests[i].body().type() == CacheReadStreamOrVoid::Tvoid_t) {
-      continue;
-    }
-    AddFeatureToStreamChild(aRequests[i].body().get_CacheReadStream(),
-                            aFeature);
-  }
-}
-
-} // namespace cache
-} // namespace dom
-} // namespace mozilla
deleted file mode 100644
--- a/dom/cache/StreamUtils.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* 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/. */
-
-#ifndef mozilla_dom_cache_StreamUtils_h
-#define mozilla_dom_cache_StreamUtils_h
-
-#include "nsTArrayForwardDeclare.h"
-
-namespace mozilla {
-namespace dom {
-namespace cache {
-
-class Feature;
-class CacheRequest;
-class CacheResponse;
-class CacheResponseOrVoid;
-
-void StartDestroyStreamChild(const CacheResponseOrVoid& aResponseOrVoid);
-void StartDestroyStreamChild(const CacheResponse& aResponse);
-void StartDestroyStreamChild(const nsTArray<CacheResponse>& aResponses);
-void StartDestroyStreamChild(const nsTArray<CacheRequest>& aRequests);
-
-void AddFeatureToStreamChild(const CacheResponseOrVoid& aResponseOrVoid,
-                             Feature* aFeature);
-void AddFeatureToStreamChild(const CacheResponse& aResponse,
-                             Feature* aFeature);
-void AddFeatureToStreamChild(const nsTArray<CacheResponse>& aResponses,
-                              Feature* aFeature);
-void AddFeatureToStreamChild(const nsTArray<CacheRequest>& aRequests,
-                              Feature* aFeature);
-
-} // namespace cache
-} // namespace dom
-} // namespace mozilla
-
-#endif // mozilla_dom_cache_StreamUtils_h
--- a/dom/cache/moz.build
+++ b/dom/cache/moz.build
@@ -32,17 +32,16 @@ EXPORTS.mozilla.dom.cache += [
     'ManagerId.h',
     'OfflineStorage.h',
     'PrincipalVerifier.h',
     'QuotaClient.h',
     'ReadStream.h',
     'SavedTypes.h',
     'StreamControl.h',
     'StreamList.h',
-    'StreamUtils.h',
     'Types.h',
     'TypeUtils.h',
 ]
 
 UNIFIED_SOURCES += [
     'Action.cpp',
     'ActorChild.cpp',
     'AutoUtils.cpp',
@@ -67,17 +66,16 @@ UNIFIED_SOURCES += [
     'Manager.cpp',
     'ManagerId.cpp',
     'OfflineStorage.cpp',
     'PrincipalVerifier.cpp',
     'QuotaClient.cpp',
     'ReadStream.cpp',
     'StreamControl.cpp',
     'StreamList.cpp',
-    'StreamUtils.cpp',
     'TypeUtils.cpp',
 ]
 
 IPDL_SOURCES += [
     'CacheTypes.ipdlh',
     'PCache.ipdl',
     'PCacheOp.ipdl',
     'PCachePushStream.ipdl',