Bug 1285036 - Part 4: Remove the 'sent' pseudostate and add support for the spec's send() flag instead. r=baku
☠☠ backed out by 8f5af40eb365 ☠ ☠
authorThomas Wisniewski <wisniewskit@gmail.com>
Thu, 07 Jul 2016 11:51:51 -0400
changeset 304157 0390a7afb12deb697239490c824fba4dc82563b4
parent 304156 b93efbaf1d7a48c99308509aaa05cdb31d8a8291
child 304158 4ae182434a6a3fc5903923a0433785288e5b9649
push id30414
push usercbook@mozilla.com
push dateFri, 08 Jul 2016 09:59:01 +0000
treeherdermozilla-central@45682df2d2d4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1285036
milestone50.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 1285036 - Part 4: Remove the 'sent' pseudostate and add support for the spec's send() flag instead. r=baku
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -159,17 +159,17 @@ XMLHttpRequestMainThread::sDontWarnAbout
 XMLHttpRequestMainThread::XMLHttpRequestMainThread()
   : mResponseBodyDecodedPos(0),
     mResponseType(XMLHttpRequestResponseType::_empty),
     mRequestObserver(nullptr),
     mState(State::unsent),
     mFlagSynchronous(false), mFlagAborted(false), mFlagParseBody(false),
     mFlagSyncLooping(false), mFlagBackgroundRequest(false),
     mFlagHadUploadListenersOnSend(false), mFlagACwithCredentials(false),
-    mFlagTimedOut(false), mFlagDeleted(false),
+    mFlagTimedOut(false), mFlagDeleted(false), mFlagSend(false),
     mUploadTransferred(0), mUploadTotal(0), mUploadComplete(true),
     mProgressSinceLastProgressEvent(false),
     mRequestSentTime(0), mTimeoutMilliseconds(0),
     mErrorLoad(false), mWaitingForOnStopRequest(false),
     mProgressTimerIsActive(false),
     mIsHtml(false),
     mWarnAboutSyncHtml(false),
     mLoadLengthComputable(false), mLoadTotal(0),
@@ -183,17 +183,18 @@ XMLHttpRequestMainThread::XMLHttpRequest
     mXPCOMifier(nullptr)
 {
 }
 
 XMLHttpRequestMainThread::~XMLHttpRequestMainThread()
 {
   mFlagDeleted = true;
 
-  if (mState == State::sent || mState == State::loading) {
+  if ((mState == State::opened && mFlagSend) ||
+      mState == State::loading) {
     Abort();
   }
 
   MOZ_ASSERT(!mFlagSyncLooping, "we rather crash than hang");
   mFlagSyncLooping = false;
 
   mResultJSON.setUndefined();
   mResultArrayBuffer = nullptr;
@@ -1015,17 +1016,17 @@ XMLHttpRequestMainThread::CloseRequestWi
 
   // If we're in the destructor, don't risk dispatching an event.
   if (mFlagDeleted) {
     mFlagSyncLooping = false;
     return;
   }
 
   if (mState != State::unsent &&
-      mState != State::opened &&
+      !(mState == State::opened && !mFlagSend) &&
       mState != State::done) {
     ChangeState(State::done, true);
 
     if (!mFlagSyncLooping) {
       DispatchProgressEvent(this, aType, mLoadLengthComputable, responseLength,
                             mLoadTotal);
       if (mUpload && !mUploadComplete) {
         mUploadComplete = true;
@@ -1127,19 +1128,17 @@ XMLHttpRequestMainThread::GetAllResponse
 void
 XMLHttpRequestMainThread::GetAllResponseHeaders(nsACString& aResponseHeaders,
                                                 ErrorResult& aRv)
 {
   aResponseHeaders.Truncate();
 
   // If the state is UNSENT or OPENED,
   // return the empty string and terminate these steps.
-  if (mState == State::unsent ||
-      mState == State::opened ||
-      mState == State::sent) {
+  if (mState == State::unsent || mState == State::opened) {
     return;
   }
 
   if (nsCOMPtr<nsIHttpChannel> httpChannel = GetCurrentHttpChannel()) {
     RefPtr<nsHeaderVisitor> visitor =
       new nsHeaderVisitor(*this, WrapNotNull(httpChannel));
     if (NS_SUCCEEDED(httpChannel->VisitResponseHeaders(visitor))) {
       aResponseHeaders = visitor->Headers();
@@ -1193,19 +1192,17 @@ XMLHttpRequestMainThread::GetResponseHea
 {
   _retval.SetIsVoid(true);
 
   nsCOMPtr<nsIHttpChannel> httpChannel = GetCurrentHttpChannel();
 
   if (!httpChannel) {
     // If the state is UNSENT or OPENED,
     // return null and terminate these steps.
-    if (mState == State::unsent ||
-        mState == State::opened ||
-        mState == State::sent) {
+    if (mState == State::unsent || mState == State::opened) {
       return;
     }
 
     // Even non-http channels supply content type and content length.
     // Remember we don't leak header information from denied cross-site
     // requests.
     nsresult status;
     if (!mChannel ||
@@ -1402,27 +1399,29 @@ XMLHttpRequestMainThread::Open(const nsA
       LogMessage("ResponseTypeSyncXHRWarning", GetOwner());
     }
     return NS_ERROR_DOM_INVALID_ACCESS_ERR;
   }
 
   nsCOMPtr<nsIURI> uri;
 
   if (mState == State::opened || mState == State::headers_received ||
-      mState == State::loading || mState == State::sent) {
+      mState == State::loading) {
     // IE aborts as well
     Abort();
 
     // XXX We should probably send a warning to the JS console
     //     that load was aborted and event listeners were cleared
     //     since this looks like a situation that could happen
     //     by accident and you could spend a lot of time wondering
     //     why things didn't work.
   }
 
+  mFlagSend = false;
+
   // Unset any pre-existing aborted and timed-out flags.
   mFlagAborted = false;
   mFlagTimedOut = false;
 
   mFlagSynchronous = !async;
 
   nsCOMPtr<nsIDocument> doc = GetDocumentIfCurrent();
   if (!doc) {
@@ -2108,16 +2107,18 @@ XMLHttpRequestMainThread::ChangeStateToD
   if (mIsHtml) {
     // In the HTML case, this has to be deferred, because the parser doesn't
     // do it's job synchronously.
     MaybeDispatchProgressEvents(true);
   }
 
   ChangeState(State::done, true);
 
+  mFlagSend = false;
+
   if (mTimeoutTimer) {
     mTimeoutTimer->Cancel();
   }
 
   DispatchProgressEvent(this,
                         mErrorLoad ? ProgressEventType::error : ProgressEventType::load,
                         !mErrorLoad,
                         mLoadTransferred,
@@ -2422,23 +2423,19 @@ XMLHttpRequestMainThread::Send(nsIVarian
 {
   NS_ENSURE_TRUE(mPrincipal, NS_ERROR_NOT_INITIALIZED);
 
   PopulateNetworkInterfaceId();
 
   nsresult rv = CheckInnerWindowCorrectness();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Return error if we're already processing a request
-  if (mState == State::sent) {
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
-  }
-
-  // Make sure we've been opened
-  if (!mChannel || mState != State::opened) {
+  if (mState != State::opened || // Step 1
+      mFlagSend || // Step 2
+      !mChannel) { // Gecko-specific
     return NS_ERROR_DOM_INVALID_STATE_ERR;
   }
 
   // nsIRequest::LOAD_BACKGROUND prevents throbber from becoming active, which
   // in turn keeps STOP button from becoming active.  If the consumer passed in
   // a progress event handler we must load with nsIRequest::LOAD_NORMAL or
   // necko won't generate any progress notifications.
   if (HasListenersFor(nsGkAtoms::onprogress) ||
@@ -2747,16 +2744,19 @@ XMLHttpRequestMainThread::Send(nsIVarian
     mChannel->SetNotificationCallbacks(mNotificationCallbacks);
     mChannel = nullptr;
 
     return rv;
   }
 
   mWaitingForOnStopRequest = true;
 
+  // Step 8
+  mFlagSend = true;
+
   // If we're synchronous, spin an event loop here and wait
   if (mFlagSynchronous) {
     mFlagSyncLooping = true;
 
     nsCOMPtr<nsIDocument> suspendedDoc;
     nsCOMPtr<nsIRunnable> resumeTimeoutRunnable;
     if (GetOwner()) {
       if (nsCOMPtr<nsPIDOMWindowOuter> topWindow = GetOwner()->GetOuterWindow()->GetTop()) {
@@ -2766,21 +2766,23 @@ XMLHttpRequestMainThread::Send(nsIVarian
             suspendedDoc->SuppressEventHandling(nsIDocument::eEvents);
           }
           topWindow->SuspendTimeouts(1, false);
           resumeTimeoutRunnable = new nsResumeTimeoutsEvent(topInner);
         }
       }
     }
 
-    ChangeState(State::sent);
+    if (mProgressNotifier) {
+      mProgressTimerIsActive = false;
+      mProgressNotifier->Cancel();
+    }
 
     {
       nsAutoSyncOperation sync(suspendedDoc);
-      // Note, calling ChangeState may have cleared mFlagSyncLooping
       nsIThread *thread = NS_GetCurrentThread();
       while (mFlagSyncLooping) {
         if (!NS_ProcessNextEvent(thread)) {
           rv = NS_ERROR_UNEXPECTED;
           break;
         }
       }
     }
@@ -2793,17 +2795,21 @@ XMLHttpRequestMainThread::Send(nsIVarian
     if (resumeTimeoutRunnable) {
       NS_DispatchToCurrentThread(resumeTimeoutRunnable);
     }
   } else {
     // Now that we've successfully opened the channel, we can change state.  Note
     // that this needs to come after the AsyncOpen() and rv check, because this
     // can run script that would try to restart this request, and that could end
     // up doing our AsyncOpen on a null channel if the reentered AsyncOpen fails.
-    ChangeState(State::sent);
+    if (mProgressNotifier) {
+      mProgressTimerIsActive = false;
+      mProgressNotifier->Cancel();
+    }
+
     if (mUpload && mUpload->HasListenersFor(nsGkAtoms::onprogress)) {
       StartProgressEventTimer();
     }
     DispatchProgressEvent(this, ProgressEventType::loadstart, false,
                           0, 0);
     if (mUpload && !mUploadComplete) {
       DispatchProgressEvent(mUpload, ProgressEventType::loadstart, true,
                             0, mUploadTotal);
@@ -2817,18 +2823,18 @@ XMLHttpRequestMainThread::Send(nsIVarian
   return rv;
 }
 
 // http://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html#dom-xmlhttprequest-setrequestheader
 NS_IMETHODIMP
 XMLHttpRequestMainThread::SetRequestHeader(const nsACString& header,
                                            const nsACString& value)
 {
-  // Step 1 and 2
-  if (mState != State::opened) {
+  // Steps 1 and 2
+  if (mState != State::opened || mFlagSend) {
     return NS_ERROR_DOM_INVALID_STATE_ERR;
   }
   NS_ASSERTION(mChannel, "mChannel must be valid if we're OPENED.");
 
   // Step 3
   // Make sure we don't store an invalid header name in mCORSUnsafeHeaders
   if (!NS_IsValidHTTPToken(header)) {
     return NS_ERROR_DOM_SYNTAX_ERR;
@@ -2982,30 +2988,31 @@ XMLHttpRequestMainThread::GetReadyState(
   *aState = ReadyState();
   return NS_OK;
 }
 
 uint16_t
 XMLHttpRequestMainThread::ReadyState() const
 {
   // Translate some of our internal states for external consumers
-  if (mState == State::unsent) {
-    return UNSENT;
-  }
-  if (mState == State::opened || mState == State::sent) {
-    return OPENED;
+  switch(mState) {
+    case State::unsent:
+      return UNSENT;
+    case State::opened:
+      return OPENED;
+    case State::headers_received:
+      return HEADERS_RECEIVED;
+    case State::loading:
+      return LOADING;
+    case State::done:
+      return DONE;
+    default:
+      MOZ_CRASH("Unknown state");
   }
-  if (mState == State::headers_received) {
-    return HEADERS_RECEIVED;
-  }
-  if (mState == State::loading) {
-    return LOADING;
-  }
-  MOZ_ASSERT(mState == State::done);
-  return DONE;
+  return 0;
 }
 
 void XMLHttpRequestMainThread::OverrideMimeType(const nsAString& aMimeType, ErrorResult& aRv)
 {
   if (mState == State::loading || mState == State::done) {
     ResetResponse();
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
@@ -3082,43 +3089,41 @@ XMLHttpRequestMainThread::SetWithCredent
 }
 
 void
 XMLHttpRequestMainThread::SetWithCredentials(bool aWithCredentials, ErrorResult& aRv)
 {
   // Return error if we're already processing a request.  Note that we can't use
   // ReadyState() here, because it can't differentiate between "opened" and
   // "sent", so we use mState directly.
-  if ((mState != State::unsent && mState != State::opened) || mIsAnon) {
+  if ((mState != State::unsent && mState != State::opened) ||
+      mFlagSend || mIsAnon) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
   mFlagACwithCredentials = aWithCredentials;
 }
 
 nsresult
 XMLHttpRequestMainThread::ChangeState(State aState, bool aBroadcast)
 {
-  bool droppingFromSENTtoOPENED = mState == State::sent &&
-                                  aState == State::opened;
-
   mState = aState;
   nsresult rv = NS_OK;
 
   if (mProgressNotifier &&
       aState != State::headers_received && aState != State::loading) {
     mProgressTimerIsActive = false;
     mProgressNotifier->Cancel();
   }
 
-  if (aState != State::sent && // SENT is internal
-      !droppingFromSENTtoOPENED && // since SENT is essentially OPENED
-      aBroadcast &&
-      (!mFlagSynchronous || aState == State::opened || aState == State::done)) {
+
+  if (aBroadcast && (!mFlagSynchronous ||
+                     aState == State::opened ||
+                     aState == State::done)) {
     nsCOMPtr<nsIDOMEvent> event;
     rv = CreateReadystatechangeEvent(getter_AddRefs(event));
     NS_ENSURE_SUCCESS(rv, rv);
 
     DispatchDOMEvent(nullptr, event, nullptr, nullptr);
   }
 
   return rv;
@@ -3208,18 +3213,18 @@ XMLHttpRequestMainThread::MaybeDispatchP
       mFlagSynchronous) {
     return;
   }
 
   if (!aFinalProgress) {
     StartProgressEventTimer();
   }
 
-  // We're uploading if our state is State::opened or State::sent
-  if (mState == State::opened || mState == State::sent) {
+  // We're in the upload phase while our state is State::opened.
+  if (mState == State::opened) {
     if (mUpload && !mUploadComplete) {
       DispatchProgressEvent(mUpload, ProgressEventType::progress,
                             mUploadLengthComputable, mUploadTransferred,
                             mUploadTotal);
     }
   } else {
     if (aFinalProgress) {
       mLoadTotal = mLoadTransferred;
@@ -3239,18 +3244,18 @@ XMLHttpRequestMainThread::MaybeDispatchP
   }
 
   mProgressSinceLastProgressEvent = false;
 }
 
 NS_IMETHODIMP
 XMLHttpRequestMainThread::OnProgress(nsIRequest *aRequest, nsISupports *aContext, int64_t aProgress, int64_t aProgressMax)
 {
-  // We're uploading if our state is State::opened or State::sent
-  bool upload = mState == State::opened || mState == State::sent;
+  // We're in the upload phase while our state is State::opened.
+  bool upload = mState == State::opened;
   // When uploading, OnProgress reports also headers in aProgress and aProgressMax.
   // So, try to remove the headers, if possible.
   bool lengthComputable = (aProgressMax != -1);
   if (upload) {
     int64_t loaded = aProgress;
     if (lengthComputable) {
       int64_t headerSize = aProgressMax - mUploadTotal;
       loaded -= headerSize;
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -556,21 +556,19 @@ public:
   }
   static bool DontWarnAboutSyncXHR()
   {
     return sDontWarnAboutSyncXHR;
   }
 protected:
   // XHR states are meant to mirror the XHR2 spec:
   //   https://xhr.spec.whatwg.org/#states
-  // Note that we currently have an extra pseudo-state called sent.
   enum class State : uint8_t {
     unsent,           // object has been constructed.
     opened,           // open() has been successfully invoked.
-    sent,             // non-spec, corresponds with "opened and the send() flag is set".
     headers_received, // redirects followed and response headers received.
     loading,          // response body is being received.
     done,             // data transfer concluded, whether success or error.
   };
 
   nsresult DetectCharset();
   nsresult AppendToResponseText(const char * aBuffer, uint32_t aBufferLen);
   static NS_METHOD StreamReaderFunc(nsIInputStream* in,
@@ -695,16 +693,22 @@ protected:
   bool mFlagParseBody;
   bool mFlagSyncLooping;
   bool mFlagBackgroundRequest;
   bool mFlagHadUploadListenersOnSend;
   bool mFlagACwithCredentials;
   bool mFlagTimedOut;
   bool mFlagDeleted;
 
+  // The XHR2 spec's send() flag. Set when the XHR begins uploading, until it
+  // finishes downloading (or an error/abort has occurred during either phase).
+  // Used to guard against the user trying to alter headers/etc when it's too
+  // late, and ensure the XHR only handles one in-flight request at once.
+  bool mFlagSend;
+
   RefPtr<XMLHttpRequestUpload> mUpload;
   int64_t mUploadTransferred;
   int64_t mUploadTotal;
   bool mUploadLengthComputable;
   bool mUploadComplete;
   bool mProgressSinceLastProgressEvent;
 
   // Timeout support