Bug 1367110 - Make XHRMainThread's mErrorLoad more descriptive. r=baku
☠☠ backed out by 1941e3304049 ☠ ☠
authorChris H-C <chutten@mozilla.com>
Wed, 24 May 2017 08:44:38 -0400
changeset 409698 9983ac05d7d1533fc13605ffc17e42d1b7fcbcf7
parent 409697 9b8b0f1e0c8043be3a2276fe98c5e5591d537c96
child 409699 1b93ec532890d3e4d88c730634339a9cfdd4000b
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1367110
milestone55.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 1367110 - Make XHRMainThread's mErrorLoad more descriptive. r=baku There are at least four ways XHRMT can error on load. Let's be specific about it. MozReview-Commit-ID: EOml2fcd1XD
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -184,17 +184,17 @@ XMLHttpRequestMainThread::XMLHttpRequest
     mState(State::unsent),
     mFlagSynchronous(false), mFlagAborted(false), mFlagParseBody(false),
     mFlagSyncLooping(false), mFlagBackgroundRequest(false),
     mFlagHadUploadListenersOnSend(false), mFlagACwithCredentials(false),
     mFlagTimedOut(false), mFlagDeleted(false), mFlagSend(false),
     mUploadTransferred(0), mUploadTotal(0), mUploadComplete(true),
     mProgressSinceLastProgressEvent(false),
     mRequestSentTime(0), mTimeoutMilliseconds(0),
-    mErrorLoad(false), mErrorParsingXML(false),
+    mErrorLoad(ErrorType::eOK), mErrorParsingXML(false),
     mWaitingForOnStopRequest(false),
     mProgressTimerIsActive(false),
     mIsHtml(false),
     mWarnAboutSyncHtml(false),
     mLoadTotal(-1),
     mIsSystem(false),
     mIsAnon(false),
     mFirstStartRequestSeen(false),
@@ -937,17 +937,17 @@ XMLHttpRequestMainThread::GetStatus(Erro
     return 0;
   }
 
   uint16_t readyState = ReadyState();
   if (readyState == UNSENT || readyState == OPENED) {
     return 0;
   }
 
-  if (mErrorLoad) {
+  if (mErrorLoad != ErrorType::eOK) {
     // Let's simulate the http protocol for jar/app requests:
     nsCOMPtr<nsIJARChannel> jarChannel = GetCurrentJARChannel();
     if (jarChannel) {
       nsresult status;
       mChannel->GetStatus(&status);
 
       if (status == NS_ERROR_FILE_NOT_FOUND) {
         return 404; // Not Found
@@ -999,17 +999,17 @@ XMLHttpRequestMainThread::GetStatusText(
   // value.  This check is to prevent the status text for redirects from being
   // available before all the redirects have been followed and HTTP headers have
   // been received.
   uint16_t readyState = ReadyState();
   if (readyState == UNSENT || readyState == OPENED) {
     return;
   }
 
-  if (mErrorLoad) {
+  if (mErrorLoad != ErrorType::eOK) {
     return;
   }
 
   nsCOMPtr<nsIHttpChannel> httpChannel = GetCurrentHttpChannel();
   if (httpChannel) {
     Unused << httpChannel->GetResponseStatusText(aStatusText);
   } else {
     aStatusText.AssignLiteral("OK");
@@ -1216,17 +1216,17 @@ XMLHttpRequestMainThread::GetAllResponse
   aResponseHeaders.Truncate();
 
   // If the state is UNSENT or OPENED,
   // return the empty string and terminate these steps.
   if (mState == State::unsent || mState == State::opened) {
     return;
   }
 
-  if (mErrorLoad) {
+  if (mErrorLoad != ErrorType::eOK) {
     return;
   }
 
   if (nsCOMPtr<nsIHttpChannel> httpChannel = GetCurrentHttpChannel()) {
     RefPtr<nsHeaderVisitor> visitor =
       new nsHeaderVisitor(*this, WrapNotNull(httpChannel));
     if (NS_SUCCEEDED(httpChannel->VisitResponseHeaders(visitor))) {
       aResponseHeaders = visitor->Headers();
@@ -1938,21 +1938,23 @@ XMLHttpRequestMainThread::OnStartRequest
     return NS_OK;
   }
 
   nsCOMPtr<nsIChannel> channel(do_QueryInterface(request));
   NS_ENSURE_TRUE(channel, NS_ERROR_UNEXPECTED);
 
   nsresult status;
   request->GetStatus(&status);
-  mErrorLoad = mErrorLoad || NS_FAILED(status);
+  if (mErrorLoad == ErrorType::eOK && NS_FAILED(status)) {
+    mErrorLoad = ErrorType::eRequest;
+  }
 
   // Upload phase is now over. If we were uploading anything,
   // stop the timer and fire any final progress events.
-  if (mUpload && !mUploadComplete && !mErrorLoad && !mFlagSynchronous) {
+  if (mUpload && !mUploadComplete && mErrorLoad == ErrorType::eOK && !mFlagSynchronous) {
     StopProgressEventTimer();
 
     mUploadTransferred = mUploadTotal;
 
     if (mProgressSinceLastProgressEvent) {
       DispatchProgressEvent(mUpload, ProgressEventType::progress,
                             mUploadTransferred, mUploadTotal);
       mProgressSinceLastProgressEvent = false;
@@ -2320,17 +2322,17 @@ XMLHttpRequestMainThread::OnStopRequest(
   // update our charset and decoder to match mResponseXML,
   // before it is possibly nulled out
   MatchCharsetAndDecoderToResponseDocument();
 
   if (NS_FAILED(status)) {
     // This can happen if the server is unreachable. Other possible
     // reasons are that the user leaves the page or hits the ESC key.
 
-    mErrorLoad = true;
+    mErrorLoad = ErrorType::eUnreachable;
     mResponseXML = nullptr;
   }
 
   // If we're uninitialized at this point, we encountered an error
   // earlier and listeners have already been notified. Also we do
   // not want to do this if we already completed.
   if (mState == State::unsent || mState == State::done) {
     return NS_OK;
@@ -2425,23 +2427,24 @@ XMLHttpRequestMainThread::ChangeStateToD
   // Per spec, if we failed in the upload phase, fire a final error
   // and loadend events for the upload after readystatechange=4/done.
   if (!mFlagSynchronous && mUpload && !mUploadComplete) {
     DispatchProgressEvent(mUpload, ProgressEventType::error, 0, -1);
   }
 
   // Per spec, fire download's load/error and loadend events after
   // readystatechange=4/done (and of course all upload events).
-  DispatchProgressEvent(this,
-                        mErrorLoad ? ProgressEventType::error :
-                                     ProgressEventType::load,
-                        mErrorLoad ? 0 : mLoadTransferred,
-                        mErrorLoad ? -1 : mLoadTotal);
-
-  if (mErrorLoad) {
+  if (mErrorLoad != ErrorType::eOK) {
+    DispatchProgressEvent(this, ProgressEventType::error, 0, -1);
+  } else {
+    DispatchProgressEvent(this, ProgressEventType::load,
+                          mLoadTransferred, mLoadTotal);
+  }
+
+  if (mErrorLoad != ErrorType::eOK) {
     // By nulling out channel here we make it so that Send() can test
     // for that and throw. Also calling the various status
     // methods/members will not throw.
     // This matches what IE does.
     mChannel = nullptr;
   }
 }
 
@@ -2780,17 +2783,17 @@ XMLHttpRequestMainThread::InitiateFetch(
   rv = mChannel->AsyncOpen2(listener);
   listener = nullptr;
   if (NS_WARN_IF(NS_FAILED(rv))) {
     // Drop our ref to the channel to avoid cycles. Also drop channel's
     // ref to us to be extra safe.
     mChannel->SetNotificationCallbacks(mNotificationCallbacks);
     mChannel = nullptr;
 
-    mErrorLoad = true;
+    mErrorLoad = ErrorType::eChannelOpen;
 
     // Per spec, we throw on sync errors, but not async.
     if (mFlagSynchronous) {
       mState = State::done;
       return NS_ERROR_DOM_NETWORK_ERR;
     }
   }
 
@@ -2928,17 +2931,17 @@ XMLHttpRequestMainThread::SendInternal(c
   // XXX We should probably send a warning to the JS console
   //     if there are no event listeners set and we are doing
   //     an asynchronous call.
 
   mUploadTransferred = 0;
   mUploadTotal = 0;
   // By default we don't have any upload, so mark upload complete.
   mUploadComplete = true;
-  mErrorLoad = false;
+  mErrorLoad = ErrorType::eOK;
   mLoadTotal = -1;
   nsCOMPtr<nsIInputStream> uploadStream;
   nsAutoCString uploadContentType;
   nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(mChannel));
   if (aBody && httpChannel &&
       !mRequestMethod.EqualsLiteral("GET") &&
       !mRequestMethod.EqualsLiteral("HEAD")) {
 
@@ -3440,17 +3443,17 @@ XMLHttpRequestMainThread::OnRedirectVeri
     mChannel = mNewRedirectChannel;
 
     nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(mChannel));
     if (httpChannel) {
       // Ensure all original headers are duplicated for the new channel (bug #553888)
       mAuthorRequestHeaders.ApplyToChannel(httpChannel);
     }
   } else {
-    mErrorLoad = true;
+    mErrorLoad = ErrorType::eRedirect;
   }
 
   mNewRedirectChannel = nullptr;
 
   mRedirectCallback->OnRedirectVerifyCallback(result);
   mRedirectCallback = nullptr;
 
   // It's important that we return success here. If we return the result code
@@ -3685,17 +3688,17 @@ XMLHttpRequestMainThread::HandleProgress
 {
   // Don't fire the progress event if mLoadTotal is 0, see XHR spec step 6.1
   if (!mLoadTotal && mLoadTransferred) {
     return;
   }
 
   mProgressTimerIsActive = false;
 
-  if (!mProgressSinceLastProgressEvent || mErrorLoad) {
+  if (!mProgressSinceLastProgressEvent || mErrorLoad != ErrorType::eOK) {
     return;
   }
 
   if (InUploadPhase()) {
     if (mUpload && !mUploadComplete) {
       DispatchProgressEvent(mUpload, ProgressEventType::progress,
                             mUploadTransferred, mUploadTotal);
     }
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -179,16 +179,25 @@ public:
     error,
     abort,
     timeout,
     load,
     loadend,
     ENUM_MAX
   };
 
+  enum class ErrorType : uint16_t {
+    eOK,
+    eRequest,
+    eUnreachable,
+    eChannelOpen,
+    eRedirect,
+    ENUM_MAX
+  };
+
   XMLHttpRequestMainThread();
 
   void Construct(nsIPrincipal* aPrincipal,
                  nsIGlobalObject* aGlobalObject,
                  nsIURI* aBaseURI = nullptr,
                  nsILoadGroup* aLoadGroup = nullptr)
   {
     MOZ_ASSERT(aPrincipal);
@@ -754,17 +763,17 @@ protected:
     eTimerStarted,
     eNoTimerNeeded
   };
 
   SyncTimeoutType MaybeStartSyncTimeoutTimer();
   void HandleSyncTimeoutTimer();
   void CancelSyncTimeoutTimer();
 
-  bool mErrorLoad;
+  ErrorType mErrorLoad;
   bool mErrorParsingXML;
   bool mWaitingForOnStopRequest;
   bool mProgressTimerIsActive;
   bool mIsHtml;
   bool mWarnAboutMultipartHtml;
   bool mWarnAboutSyncHtml;
   int64_t mLoadTotal; // -1 if not known.
   // Amount of script-exposed (i.e. after undoing gzip compresion) data