Backed out changeset 64d0dbf9706b (bug 1378342)
authorSebastian Hengst <archaeopteryx@coole-files.de>
Tue, 29 Aug 2017 09:16:48 +0200
changeset 429448 22b6d264d711ed5060160966a277c835686ace80
parent 429447 cccc590388390ccd8f3ebd9c6a53480c51efb6fb
child 429449 a47c8ac7bc41f4b744f4f9917a03817ebc7df1ee
push id1567
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 12:36:05 +0000
treeherdermozilla-release@e512c14a0406 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1378342
milestone57.0a1
backs out64d0dbf9706bf31d5809322abb97d3cecfcac89f
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 64d0dbf9706b (bug 1378342)
dom/abort/AbortSignal.cpp
dom/abort/AbortSignal.h
dom/fetch/Fetch.cpp
dom/fetch/Fetch.h
dom/fetch/FetchStream.cpp
dom/fetch/Response.cpp
testing/web-platform/meta/fetch/api/abort/__dir__.ini
testing/web-platform/meta/fetch/api/abort/general.html.ini
--- a/dom/abort/AbortSignal.cpp
+++ b/dom/abort/AbortSignal.cpp
@@ -114,16 +114,10 @@ void
 AbortSignal::Follower::Unfollow()
 {
   if (mFollowingSignal) {
     mFollowingSignal->RemoveFollower(this);
     mFollowingSignal = nullptr;
   }
 }
 
-bool
-AbortSignal::Follower::IsFollowing() const
-{
-  return !!mFollowingSignal;
-}
-
 } // dom namespace
 } // mozilla namespace
--- a/dom/abort/AbortSignal.h
+++ b/dom/abort/AbortSignal.h
@@ -28,19 +28,16 @@ public:
     virtual ~Follower();
 
     void
     Follow(AbortSignal* aSignal);
 
     void
     Unfollow();
 
-    bool
-    IsFollowing() const;
-
     RefPtr<AbortSignal> mFollowingSignal;
   };
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(AbortSignal, DOMEventTargetHelper)
 
   AbortSignal(AbortController* aController, bool aAborted);
   explicit AbortSignal(bool aAborted);
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -21,17 +21,17 @@
 #include "nsStreamUtils.h"
 #include "nsStringStream.h"
 #include "nsProxyRelease.h"
 
 #include "mozilla/ErrorResult.h"
 #include "mozilla/dom/BindingDeclarations.h"
 #include "mozilla/dom/BodyUtil.h"
 #include "mozilla/dom/Exceptions.h"
-#include "mozilla/dom/DOMException.h"
+#include "mozilla/dom/DOMError.h"
 #include "mozilla/dom/FetchDriver.h"
 #include "mozilla/dom/File.h"
 #include "mozilla/dom/FormData.h"
 #include "mozilla/dom/Headers.h"
 #include "mozilla/dom/MutableBlobStreamListener.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/PromiseWorkerProxy.h"
 #include "mozilla/dom/Request.h"
@@ -51,37 +51,16 @@
 #include "WorkerScope.h"
 #include "Workers.h"
 
 namespace mozilla {
 namespace dom {
 
 using namespace workers;
 
-namespace {
-
-void
-AbortStream(JSContext* aCx, JS::Handle<JSObject*> aStream)
-{
-  if (!JS::ReadableStreamIsReadable(aStream)) {
-    return;
-  }
-
-  RefPtr<DOMException> e = DOMException::Create(NS_ERROR_DOM_ABORT_ERR);
-
-  JS::Rooted<JS::Value> value(aCx);
-  if (!GetOrCreateDOMReflector(aCx, e, &value)) {
-    return;
-  }
-
-  JS::ReadableStreamError(aCx, aStream, value);
-}
-
-} // anonymous
-
 // This class helps the proxying of AbortSignal changes cross threads.
 class AbortSignalProxy final : public AbortSignal::Follower
 {
   // This is created and released on the main-thread.
   RefPtr<AbortSignal> mSignalMainThread;
 
   // The main-thread event target for runnable dispatching.
   nsCOMPtr<nsIEventTarget> mMainThreadEventTarget;
@@ -959,17 +938,16 @@ template
 FetchBody<Request>::FetchBody(nsIGlobalObject* aOwner);
 
 template
 FetchBody<Response>::FetchBody(nsIGlobalObject* aOwner);
 
 template <class Derived>
 FetchBody<Derived>::~FetchBody()
 {
-  Unfollow();
 }
 
 template
 FetchBody<Request>::~FetchBody();
 
 template
 FetchBody<Response>::~FetchBody();
 
@@ -1123,43 +1101,30 @@ void
 FetchBody<Request>::SetMimeType();
 
 template
 void
 FetchBody<Response>::SetMimeType();
 
 template <class Derived>
 void
-FetchBody<Derived>::SetReadableStreamBody(JSContext* aCx, JSObject* aBody)
+FetchBody<Derived>::SetReadableStreamBody(JSObject* aBody)
 {
   MOZ_ASSERT(!mReadableStreamBody);
   MOZ_ASSERT(aBody);
   mReadableStreamBody = aBody;
-
-  RefPtr<AbortSignal> signal = DerivedClass()->GetSignal();
-  if (!signal) {
-    return;
-  }
-
-  bool aborted = signal->Aborted();
-  if (aborted) {
-    JS::Rooted<JSObject*> body(aCx, mReadableStreamBody);
-    AbortStream(aCx, body);
-  } else if (!IsFollowing()) {
-    Follow(signal);
-  }
 }
 
 template
 void
-FetchBody<Request>::SetReadableStreamBody(JSContext* aCx, JSObject* aBody);
+FetchBody<Request>::SetReadableStreamBody(JSObject* aBody);
 
 template
 void
-FetchBody<Response>::SetReadableStreamBody(JSContext* aCx, JSObject* aBody);
+FetchBody<Response>::SetReadableStreamBody(JSObject* aBody);
 
 template <class Derived>
 void
 FetchBody<Derived>::GetBody(JSContext* aCx,
                             JS::MutableHandle<JSObject*> aBodyOut,
                             ErrorResult& aRv)
 {
   if (mReadableStreamBody) {
@@ -1187,25 +1152,16 @@ FetchBody<Derived>::GetBody(JSContext* a
   // If the body has been already consumed, we lock the stream.
   if (BodyUsed()) {
     LockStream(aCx, body, aRv);
     if (NS_WARN_IF(aRv.Failed())) {
       return;
     }
   }
 
-  RefPtr<AbortSignal> signal = DerivedClass()->GetSignal();
-  if (signal) {
-    if (signal->Aborted()) {
-      AbortStream(aCx, body);
-    } else if (!IsFollowing()) {
-      Follow(signal);
-    }
-  }
-
   mReadableStreamBody = body;
   aBodyOut.set(mReadableStreamBody);
 }
 
 template
 void
 FetchBody<Request>::GetBody(JSContext* aCx,
                             JS::MutableHandle<JSObject*> aMessage,
@@ -1308,35 +1264,10 @@ FetchBody<Request>::MaybeTeeReadableStre
 template
 void
 FetchBody<Response>::MaybeTeeReadableStreamBody(JSContext* aCx,
                                                 JS::MutableHandle<JSObject*> aMessage,
                                                 FetchStreamReader** aStreamReader,
                                                 nsIInputStream** aInputStream,
                                                 ErrorResult& aRv);
 
-template <class Derived>
-void
-FetchBody<Derived>::Aborted()
-{
-  MOZ_ASSERT(mReadableStreamBody);
-
-  AutoJSAPI jsapi;
-  if (!jsapi.Init(mOwner)) {
-    return;
-  }
-
-  JSContext* cx = jsapi.cx();
-
-  JS::Rooted<JSObject*> body(cx, mReadableStreamBody);
-  AbortStream(cx, body);
-}
-
-template
-void
-FetchBody<Request>::Aborted();
-
-template
-void
-FetchBody<Response>::Aborted();
-
 } // namespace dom
 } // namespace mozilla
--- a/dom/fetch/Fetch.h
+++ b/dom/fetch/Fetch.h
@@ -12,17 +12,16 @@
 
 #include "nsCOMPtr.h"
 #include "nsError.h"
 #include "nsProxyRelease.h"
 #include "nsString.h"
 
 #include "mozilla/DebugOnly.h"
 #include "mozilla/ErrorResult.h"
-#include "mozilla/dom/AbortSignal.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/FetchStreamReader.h"
 #include "mozilla/dom/RequestBinding.h"
 
 class nsIGlobalObject;
 class nsIEventTarget;
 
 namespace mozilla {
@@ -135,17 +134,16 @@ public:
  *    held by it) until pump->Cancel() is called. OnStreamComplete() will not
  *    do anything if the error code is NS_BINDING_ABORTED, so we don't have to
  *    worry about keeping anything alive.
  *
  * The pump is always released on the main thread.
  */
 template <class Derived>
 class FetchBody : public FetchStreamHolder
-                , public AbortSignal::Follower
 {
 public:
   friend class FetchBodyConsumer<Derived>;
 
   bool
   BodyUsed() const;
 
   already_AddRefed<Promise>
@@ -230,20 +228,16 @@ public:
     mReadableStreamBody = nullptr;
     mReadableStreamReader = nullptr;
     mFetchStreamReader = nullptr;
   }
 
   virtual AbortSignal*
   GetSignal() const = 0;
 
-  // AbortSignal::Follower
-  void
-  Aborted() override;
-
 protected:
   nsCOMPtr<nsIGlobalObject> mOwner;
 
   // Always set whenever the FetchBody is created on the worker thread.
   workers::WorkerPrivate* mWorkerPrivate;
 
   // This is the ReadableStream exposed to content. It's underlying source is a
   // FetchStream object.
@@ -256,17 +250,17 @@ protected:
   explicit FetchBody(nsIGlobalObject* aOwner);
 
   virtual ~FetchBody();
 
   void
   SetMimeType();
 
   void
-  SetReadableStreamBody(JSContext* aCx, JSObject* aBody);
+  SetReadableStreamBody(JSObject* aBody);
 
 private:
   Derived*
   DerivedClass() const
   {
     return static_cast<Derived*>(const_cast<FetchBody*>(this));
   }
 
--- a/dom/fetch/FetchStream.cpp
+++ b/dom/fetch/FetchStream.cpp
@@ -1,16 +1,15 @@
 /* -*- 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 "FetchStream.h"
-#include "mozilla/dom/DOMError.h"
 #include "nsITransport.h"
 #include "nsIStreamTransportService.h"
 #include "nsProxyRelease.h"
 #include "WorkerPrivate.h"
 #include "Workers.h"
 
 #define FETCH_STREAM_FLAG 0
 
@@ -335,28 +334,16 @@ FetchStream::ClosedCallback(JSContext* a
 
 /* static */ void
 FetchStream::ErroredCallback(JSContext* aCx, JS::HandleObject aStream,
                              void* aUnderlyingSource, uint8_t aFlags,
                              JS::HandleValue aReason)
 {
   MOZ_DIAGNOSTIC_ASSERT(aUnderlyingSource);
   MOZ_DIAGNOSTIC_ASSERT(aFlags == FETCH_STREAM_FLAG);
-
-  // This is safe because we created an extra reference in FetchStream::Create()
-  // that won't be released until FetchStream::FinalizeCallback() is called.
-  // We are guaranteed that won't happen until the js ReadableStream object
-  // is finalized.
-  FetchStream* stream = static_cast<FetchStream*>(aUnderlyingSource);
-
-  if (stream->mInputStream) {
-    stream->mInputStream->CloseWithStatus(NS_BASE_STREAM_CLOSED);
-  }
-
-  stream->ReleaseObjects();
 }
 
 void
 FetchStream::FinalizeCallback(void* aUnderlyingSource, uint8_t aFlags)
 {
   MOZ_DIAGNOSTIC_ASSERT(aUnderlyingSource);
   MOZ_DIAGNOSTIC_ASSERT(aFlags == FETCH_STREAM_FLAG);
 
--- a/dom/fetch/Response.cpp
+++ b/dom/fetch/Response.cpp
@@ -246,17 +246,17 @@ Response::Constructor(const GlobalObject
 
       if (JS::ReadableStreamIsDisturbed(readableStreamObj) ||
           JS::ReadableStreamIsLocked(readableStreamObj) ||
           !JS::ReadableStreamIsReadable(readableStreamObj)) {
         aRv.ThrowTypeError<MSG_FETCH_BODY_CONSUMED_ERROR>();
         return nullptr;
       }
 
-      r->SetReadableStreamBody(aGlobal.Context(), readableStreamObj);
+      r->SetReadableStreamBody(readableStreamObj);
 
       if (JS::ReadableStreamGetMode(readableStreamObj) ==
             JS::ReadableStreamMode::ExternalSource) {
         // If this is a DOM generated ReadableStream, we can extract the
         // inputStream directly.
         void* underlyingSource = nullptr;
         if (!JS::ReadableStreamGetExternalUnderlyingSource(aGlobal.Context(),
                                                            readableStreamObj,
@@ -349,17 +349,17 @@ Response::Clone(JSContext* aCx, ErrorRes
 
   RefPtr<Response> response = new Response(mOwner, ir, mSignal);
 
   if (body) {
     // Maybe we have a body, but we receive null from MaybeTeeReadableStreamBody
     // if this body is a native stream.   In this case the InternalResponse will
     // have a clone of the native body and the ReadableStream will be created
     // lazily if needed.
-    response->SetReadableStreamBody(aCx, body);
+    response->SetReadableStreamBody(body);
     response->mFetchStreamReader = streamReader;
     ir->SetBody(inputStream, InternalResponse::UNKNOWN_BODY_SIZE);
   }
 
   return response.forget();
 }
 
 already_AddRefed<Response>
@@ -392,17 +392,17 @@ Response::CloneUnfiltered(JSContext* aCx
   RefPtr<InternalResponse> ir = clone->Unfiltered();
   RefPtr<Response> ref = new Response(mOwner, ir, mSignal);
 
   if (body) {
     // Maybe we have a body, but we receive null from MaybeTeeReadableStreamBody
     // if this body is a native stream.   In this case the InternalResponse will
     // have a clone of the native body and the ReadableStream will be created
     // lazily if needed.
-    ref->SetReadableStreamBody(aCx, body);
+    ref->SetReadableStreamBody(body);
     ref->mFetchStreamReader = streamReader;
     ir->SetBody(inputStream, InternalResponse::UNKNOWN_BODY_SIZE);
   }
 
   return ref.forget();
 }
 
 void
deleted file mode 100644
--- a/testing/web-platform/meta/fetch/api/abort/__dir__.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-prefs: [javascript.options.streams:true,
-        dom.streams.enabled:true]
--- a/testing/web-platform/meta/fetch/api/abort/general.html.ini
+++ b/testing/web-platform/meta/fetch/api/abort/general.html.ini
@@ -2,44 +2,62 @@
   type: testharness
   expected: TIMEOUT
   [Window: Signal on request object]
     expected: FAIL
 
   [Window: Signal removed by setting to null]
     expected: FAIL
 
+  [Window: Stream errors once aborted. Underlying connection closed.]
+    expected: FAIL
+
+  [Window: Stream errors once aborted, after reading. Underlying connection closed.]
+    expected: FAIL
+
   [Window: Stream will not error if body is empty. It's closed with an empty queue before it errors.]
     expected: FAIL
 
   [Window: Readable stream synchronously cancels with AbortError if aborted before reading]
     expected: FAIL
 
   [DedicatedWorkerGlobalScope: Signal on request object]
     expected: FAIL
 
   [DedicatedWorkerGlobalScope: Signal removed by setting to null]
     expected: FAIL
 
   [DedicatedWorkerGlobalScope: Already aborted signal rejects immediately]
     expected: FAIL
 
+  [DedicatedWorkerGlobalScope: Stream errors once aborted. Underlying connection closed.]
+    expected: FAIL
+
+  [DedicatedWorkerGlobalScope: Stream errors once aborted, after reading. Underlying connection closed.]
+    expected: FAIL
+
   [DedicatedWorkerGlobalScope: Stream will not error if body is empty. It's closed with an empty queue before it errors.]
     expected: FAIL
 
   [DedicatedWorkerGlobalScope: Readable stream synchronously cancels with AbortError if aborted before reading]
     expected: FAIL
 
   [SharedWorkerGlobalScope: Signal on request object]
     expected: FAIL
 
   [SharedWorkerGlobalScope: Signal removed by setting to null]
     expected: FAIL
 
   [SharedWorkerGlobalScope: Already aborted signal rejects immediately]
     expected: FAIL
 
+  [SharedWorkerGlobalScope: Stream errors once aborted. Underlying connection closed.]
+    expected: FAIL
+
+  [SharedWorkerGlobalScope: Stream errors once aborted, after reading. Underlying connection closed.]
+    expected: FAIL
+
   [SharedWorkerGlobalScope: Stream will not error if body is empty. It's closed with an empty queue before it errors.]
     expected: FAIL
 
   [SharedWorkerGlobalScope: Readable stream synchronously cancels with AbortError if aborted before reading]
     expected: FAIL