Bug 1303121 - Do not fire one last progress event on XHR errors, to match a spec change. r=annevk

--HG--
extra : rebase_source : 9a59934cfe8fc7f2ee8ef7788813f97e2355ce2a

dom/base/test/test_bug435425.html

@@ -217,7 +217,7 @@ var tests =
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "GET", withUpload: none, testAbort: true, testRedirectError: false, testNetworkError: false,
       expectedEvents: [{target: XHR, type: "loadstart", optional: false},
-                       {target: XHR, type: "progress", optional: false},
+                       {target: XHR, type: "progress", optional: true},
                        {target: XHR, type: "abort", optional: false},
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "GET", withUpload: none, testAbort: false, testRedirectError: true, testNetworkError: false,
@@ -236,7 +236,7 @@ var tests =
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "GET", withUpload: small, testAbort: true, testRedirectError: false, testNetworkError: false,
       expectedEvents: [{target: XHR, type: "loadstart", optional: false},
-                       {target: XHR, type: "progress", optional: false},
+                       {target: XHR, type: "progress", optional: true},
                        {target: XHR, type: "abort", optional: false},
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "GET", withUpload: small, testAbort: false, testRedirectError: true, testNetworkError: false,
@@ -255,7 +255,7 @@ var tests =
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "GET", withUpload: mid, testAbort: true, testRedirectError: false, testNetworkError: false,
       expectedEvents: [{target: XHR, type: "loadstart", optional: false},
-                       {target: XHR, type: "progress", optional: false},
+                       {target: XHR, type: "progress", optional: true},
                        {target: XHR, type: "abort", optional: false},
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "GET", withUpload: mid, testAbort: false, testRedirectError: true, testNetworkError: false,
@@ -274,7 +274,7 @@ var tests =
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "GET", withUpload: large, testAbort: true, testRedirectError: false, testNetworkError: false,
       expectedEvents: [{target: XHR, type: "loadstart", optional: false},
-                       {target: XHR, type: "progress", optional: false},
+                       {target: XHR, type: "progress", optional: true},
                        {target: XHR, type: "abort", optional: false},
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "GET", withUpload: large, testAbort: false, testRedirectError: true, testNetworkError: false,
@@ -293,7 +293,7 @@ var tests =
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "POST", withUpload: none, testAbort: true, testRedirectError: false, testNetworkError: false,
       expectedEvents: [{target: XHR, type: "loadstart", optional: false},
-                       {target: XHR, type: "progress", optional: false},
+                       {target: XHR, type: "progress", optional: true},
                        {target: XHR, type: "abort", optional: false},
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "POST", withUpload: none, testAbort: false, testRedirectError: true, testNetworkError: false,
@@ -317,10 +317,10 @@ var tests =
     { method: "POST", withUpload: small, testAbort: true, testRedirectError: false, testNetworkError: false,
       expectedEvents: [{target: XHR, type: "loadstart", optional: false},
                        {target: UPLOAD, type: "loadstart", optional: false},
-                       {target: UPLOAD, type: "progress", optional: false},
+                       {target: UPLOAD, type: "progress", optional: true},
                        {target: UPLOAD, type: "abort", optional: false},
                        {target: UPLOAD, type: "loadend", optional: false},
-                       {target: XHR, type: "progress", optional: false},
+                       {target: XHR, type: "progress", optional: true},
                        {target: XHR, type: "abort", optional: false},
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "POST", withUpload: small, testAbort: false, testRedirectError: true, testNetworkError: false,
@@ -351,10 +351,10 @@ var tests =
     { method: "POST", withUpload: mid, testAbort: true, testRedirectError: false, testNetworkError: false,
       expectedEvents: [{target: XHR, type: "loadstart", optional: false},
                        {target: UPLOAD, type: "loadstart", optional: false},
-                       {target: UPLOAD, type: "progress", optional: false},
+                       {target: UPLOAD, type: "progress", optional: true},
                        {target: UPLOAD, type: "abort", optional: false},
                        {target: UPLOAD, type: "loadend", optional: false},
-                       {target: XHR, type: "progress", optional: false},
+                       {target: XHR, type: "progress", optional: true},
                        {target: XHR, type: "abort", optional: false},
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "POST", withUpload: mid, testAbort: false, testRedirectError: true, testNetworkError: false,
@@ -385,10 +385,10 @@ var tests =
     { method: "POST", withUpload: large, testAbort: true, testRedirectError: false, testNetworkError: false,
       expectedEvents: [{target: XHR, type: "loadstart", optional: false},
                        {target: UPLOAD, type: "loadstart", optional: false},
-                       {target: UPLOAD, type: "progress", optional: false},
+                       {target: UPLOAD, type: "progress", optional: true},
                        {target: UPLOAD, type: "abort", optional: false},
                        {target: UPLOAD, type: "loadend", optional: false},
-                       {target: XHR, type: "progress", optional: false},
+                       {target: XHR, type: "progress", optional: true},
                        {target: XHR, type: "abort", optional: false},
                        {target: XHR, type: "loadend", optional: false}]},
     { method: "POST", withUpload: large, testAbort: false, testRedirectError: true, testNetworkError: false,

dom/security/test/cors/test_CrossSiteXHR.html

@@ -759,10 +759,8 @@ function runTest() {
          "wrong responseXML in test for " + test.toSource());
       is(res.responseText, "",
          "wrong responseText in test for " + test.toSource());
-      var expectedProgressCount = 0;
       if (!res.sendThrew) {
         if (test.username) {
-          expectedProgressCount = 1;
           is(res.events.join(","),
              "opening,rs1,sending,loadstart,rs4,error,loadend",
              "wrong events in test for " + test.toSource());
@@ -772,7 +770,7 @@ function runTest() {
              "wrong events in test for " + test.toSource());
         }
       }
-      is(res.progressEvents, expectedProgressCount,
+      is(res.progressEvents, 0,
          "wrong events in test for " + test.toSource());
       if (test.responseHeaders) {
         for (header in test.responseHeaders) {

dom/xhr/XMLHttpRequestMainThread.cpp

@@ -1036,10 +1036,8 @@ XMLHttpRequestMainThread::CloseRequestWithError(const ProgressEventType aType)
     if (!mFlagSyncLooping) {
       if (mUpload && !mUploadComplete) {
         mUploadComplete = true;
-        DispatchProgressEvent(mUpload, ProgressEventType::progress, 0, 0);
         DispatchProgressEvent(mUpload, aType, 0, 0);
       }
-      DispatchProgressEvent(this, ProgressEventType::progress, 0, 0);
       DispatchProgressEvent(this, aType, 0, 0);
     }
   }
@@ -2182,10 +2180,9 @@ XMLHttpRequestMainThread::ChangeStateToDone()
   // Per spec, fire readystatechange=4/done before final error events.
   ChangeState(State::done, true);
 
-  // Per spec, if we failed in the upload phase, fire a final progress, error,
+  // Per spec, if we failed in the upload phase, fire a final error
-  // and loadend event for the upload after readystatechange=4/done.
+  // and loadend events for the upload after readystatechange=4/done.
   if (!mFlagSynchronous && mUpload && !mUploadComplete) {
-    DispatchProgressEvent(mUpload, ProgressEventType::progress, 0, 0);
     DispatchProgressEvent(mUpload, ProgressEventType::error, 0, 0);
   }
 

testing/web-platform/meta/XMLHttpRequest/progress-events-response-data-gzip.htm.ini

@@ -1,5 +0,0 @@
-[progress-events-response-data-gzip.htm]
-  type: testharness
-  [XMLHttpRequest: progress events and GZIP encoding]
-    expected: FAIL
-

testing/web-platform/tests/XMLHttpRequest/abort-after-send.htm

@@ -4,6 +4,7 @@
     <title>XMLHttpRequest: abort() after send()</title>
     <script src="/resources/testharness.js"></script>
     <script src="/resources/testharnessreport.js"></script>
+    <script src="resources/xmlhttprequest-event-order.js"></script>
     <link rel="help" href="https://xhr.spec.whatwg.org/#the-abort()-method" data-tested-assertations="following-sibling::ol/li[1] following-sibling::ol/li[3] following-sibling::ol/li[4] following-sibling::ol/li[4]/ol/li[1] following-sibling::ol/li[4]/ol/li[3] following-sibling::ol/li[4]/ol/li[4] following-sibling::ol/li[4]/ol/li[5] following-sibling::ol/li[4]/ol/li[6] following-sibling::ol/li[5]" />
     <link rel="help" href="https://xhr.spec.whatwg.org/#the-responsetext-attribute" data-tested-assertations="following::ol/li[3]" />
     <link rel="help" href="https://xhr.spec.whatwg.org/#the-responsexml-attribute" data-tested-assertations="following::ol/li[3]" />
@@ -19,36 +20,26 @@
       var test = async_test()
       test.step(function() {
         var client = new XMLHttpRequest(),
-            control_flag = false,
+            control_flag = false;
-            result = [],
+        prepare_xhr_for_event_order_test(client);
-            expected = [1, 4, 'progress', 'abort', 'loadend'] // open() -> 1, abort() -> 4
+        client.addEventListener("readystatechange", test.step_func(function() {
-        client.onreadystatechange = function() {
+          if(client.readyState == 4) {
-          test.step(function() {
+            control_flag = true
-            result.push(client.readyState)
+            assert_equals(client.responseXML, null)
-            if(client.readyState == 4) {
+            assert_equals(client.responseText, "")
-              control_flag = true
+            assert_equals(client.status, 0)
-              assert_equals(client.responseXML, null)
+            assert_equals(client.statusText, "")
-              assert_equals(client.responseText, "")
+            assert_equals(client.getAllResponseHeaders(), "")
-              assert_equals(client.status, 0)
+            assert_equals(client.getResponseHeader('Content-Type'), null)
-              assert_equals(client.statusText, "")
+          }
-              assert_equals(client.getAllResponseHeaders(), "")
+        }))
-              assert_equals(client.getResponseHeader('Content-Type'), null)
-            }
-          })
-        }
         client.open("GET", "resources/well-formed.xml", true)
         client.send(null)
-        client.addEventListener('progress', logEvt)
-        client.addEventListener('abort', logEvt)
-        client.addEventListener('loadend', logEvt)
         client.abort()
         assert_true(control_flag)
         assert_equals(client.readyState, 0)
-        assert_array_equals(result, expected)
+        assert_xhr_event_order_matches([1, "loadstart(0,0,false)", 4, "abort(0,0,false)", "loadend(0,0,false)"])
         test.done()
-        function logEvt (e) {
-          result.push(e.type)
-        }
       })
     </script>
   </body>

testing/web-platform/tests/XMLHttpRequest/abort-during-upload.htm

@@ -18,7 +18,7 @@
         client.open("POST", "resources/delay.py?ms=1000")
         client.addEventListener("loadend", function(e) {
           test.step(function() {
-            assert_xhr_event_order_matches([1, "loadstart(0,0,false)", "upload.loadstart(0,9999,true)", 4, "upload.progress(0,0,false)", "upload.abort(0,0,false)", "upload.loadend(0,0,false)", "progress(0,0,false)", "abort(0,0,false)", "loadend(0,0,false)"]);
+            assert_xhr_event_order_matches([1, "loadstart(0,0,false)", "upload.loadstart(0,9999,true)", 4, "upload.abort(0,0,false)", "upload.loadend(0,0,false)", "abort(0,0,false)", "loadend(0,0,false)"]);
             test.done()
           })
         });

testing/web-platform/tests/XMLHttpRequest/abort-event-order.htm

@@ -37,7 +37,7 @@
             {
                 test.step(function()
                 {
-                    assert_xhr_event_order_matches([1, "loadstart(0,0,false)", 4, "upload.progress(0,0,false)", "upload.abort(0,0,false)", "upload.loadend(0,0,false)", "progress(0,0,false)", "abort(0,0,false)", "loadend(0,0,false)"]);
+                    assert_xhr_event_order_matches([1, "loadstart(0,0,false)", 4, "upload.abort(0,0,false)", "upload.loadend(0,0,false)", "abort(0,0,false)", "loadend(0,0,false)"]);
 
                     assert_equals(xhr.readyState, 0, 'state should be UNSENT');
                     test.done();

testing/web-platform/tests/XMLHttpRequest/event-timeout-order.htm

@@ -20,7 +20,7 @@
             prepare_xhr_for_event_order_test(xhr);
             xhr.addEventListener("loadend", function() {
                 test.step(function() {
-                    assert_xhr_event_order_matches([1, "loadstart(0,0,false)", "upload.loadstart(0,12,true)", 4, "upload.progress(0,0,false)", "upload.timeout(0,0,false)", "upload.loadend(0,0,false)", "progress(0,0,false)", "timeout(0,0,false)", "loadend(0,0,false)"]);
+                    assert_xhr_event_order_matches([1, "loadstart(0,0,false)", "upload.loadstart(0,12,true)", 4, "upload.timeout(0,0,false)", "upload.loadend(0,0,false)", "timeout(0,0,false)", "loadend(0,0,false)"]);
                     test.done();
                 });
             });

testing/web-platform/tests/XMLHttpRequest/progress-events-response-data-gzip.htm

@@ -36,17 +36,20 @@
 
           * If lengthComputable is true:
               * Event.total must match Content-length header
-              * event.loaded should be a smaller number while resource is loading
+              * event.loaded must only ever increase in progress events
-                and match Content-length when loading is finished
+                (and may never repeat its value).
-              * Setting event.loaded to equal event.total for each progress event if the
+              * event.loaded must never exceed the Content-length.
-                resource is not fully downloaded would be cheating
 
           * If lengthComputable is false:
               * event.total should be 0
+              * event.loaded must only ever increase in progress events
+                (and may never repeat its value).
               * event.loaded should be the length of the decompressed content, i.e.
                 bigger than Content-length header value when finished loading
 
         */
+        var lastTotal;
+        var lastLoaded = -1;
         client.addEventListener('loadend', test.step_func(function(e){
           var len = parseInt(client.getResponseHeader('content-length'), 10)
           if(e.lengthComputable){
@@ -59,14 +62,17 @@
           test.done();
         }), false)
         client.addEventListener('progress', test.step_func(function(e){
-          if(e.lengthComputable && e.total && e.loaded && e.target.readyState < 4){
+          if(lastTotal === undefined){
-            assert_not_equals(e.total, e.loaded, 'total should not equal loaded while download/decode is incomplete')
+            lastTotal = e.total;
-            // We should only do this assertation once
-            // it's theoretically possible that all the data would get in
-            // and a progress event fire before the readyState switches from 3 to 4 -
-            // in this case we might report bogus and random failures. Better to remove the event listener again..
-            client.removeEventListener('progress', arguments.callee, false);
           }
+          if(e.lengthComputable && e.total && e.loaded){
+            assert_equals(e.total, lastTotal, 'event.total should remain invariant')
+            assert_less_than_equal(e.loaded, lastTotal, 'event.loaded should not exceed content-length')
+          }else{
+            assert_equals(e.total, 0, 'event.total should be 0')
+          }
+          assert_greater_than(e.loaded, lastLoaded, 'event.loaded should only ever increase')
+          lastLoaded = e.loaded;
         }), false)
         // image.gif is 165375 bytes compressed. Sending 45000 bytes at a time with 1 second delay will load it in 4 seconds
         client.open("GET", "resources/image.gif?pipe=gzip|trickle(45000:d1:r2)", true)

testing/web-platform/tests/XMLHttpRequest/resources/xmlhttprequest-event-order.js

@@ -21,13 +21,60 @@
     }
   }
 
+  function getNextEvent(arr) {
+    var eventStr = arr.shift();
+
+    // we can only handle strings, numbers (readystates) and undefined
+    if (eventStr === undefined) {
+      return event;
+    }
+    if (typeof eventStr !== "string") {
+      if (Number.isInteger(eventStr)) {
+        eventStr = "readystatechange(" + eventStr + ")";
+      } else {
+        throw "Test error: unexpected event type " + eventStr;
+      }
+    }
+
+    // parse out the general type, loaded and total values
+    var type = eventStr.type = eventStr.split("(")[0].split(".").pop();
+    eventStr.mayFollowOptionalProgressEvents = type == "progress" ||
+      type == "load" || type == "abort" || type == "error";
+    var loadedAndTotal = eventStr.match(/\((\d)+,(\d)+/);
+    if (loadedAndTotal) {
+      eventStr.loaded = parseInt(loadedAndTotal[0]);
+      eventStr.total = parseInt(loadedAndTotal[1]);
+    }
+
+    return eventStr;
+  }
+
   global.assert_xhr_event_order_matches = function(expected) {
-    try {
+    var recorded = recorded_xhr_events;
-      assert_array_equals(recorded_xhr_events, expected);
+    var lastRecordedLoaded = -1;
-    } catch(e) {
+
-      e.message += "\nRecorded events were:" + recorded_xhr_events.join(", ");
+    while(expected.length && recorded.length) {
-      e.message += "\nExpected events were:" + expected.join(", ");
+      var currentExpected = getNextEvent(expected),
-      throw e;
+          currentRecorded = getNextEvent(recorded);
+
+      // skip to the last progress event if we've hit one
+      while (recorded.length && currentRecorded.type == "progress") {
+        assert_greater(currentRecorded.loaded, lastRecordedLoaded,
+                       "progress event 'loaded' values must only increase");
+        lastRecordedLoaded = currentRecorded.loaded;
+        currentRecorded = getNextEvent(recorded);
+      }
+      if (currentRecorded.type == "loadstart") {
+        lastRecordedLoaded = -1;
+      }
+
+      assert_equals(currentRecorded, currentExpected);
+    }
+    if (recorded.length) {
+      throw "\nUnexpected extra events: " + recorded.join(", ");
+    }
+    if (expected.length) {
+      throw "\nExpected more events: " + expected.join(", ");
     }
   }
 }(this));

testing/web-platform/tests/XMLHttpRequest/send-response-event-order.htm

@@ -12,6 +12,7 @@
     <link rel="help" href="https://xhr.spec.whatwg.org/#switch-done" data-tested-assertations="following::ol[1]/li[3] following::ol[1]/li[4] following::ol[1]/li[5] following::ol[1]/li[6] following::ol[1]/li[7]" />
     <script src="/resources/testharness.js"></script>
     <script src="/resources/testharnessreport.js"></script>
+    <script src="resources/xmlhttprequest-event-order.js"></script>
     <title>XMLHttpRequest: The send() method: event order when synchronous flag is unset</title>
 </head>
 
@@ -24,38 +25,12 @@
         test.step(function()
         {
             var xhr = new XMLHttpRequest();
-            var expect =  ["loadstart", "upload.loadstart", "upload.progress", "upload.load", "upload.loadend", "progress", 4, "load", "loadend"];
+            prepare_xhr_for_event_order_test(xhr);
-            var actual = [];
 
-            xhr.onreadystatechange = function()
+            xhr.addEventListener("loadend", test.step_func(function() {
-            {
+                assert_xhr_event_order_matches([1, "loadstart(0,0,false)", "upload.loadstart(0,12,true)", "upload.progress(12,12,true)", "upload.load(12,12,true)", "upload.loadend(12,12,true)", 2, 3, "progress(12,12,true)", 4, "load(12,12,true)", "loadend(12,12,true)"]);
-                test.step(function()
+                test.done();
-                {
+            }));
-                    if (xhr.readyState == 4)
-                    {
-                        actual.push(xhr.readyState);
-                    }
-                });
-            };
-
-            xhr.onloadstart        = function(e){ actual.push(e.type); };
-            xhr.onload             = function(e){ actual.push(e.type); };
-            xhr.onloadend          = function(e){ actual.push(e.type); VerifyResult()};
-            xhr.onprogress         = function(e){ actual.push(e.type);};
-
-            xhr.upload.onloadstart = function(e){ actual.push("upload." + e.type); };
-            xhr.upload.onload      = function(e){ actual.push("upload." + e.type); };
-            xhr.upload.onloadend   = function(e){ actual.push("upload." + e.type);};
-            xhr.upload.onprogress  = function(e){ actual.push("upload." + e.type);};
-
-            function VerifyResult()
-            {
-                test.step(function()
-                {
-                    assert_array_equals(actual, expect);
-                    test.done();
-                });
-            };
 
             xhr.open("POST", "./resources/content.py", true);
             xhr.send("Test Message");