From c190891418ed69d73cd883175802618ba1d3dd52 Mon Sep 17 00:00:00 2001 From: Thomas Wisniewski Date: Wed, 28 Sep 2016 13:05:32 -0400 Subject: [PATCH] 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 | 22 +++---- dom/security/test/cors/test_CrossSiteXHR.html | 4 +- dom/xhr/XMLHttpRequestMainThread.cpp | 7 +-- ...progress-events-response-data-gzip.htm.ini | 5 -- .../tests/XMLHttpRequest/abort-after-send.htm | 39 +++++------- .../XMLHttpRequest/abort-during-upload.htm | 2 +- .../XMLHttpRequest/abort-event-order.htm | 2 +- .../XMLHttpRequest/event-timeout-order.htm | 2 +- .../progress-events-response-data-gzip.htm | 28 +++++---- .../resources/xmlhttprequest-event-order.js | 59 +++++++++++++++++-- .../send-response-event-order.htm | 37 ++---------- 11 files changed, 108 insertions(+), 99 deletions(-) delete mode 100644 testing/web-platform/meta/XMLHttpRequest/progress-events-response-data-gzip.htm.ini diff --git a/dom/base/test/test_bug435425.html b/dom/base/test/test_bug435425.html index 0e4e3a4ebd28..828931d67d17 100644 --- a/dom/base/test/test_bug435425.html +++ b/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, diff --git a/dom/security/test/cors/test_CrossSiteXHR.html b/dom/security/test/cors/test_CrossSiteXHR.html index 29f5b1f34a21..b3cda3b871f3 100644 --- a/dom/security/test/cors/test_CrossSiteXHR.html +++ b/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) { diff --git a/dom/xhr/XMLHttpRequestMainThread.cpp b/dom/xhr/XMLHttpRequestMainThread.cpp index a3df1803caec..ded654267781 100644 --- a/dom/xhr/XMLHttpRequestMainThread.cpp +++ b/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, - // and loadend event for the upload after readystatechange=4/done. + // 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::progress, 0, 0); DispatchProgressEvent(mUpload, ProgressEventType::error, 0, 0); } diff --git a/testing/web-platform/meta/XMLHttpRequest/progress-events-response-data-gzip.htm.ini b/testing/web-platform/meta/XMLHttpRequest/progress-events-response-data-gzip.htm.ini deleted file mode 100644 index 5adf572e37d3..000000000000 --- a/testing/web-platform/meta/XMLHttpRequest/progress-events-response-data-gzip.htm.ini +++ /dev/null @@ -1,5 +0,0 @@ -[progress-events-response-data-gzip.htm] - type: testharness - [XMLHttpRequest: progress events and GZIP encoding] - expected: FAIL - diff --git a/testing/web-platform/tests/XMLHttpRequest/abort-after-send.htm b/testing/web-platform/tests/XMLHttpRequest/abort-after-send.htm index c4885c9911b0..523a0d616b64 100644 --- a/testing/web-platform/tests/XMLHttpRequest/abort-after-send.htm +++ b/testing/web-platform/tests/XMLHttpRequest/abort-after-send.htm @@ -4,6 +4,7 @@ XMLHttpRequest: abort() after send() + @@ -19,36 +20,26 @@ var test = async_test() test.step(function() { var client = new XMLHttpRequest(), - control_flag = false, - result = [], - expected = [1, 4, 'progress', 'abort', 'loadend'] // open() -> 1, abort() -> 4 - client.onreadystatechange = function() { - test.step(function() { - result.push(client.readyState) - if(client.readyState == 4) { - control_flag = true - assert_equals(client.responseXML, null) - assert_equals(client.responseText, "") - assert_equals(client.status, 0) - assert_equals(client.statusText, "") - assert_equals(client.getAllResponseHeaders(), "") - assert_equals(client.getResponseHeader('Content-Type'), null) - } - }) - } + control_flag = false; + prepare_xhr_for_event_order_test(client); + client.addEventListener("readystatechange", test.step_func(function() { + if(client.readyState == 4) { + control_flag = true + assert_equals(client.responseXML, null) + assert_equals(client.responseText, "") + assert_equals(client.status, 0) + 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) - } }) diff --git a/testing/web-platform/tests/XMLHttpRequest/abort-during-upload.htm b/testing/web-platform/tests/XMLHttpRequest/abort-during-upload.htm index 766dcc4693d8..9fbc8b9bbb47 100644 --- a/testing/web-platform/tests/XMLHttpRequest/abort-during-upload.htm +++ b/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() }) }); diff --git a/testing/web-platform/tests/XMLHttpRequest/abort-event-order.htm b/testing/web-platform/tests/XMLHttpRequest/abort-event-order.htm index cb405a71accf..f05c20628c4a 100644 --- a/testing/web-platform/tests/XMLHttpRequest/abort-event-order.htm +++ b/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(); diff --git a/testing/web-platform/tests/XMLHttpRequest/event-timeout-order.htm b/testing/web-platform/tests/XMLHttpRequest/event-timeout-order.htm index 1d9ba31d6e06..7376ca2f8b14 100644 --- a/testing/web-platform/tests/XMLHttpRequest/event-timeout-order.htm +++ b/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(); }); }); diff --git a/testing/web-platform/tests/XMLHttpRequest/progress-events-response-data-gzip.htm b/testing/web-platform/tests/XMLHttpRequest/progress-events-response-data-gzip.htm index dc166a2396a4..058064636d43 100644 --- a/testing/web-platform/tests/XMLHttpRequest/progress-events-response-data-gzip.htm +++ b/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 - and match Content-length when loading is finished - * Setting event.loaded to equal event.total for each progress event if the - resource is not fully downloaded would be cheating + * event.loaded must only ever increase in progress events + (and may never repeat its value). + * event.loaded must never exceed the Content-length. * 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){ - assert_not_equals(e.total, e.loaded, 'total should not equal loaded while download/decode is incomplete') - // 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(lastTotal === undefined){ + lastTotal = e.total; } + 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) diff --git a/testing/web-platform/tests/XMLHttpRequest/resources/xmlhttprequest-event-order.js b/testing/web-platform/tests/XMLHttpRequest/resources/xmlhttprequest-event-order.js index 820f9ee22143..77fc0e784ef9 100644 --- a/testing/web-platform/tests/XMLHttpRequest/resources/xmlhttprequest-event-order.js +++ b/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 { - assert_array_equals(recorded_xhr_events, expected); - } catch(e) { - e.message += "\nRecorded events were:" + recorded_xhr_events.join(", "); - e.message += "\nExpected events were:" + expected.join(", "); - throw e; + var recorded = recorded_xhr_events; + var lastRecordedLoaded = -1; + + while(expected.length && recorded.length) { + var currentExpected = getNextEvent(expected), + 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)); diff --git a/testing/web-platform/tests/XMLHttpRequest/send-response-event-order.htm b/testing/web-platform/tests/XMLHttpRequest/send-response-event-order.htm index 64dfaa670f82..041cb23c6ea8 100644 --- a/testing/web-platform/tests/XMLHttpRequest/send-response-event-order.htm +++ b/testing/web-platform/tests/XMLHttpRequest/send-response-event-order.htm @@ -12,6 +12,7 @@ + XMLHttpRequest: The send() method: event order when synchronous flag is unset @@ -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"]; - var actual = []; + prepare_xhr_for_event_order_test(xhr); - xhr.onreadystatechange = function() - { - test.step(function() - { - 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.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.done(); + })); xhr.open("POST", "./resources/content.py", true); xhr.send("Test Message");