Bug 709991 - Fire onerror instead of throwing on network errors for async XHRs. r=bz
authorThomas Wisniewski <wisniewskit@gmail.com>
Sat, 30 Jul 2016 00:24:56 -0400
changeset 332554 59ddf661a7ee6f05e59c296981f125f49a7478ae
parent 332553 405ec13645471f1e72312be99220eb66eaf2af5d
child 332555 ce119faadbc5999895c73ecbdab003d761ce296e
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs709991
milestone50.0a1
Bug 709991 - Fire onerror instead of throwing on network errors for async XHRs. r=bz
browser/components/contextualidentity/test/browser/browser_blobUrl.js
browser/components/privatebrowsing/test/browser/browser_privatebrowsing_blobUrl.js
browser/modules/ContentSearch.jsm
browser/modules/test/browser_ContentSearch.js
caps/tests/mochitest/test_extensionURL.html
dom/base/test/file_mozfiledataurl_inner.html
dom/base/test/test_mozfiledataurl.html
dom/security/test/cors/test_CrossSiteXHR.html
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/tests/test_XHR_system.html
modules/libjar/test/mochitest/test_bug1173171.html
security/manager/tools/getHSTSPreloadList.js
testing/web-platform/meta/XMLHttpRequest/send-authentication-basic-cors.htm.ini
--- a/browser/components/contextualidentity/test/browser/browser_blobUrl.js
+++ b/browser/components/contextualidentity/test/browser/browser_blobUrl.js
@@ -36,26 +36,23 @@ add_task(function* test() {
   is(tab2.getAttribute('usercontextid'), 2, "New tab has UCI equal 2");
 
   let browser2 = gBrowser.getBrowserForTab(tab2);
   yield BrowserTestUtils.browserLoaded(browser2);
 
   yield ContentTask.spawn(browser2, blobURL, function(url) {
     return new Promise(resolve => {
       var xhr = new content.window.XMLHttpRequest();
+      xhr.onerror = function() { resolve("SendErrored"); }
+      xhr.onload = function() { resolve("SendLoaded"); }
       xhr.open("GET", url);
-      try {
-        xhr.send();
-        resolve("SendSucceeded");
-      } catch(e) {
-        resolve("SendThrew");
-      }
+      xhr.send();
     });
   }).then(status => {
-    is(status, "SendThrew", "Using a blob URI from one user context id in another should not work");
+    is(status, "SendErrored", "Using a blob URI from one user context id in another should not work");
   });
 
   info("Creating a tab with UCI = 1...");
   let tab3 = gBrowser.addTab(BASE_URI, {userContextId: 1});
   is(tab3.getAttribute('usercontextid'), 1, "New tab has UCI equal 1");
 
   let browser3 = gBrowser.getBrowserForTab(tab3);
   yield BrowserTestUtils.browserLoaded(browser3);
--- a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_blobUrl.js
+++ b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_blobUrl.js
@@ -26,23 +26,20 @@ add_task(function* test() {
   let privateWin = yield BrowserTestUtils.openNewBrowserWindow({ private: true });
   let privateTab = privateWin.gBrowser.selectedBrowser;
   privateTab.loadURI(BASE_URI);
   yield BrowserTestUtils.browserLoaded(privateTab);
 
   yield ContentTask.spawn(privateTab, blobURL, function(url) {
     return new Promise(resolve => {
       var xhr = new content.window.XMLHttpRequest();
+      xhr.onerror = function() { resolve("SendErrored"); }
+      xhr.onload = function() { resolve("SendLoaded"); }
       xhr.open("GET", url);
-      try {
-        xhr.send();
-        resolve("OpenSucceeded");
-      } catch(e) {
-        resolve("OpenThrew");
-      }
+      xhr.send();
     });
   }).then(status => {
-    is(status, "OpenThrew", "Using a blob URI from one user context id in another should not work");
+    is(status, "SendErrored", "Using a blob URI from one user context id in another should not work");
   });
 
   yield BrowserTestUtils.closeWindow(win);
   yield BrowserTestUtils.closeWindow(privateWin);
-});
\ No newline at end of file
+});
--- a/browser/modules/ContentSearch.jsm
+++ b/browser/modules/ContentSearch.jsm
@@ -524,19 +524,22 @@ this.ContentSearch = {
     if (!uri) {
       return Promise.resolve(null);
     }
     let deferred = Promise.defer();
     let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].
               createInstance(Ci.nsIXMLHttpRequest);
     xhr.open("GET", uri, true);
     xhr.responseType = "arraybuffer";
-    xhr.onloadend = () => {
+    xhr.onload = () => {
       deferred.resolve(xhr.response);
     };
+    xhr.onerror = xhr.onabort = xhr.ontimeout = () => {
+      deferred.resolve(null);
+    };
     try {
       // This throws if the URI is erroneously encoded.
       xhr.send();
     }
     catch (err) {
       return Promise.resolve(null);
     }
     return deferred.promise;
--- a/browser/modules/test/browser_ContentSearch.js
+++ b/browser/modules/test/browser_ContentSearch.js
@@ -387,17 +387,20 @@ function arrayBufferFromDataURI(uri) {
   if (!uri) {
     return Promise.resolve(null);
   }
   let deferred = Promise.defer();
   let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].
             createInstance(Ci.nsIXMLHttpRequest);
   xhr.open("GET", uri, true);
   xhr.responseType = "arraybuffer";
-  xhr.onloadend = () => {
+  xhr.onerror = () => {
+    deferred.resolve(null);
+  };
+  xhr.onload = () => {
     deferred.resolve(xhr.response);
   };
   try {
     xhr.send();
   }
   catch (err) {
     return Promise.resolve(null);
   }
--- a/caps/tests/mochitest/test_extensionURL.html
+++ b/caps/tests/mochitest/test_extensionURL.html
@@ -116,43 +116,38 @@ https://bugzilla.mozilla.org/show_bug.cg
         }
         is(threw, !!shouldThrow, "Correct throwing behavior for: " + url);
         !threw || resolve();
       });
 
       return p;
     }
 
-    function testXHR(url, shouldThrow) {
+    function testXHR(url, shouldError) {
       return new Promise(function(resolve, reject) {
         var xhr = new XMLHttpRequest();
-        xhr.addEventListener("load", () => { ok(!shouldThrow, "XHR succeeded for " + url); resolve(); });
-        xhr.addEventListener("error", () => { ok(false, "Unexpected XHR error: " + url); resolve(); });
-        try {
-          xhr.open("GET", url, true);
-          xhr.send();
-        } catch (e) {
-          ok(shouldThrow, "XHR threw for " + url);
-          resolve();
-        }
+        xhr.addEventListener("load", () => { ok(!shouldError, `XHR to ${url} should succeed`); resolve(); });
+        xhr.addEventListener("error", () => { ok(shouldError, `XHR to ${url} should fail`); resolve(); });
+        xhr.open("GET", url, true);
+        xhr.send();
       });
     }
 
     //
     // Perform some loads and make sure they work correctly.
     //
     testLoad.bind(null, 'moz-extension://cherise', navigateFromChromeWithLocation)()
     .then(testLoad.bind(null, 'moz-extension://cherise', navigateFromChromeWithWebNav))
     .then(testLoad.bind(null, 'moz-extension://cherise', navigateWithLocation, /* shouldThrow = */ true))
-    .then(testXHR.bind(null, 'moz-extension://cherise', /* shouldThrow = */ true))
+    .then(testXHR.bind(null, 'moz-extension://cherise', /* shouldError = */ true))
     .then(setWhitelistCallback.bind(null, /cherise/))
     .then(testLoad.bind(null, 'moz-extension://cherise', navigateWithLocation))
     .then(testXHR.bind(null, 'moz-extension://cherise'))
     .then(testLoad.bind(null, 'moz-extension://liebchen', navigateWithLocation, /* shouldThrow = */ true))
-    .then(testXHR.bind(null, 'moz-extension://liebchen', /* shouldThrow = */ true))
+    .then(testXHR.bind(null, 'moz-extension://liebchen', /* shouldError = */ true))
     .then(setWhitelistCallback.bind(null, /cherise|liebchen/))
     .then(testLoad.bind(null, 'moz-extension://liebchen', navigateWithLocation))
     .then(testLoad.bind(null, 'moz-extension://liebchen', navigateWithSrc))
     .then(testLoad.bind(null, 'moz-extension://cherise', navigateWithSrc))
     .then(testLoad.bind(null, 'moz-extension://cherise/_blank.html', navigateWithSrc))
     .then(SimpleTest.finish.bind(SimpleTest),
           function(e) { ok(false, "rejected promise: " + e); SimpleTest.finish() }
     );
--- a/dom/base/test/file_mozfiledataurl_inner.html
+++ b/dom/base/test/file_mozfiledataurl_inner.html
@@ -11,16 +11,19 @@ addEventListener("message", function(e) 
   if ("img" in mess)
     img.src = mess.img;
   else if ("audio" in mess)
     audio.src = mess.audio
   else if ("iframe" in mess)
     iframe.src = mess.iframe;
   else if ("xhr" in mess) {
     let xhr = new XMLHttpRequest();
+    xhr.onerror = function() {
+      sendItUp({ didError: true });
+    }
     xhr.onload = function() {
       sendItUp({ text: xhr.responseText });
     }
     try {
       xhr.open("GET", mess.xhr);
       xhr.send();
     }
     catch (ex) {
--- a/dom/base/test/test_mozfiledataurl.html
+++ b/dom/base/test/test_mozfiledataurl.html
@@ -208,17 +208,17 @@ function runTest([imgFile, audioFile, do
   is(res.didThrow, undefined, "load successful");
   is(res.text, "Yarr, here be plaintext file, ya landlubber\n", "load successful");
 
   // Attempt to load file url using XHR
   inner.src = innerCrossSiteURI;
   is((yield), "inner loaded", "correct gen.next()");
   inner.contentWindow.postMessage(JSON.stringify({xhr:fileurl}), "*");
   var res = (yield);
-  is(res.didThrow, true, "load failed successfully");
+  is(res.didError, true, "load failed successfully");
 
   SimpleTest.finish();
 
   yield undefined;
 }
 </script>
 </pre>
 </body>
--- a/dom/security/test/cors/test_CrossSiteXHR.html
+++ b/dom/security/test/cors/test_CrossSiteXHR.html
@@ -755,19 +755,25 @@ function runTest() {
         "should have failed in test for " + test.toSource());
       is(res.status, 0, "wrong status in test for " + test.toSource());
       is(res.statusText, "", "wrong status text for " + test.toSource());
       is(res.responseXML, null,
          "wrong responseXML in test for " + test.toSource());
       is(res.responseText, "",
          "wrong responseText in test for " + test.toSource());
       if (!res.sendThrew) {
-        is(res.events.join(","),
-           "opening,rs1,sending,loadstart,rs2,rs4,error,loadend",
-           "wrong events in test for " + test.toSource());
+        if (test.username) {
+          is(res.events.join(","),
+             "opening,rs1,sending,loadstart,rs4,error,loadend",
+             "wrong events in test for " + test.toSource());
+        } else {
+          is(res.events.join(","),
+             "opening,rs1,sending,loadstart,rs2,rs4,error,loadend",
+             "wrong events in test for " + test.toSource());
+        }
       }
       is(res.progressEvents, 0,
          "wrong events in test for " + test.toSource());
       if (test.responseHeaders) {
         for (header in test.responseHeaders) {
           is(res.responseHeaders[header], null,
              "wrong response header (" + header + ") in test for " +
              test.toSource());
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -2761,17 +2761,22 @@ XMLHttpRequestMainThread::Send(nsIVarian
   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;
 
-    return rv;
+    mErrorLoad = true;
+
+    // Per spec, we throw on sync errors, but not async.
+    if (mFlagSynchronous) {
+      return rv;
+    }
   }
 
   mWaitingForOnStopRequest = true;
 
   // Step 8
   mFlagSend = true;
 
   // If we're synchronous, spin an event loop here and wait
@@ -2834,17 +2839,28 @@ XMLHttpRequestMainThread::Send(nsIVarian
                           0, 0);
     if (mUpload && !mUploadComplete) {
       DispatchProgressEvent(mUpload, ProgressEventType::loadstart, true,
                             0, mUploadTotal);
     }
   }
 
   if (!mChannel) {
-    return NS_ERROR_FAILURE;
+    // Per spec, silently fail on async request failures; throw for sync.
+    if (mFlagSynchronous) {
+      return NS_ERROR_FAILURE;
+    } else {
+      // Defer the actual sending of async events just in case listeners
+      // are attached after the send() method is called.
+      NS_DispatchToCurrentThread(
+        NewRunnableMethod<ProgressEventType>(this,
+          &XMLHttpRequestMainThread::CloseRequestWithError,
+          ProgressEventType::error));
+      return NS_OK;
+    }
   }
 
   return rv;
 }
 
 // http://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html#dom-xmlhttprequest-setrequestheader
 NS_IMETHODIMP
 XMLHttpRequestMainThread::SetRequestHeader(const nsACString& header,
--- a/dom/xhr/tests/test_XHR_system.html
+++ b/dom/xhr/tests/test_XHR_system.html
@@ -43,27 +43,26 @@ tests.push(function test_cross_origin() 
 });
 
 tests.push(function test_file_uri() {
   // System XHR is not permitted to access file:/// URIs.
 
   let xhr = new XMLHttpRequest({mozSystem: true});
   is(xhr.mozSystem, true, ".mozSystem == true");
   xhr.open("GET", PROTECTED_URL);
-  let error;
-  try {
-    xhr.send();
-  } catch (ex) {
-    error = ex;
+  xhr.onload = function() {
+    ok(false, "Should not have loaded");
+    runNextTest();
   }
-  ok(!!error, "got exception");
-  is(error.name, "NS_ERROR_DOM_BAD_URI");
-  is(error.message, "Access to restricted URI denied");
-
-  runNextTest();
+  xhr.onerror = function(event) {
+    ok(true, "Got an error event: " + event);
+    is(xhr.status, 0, "HTTP status is 0");
+    runNextTest();
+  }
+  xhr.send();
 });
 
 tests.push(function test_redirect_to_file_uri() {
   // System XHR won't load file:/// URIs even if an HTTP resource redirects there.
 
   let xhr = new XMLHttpRequest({mozSystem: true});
   is(xhr.mozSystem, true, ".mozSystem == true");
   xhr.open("GET", REDIRECT_URL);
--- a/modules/libjar/test/mochitest/test_bug1173171.html
+++ b/modules/libjar/test/mochitest/test_bug1173171.html
@@ -32,34 +32,32 @@ let pushPref = function (key, value) {
 // Returns a promise with the response.
 let xhr = function (method, url, responseType) {
   return new Promise(function (resolve, reject) {
     let xhr = new XMLHttpRequest();
     xhr.open(method, url, true);
     xhr.onload = function () {
       resolve(xhr.response);
     };
+    xhr.onerror = function() {
+      resolve(null);
+    };
     xhr.responseType = responseType;
     xhr.send();
   });
 };
 
 let jarURL = "jar:http://mochi.test:8888/tests/modules/libjar/test/mochitest/bug403331.zip!/test.html";
 
 // Test behavior when blocking is deactivated and activated.
 add_task(function* () {
   for (let shouldBlock of [false, true]) {
     yield pushPref("network.jar.block-remote-files", shouldBlock);
-    try {
-      let response = yield xhr("GET", jarURL, "document");
-      didBlock = false;
-    } catch (e) {
-      didBlock = true;
-    }
-    ok(didBlock === shouldBlock,
+    let response = yield xhr("GET", jarURL, "document");
+    ok(shouldBlock === (response === null),
        "Remote jars should be blocked if and only if the 'network.jar.block-remote-files' pref is active.");
   }
 });
 
 </script>
 </pre>
 
 </body>
--- a/security/manager/tools/getHSTSPreloadList.js
+++ b/security/manager/tools/getHSTSPreloadList.js
@@ -177,25 +177,27 @@ RedirectAndAuthStopper.prototype = {
 function getHSTSStatus(host, resultList) {
   var req = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"]
             .createInstance(Ci.nsIXMLHttpRequest);
   var inResultList = false;
   var uri = "https://" + host.name + "/";
   req.open("GET", uri, true);
   req.timeout = REQUEST_TIMEOUT;
   req.channel.notificationCallbacks = new RedirectAndAuthStopper();
-  req.onreadystatechange = function(event) {
-    if (!inResultList && req.readyState == 4) {
+  req.onload = function(event) {
+    if (!inResultList) {
       inResultList = true;
       var header = req.getResponseHeader("strict-transport-security");
       resultList.push(processStsHeader(host, header, req.status,
                                        req.channel.securityInfo));
     }
   };
-
+  req.onerror = function(e) {
+    dump("ERROR: network error making request to " + host.name + ": " + e + "\n");
+  };
   try {
     req.send();
   }
   catch (e) {
     dump("ERROR: exception making request to " + host.name + ": " + e + "\n");
   }
 }
 
deleted file mode 100644
--- a/testing/web-platform/meta/XMLHttpRequest/send-authentication-basic-cors.htm.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[send-authentication-basic-cors.htm]
-  type: testharness
-  [XMLHttpRequest: send() - "Basic" authenticated CORS requests with user name and password passed to open() (asserts failure)]
-    expected: FAIL
-