Bug 918719 - Only fire one loading readystatechange per XHR, but keep the old behavior available behind the preference dom.send_multiple_xhr_loading_readystatechanges. r=smaug a=smaug

wisniewskit · wisniewskit · commit 89a07725ed51 · 2016-09-08T12:01:58.000-04:00
--HG--
extra : source : ea777ccca86cceeb53d2fc7252bea2a3bad3e405
diff --git a/dom/tests/mochitest/bugs/bug918719.sjs b/dom/tests/mochitest/bugs/bug918719.sjs
@@ -0,0 +1,13 @@
+// Send a payload that's over 32k in size, which for a plain response should be
+// large enough to ensure that OnDataAvailable is called more than once (and so
+// the XHR will be triggered to send more than one "loading" event if desired).
+
+function handleRequest(request, response)
+{
+  let data = (new Array(1 << 18)).join("A");
+  response.processAsync();
+  response.setHeader("Content-Type", "text/plain", false);
+  response.setHeader("Content-Length", "" + data.length, false);
+  response.write(data, data.length);
+  response.finish();
+}
diff --git a/dom/tests/mochitest/bugs/mochitest.ini b/dom/tests/mochitest/bugs/mochitest.ini
@@ -7,6 +7,7 @@ support-files =
   bug346659-parent-echoer.html
   bug346659-parent.html
   bug458091_child.html
+  bug918719.sjs
   child_bug260264.html
   devicemotion_inner.html
   devicemotion_outer.html
@@ -155,6 +156,7 @@ skip-if = (buildapp == 'b2g' && toolkit != 'gonk') #Bug 931116, b2g desktop spec
 [test_bug857555.html]
 [test_bug862540.html]
 [test_bug876098.html]
+[test_bug918719.html]
 [test_bug927901.html]
 [test_devicemotion_multiple_listeners.html]
 skip-if = toolkit == 'android' #bug 775227
diff --git a/dom/tests/mochitest/bugs/test_bug918719.html b/dom/tests/mochitest/bugs/test_bug918719.html
@@ -0,0 +1,62 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=918719
+-->
+<head>
+  <title>Test for Bug 918719</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=918719">Mozilla Bug 918719</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+SimpleTest.waitForExplicitFinish();
+
+function runTest() {
+  return new Promise(function(resolve) {
+    let loadingEventCount = 0,
+        xhr = new XMLHttpRequest();
+    xhr.open("GET", "bug918719.sjs");
+    xhr.onreadystatechange = function() {
+      if (xhr.readyState == 3) {
+        loadingEventCount++;
+      } else if (xhr.readyState == 4) {
+        resolve(loadingEventCount);
+      }
+    }
+    xhr.send();
+  });
+}
+
+function prefChangePromise(args) {
+  return new Promise(function(resolve) {
+    SpecialPowers.pushPrefEnv(args, resolve);
+  });
+}
+
+runTest().then(function(count) {
+  ok(count === 1, "Only one loading readystatechange event should have been fired with the pref off.");
+}).then(function() {
+  return prefChangePromise({"set": [["dom.fire_extra_xhr_loading_readystatechanges", true]]});
+}).then(function() {
+  return runTest();
+}).then(function(count) {
+  // Note this case is tricky to test, as there is no hard guarantee that the
+  // server will send the data in more than one chunk, even with the trick
+  // we're using in the sjs. As such this case *may* intermittently fail.
+  ok(count > 1, "Multiple loading readystatechange events should have been fired with the pref on.");
+  SimpleTest.finish();
+});
+
+</script>
+</pre>
+</body>
+</html>
+
diff --git a/dom/xhr/XMLHttpRequestMainThread.cpp b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -169,6 +169,7 @@ XMLHttpRequestMainThread::XMLHttpRequestMainThread()
     mFlagSyncLooping(false), mFlagBackgroundRequest(false),
     mFlagHadUploadListenersOnSend(false), mFlagACwithCredentials(false),
     mFlagTimedOut(false), mFlagDeleted(false), mFlagSend(false),
+    mSendExtraLoadingEvents(Preferences::GetBool("dom.fire_extra_xhr_loading_readystatechanges", true)),
     mUploadTransferred(0), mUploadTotal(0), mUploadComplete(true),
     mProgressSinceLastProgressEvent(false),
     mRequestSentTime(0), mTimeoutMilliseconds(0),
@@ -1719,7 +1720,9 @@ XMLHttpRequestMainThread::OnDataAvailable(nsIRequest *request,
 
   mDataAvailable += totalRead;
 
-  ChangeState(State::loading);
+  if (mState == State::headers_received || mSendExtraLoadingEvents) {
+    ChangeState(State::loading);
+  }
 
   if (!mFlagSynchronous && !mProgressTimerIsActive) {
     StartProgressEventTimer();
diff --git a/dom/xhr/XMLHttpRequestMainThread.h b/dom/xhr/XMLHttpRequestMainThread.h
@@ -696,6 +696,14 @@ class XMLHttpRequestMainThread final : public XMLHttpRequest,
   // late, and ensure the XHR only handles one in-flight request at once.
   bool mFlagSend;
 
+  // Before ProgressEvents were a thing, multiple readystatechange events were
+  // fired during the loading state to give sites a way to monitor XHR progress.
+  // The XHR spec now has proper progress events and dictates that only one
+  // "loading" readystatechange should be fired per send. However, it's possible
+  // that some content still relies on this old behavior, so we're keeping it
+  // (behind a preference) for now. See bug 918719.
+  bool mSendExtraLoadingEvents;
+
   RefPtr<XMLHttpRequestUpload> mUpload;
   int64_t mUploadTransferred;
   int64_t mUploadTotal;
diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js
@@ -5080,6 +5080,10 @@ pref("dom.voicemail.defaultServiceId", 0);
 // Enable mapped array buffer by default.
 pref("dom.mapped_arraybuffer.enabled", true);
 
+// Whether to send more than one "loading" readystatechange during XHRs to
+// simulate progress events for sites still not using modern progress events.
+pref("dom.fire_extra_xhr_loading_readystatechanges", false);
+
 // The tables used for Safebrowsing phishing and malware checks.
 pref("urlclassifier.malwareTable", "goog-malware-shavar,goog-unwanted-shavar,test-malware-simple,test-unwanted-simple");
 
diff --git a/testing/web-platform/meta/XMLHttpRequest/event-readystatechange-loaded.htm.ini b/testing/web-platform/meta/XMLHttpRequest/event-readystatechange-loaded.htm.ini
