Bug 1567419 - Ensure the BodyStreamHolder has a valid stream always, r=smaug

Differential Revision: https://phabricator.services.mozilla.com/D38814

--HG--
extra : moz-landing-system : lando

dom/base/BodyStream.cpp

@@ -67,7 +67,6 @@ NS_IMPL_ISUPPORTS(BodyStream, nsIInputStreamCallback, nsIObserver,
 /* static */
 void BodyStream::Create(JSContext* aCx, BodyStreamHolder* aStreamHolder,
                         nsIGlobalObject* aGlobal, nsIInputStream* aInputStream,
-                        JS::MutableHandle<JSObject*> aStream,
                         ErrorResult& aRv) {
   MOZ_DIAGNOSTIC_ASSERT(aCx);
   MOZ_DIAGNOSTIC_ASSERT(aStreamHolder);
@@ -119,7 +118,7 @@ void BodyStream::Create(JSContext* aCx, BodyStreamHolder* aStreamHolder,
   // js object is finalized.
   NS_ADDREF(stream.get());
 
-  aStream.set(body);
+  aStreamHolder->SetReadableStreamBody(body);
 }
 
 void BodyStream::requestData(JSContext* aCx, JS::HandleObject aStream,

dom/base/BodyStream.h

@@ -32,6 +32,8 @@ class BodyStreamHolder : public nsISupports {
 
   virtual void MarkAsRead() = 0;
 
+  virtual void SetReadableStreamBody(JSObject* aBody) = 0;
+
   virtual JSObject* GetReadableStreamBody() = 0;
 
  protected:
@@ -47,19 +49,11 @@ class BodyStream final : public nsIInputStreamCallback,
   NS_DECL_NSIINPUTSTREAMCALLBACK
   NS_DECL_NSIOBSERVER
 
-  // This method creates a JS ReadableStream object and it makes a CC cycle
-  // between it and the aStreamHolder.
-  // The aStreamHolder must do the following operations after calling this
-  // method:
-  // - it must store the JS ReadableStream and returns it in
-  //   BodyStreamHolder::GetReadableStreamBody().
-  // - it must trace the JS ReadableStream.
-  // - if the operation has to be aborted, or the stream has to be released for
-  //   any reason, the aStreamHolder must call
-  //   JS::ReadableStreamReleaseCCObject().
+  // This method creates a JS ReadableStream object and it assigns it to the
+  // aStreamHolder calling SetReadableStreamBody().
   static void Create(JSContext* aCx, BodyStreamHolder* aStreamHolder,
                      nsIGlobalObject* aGlobal, nsIInputStream* aInputStream,
-                     JS::MutableHandle<JSObject*> aStream, ErrorResult& aRv);
+                     ErrorResult& aRv);
 
   void Close();
 

dom/fetch/Fetch.cpp

@@ -1319,19 +1319,15 @@ void FetchBody<Derived>::GetBody(JSContext* aCx,
     return;
   }
 
-  JS::Rooted<JSObject*> body(aCx);
   BodyStream::Create(aCx, this, DerivedClass()->GetParentObject(), inputStream,
-                     &body, aRv);
+                     aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
 
-  MOZ_ASSERT(body);
+  MOZ_ASSERT(mReadableStreamBody);
 
-  // We must break the cycle between the body and the stream in case we do an
-  // early return.
-  auto raii =
-      mozilla::MakeScopeExit([&] { JS::ReadableStreamReleaseCCObject(body); });
+  JS::Rooted<JSObject*> body(aCx, mReadableStreamBody);
 
   // If the body has been already consumed, we lock the stream.
   bool bodyUsed = GetBodyUsed(aRv);
@@ -1357,9 +1353,6 @@ void FetchBody<Derived>::GetBody(JSContext* aCx,
     }
   }
 
-  raii.release();
-
-  mReadableStreamBody = body;
   aBodyOut.set(mReadableStreamBody);
 }
 

dom/fetch/Fetch.h

@@ -199,6 +199,10 @@ class FetchBody : public BodyStreamHolder, public AbortFollower {
     mFetchStreamReader = nullptr;
   }
 
+  void SetReadableStreamBody(JSObject* aBody) override {
+    mReadableStreamBody = aBody;
+  }
+
   JSObject* GetReadableStreamBody() override { return mReadableStreamBody; }
 
   void MarkAsRead() override { mBodyUsed = true; }

dom/file/Blob.cpp

@@ -298,13 +298,13 @@ class BlobBodyStreamHolder final : public BodyStreamHolder {
 
   void MarkAsRead() override {}
 
-  JSObject* GetReadableStreamBody() override { return mStream; }
-
-  void SetStream(JSObject* aObject) {
-    MOZ_ASSERT(aObject);
-    mStream = aObject;
+  void SetReadableStreamBody(JSObject* aBody) override {
+    MOZ_ASSERT(aBody);
+    mStream = aBody;
   }
 
+  JSObject* GetReadableStreamBody() override { return mStream; }
+
   // Public to make trace happy.
   JS::Heap<JSObject*> mStream;
 
@@ -352,16 +352,12 @@ void Blob::Stream(JSContext* aCx, JS::MutableHandle<JSObject*> aStream,
 
   RefPtr<BlobBodyStreamHolder> holder = new BlobBodyStreamHolder();
 
-  JS::Rooted<JSObject*> body(aCx);
-  BodyStream::Create(aCx, holder, global, stream, &body, aRv);
+  BodyStream::Create(aCx, holder, global, stream, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     return;
   }
 
-  MOZ_ASSERT(body);
-
-  holder->SetStream(body);
-  aStream.set(body);
+  aStream.set(holder->GetReadableStreamBody());
 }
 
 }  // namespace dom