Bug 1378342 - AbortSignal/AbortController - part 9 - Request.signal should not be a reference of RequestInit.signal, r=bkelly

dom/abort/AbortSignal.cpp

@@ -60,7 +60,7 @@ AbortSignal::Abort()
 
   // Let's inform the followers.
   for (uint32_t i = 0; i < mFollowers.Length(); ++i) {
-    mFollowers[i]->Aborted();
+    mFollowers[i]->Abort();
   }
 
   EventInit init;
@@ -76,7 +76,7 @@ AbortSignal::Abort()
 }
 
 void
-AbortSignal::AddFollower(AbortSignal::Follower* aFollower)
+AbortSignal::AddFollower(AbortFollower* aFollower)
 {
   MOZ_DIAGNOSTIC_ASSERT(aFollower);
   if (!mFollowers.Contains(aFollower)) {
@@ -85,22 +85,22 @@ AbortSignal::AddFollower(AbortSignal::Follower* 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);
 
@@ -111,7 +111,7 @@ AbortSignal::Follower::Follow(AbortSignal* aSignal)
 }
 
 void
-AbortSignal::Follower::Unfollow()
+AbortFollower::Unfollow()
 {
   if (mFollowingSignal) {
     mFollowingSignal->RemoveFollower(this);
@@ -120,7 +120,7 @@ AbortSignal::Follower::Unfollow()
 }
 
 bool
-AbortSignal::Follower::IsFollowing() const
+AbortFollower::IsFollowing() const
 {
   return !!mFollowingSignal;
 }

dom/abort/AbortSignal.h

@@ -15,30 +15,31 @@ 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;
 
-  protected:
-    virtual ~Follower();
+  void
+  Follow(AbortSignal* aSignal);
 
-    void
-    Follow(AbortSignal* aSignal);
+  void
+  Unfollow();
 
-    void
-    Unfollow();
+  bool
+  IsFollowing() const;
 
-    bool
-    IsFollowing() const;
+protected:
+  virtual ~AbortFollower();
 
-    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)
 
@@ -52,23 +53,23 @@ public:
   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/fetch/Fetch.cpp

@@ -78,7 +78,7 @@ AbortStream(JSContext* aCx, JS::Handle<JSObject*> aStream)
 } // 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;
@@ -124,7 +124,7 @@ public:
   }
 
   void
-  Aborted() override
+  Abort() override
   {
     RefPtr<AbortSignalProxyRunnable> runnable =
       new AbortSignalProxyRunnable(this);
@@ -1315,7 +1315,7 @@ FetchBody<Response>::MaybeTeeReadableStreamBody(JSContext* aCx,
 
 template <class Derived>
 void
-FetchBody<Derived>::Aborted()
+FetchBody<Derived>::Abort()
 {
   MOZ_ASSERT(mReadableStreamBody);
 
@@ -1332,11 +1332,11 @@ FetchBody<Derived>::Aborted()
 
 template
 void
-FetchBody<Request>::Aborted();
+FetchBody<Request>::Abort();
 
 template
 void
-FetchBody<Response>::Aborted();
+FetchBody<Response>::Abort();
 
 } // namespace dom
 } // namespace mozilla

dom/fetch/Fetch.h

@@ -140,7 +140,7 @@ public:
  */
 template <class Derived>
 class FetchBody : public FetchStreamHolder
-                , public AbortSignal::Follower
+                , public AbortFollower
 {
 public:
   friend class FetchBodyConsumer<Derived>;
@@ -235,9 +235,9 @@ public:
   virtual AbortSignal*
   GetSignal() const = 0;
 
-  // AbortSignal::Follower
+  // AbortFollower
   void
-  Aborted() override;
+  Abort() override;
 
 protected:
   nsCOMPtr<nsIGlobalObject> mOwner;

dom/fetch/FetchConsumer.cpp

@@ -733,7 +733,7 @@ FetchBodyConsumer<Derived>::Observe(nsISupports* aSubject,
 
 template <class Derived>
 void
-FetchBodyConsumer<Derived>::Aborted()
+FetchBodyConsumer<Derived>::Abort()
 {
   AssertIsOnTargetThread();
   ContinueConsumeBody(NS_ERROR_DOM_ABORT_ERR, 0, nullptr);

dom/fetch/FetchConsumer.h

@@ -32,7 +32,7 @@ template <class Derived> class FetchBody;
 template <class Derived>
 class FetchBodyConsumer final : public nsIObserver
                               , public nsSupportsWeakReference
-                              , public AbortSignal::Follower
+                              , public AbortFollower
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
@@ -79,8 +79,8 @@ public:
     mConsumeBodyPump = nullptr;
   }
 
-  // AbortSignal::Follower
-  void Aborted() override;
+  // AbortFollower
+  void Abort() override;
 
 private:
   FetchBodyConsumer(nsIEventTarget* aMainThreadEventTarget,

dom/fetch/FetchDriver.cpp

@@ -104,7 +104,7 @@ FetchDriver::Fetch(AbortSignal* aSignal, FetchDriverObserver* aObserver)
   // the operation.
   if (aSignal) {
     if (aSignal->Aborted()) {
-      Aborted();
+      Abort();
       return NS_OK;
     }
 
@@ -967,7 +967,7 @@ FetchDriver::SetRequestHeaders(nsIHttpChannel* aChannel) const
 }
 
 void
-FetchDriver::Aborted()
+FetchDriver::Abort()
 {
   if (mObserver) {
   #ifdef DEBUG

dom/fetch/FetchDriver.h

@@ -85,7 +85,7 @@ class FetchDriver final : public nsIStreamListener,
                           public nsIChannelEventSink,
                           public nsIInterfaceRequestor,
                           public nsIThreadRetargetableStreamListener,
-                          public AbortSignal::Follower
+                          public AbortFollower
 {
 public:
   NS_DECL_ISUPPORTS
@@ -114,10 +114,9 @@ public:
     mWorkerScript = aWorkerScirpt;
   }
 
-  // AbortSignal::Follower
-
+  // AbortFollower
   void
-  Aborted() override;
+  Abort() override;
 
 private:
   nsCOMPtr<nsIPrincipal> mPrincipal;

dom/fetch/FetchObserver.cpp

@@ -67,7 +67,7 @@ FetchObserver::State() const
 }
 
 void
-FetchObserver::Aborted()
+FetchObserver::Abort()
 {
   SetState(FetchState::Aborted);
 }

dom/fetch/FetchObserver.h

@@ -15,7 +15,7 @@ namespace mozilla {
 namespace dom {
 
 class FetchObserver final : public DOMEventTargetHelper
-                          , public AbortSignal::Follower
+                          , public AbortFollower
 {
 public:
   NS_DECL_ISUPPORTS_INHERITED
@@ -37,7 +37,7 @@ public:
   IMPL_EVENT_HANDLER(responseprogress);
 
   void
-  Aborted() override;
+  Abort() override;
 
   void
   SetState(FetchState aState);

dom/fetch/Request.cpp

@@ -57,14 +57,20 @@ 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()

testing/web-platform/meta/fetch/api/abort/general.html.ini

@@ -1,9 +1,6 @@
 [general.html]
   type: testharness
   expected: TIMEOUT
-  [Window: Signal on request object]
-    expected: FAIL
-
   [Window: Signal removed by setting to null]
     expected: FAIL
 
@@ -13,9 +10,6 @@
   [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
 
@@ -28,9 +22,6 @@
   [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