Bug 1400748 - Correct our handling of XHR.abort edge-cases; r?baku draft
authorThomas Wisniewski <wisniewskit@gmail.com>
Mon, 25 Sep 2017 12:58:23 -0400
changeset 669932 cb88cf5f585347b3ea2180da4ec103653dcf9071
parent 669742 5f3f19824efa14cc6db546baf59c54a0fc15ddc9
child 669933 64405a75bb33f4e4e232525eb50d36c843a6a71e
push id81472
push userwisniewskit@gmail.com
push dateMon, 25 Sep 2017 17:07:38 +0000
reviewersbaku
bugs1400748
milestone58.0a1
Bug 1400748 - Correct our handling of XHR.abort edge-cases; r?baku 1. Handle the "terminate the ongoing fetch" cases in the spec-text - do not CloseRequest in Abort/Open if the state is UNSENT or DONE). - ensure we don't fire extra events after terminating this way if a stray OnDataAvailable happens afterward. 2. Ensure that status/statusText correctly return 0/"" to mimic the spec's "set response to a network error" steps (requires special handling in the worker XHR code). MozReview-Commit-ID: 5kMyGgD7uUU
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
dom/xhr/XMLHttpRequestWorker.cpp
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -979,19 +979,28 @@ XMLHttpRequestMainThread::GetStatusText(
   if (httpChannel) {
     Unused << httpChannel->GetResponseStatusText(aStatusText);
   } else {
     aStatusText.AssignLiteral("OK");
   }
 }
 
 void
+XMLHttpRequestMainThread::TerminateOngoingFetch() {
+  if ((mState == State::opened && mFlagSend) ||
+      mState == State::headers_received || mState == State::loading) {
+    CloseRequest();
+  }
+}
+
+void
 XMLHttpRequestMainThread::CloseRequest()
 {
   mWaitingForOnStopRequest = false;
+  mErrorLoad = ErrorType::eTerminated;
   if (mChannel) {
     mChannel->Cancel(NS_BINDING_ABORTED);
   }
   if (mTimeoutTimer) {
     mTimeoutTimer->Cancel();
   }
 }
 
@@ -1082,17 +1091,17 @@ XMLHttpRequestMainThread::RequestErrorSt
 }
 
 void
 XMLHttpRequestMainThread::Abort(ErrorResult& aRv)
 {
   mFlagAborted = true;
 
   // Step 1
-  CloseRequest();
+  TerminateOngoingFetch();
 
   // Step 2
   if ((mState == State::opened && mFlagSend) ||
        mState == State::headers_received ||
        mState == State::loading) {
     RequestErrorSteps(ProgressEventType::abort, NS_OK, aRv);
   }
 
@@ -1324,16 +1333,17 @@ XMLHttpRequestMainThread::GetLoadGroup()
   }
 
   return nullptr;
 }
 
 nsresult
 XMLHttpRequestMainThread::FireReadystatechangeEvent()
 {
+  MOZ_ASSERT(mState != State::unsent);
   RefPtr<Event> event = NS_NewDOMEvent(this, nullptr, nullptr);
   event->InitEvent(kLiteralString_readystatechange, false, false);
   // We assume anyone who managed to call CreateReadystatechangeEvent is trusted
   event->SetTrusted(true);
   DispatchOrStoreEvent(this, event);
   return NS_OK;
 }
 
@@ -1579,17 +1589,17 @@ XMLHttpRequestMainThread::Open(const nsA
     }
     if (mResponseType != XMLHttpRequestResponseType::_empty) {
       LogMessage("ResponseTypeSyncXHRWarning", GetOwner());
     }
     return NS_ERROR_DOM_INVALID_ACCESS_XHR_TIMEOUT_AND_RESPONSETYPE_UNSUPPORTED_FOR_SYNC;
   }
 
   // Step 10
-  CloseRequest();
+  TerminateOngoingFetch();
 
   // Step 11
   // timeouts are handled without a flag
   mFlagSend = false;
   mRequestMethod.Assign(method);
   mRequestURL = parsedURL;
   mFlagSynchronous = !aAsync;
   mAuthorRequestHeaders.Clear();
@@ -1876,17 +1886,17 @@ XMLHttpRequestMainThread::OnDataAvailabl
   }
 
   uint32_t totalRead;
   rv = inStr->ReadSegments(XMLHttpRequestMainThread::StreamReaderFunc,
                            (void*)this, count, &totalRead);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Fire the first progress event/loading state change
-  if (mState != State::loading) {
+  if (mState == State::headers_received) {
     ChangeState(State::loading);
     if (!mFlagSynchronous) {
       DispatchProgressEvent(this, ProgressEventType::progress,
                             mLoadTransferred, mLoadTotal);
     }
     mProgressSinceLastProgressEvent = false;
   }
 
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -184,16 +184,17 @@ public:
   };
 
   enum class ErrorType : uint16_t {
     eOK,
     eRequest,
     eUnreachable,
     eChannelOpen,
     eRedirect,
+    eTerminated,
     ENUM_MAX
   };
 
   XMLHttpRequestMainThread();
 
   void Construct(nsIPrincipal* aPrincipal,
                  nsIGlobalObject* aGlobalObject,
                  nsIURI* aBaseURI = nullptr,
@@ -759,16 +760,18 @@ protected:
   // that this request is associated with.
   nsCString mNetworkInterfaceId;
 
   /**
    * Close the XMLHttpRequest's channels.
    */
   void CloseRequest();
 
+  void TerminateOngoingFetch();
+
   /**
    * Close the XMLHttpRequest's channels and dispatch appropriate progress
    * events.
    *
    * @param aType The progress event type.
    */
   void CloseRequestWithError(const ProgressEventType aType);
 
--- a/dom/xhr/XMLHttpRequestWorker.cpp
+++ b/dom/xhr/XMLHttpRequestWorker.cpp
@@ -2164,16 +2164,27 @@ XMLHttpRequestWorker::Abort(ErrorResult&
     aRv.ThrowUncatchableException();
     return;
   }
 
   if (!mProxy) {
     return;
   }
 
+  // Set our status to 0 and statusText to "" if we
+  // will be aborting an ongoing fetch, so the upcoming
+  // abort events we dispatch have the correct info.
+  if ((mStateData.mReadyState == nsIXMLHttpRequest::OPENED && mStateData.mFlagSend) ||
+      mStateData.mReadyState == nsIXMLHttpRequest::HEADERS_RECEIVED ||
+      mStateData.mReadyState == nsIXMLHttpRequest::LOADING ||
+      mStateData.mReadyState == nsIXMLHttpRequest::DONE) {
+    mStateData.mStatus = 0;
+    mStateData.mStatusText.Truncate();
+  }
+
   MaybeDispatchPrematureAbortEvents(aRv);
   if (aRv.Failed()) {
     return;
   }
 
   if (mStateData.mReadyState == 4) {
     // No one did anything to us while we fired abort events, so reset our state
     // to "unsent"