Bug 1271905 P1 Don't crash if a closed cache::ReadableStream is serialized. r=asuth a=ritu
authorBen Kelly <ben@wanderview.com>
Fri, 13 May 2016 11:44:38 -0700
changeset 332847 d768e95103ff967c108496bc035b3be5fbef50b8
parent 332846 ac470f026c69f90c82798f15dbf9d538fc99b68e
child 332848 805051b9a46259319d5c56424cb5249efc811363
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth, ritu
bugs1271905
milestone48.0a2
Bug 1271905 P1 Don't crash if a closed cache::ReadableStream is serialized. r=asuth a=ritu
dom/bindings/Errors.msg
dom/cache/AutoUtils.cpp
dom/cache/ReadStream.cpp
dom/cache/ReadStream.h
dom/cache/TypeUtils.cpp
--- a/dom/bindings/Errors.msg
+++ b/dom/bindings/Errors.msg
@@ -91,8 +91,9 @@ MSG_DEF(MSG_IS_NOT_PROMISE, 1, JSEXN_TYP
 MSG_DEF(MSG_SW_INSTALL_ERROR, 2, JSEXN_TYPEERR, "ServiceWorker script at {0} for scope {1} encountered an error during installation.")
 MSG_DEF(MSG_SW_SCRIPT_THREW, 2, JSEXN_TYPEERR, "ServiceWorker script at {0} for scope {1} threw an exception during script evaluation.")
 MSG_DEF(MSG_TYPEDARRAY_IS_SHARED, 1, JSEXN_TYPEERR, "{0} can't be a typed array on SharedArrayBuffer")
 MSG_DEF(MSG_CACHE_ADD_FAILED_RESPONSE, 3, JSEXN_TYPEERR, "Cache got {0} response with bad status {1} while trying to add request {2}")
 MSG_DEF(MSG_SW_UPDATE_BAD_REGISTRATION, 2, JSEXN_TYPEERR, "Failed to update the ServiceWorker for scope {0] because the registration has been {1} since the update was scheduled.")
 MSG_DEF(MSG_INVALID_DURATION_ERROR, 1, JSEXN_TYPEERR, "Invalid duration '{0}'.")
 MSG_DEF(MSG_INVALID_EASING_ERROR, 1, JSEXN_TYPEERR, "Invalid easing '{0}'.")
 MSG_DEF(MSG_USELESS_SETTIMEOUT, 1, JSEXN_TYPEERR, "Useless {0} call (missing quotes around argument?)")
+MSG_DEF(MSG_CACHE_STREAM_CLOSED, 0, JSEXN_TYPEERR, "Response body is a cache file stream that has already been closed.")
--- a/dom/cache/AutoUtils.cpp
+++ b/dom/cache/AutoUtils.cpp
@@ -617,14 +617,16 @@ AutoParentOpResult::SerializeReadStream(
       return;
     }
   }
 
   aStreamList->SetStreamControl(mStreamControl);
 
   RefPtr<ReadStream> readStream = ReadStream::Create(mStreamControl,
                                                        aId, stream);
-  readStream->Serialize(aReadStreamOut);
+  ErrorResult rv;
+  readStream->Serialize(aReadStreamOut, rv);
+  MOZ_ASSERT(!rv.Failed());
 }
 
 } // namespace cache
 } // namespace dom
 } // namespace mozilla
--- a/dom/cache/ReadStream.cpp
+++ b/dom/cache/ReadStream.cpp
@@ -30,20 +30,20 @@ using mozilla::ipc::FileDescriptor;
 // guaranteed by our outer ReadStream class.
 class ReadStream::Inner final : public ReadStream::Controllable
 {
 public:
   Inner(StreamControl* aControl, const nsID& aId,
         nsIInputStream* aStream);
 
   void
-  Serialize(CacheReadStreamOrVoid* aReadStreamOut);
+  Serialize(CacheReadStreamOrVoid* aReadStreamOut, ErrorResult& aRv);
 
   void
-  Serialize(CacheReadStream* aReadStreamOut);
+  Serialize(CacheReadStream* aReadStreamOut, ErrorResult& aRv);
 
   // ReadStream::Controllable methods
   virtual void
   CloseStream() override;
 
   virtual void
   CloseStreamWithoutReporting() override;
 
@@ -192,31 +192,37 @@ ReadStream::Inner::Inner(StreamControl* 
   , mState(Open)
 {
   MOZ_ASSERT(mStream);
   MOZ_ASSERT(mControl);
   mControl->AddReadStream(this);
 }
 
 void
-ReadStream::Inner::Serialize(CacheReadStreamOrVoid* aReadStreamOut)
+ReadStream::Inner::Serialize(CacheReadStreamOrVoid* aReadStreamOut,
+                             ErrorResult& aRv)
 {
   MOZ_ASSERT(NS_GetCurrentThread() == mOwningThread);
   MOZ_ASSERT(aReadStreamOut);
   CacheReadStream stream;
-  Serialize(&stream);
+  Serialize(&stream, aRv);
   *aReadStreamOut = stream;
 }
 
 void
-ReadStream::Inner::Serialize(CacheReadStream* aReadStreamOut)
+ReadStream::Inner::Serialize(CacheReadStream* aReadStreamOut, ErrorResult& aRv)
 {
   MOZ_ASSERT(NS_GetCurrentThread() == mOwningThread);
   MOZ_ASSERT(aReadStreamOut);
-  MOZ_ASSERT(mState == Open);
+
+  if (mState != Open) {
+    aRv.ThrowTypeError<MSG_CACHE_STREAM_CLOSED>();
+    return;
+  }
+
   MOZ_ASSERT(mControl);
 
   // If we are sending a ReadStream, then we never want to set the
   // pushStream actors at the same time.
   aReadStreamOut->pushStreamChild() = nullptr;
   aReadStreamOut->pushStreamParent() = nullptr;
 
   aReadStreamOut->id() = mId;
@@ -477,25 +483,25 @@ ReadStream::Create(PCacheStreamControlPa
   MOZ_ASSERT(aControl);
   auto actor = static_cast<CacheStreamControlParent*>(aControl);
   RefPtr<Inner> inner = new Inner(actor, aId, aStream);
   RefPtr<ReadStream> ref = new ReadStream(inner);
   return ref.forget();
 }
 
 void
-ReadStream::Serialize(CacheReadStreamOrVoid* aReadStreamOut)
+ReadStream::Serialize(CacheReadStreamOrVoid* aReadStreamOut, ErrorResult& aRv)
 {
-  mInner->Serialize(aReadStreamOut);
+  mInner->Serialize(aReadStreamOut, aRv);
 }
 
 void
-ReadStream::Serialize(CacheReadStream* aReadStreamOut)
+ReadStream::Serialize(CacheReadStream* aReadStreamOut, ErrorResult& aRv)
 {
-  mInner->Serialize(aReadStreamOut);
+  mInner->Serialize(aReadStreamOut, aRv);
 }
 
 ReadStream::ReadStream(ReadStream::Inner* aInner)
   : mInner(aInner)
 {
   MOZ_ASSERT(mInner);
 }
 
--- a/dom/cache/ReadStream.h
+++ b/dom/cache/ReadStream.h
@@ -76,18 +76,18 @@ public:
 
   static already_AddRefed<ReadStream>
   Create(const CacheReadStream& aReadStream);
 
   static already_AddRefed<ReadStream>
   Create(PCacheStreamControlParent* aControl, const nsID& aId,
          nsIInputStream* aStream);
 
-  void Serialize(CacheReadStreamOrVoid* aReadStreamOut);
-  void Serialize(CacheReadStream* aReadStreamOut);
+  void Serialize(CacheReadStreamOrVoid* aReadStreamOut, ErrorResult& aRv);
+  void Serialize(CacheReadStream* aReadStreamOut, ErrorResult& aRv);
 
 private:
   class Inner;
 
   explicit ReadStream(Inner* aInner);
   ~ReadStream();
 
   // Hold a strong ref to an inner class that actually implements the
--- a/dom/cache/TypeUtils.cpp
+++ b/dom/cache/TypeUtils.cpp
@@ -506,17 +506,17 @@ TypeUtils::SerializeCacheStream(nsIInput
   *aStreamOut = void_t();
   if (!aStream) {
     return;
   }
 
   // Option 1: Send a cache-specific ReadStream if we can.
   RefPtr<ReadStream> controlled = do_QueryObject(aStream);
   if (controlled) {
-    controlled->Serialize(aStreamOut);
+    controlled->Serialize(aStreamOut, aRv);
     return;
   }
 
   CacheReadStream readStream;
   readStream.controlChild() = nullptr;
   readStream.controlParent() = nullptr;
   readStream.pushStreamChild() = nullptr;
   readStream.pushStreamParent() = nullptr;