Bug 1378342 - AbortSignal/AbortController - part 9 - Request.signal should not be a reference of RequestInit.signal, r=bkelly
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 29 Aug 2017 11:31:07 +0200
changeset 377505 9b10392f26bf1c4881248ad761dcef28446ab507
parent 377504 4ccd7926f66b34022bd9a36ffcaf439717bdd84b
child 377506 52519801d0bc68b9594afea4d24dc64c2b3ca124
push id32408
push userarchaeopteryx@coole-files.de
push dateTue, 29 Aug 2017 18:31:12 +0000
treeherdermozilla-central@6c3510bac832 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1378342
milestone57.0a1
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
Bug 1378342 - AbortSignal/AbortController - part 9 - Request.signal should not be a reference of RequestInit.signal, r=bkelly
dom/abort/AbortSignal.cpp
dom/abort/AbortSignal.h
dom/fetch/Fetch.cpp
dom/fetch/Fetch.h
dom/fetch/FetchConsumer.cpp
dom/fetch/FetchConsumer.h
dom/fetch/FetchDriver.cpp
dom/fetch/FetchDriver.h
dom/fetch/FetchObserver.cpp
dom/fetch/FetchObserver.h
dom/fetch/Request.cpp
testing/web-platform/meta/fetch/api/abort/general.html.ini
--- a/dom/abort/AbortSignal.cpp
+++ b/dom/abort/AbortSignal.cpp
@@ -55,75 +55,75 @@ AbortSignal::Aborted() const
 void
 AbortSignal::Abort()
 {
   MOZ_ASSERT(!mAborted);
   mAborted = true;
 
   // Let's inform the followers.
   for (uint32_t i = 0; i < mFollowers.Length(); ++i) {
-    mFollowers[i]->Aborted();
+    mFollowers[i]->Abort();
   }
 
   EventInit init;
   init.mBubbles = false;
   init.mCancelable = false;
 
   RefPtr<Event> event =
     Event::Constructor(this, NS_LITERAL_STRING("abort"), init);
   event->SetTrusted(true);
 
   bool dummy;
   DispatchEvent(event, &dummy);
 }
 
 void
-AbortSignal::AddFollower(AbortSignal::Follower* aFollower)
+AbortSignal::AddFollower(AbortFollower* aFollower)
 {
   MOZ_DIAGNOSTIC_ASSERT(aFollower);
   if (!mFollowers.Contains(aFollower)) {
     mFollowers.AppendElement(aFollower);
   }
 }
 
 void
-AbortSignal::RemoveFollower(AbortSignal::Follower* aFollower)
+AbortSignal::RemoveFollower(AbortFollower* aFollower)
 {
   MOZ_DIAGNOSTIC_ASSERT(aFollower);
   mFollowers.RemoveElement(aFollower);
 }
 
-// AbortSignal::Follower
+// AbortFollower
 // ----------------------------------------------------------------------------
 
-AbortSignal::Follower::~Follower()
+AbortFollower::~AbortFollower()
 {
   Unfollow();
 }
 
 void
-AbortSignal::Follower::Follow(AbortSignal* aSignal)
+AbortFollower::Follow(AbortSignal* aSignal)
 {
   MOZ_DIAGNOSTIC_ASSERT(aSignal);
 
   Unfollow();
 
   mFollowingSignal = aSignal;
   aSignal->AddFollower(this);
 }
 
 void
-AbortSignal::Follower::Unfollow()
+AbortFollower::Unfollow()
 {
   if (mFollowingSignal) {
     mFollowingSignal->RemoveFollower(this);
     mFollowingSignal = nullptr;
   }
 }
 
 bool
-AbortSignal::Follower::IsFollowing() const
+AbortFollower::IsFollowing() const
 {
   return !!mFollowingSignal;
 }
 
 } // dom namespace
 } // mozilla namespace
--- a/dom/abort/AbortSignal.h
+++ b/dom/abort/AbortSignal.h
@@ -10,70 +10,71 @@
 #include "mozilla/DOMEventTargetHelper.h"
 
 namespace mozilla {
 namespace dom {
 
 class AbortController;
 class AbortSignal;
 
-class AbortSignal final : public DOMEventTargetHelper
+// This class must be implemented by objects who want to follow a AbortSignal.
+class AbortFollower
 {
 public:
-  // This class must be implemented by objects who want to follow a AbortSignal.
-  class Follower
-  {
-  public:
-    virtual void Aborted() = 0;
+  virtual void Abort() = 0;
+
+  void
+  Follow(AbortSignal* aSignal);
 
-  protected:
-    virtual ~Follower();
+  void
+  Unfollow();
+
+  bool
+  IsFollowing() const;
 
-    void
-    Follow(AbortSignal* aSignal);
-
-    void
-    Unfollow();
+protected:
+  virtual ~AbortFollower();
 
-    bool
-    IsFollowing() const;
+  RefPtr<AbortSignal> mFollowingSignal;
+};
 
-    RefPtr<AbortSignal> mFollowingSignal;
-  };
-
+class AbortSignal final : public DOMEventTargetHelper
+                        , public AbortFollower
+{
+public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(AbortSignal, DOMEventTargetHelper)
 
   AbortSignal(AbortController* aController, bool aAborted);
   explicit AbortSignal(bool aAborted);
 
   JSObject*
   WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   bool
   Aborted() const;
 
   void
-  Abort();
+  Abort() override;
 
   IMPL_EVENT_HANDLER(abort);
 
   void
-  AddFollower(Follower* aFollower);
+  AddFollower(AbortFollower* aFollower);
 
   void
-  RemoveFollower(Follower* aFollower);
+  RemoveFollower(AbortFollower* aFollower);
 
 private:
   ~AbortSignal() = default;
 
   RefPtr<AbortController> mController;
 
-  // Raw pointers. Follower unregisters itself in the DTOR.
-  nsTArray<Follower*> mFollowers;
+  // Raw pointers. AbortFollower unregisters itself in the DTOR.
+  nsTArray<AbortFollower*> mFollowers;
 
   bool mAborted;
 };
 
 } // dom namespace
 } // mozilla namespace
 
 #endif // mozilla_dom_AbortSignal_h
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -73,17 +73,17 @@ AbortStream(JSContext* aCx, JS::Handle<J
   }
 
   JS::ReadableStreamError(aCx, aStream, value);
 }
 
 } // anonymous
 
 // This class helps the proxying of AbortSignal changes cross threads.
-class AbortSignalProxy final : public AbortSignal::Follower
+class AbortSignalProxy final : public AbortFollower
 {
   // This is created and released on the main-thread.
   RefPtr<AbortSignal> mSignalMainThread;
 
   // The main-thread event target for runnable dispatching.
   nsCOMPtr<nsIEventTarget> mMainThreadEventTarget;
 
   // This value is used only for the creation of AbortSignal on the
@@ -119,17 +119,17 @@ public:
     : mMainThreadEventTarget(aMainThreadEventTarget)
     , mAborted(aSignal->Aborted())
   {
     MOZ_ASSERT(mMainThreadEventTarget);
     Follow(aSignal);
   }
 
   void
-  Aborted() override
+  Abort() override
   {
     RefPtr<AbortSignalProxyRunnable> runnable =
       new AbortSignalProxyRunnable(this);
     mMainThreadEventTarget->Dispatch(runnable.forget(), NS_DISPATCH_NORMAL);
   }
 
   AbortSignal*
   GetOrCreateSignalForMainThread()
@@ -1310,33 +1310,33 @@ void
 FetchBody<Response>::MaybeTeeReadableStreamBody(JSContext* aCx,
                                                 JS::MutableHandle<JSObject*> aMessage,
                                                 FetchStreamReader** aStreamReader,
                                                 nsIInputStream** aInputStream,
                                                 ErrorResult& aRv);
 
 template <class Derived>
 void
-FetchBody<Derived>::Aborted()
+FetchBody<Derived>::Abort()
 {
   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();
+FetchBody<Request>::Abort();
 
 template
 void
-FetchBody<Response>::Aborted();
+FetchBody<Response>::Abort();
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/fetch/Fetch.h
+++ b/dom/fetch/Fetch.h
@@ -135,17 +135,17 @@ 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 AbortFollower
 {
 public:
   friend class FetchBodyConsumer<Derived>;
 
   bool
   BodyUsed() const;
 
   already_AddRefed<Promise>
@@ -230,19 +230,19 @@ public:
     mReadableStreamBody = nullptr;
     mReadableStreamReader = nullptr;
     mFetchStreamReader = nullptr;
   }
 
   virtual AbortSignal*
   GetSignal() const = 0;
 
-  // AbortSignal::Follower
+  // AbortFollower
   void
-  Aborted() override;
+  Abort() 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
--- a/dom/fetch/FetchConsumer.cpp
+++ b/dom/fetch/FetchConsumer.cpp
@@ -728,17 +728,17 @@ FetchBodyConsumer<Derived>::Observe(nsIS
     ContinueConsumeBody(NS_BINDING_ABORTED, 0, nullptr);
   }
 
   return NS_OK;
 }
 
 template <class Derived>
 void
-FetchBodyConsumer<Derived>::Aborted()
+FetchBodyConsumer<Derived>::Abort()
 {
   AssertIsOnTargetThread();
   ContinueConsumeBody(NS_ERROR_DOM_ABORT_ERR, 0, nullptr);
 }
 
 template <class Derived>
 NS_IMPL_ADDREF(FetchBodyConsumer<Derived>)
 
--- a/dom/fetch/FetchConsumer.h
+++ b/dom/fetch/FetchConsumer.h
@@ -27,17 +27,17 @@ class WorkerHolder;
 template <class Derived> class FetchBody;
 
 // FetchBody is not thread-safe but we need to move it around threads.
 // In order to keep it alive all the time, we use a WorkerHolder, if created on
 // workers, plus a this consumer.
 template <class Derived>
 class FetchBodyConsumer final : public nsIObserver
                               , public nsSupportsWeakReference
-                              , public AbortSignal::Follower
+                              , public AbortFollower
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
   static already_AddRefed<Promise>
   Create(nsIGlobalObject* aGlobal,
          nsIEventTarget* aMainThreadEventTarget,
@@ -74,18 +74,18 @@ public:
   }
 
   void
   NullifyConsumeBodyPump()
   {
     mConsumeBodyPump = nullptr;
   }
 
-  // AbortSignal::Follower
-  void Aborted() override;
+  // AbortFollower
+  void Abort() override;
 
 private:
   FetchBodyConsumer(nsIEventTarget* aMainThreadEventTarget,
                     nsIGlobalObject* aGlobalObject,
                     workers::WorkerPrivate* aWorkerPrivate,
                     FetchBody<Derived>* aBody,
                     Promise* aPromise,
                     FetchConsumeType aType);
--- a/dom/fetch/FetchDriver.cpp
+++ b/dom/fetch/FetchDriver.cpp
@@ -99,17 +99,17 @@ FetchDriver::Fetch(AbortSignal* aSignal,
   }
 
   mRequest->SetPrincipalInfo(Move(principalInfo));
 
   // If the signal is aborted, it's time to inform the observer and terminate
   // the operation.
   if (aSignal) {
     if (aSignal->Aborted()) {
-      Aborted();
+      Abort();
       return NS_OK;
     }
 
     Follow(aSignal);
   }
 
   if (NS_FAILED(HttpFetch())) {
     FailWithNetworkError();
@@ -962,17 +962,17 @@ FetchDriver::SetRequestHeaders(nsIHttpCh
                                    NS_ConvertUTF16toUTF8(origin),
                                    false /* merge */);
       MOZ_ASSERT(NS_SUCCEEDED(rv));
     }
   }
 }
 
 void
-FetchDriver::Aborted()
+FetchDriver::Abort()
 {
   if (mObserver) {
   #ifdef DEBUG
     mResponseAvailableCalled = true;
   #endif
     mObserver->OnResponseEnd(FetchDriverObserver::eAborted);
     mObserver = nullptr;
   }
--- a/dom/fetch/FetchDriver.h
+++ b/dom/fetch/FetchDriver.h
@@ -80,17 +80,17 @@ protected:
 private:
   bool mGotResponseAvailable;
 };
 
 class FetchDriver final : public nsIStreamListener,
                           public nsIChannelEventSink,
                           public nsIInterfaceRequestor,
                           public nsIThreadRetargetableStreamListener,
-                          public AbortSignal::Follower
+                          public AbortFollower
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSICHANNELEVENTSINK
   NS_DECL_NSIINTERFACEREQUESTOR
   NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
@@ -109,20 +109,19 @@ public:
 
   void
   SetWorkerScript(const nsACString& aWorkerScirpt)
   {
     MOZ_ASSERT(!aWorkerScirpt.IsEmpty());
     mWorkerScript = aWorkerScirpt;
   }
 
-  // AbortSignal::Follower
-
+  // AbortFollower
   void
-  Aborted() override;
+  Abort() override;
 
 private:
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsCOMPtr<nsILoadGroup> mLoadGroup;
   RefPtr<InternalRequest> mRequest;
   RefPtr<InternalResponse> mResponse;
   nsCOMPtr<nsIOutputStream> mPipeOutputStream;
   RefPtr<FetchDriverObserver> mObserver;
--- a/dom/fetch/FetchObserver.cpp
+++ b/dom/fetch/FetchObserver.cpp
@@ -62,17 +62,17 @@ FetchObserver::WrapObject(JSContext* aCx
 
 FetchState
 FetchObserver::State() const
 {
   return mState;
 }
 
 void
-FetchObserver::Aborted()
+FetchObserver::Abort()
 {
   SetState(FetchState::Aborted);
 }
 
 void
 FetchObserver::SetState(FetchState aState)
 {
   MOZ_ASSERT(mState < aState);
--- a/dom/fetch/FetchObserver.h
+++ b/dom/fetch/FetchObserver.h
@@ -10,17 +10,17 @@
 #include "mozilla/DOMEventTargetHelper.h"
 #include "mozilla/dom/FetchObserverBinding.h"
 #include "mozilla/dom/AbortSignal.h"
 
 namespace mozilla {
 namespace dom {
 
 class FetchObserver final : public DOMEventTargetHelper
-                          , public AbortSignal::Follower
+                          , public AbortFollower
 {
 public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(FetchObserver, DOMEventTargetHelper)
 
   static bool
   IsEnabled(JSContext* aCx, JSObject* aGlobal);
 
@@ -32,17 +32,17 @@ public:
   FetchState
   State() const;
 
   IMPL_EVENT_HANDLER(statechange);
   IMPL_EVENT_HANDLER(requestprogress);
   IMPL_EVENT_HANDLER(responseprogress);
 
   void
-  Aborted() override;
+  Abort() override;
 
   void
   SetState(FetchState aState);
 
 private:
   ~FetchObserver() = default;
 
   FetchState mState;
--- a/dom/fetch/Request.cpp
+++ b/dom/fetch/Request.cpp
@@ -52,24 +52,30 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
 
 Request::Request(nsIGlobalObject* aOwner, InternalRequest* aRequest,
                  AbortSignal* aSignal)
   : FetchBody<Request>(aOwner)
   , mRequest(aRequest)
-  , mSignal(aSignal)
 {
   MOZ_ASSERT(aRequest->Headers()->Guard() == HeadersGuardEnum::Immutable ||
              aRequest->Headers()->Guard() == HeadersGuardEnum::Request ||
              aRequest->Headers()->Guard() == HeadersGuardEnum::Request_no_cors);
   SetMimeType();
 
-  // aSignal can be null.
+  if (aSignal) {
+    // If we don't have a signal as argument, we will create it when required by
+    // content, otherwise the Request's signal must follow what has been passed.
+    mSignal = new AbortSignal(aSignal->Aborted());
+    if (!mSignal->Aborted()) {
+      mSignal->Follow(aSignal);
+    }
+  }
 }
 
 Request::~Request()
 {
 }
 
 // static
 bool
--- a/testing/web-platform/meta/fetch/api/abort/general.html.ini
+++ b/testing/web-platform/meta/fetch/api/abort/general.html.ini
@@ -1,41 +1,32 @@
 [general.html]
   type: testharness
   expected: TIMEOUT
-  [Window: Signal on request object]
-    expected: FAIL
-
   [Window: Signal removed by setting to null]
     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 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 will not error if body is empty. It's closed with an empty queue before it errors.]
     expected: FAIL