Bug 1403771 - SlicedInputStream takes ownership of the underlying stream and it propagates the Close() call, r=smaug

dom/file/FileBlobImpl.cpp

@@ -254,7 +254,7 @@ FileBlobImpl::CreateInputStream(nsIInputStream** aStream, ErrorResult& aRv)
   }
 
   RefPtr<SlicedInputStream> slicedInputStream =
-    new SlicedInputStream(stream, mStart, mLength);
+    new SlicedInputStream(stream.forget(), mStart, mLength);
   slicedInputStream.forget(aStream);
 }
 

dom/file/StreamBlobImpl.cpp

@@ -49,17 +49,6 @@ StreamBlobImpl::StreamBlobImpl(nsIInputStream* aInputStream,
   mImmutable = true;
 }
 
-StreamBlobImpl::StreamBlobImpl(StreamBlobImpl* aOther,
-                               const nsAString& aContentType,
-                               uint64_t aStart, uint64_t aLength)
-  : BaseBlobImpl(aContentType, aOther->mStart + aStart, aLength)
-  , mInputStream(new SlicedInputStream(aOther->mInputStream, aStart, aLength))
-  , mIsDirectory(false)
-  , mFileId(-1)
-{
-  mImmutable = true;
-}
-
 StreamBlobImpl::StreamBlobImpl(nsIInputStream* aInputStream,
                                const nsAString& aName,
                                const nsAString& aContentType,
@@ -106,22 +95,29 @@ StreamBlobImpl::CreateSlice(uint64_t aStart, uint64_t aLength,
     return impl.forget();
   }
 
+  nsCOMPtr<nsIInputStream> clonedStream;
+
   nsCOMPtr<nsICloneableInputStreamWithRange> stream =
     do_QueryInterface(mInputStream);
   if (stream) {
-    nsCOMPtr<nsIInputStream> clonedStream;
     aRv = stream->CloneWithRange(aStart, aLength, getter_AddRefs(clonedStream));
     if (NS_WARN_IF(aRv.Failed())) {
       return nullptr;
     }
+  } else {
+    CreateInputStream(getter_AddRefs(clonedStream), aRv);
+    if (NS_WARN_IF(aRv.Failed())) {
+      return nullptr;
+    }
 
-    RefPtr<BlobImpl> impl =
-      new StreamBlobImpl(clonedStream, aContentType, aLength);
-    return impl.forget();
+    clonedStream =
+      new SlicedInputStream(clonedStream.forget(), aStart, aLength);
   }
 
-  RefPtr<BlobImpl> impl;
-    impl = new StreamBlobImpl(this, aContentType, aStart, aLength);
+  MOZ_ASSERT(clonedStream);
+
+  RefPtr<BlobImpl> impl =
+    new StreamBlobImpl(clonedStream, aContentType, aLength);
   return impl.forget();
 }
 

dom/file/StreamBlobImpl.h

@@ -90,11 +90,6 @@ private:
                  int64_t aLastModifiedDate,
                  uint64_t aLength);
 
-  StreamBlobImpl(StreamBlobImpl* aOther,
-                 const nsAString& aContentType,
-                 uint64_t aStart,
-                 uint64_t aLength);
-
   ~StreamBlobImpl();
 
   void MaybeRegisterMemoryReporter();

dom/file/ipc/IPCBlobInputStream.cpp

@@ -382,30 +382,33 @@ IPCBlobInputStream::AsyncWait(nsIInputStreamCallback* aCallback,
 }
 
 void
-IPCBlobInputStream::StreamReady(nsIInputStream* aInputStream)
+IPCBlobInputStream::StreamReady(already_AddRefed<nsIInputStream> aInputStream)
 {
+  nsCOMPtr<nsIInputStream> inputStream = Move(aInputStream);
+
   // We have been closed in the meantime.
   if (mState == eClosed) {
-    if (aInputStream) {
-      aInputStream->Close();
+    if (inputStream) {
+      inputStream->Close();
     }
     return;
   }
 
-  // If aInputStream is null, it means that the serialization went wrong or the
+  // If inputStream is null, it means that the serialization went wrong or the
   // stream is not available anymore. We keep the state as pending just to block
   // any additional operation.
 
-  if (!aInputStream) {
+  if (!inputStream) {
     return;
   }
 
   // Now it's the right time to apply a slice if needed.
   if (mStart > 0 || mLength < mActor->Size()) {
-    aInputStream = new SlicedInputStream(aInputStream, mStart, mLength);
+    inputStream =
+      new SlicedInputStream(inputStream.forget(), mStart, mLength);
   }
 
-  mRemoteStream = aInputStream;
+  mRemoteStream = inputStream;
 
   MOZ_ASSERT(mState == ePending);
   mState = eRunning;
@@ -476,7 +479,8 @@ IPCBlobInputStream::InitWithExistingRange(uint64_t aStart, uint64_t aLength)
   // because the stream is immediately consumable.
   if (mState == eRunning && mRemoteStream && XRE_IsParentProcess() &&
       (mStart > 0 || mLength < mActor->Size())) {
-    mRemoteStream = new SlicedInputStream(mRemoteStream, mStart, mLength);
+    mRemoteStream =
+      new SlicedInputStream(mRemoteStream.forget(), mStart, mLength);
   }
 }
 

dom/file/ipc/IPCBlobInputStream.h

@@ -38,7 +38,7 @@ public:
   explicit IPCBlobInputStream(IPCBlobInputStreamChild* aActor);
 
   void
-  StreamReady(nsIInputStream* aInputStream);
+  StreamReady(already_AddRefed<nsIInputStream> aInputStream);
 
 private:
   ~IPCBlobInputStream();

dom/file/ipc/IPCBlobInputStreamChild.cpp

@@ -71,10 +71,10 @@ class StreamReadyRunnable final : public CancelableRunnable
 {
 public:
   StreamReadyRunnable(IPCBlobInputStream* aDestinationStream,
-                      nsIInputStream* aCreatedStream)
+                      already_AddRefed<nsIInputStream> aCreatedStream)
     : CancelableRunnable("dom::StreamReadyRunnable")
     , mDestinationStream(aDestinationStream)
-    , mCreatedStream(aCreatedStream)
+    , mCreatedStream(Move(aCreatedStream))
   {
     MOZ_ASSERT(mDestinationStream);
     // mCreatedStream can be null.
@@ -83,7 +83,7 @@ public:
   NS_IMETHOD
   Run() override
   {
-    mDestinationStream->StreamReady(mCreatedStream);
+    mDestinationStream->StreamReady(mCreatedStream.forget());
     return NS_OK;
   }
 
@@ -317,7 +317,7 @@ IPCBlobInputStreamChild::RecvStreamReady(const OptionalIPCStream& aStream)
   }
 
   RefPtr<StreamReadyRunnable> runnable =
-    new StreamReadyRunnable(pendingStream, stream);
+    new StreamReadyRunnable(pendingStream, stream.forget());
 
   // If IPCBlobInputStream::AsyncWait() has been executed without passing an
   // event target, we run the callback synchronous because any thread could be

dom/file/ipc/IPCBlobInputStreamStorage.cpp

@@ -172,7 +172,8 @@ IPCBlobInputStreamStorage::GetStream(const nsID& aID,
 
   // Now it's the right time to apply a slice if needed.
   if (aStart > 0 || aLength < size) {
-    clonedStream = new SlicedInputStream(clonedStream, aStart, aLength);
+    clonedStream =
+      new SlicedInputStream(clonedStream.forget(), aStart, aLength);
   }
 
   clonedStream.forget(aInputStream);

xpcom/io/SlicedInputStream.cpp

@@ -28,7 +28,7 @@ NS_INTERFACE_MAP_BEGIN(SlicedInputStream)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIInputStream)
 NS_INTERFACE_MAP_END
 
-SlicedInputStream::SlicedInputStream(nsIInputStream* aInputStream,
+SlicedInputStream::SlicedInputStream(already_AddRefed<nsIInputStream> aInputStream,
                                      uint64_t aStart, uint64_t aLength)
   : mWeakCloneableInputStream(nullptr)
   , mWeakIPCSerializableInputStream(nullptr)
@@ -41,8 +41,8 @@ SlicedInputStream::SlicedInputStream(nsIInputStream* aInputStream,
   , mAsyncWaitFlags(0)
   , mAsyncWaitRequestedCount(0)
 {
-  MOZ_ASSERT(aInputStream);
-  SetSourceStream(aInputStream);
+  nsCOMPtr<nsIInputStream> inputStream = mozilla::Move(aInputStream);
+  SetSourceStream(inputStream.forget());
 }
 
 SlicedInputStream::SlicedInputStream()
@@ -62,35 +62,34 @@ SlicedInputStream::~SlicedInputStream()
 {}
 
 void
-SlicedInputStream::SetSourceStream(nsIInputStream* aInputStream)
+SlicedInputStream::SetSourceStream(already_AddRefed<nsIInputStream> aInputStream)
 {
   MOZ_ASSERT(!mInputStream);
-  MOZ_ASSERT(aInputStream);
 
-  mInputStream = aInputStream;
+  mInputStream = mozilla::Move(aInputStream);
 
   nsCOMPtr<nsICloneableInputStream> cloneableStream =
-    do_QueryInterface(aInputStream);
-  if (cloneableStream && SameCOMIdentity(aInputStream, cloneableStream)) {
+    do_QueryInterface(mInputStream);
+  if (cloneableStream && SameCOMIdentity(mInputStream, cloneableStream)) {
     mWeakCloneableInputStream = cloneableStream;
   }
 
   nsCOMPtr<nsIIPCSerializableInputStream> serializableStream =
-    do_QueryInterface(aInputStream);
+    do_QueryInterface(mInputStream);
   if (serializableStream &&
-      SameCOMIdentity(aInputStream, serializableStream)) {
+      SameCOMIdentity(mInputStream, serializableStream)) {
     mWeakIPCSerializableInputStream = serializableStream;
   }
 
   nsCOMPtr<nsISeekableStream> seekableStream =
-    do_QueryInterface(aInputStream);
-  if (seekableStream && SameCOMIdentity(aInputStream, seekableStream)) {
+    do_QueryInterface(mInputStream);
+  if (seekableStream && SameCOMIdentity(mInputStream, seekableStream)) {
     mWeakSeekableInputStream = seekableStream;
   }
 
   nsCOMPtr<nsIAsyncInputStream> asyncInputStream =
-    do_QueryInterface(aInputStream);
-  if (asyncInputStream && SameCOMIdentity(aInputStream, asyncInputStream)) {
+    do_QueryInterface(mInputStream);
+  if (asyncInputStream && SameCOMIdentity(mInputStream, asyncInputStream)) {
     mWeakAsyncInputStream = asyncInputStream;
   }
 }
@@ -101,7 +100,7 @@ SlicedInputStream::Close()
   NS_ENSURE_STATE(mInputStream);
 
   mClosed = true;
-  return NS_OK;
+  return mInputStream->Close();
 }
 
 // nsIInputStream interface
@@ -226,7 +225,7 @@ SlicedInputStream::Clone(nsIInputStream** aResult)
   }
 
   nsCOMPtr<nsIInputStream> sis =
-    new SlicedInputStream(clonedStream, mStart, mLength);
+    new SlicedInputStream(clonedStream.forget(), mStart, mLength);
 
   sis.forget(aResult);
   return NS_OK;
@@ -240,6 +239,7 @@ SlicedInputStream::CloseWithStatus(nsresult aStatus)
   NS_ENSURE_STATE(mInputStream);
   NS_ENSURE_STATE(mWeakAsyncInputStream);
 
+  mClosed = true;
   return mWeakAsyncInputStream->CloseWithStatus(aStatus);
 }
 
@@ -384,7 +384,7 @@ SlicedInputStream::Deserialize(const mozilla::ipc::InputStreamParams& aParams,
     return false;
   }
 
-  SetSourceStream(stream);
+  SetSourceStream(stream.forget());
 
   mStart = params.start();
   mLength = params.length();

xpcom/io/SlicedInputStream.h

@@ -41,7 +41,7 @@ public:
   // aInputStream should not be read from after constructing a
   // SlicedInputStream wrapper around it.
 
-  SlicedInputStream(nsIInputStream* aInputStream,
+  SlicedInputStream(already_AddRefed<nsIInputStream> aInputStream,
                     uint64_t aStart, uint64_t aLength);
 
   // This CTOR is meant to be used just for IPC.
@@ -51,7 +51,7 @@ private:
   ~SlicedInputStream();
 
   void
-  SetSourceStream(nsIInputStream* aInputStream);
+  SetSourceStream(already_AddRefed<nsIInputStream> aInputStream);
 
   nsresult
   RunAsyncWaitCallback();

xpcom/tests/gtest/TestSlicedInputStream.cpp

@@ -137,7 +137,7 @@ CreateSeekableStreams(uint32_t aSize, uint64_t aStart, uint64_t aLength,
 
   nsCOMPtr<nsIInputStream> stream;
   NS_NewCStringInputStream(getter_AddRefs(stream), aBuffer);
-  return new SlicedInputStream(stream, aStart, aLength);
+  return new SlicedInputStream(stream.forget(), aStart, aLength);
 }
 
 // Helper function for creating a non-seekable nsIInputStream + a
@@ -152,7 +152,7 @@ CreateNonSeekableStreams(uint32_t aSize, uint64_t aStart, uint64_t aLength,
   }
 
   RefPtr<NonSeekableStringStream> stream = new NonSeekableStringStream(aBuffer);
-  return new SlicedInputStream(stream, aStart, aLength);
+  return new SlicedInputStream(stream.forget(), aStart, aLength);
 }
 
 // Same start, same length.
@@ -334,10 +334,12 @@ TEST(TestSlicedInputStream, Seek_SET) {
   nsCString buf;
   buf.AssignLiteral("Hello world");
 
-  nsCOMPtr<nsIInputStream> stream;
-  NS_NewCStringInputStream(getter_AddRefs(stream), buf);
-
-  RefPtr<SlicedInputStream> sis = new SlicedInputStream(stream, 1, buf.Length());
+  RefPtr<SlicedInputStream> sis;
+  {
+    nsCOMPtr<nsIInputStream> stream;
+    NS_NewCStringInputStream(getter_AddRefs(stream), buf);
+    sis = new SlicedInputStream(stream.forget(), 1, buf.Length());
+  }
 
   ASSERT_EQ(NS_OK, sis->Seek(nsISeekableStream::NS_SEEK_SET, 1));
 
@@ -357,10 +359,13 @@ TEST(TestSlicedInputStream, Seek_CUR) {
   nsCString buf;
   buf.AssignLiteral("Hello world");
 
-  nsCOMPtr<nsIInputStream> stream;
-  NS_NewCStringInputStream(getter_AddRefs(stream), buf);
+  RefPtr<SlicedInputStream> sis;
+  {
+    nsCOMPtr<nsIInputStream> stream;
+    NS_NewCStringInputStream(getter_AddRefs(stream), buf);
 
-  RefPtr<SlicedInputStream> sis = new SlicedInputStream(stream, 1, buf.Length());
+    sis = new SlicedInputStream(stream.forget(), 1, buf.Length());
+  }
 
   ASSERT_EQ(NS_OK, sis->Seek(nsISeekableStream::NS_SEEK_CUR, 1));
 
@@ -386,13 +391,17 @@ TEST(TestSlicedInputStream, Seek_END_Bigger) {
   nsCString buf;
   buf.AssignLiteral("Hello world");
 
-  nsCOMPtr<nsIInputStream> stream;
-  NS_NewCStringInputStream(getter_AddRefs(stream), buf);
+  RefPtr<SlicedInputStream> sis;
+  {
+    nsCOMPtr<nsIInputStream> stream;
+    NS_NewCStringInputStream(getter_AddRefs(stream), buf);
 
-  RefPtr<SlicedInputStream> sis = new SlicedInputStream(stream, 2, buf.Length());
+    sis = new SlicedInputStream(stream.forget(), 2, buf.Length());
+  }
 
   ASSERT_EQ(NS_OK, sis->Seek(nsISeekableStream::NS_SEEK_END, -5));
 
+  nsCOMPtr<nsIInputStream> stream;
   NS_NewCStringInputStream(getter_AddRefs(stream), buf);
   nsCOMPtr<nsISeekableStream> seekStream = do_QueryInterface(stream);
   ASSERT_EQ(NS_OK, seekStream->Seek(nsISeekableStream::NS_SEEK_END, -5));
@@ -420,10 +429,13 @@ TEST(TestSlicedInputStream, Seek_END_Lower) {
   nsCString buf;
   buf.AssignLiteral("Hello world");
 
-  nsCOMPtr<nsIInputStream> stream;
-  NS_NewCStringInputStream(getter_AddRefs(stream), buf);
+  RefPtr<SlicedInputStream> sis;
+  {
+    nsCOMPtr<nsIInputStream> stream;
+    NS_NewCStringInputStream(getter_AddRefs(stream), buf);
 
-  RefPtr<SlicedInputStream> sis = new SlicedInputStream(stream, 2, 6);
+    sis = new SlicedInputStream(stream.forget(), 2, 6);
+  }
 
   ASSERT_EQ(NS_OK, sis->Seek(nsISeekableStream::NS_SEEK_END, -3));
 
@@ -468,10 +480,14 @@ TEST(TestSlicedInputStream, AsyncInputStream) {
 
   // We have to wrap the reader because it implements only a partial
   // nsISeekableStream interface. When ::Seek() is called, it does a MOZ_CRASH.
-  RefPtr<NonSeekableStringStream> wrapper =
-    new NonSeekableStringStream(reader);
+  nsCOMPtr<nsIInputStream> sis;
+  {
+    RefPtr<NonSeekableStringStream> wrapper =
+      new NonSeekableStringStream(reader);
+
+    sis = new SlicedInputStream(wrapper.forget(), 500, 500);
+  }
 
-  nsCOMPtr<nsIInputStream> sis = new SlicedInputStream(wrapper, 500, 500);
   nsCOMPtr<nsIAsyncInputStream> async = do_QueryInterface(sis);
   ASSERT_TRUE(!!async);