Bug 1341609 - Don't throw on async XHR send() failures when open() created no channel; r=baku draft
authorThomas Wisniewski <wisniewskit@gmail.com>
Fri, 29 Sep 2017 11:36:55 -0400
changeset 672687 6f85751dae1bf7413598a3ddc381bee86545de9b
parent 672686 935eca685536793ab7df8bfc9fb52d70128f7756
child 733885 d333fc36869abce3bc22d1d4ac4a8c05c3fe3647
push id82336
push userwisniewskit@gmail.com
push dateFri, 29 Sep 2017 16:24:39 +0000
reviewersbaku
bugs1341609
milestone58.0a1
Bug 1341609 - Don't throw on async XHR send() failures when open() created no channel; r=baku MozReview-Commit-ID: IKVSbdRXoP8
dom/file/tests/test_blobURL_expiring.html
dom/file/tests/test_blob_fragment_and_query.html
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
testing/web-platform/meta/FileAPI/blob/Blob-XHR-revoke.html.ini
--- a/dom/file/tests/test_blobURL_expiring.html
+++ b/dom/file/tests/test_blobURL_expiring.html
@@ -20,24 +20,25 @@ onmessage = function(e) {
       is(xhr.response, "123", "Response matches!");
       resolve();
     }
   })).then(function() {
     document.body.removeChild(iframe);
   }).then(function() {
     var xhr = new XMLHttpRequest();
     xhr.open("GET", blobURL);
-    try {
-      xhr.send();
+    xhr.onerror = function() {
+      ok(true, "The URL should be done!");
+      SimpleTest.finish();
+    }
+    xhr.onload = function() {
       ok(false, "The URL should be done!");
-    } catch(e) {
-      ok(true, "The URL should be done!");
+      SimpleTest.finish();
     }
-
-    SimpleTest.finish();
+    xhr.send();
   });
 }
 
 var iframe = document.createElement('iframe');
 iframe.src = 'file_blobURL_expiring.html';
 document.body.appendChild(iframe);
 
 SimpleTest.waitForExplicitFinish();
--- a/dom/file/tests/test_blob_fragment_and_query.html
+++ b/dom/file/tests/test_blob_fragment_and_query.html
@@ -26,34 +26,35 @@ function runTest() {
     return;
   }
 
   var url = URL.createObjectURL(blob);
   ok(url, "We have a URI");
 
   var test = tests.shift();
 
-  URL.revokeObjectURL(url + test.part);
+  if (test.revoke) {
+    URL.revokeObjectURL(url + test.part);
+  }
 
   var xhr = new XMLHttpRequest();
   xhr.open('GET', url + test.part);
 
   xhr.onload = function() {
+    ok(!test.revoke, "Not-revoked URL should send()");
     is(xhr.responseText, 'hello world', 'URL: ' + url + test.part);
     runTest();
   }
 
-  try {
-    xhr.send();
-  } catch(e) {
-    ok(test.revoke, "This should fail!");
+  xhr.onerror = function() {
+    ok(test.revoke, "Revoked URL should fail on send()");
     runTest();
-    return;
   }
-  ok(!test.revoke, "This should succeed!");
+
+  xhr.send();
 }
 
 SimpleTest.waitForExplicitFinish();
 runTest();
 
   </script>
 </body>
 </html>
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -2928,16 +2928,36 @@ XMLHttpRequestMainThread::Send(JSContext
   if (aData.Value().IsUSVString()) {
     BodyExtractor<const nsAString> body(&aData.Value().GetAsUSVString());
     aRv = SendInternal(&body);
     return;
   }
 }
 
 nsresult
+XMLHttpRequestMainThread::MaybeSilentSendFailure(nsresult aRv)
+{
+  // Per spec, silently fail on async request failures; throw for sync.
+  if (mFlagSynchronous) {
+    mState = State::done;
+    return NS_ERROR_DOM_NETWORK_ERR;
+  }
+
+  // Defer the actual sending of async events just in case listeners
+  // are attached after the send() method is called.
+  Unused << NS_WARN_IF(NS_FAILED(
+    DispatchToMainThread(NewRunnableMethod<ProgressEventType>(
+      "dom::XMLHttpRequestMainThread::CloseRequestWithError",
+      this,
+      &XMLHttpRequestMainThread::CloseRequestWithError,
+      ProgressEventType::error))));
+  return NS_OK;
+}
+
+nsresult
 XMLHttpRequestMainThread::SendInternal(const BodyExtractorBase* aBody)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   NS_ENSURE_TRUE(mPrincipal, NS_ERROR_NOT_INITIALIZED);
 
   // Step 1
   if (mState != State::opened) {
@@ -2953,17 +2973,18 @@ XMLHttpRequestMainThread::SendInternal(c
   if (NS_FAILED(rv)) {
     return NS_ERROR_DOM_INVALID_STATE_XHR_HAS_INVALID_CONTEXT;
   }
 
   // If open() failed to create the channel, then throw a network error
   // as per spec. We really should create the channel here in send(), but
   // we have internal code relying on the channel being created in open().
   if (!mChannel) {
-    return NS_ERROR_DOM_NETWORK_ERR;
+    mFlagSend = true; // so CloseRequestWithError sets us to DONE.
+    return MaybeSilentSendFailure(NS_ERROR_DOM_NETWORK_ERR);
   }
 
   PopulateNetworkInterfaceId();
 
   // 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.
 
@@ -3106,29 +3127,17 @@ XMLHttpRequestMainThread::SendInternal(c
     DispatchProgressEvent(this, ProgressEventType::loadstart, 0, -1);
     if (mUpload && !mUploadComplete) {
       DispatchProgressEvent(mUpload, ProgressEventType::loadstart,
                             0, mUploadTotal);
     }
   }
 
   if (!mChannel) {
-    // Per spec, silently fail on async request failures; throw for sync.
-    if (mFlagSynchronous) {
-      mState = State::done;
-      return NS_ERROR_DOM_NETWORK_ERR;
-    } else {
-      // Defer the actual sending of async events just in case listeners
-      // are attached after the send() method is called.
-      return DispatchToMainThread(NewRunnableMethod<ProgressEventType>(
-        "dom::XMLHttpRequestMainThread::CloseRequestWithError",
-        this,
-        &XMLHttpRequestMainThread::CloseRequestWithError,
-        ProgressEventType::error));
-    }
+    return MaybeSilentSendFailure(NS_ERROR_DOM_NETWORK_ERR);
   }
 
   return rv;
 }
 
 /* static */
 bool
 XMLHttpRequestMainThread::IsMappedArrayBufferEnabled()
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -303,16 +303,17 @@ public:
   SetWithCredentials(bool aWithCredentials, ErrorResult& aRv) override;
 
   virtual XMLHttpRequestUpload*
   GetUpload(ErrorResult& aRv) override;
 
 private:
   virtual ~XMLHttpRequestMainThread();
 
+  nsresult MaybeSilentSendFailure(nsresult aRv);
   nsresult SendInternal(const BodyExtractorBase* aBody);
 
   bool IsCrossSiteCORSRequest() const;
   bool IsDeniedCrossSiteCORSRequest();
 
   // Tell our channel what network interface ID we were told to use.
   // If it's an HTTP channel and we were told to use a non-default
   // interface ID.
deleted file mode 100644
--- a/testing/web-platform/meta/FileAPI/blob/Blob-XHR-revoke.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[Blob-XHR-revoke.html]
-  type: testharness
-  [Revoke blob URL before open(), network error (after send())]
-    expected: FAIL
-