Title: [243765] trunk
Revision
243765
Author
[email protected]
Date
2019-04-02 16:00:47 -0700 (Tue, 02 Apr 2019)

Log Message

XMLHttpRequestUpload's loadstart event not correct initialized
https://bugs.webkit.org/show_bug.cgi?id=196174
<rdar://problem/49191412>

Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

* web-platform-tests/xhr/event-error-order.sub.html:
Update test after https://github.com/web-platform-tests/wpt/pull/13365

* web-platform-tests/xhr/abort-during-upload-expected.txt:
* web-platform-tests/xhr/event-error-order.sub-expected.txt:
* web-platform-tests/xhr/event-loadstart-upload-expected.txt:
* web-platform-tests/xhr/event-timeout-order-expected.txt:
* web-platform-tests/xhr/send-response-event-order-expected.txt:
Rebaseline several WPT tests that are now passing.

Source/WebCore:

Align progress event firing with the XHR specification.

No new tests, rebaselined existing tests.

* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::createRequest):
As per [1], the loadstart event fired on the XMLHttpRequestUpload object should use
loaded=0 and total=`req’s body’s total bytes`.
[1] https://xhr.spec.whatwg.org/#the-send()-method (step 11.2.)

(WebCore::XMLHttpRequest::didSendData):
As per [2], the progress / load / loadend should use loaded=transmitted and total=length.
[2] https://xhr.spec.whatwg.org/#ref-for-process-request-end-of-body (steps 5, 6 and 7)

(WebCore::XMLHttpRequest::didReceiveData):
As per [3], we should fire the readystatechange event *before* the progress event.
This is covered by web-platform-tests/xhr/send-response-event-order.htm which was failing
differently after the other changes in this patch.
[3] https://xhr.spec.whatwg.org/#ref-for-process-response (steps 9.4 and 9.5)

(WebCore::XMLHttpRequest::dispatchErrorEvents):
As per [4], in case of an error, we should fire the provided 'event' and 'loadend' with
loaded=0 and total=0.
[4] https://xhr.spec.whatwg.org/#request-error-steps (steps 7 and 8)

* xml/XMLHttpRequestUpload.cpp:
(WebCore::XMLHttpRequestUpload::dispatchProgressEvent):
* xml/XMLHttpRequestUpload.h:
Simplify XMLHttpRequestUpload. It no longer needs to store loaded / total as data
members now that they are always passed by the call site. lengthComputable is set
to !!total as [5] says to set it to true if length/total is not 0.
[5] https://xhr.spec.whatwg.org/#concept-event-fire-progress

Modified Paths

Diff

Modified: trunk/LayoutTests/TestExpectations (243764 => 243765)


--- trunk/LayoutTests/TestExpectations	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/LayoutTests/TestExpectations	2019-04-02 23:00:47 UTC (rev 243765)
@@ -217,6 +217,7 @@
 imported/w3c/web-platform-tests/service-workers/service-worker/navigation-redirect-to-http.https.html [ Pass Failure ]
 imported/w3c/web-platform-tests/service-workers/service-worker/resource-timing.https.html [ Pass Failure ]
 imported/w3c/web-platform-tests/service-workers/service-worker/worker-client-id.https.html [ Pass Failure ]
+imported/w3c/web-platform-tests/xhr/event-timeout-order.htm [ Pass Failure ]
 
 # This test is timing out because it relies on http://www1.localhost:8800 host for cross origin checks.
 imported/w3c/web-platform-tests/cors/remote-origin.htm [ Skip ]

Modified: trunk/LayoutTests/http/tests/xmlhttprequest/upload-onabort-progressevent-attributes.html (243764 => 243765)


--- trunk/LayoutTests/http/tests/xmlhttprequest/upload-onabort-progressevent-attributes.html	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/upload-onabort-progressevent-attributes.html	2019-04-02 23:00:47 UTC (rev 243765)
@@ -26,6 +26,15 @@
             + "(" + e.loaded + " / " + e.total + "), expected (" + loaded + " / " + total + ")");
 }
 
+function onErrorProgressEvent(e)
+{
+    if (e.lengthComputable)
+        fail("Event " + e.type + " lengthComputable is true");
+    if (e.total != 0 || e.loaded != 0)
+        fail("Event " + e.type + " total/loaded values not matching: "
+            + "(" + e.loaded + " / " + e.total + "), expected (0 / 0)");
+}
+
 function onUnexpectedProgressEvent(e)
 {
     fail("unexpected ProgressEvent: " + e.type);
@@ -50,13 +59,13 @@
     var req = new XMLHttpRequest();
     req.upload._onerror_ = onUnexpectedProgressEvent;
     req.upload._onload_ = onUnexpectedProgressEvent;
-    req.upload._onabort_ = onProgressEvent;
+    req.upload._onabort_ = onErrorProgressEvent;
     req.upload._onprogress_ = function(e) {
         onProgressEvent(e);
         req.abort();
     }
     req.upload._onloadend_ = function(e) {
-        onProgressEvent(e);
+        onErrorProgressEvent(e);
         completeTest();
     }
 

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (243764 => 243765)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2019-04-02 23:00:47 UTC (rev 243765)
@@ -1,5 +1,23 @@
 2019-04-02  Chris Dumez  <[email protected]>
 
+        XMLHttpRequestUpload's loadstart event not correct initialized
+        https://bugs.webkit.org/show_bug.cgi?id=196174
+        <rdar://problem/49191412>
+
+        Reviewed by Alex Christensen.
+
+        * web-platform-tests/xhr/event-error-order.sub.html:
+        Update test after https://github.com/web-platform-tests/wpt/pull/13365
+
+        * web-platform-tests/xhr/abort-during-upload-expected.txt:
+        * web-platform-tests/xhr/event-error-order.sub-expected.txt:
+        * web-platform-tests/xhr/event-loadstart-upload-expected.txt:
+        * web-platform-tests/xhr/event-timeout-order-expected.txt:
+        * web-platform-tests/xhr/send-response-event-order-expected.txt:
+        Rebaseline several WPT tests that are now passing.
+
+2019-04-02  Chris Dumez  <[email protected]>
+
         [WK2] Add support for Window's beforeprint / afterprint events
         https://bugs.webkit.org/show_bug.cgi?id=196478
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/abort-during-upload-expected.txt (243764 => 243765)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/abort-during-upload-expected.txt	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/abort-during-upload-expected.txt	2019-04-02 23:00:47 UTC (rev 243765)
@@ -1,3 +1,3 @@
 
-FAIL XMLHttpRequest: abort() while sending data assert_equals: expected "upload.loadstart(0,9999,true)" but got "upload.loadstart(0,0,false)"
+PASS XMLHttpRequest: abort() while sending data 
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt (243764 => 243765)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt	2019-04-02 23:00:47 UTC (rev 243765)
@@ -1,4 +1,4 @@
 Blocked access to external URL http://nonexistent.localhost:8800/
 
-FAIL XMLHttpRequest: event - error (order of events) assert_equals: expected "upload.loadstart(0,12,true)" but got "upload.loadstart(0,0,false)"
+PASS XMLHttpRequest: event - error (order of events) 
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub.html (243764 => 243765)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub.html	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub.html	2019-04-02 23:00:47 UTC (rev 243765)
@@ -22,7 +22,7 @@
             xhr.addEventListener("loadend", function() {
                 test.step(function() {
                     // no progress events due to CORS failure
-                    assert_xhr_event_order_matches([1, "loadstart(0,0,false)", "upload.loadstart(0,12,true)", 2, 4, "upload.error(0,0,false)", "upload.loadend(0,0,false)", "error(0,0,false)", "loadend(0,0,false)"]);
+                    assert_xhr_event_order_matches([1, "loadstart(0,0,false)", "upload.loadstart(0,12,true)", 4, "upload.error(0,0,false)", "upload.loadend(0,0,false)", "error(0,0,false)", "loadend(0,0,false)"]);
                     test.done();
                 });
             });

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-loadstart-upload-expected.txt (243764 => 243765)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-loadstart-upload-expected.txt	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-loadstart-upload-expected.txt	2019-04-02 23:00:47 UTC (rev 243765)
@@ -1,3 +1,3 @@
 
-FAIL XMLHttpRequest: The send() method: Fire a progress event named loadstart on upload object (synchronous flag is unset) assert_equals: upload.onloadstart: event.total expected 7 but got 0
+PASS XMLHttpRequest: The send() method: Fire a progress event named loadstart on upload object (synchronous flag is unset) 
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-timeout-order-expected.txt (243764 => 243765)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-timeout-order-expected.txt	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-timeout-order-expected.txt	2019-04-02 23:00:47 UTC (rev 243765)
@@ -1,3 +1,3 @@
 
-FAIL XMLHttpRequest: event - timeout (order of events) assert_equals: expected "upload.loadstart(0,12,true)" but got "upload.loadstart(0,0,false)"
+PASS XMLHttpRequest: event - timeout (order of events) 
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/send-response-event-order-expected.txt (243764 => 243765)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/send-response-event-order-expected.txt	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/send-response-event-order-expected.txt	2019-04-02 23:00:47 UTC (rev 243765)
@@ -1,3 +1,3 @@
 
-FAIL XMLHttpRequest: The send() method: event order when synchronous flag is unset assert_equals: expected "upload.loadstart(0,12,true)" but got "upload.loadstart(0,0,false)"
+PASS XMLHttpRequest: The send() method: event order when synchronous flag is unset 
 

Modified: trunk/LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt (243764 => 243765)


--- trunk/LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt	2019-04-02 23:00:47 UTC (rev 243765)
@@ -1,3 +1,3 @@
 
-FAIL XMLHttpRequest: event - error (order of events) assert_equals: expected "upload.loadstart(0,12,true)" but got "upload.loadstart(0,0,false)"
+PASS XMLHttpRequest: event - error (order of events) 
 

Modified: trunk/Source/WebCore/ChangeLog (243764 => 243765)


--- trunk/Source/WebCore/ChangeLog	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/Source/WebCore/ChangeLog	2019-04-02 23:00:47 UTC (rev 243765)
@@ -1,3 +1,44 @@
+2019-04-02  Chris Dumez  <[email protected]>
+
+        XMLHttpRequestUpload's loadstart event not correct initialized
+        https://bugs.webkit.org/show_bug.cgi?id=196174
+        <rdar://problem/49191412>
+
+        Reviewed by Alex Christensen.
+
+        Align progress event firing with the XHR specification.
+
+        No new tests, rebaselined existing tests.
+
+        * xml/XMLHttpRequest.cpp:
+        (WebCore::XMLHttpRequest::createRequest):
+        As per [1], the loadstart event fired on the XMLHttpRequestUpload object should use
+        loaded=0 and total=`req’s body’s total bytes`.
+        [1] https://xhr.spec.whatwg.org/#the-send()-method (step 11.2.)
+
+        (WebCore::XMLHttpRequest::didSendData):
+        As per [2], the progress / load / loadend should use loaded=transmitted and total=length.
+        [2] https://xhr.spec.whatwg.org/#ref-for-process-request-end-of-body (steps 5, 6 and 7)
+
+        (WebCore::XMLHttpRequest::didReceiveData):
+        As per [3], we should fire the readystatechange event *before* the progress event.
+        This is covered by web-platform-tests/xhr/send-response-event-order.htm which was failing
+        differently after the other changes in this patch.
+        [3] https://xhr.spec.whatwg.org/#ref-for-process-response (steps 9.4 and 9.5)
+
+        (WebCore::XMLHttpRequest::dispatchErrorEvents):
+        As per [4], in case of an error, we should fire the provided 'event' and 'loadend' with
+        loaded=0 and total=0.
+        [4] https://xhr.spec.whatwg.org/#request-error-steps (steps 7 and 8)
+
+        * xml/XMLHttpRequestUpload.cpp:
+        (WebCore::XMLHttpRequestUpload::dispatchProgressEvent):
+        * xml/XMLHttpRequestUpload.h:
+        Simplify XMLHttpRequestUpload. It no longer needs to store loaded / total as data
+        members now that they are always passed by the call site. lengthComputable is set
+        to !!total as [5] says to set it to true if length/total is not 0. 
+        [5] https://xhr.spec.whatwg.org/#concept-event-fire-progress
+
 2019-04-02  Devin Rousso  <[email protected]>
 
         Web Inspector: Canvas: add support for showing WebGPU contexts

Modified: trunk/Source/WebCore/xml/XMLHttpRequest.cpp (243764 => 243765)


--- trunk/Source/WebCore/xml/XMLHttpRequest.cpp	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.cpp	2019-04-02 23:00:47 UTC (rev 243765)
@@ -626,7 +626,7 @@
     if (m_async) {
         m_progressEventThrottle.dispatchProgressEvent(eventNames().loadstartEvent);
         if (!m_uploadComplete && m_uploadListenerFlag)
-            m_upload->dispatchProgressEvent(eventNames().loadstartEvent);
+            m_upload->dispatchProgressEvent(eventNames().loadstartEvent, 0, request.httpBody()->lengthInBytes());
 
         if (readyState() != OPENED || !m_sendFlag || m_loader)
             return { };
@@ -946,13 +946,13 @@
         return;
 
     if (m_uploadListenerFlag)
-        m_upload->dispatchThrottledProgressEvent(true, bytesSent, totalBytesToBeSent);
+        m_upload->dispatchProgressEvent(eventNames().progressEvent, bytesSent, totalBytesToBeSent);
 
     if (bytesSent == totalBytesToBeSent && !m_uploadComplete) {
         m_uploadComplete = true;
         if (m_uploadListenerFlag) {
-            m_upload->dispatchProgressEvent(eventNames().loadEvent);
-            m_upload->dispatchProgressEvent(eventNames().loadendEvent);
+            m_upload->dispatchProgressEvent(eventNames().loadEvent, bytesSent, totalBytesToBeSent);
+            m_upload->dispatchProgressEvent(eventNames().loadendEvent, bytesSent, totalBytesToBeSent);
         }
     }
 }
@@ -1047,6 +1047,13 @@
     if (!m_error) {
         m_receivedLength += len;
 
+        if (readyState() != LOADING)
+            changeState(LOADING);
+        else {
+            // Firefox calls readyStateChanged every time it receives data, 4449442
+            callReadyStateChangeListener();
+        }
+
         if (m_async) {
             long long expectedLength = m_response.expectedContentLength();
             bool lengthComputable = expectedLength > 0 && m_receivedLength <= expectedLength;
@@ -1053,12 +1060,6 @@
             unsigned long long total = lengthComputable ? expectedLength : 0;
             m_progressEventThrottle.dispatchThrottledProgressEvent(lengthComputable, m_receivedLength, total);
         }
-
-        if (readyState() != LOADING)
-            changeState(LOADING);
-        else
-            // Firefox calls readyStateChanged every time it receives data, 4449442
-            callReadyStateChangeListener();
     }
 }
 
@@ -1067,8 +1068,8 @@
     if (!m_uploadComplete) {
         m_uploadComplete = true;
         if (m_upload && m_uploadListenerFlag) {
-            m_upload->dispatchProgressEvent(type);
-            m_upload->dispatchProgressEvent(eventNames().loadendEvent);
+            m_upload->dispatchProgressEvent(type, 0, 0);
+            m_upload->dispatchProgressEvent(eventNames().loadendEvent, 0, 0);
         }
     }
     m_progressEventThrottle.dispatchProgressEvent(type);

Modified: trunk/Source/WebCore/xml/XMLHttpRequestUpload.cpp (243764 => 243765)


--- trunk/Source/WebCore/xml/XMLHttpRequestUpload.cpp	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/Source/WebCore/xml/XMLHttpRequestUpload.cpp	2019-04-02 23:00:47 UTC (rev 243765)
@@ -38,26 +38,12 @@
 {
 }
 
-void XMLHttpRequestUpload::dispatchThrottledProgressEvent(bool lengthComputable, unsigned long long loaded, unsigned long long total)
+void XMLHttpRequestUpload::dispatchProgressEvent(const AtomicString& type, unsigned long long loaded, unsigned long long total)
 {
-    m_lengthComputable = lengthComputable;
-    m_loaded = loaded;
-    m_total = total;
-
-    dispatchEvent(XMLHttpRequestProgressEvent::create(eventNames().progressEvent, lengthComputable, loaded, total));
-}
-
-void XMLHttpRequestUpload::dispatchProgressEvent(const AtomicString& type)
-{
     ASSERT(type == eventNames().loadstartEvent || type == eventNames().progressEvent || type == eventNames().loadEvent || type == eventNames().loadendEvent || type == eventNames().abortEvent || type == eventNames().errorEvent || type == eventNames().timeoutEvent);
 
-    if (type == eventNames().loadstartEvent) {
-        m_lengthComputable = false;
-        m_loaded = 0;
-        m_total = 0;
-    }
-
-    dispatchEvent(XMLHttpRequestProgressEvent::create(type, m_lengthComputable, m_loaded, m_total));
+    // https://xhr.spec.whatwg.org/#firing-events-using-the-progressevent-interface
+    dispatchEvent(XMLHttpRequestProgressEvent::create(type, !!total, loaded, total));
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/xml/XMLHttpRequestUpload.h (243764 => 243765)


--- trunk/Source/WebCore/xml/XMLHttpRequestUpload.h	2019-04-02 22:39:11 UTC (rev 243764)
+++ trunk/Source/WebCore/xml/XMLHttpRequestUpload.h	2019-04-02 23:00:47 UTC (rev 243765)
@@ -38,8 +38,7 @@
     void ref() { m_request.ref(); }
     void deref() { m_request.deref(); }
 
-    void dispatchThrottledProgressEvent(bool lengthComputable, unsigned long long loaded, unsigned long long total);
-    void dispatchProgressEvent(const AtomicString& type);
+    void dispatchProgressEvent(const AtomicString& type, unsigned long long loaded, unsigned long long total);
 
 private:
     void refEventTarget() final { ref(); }
@@ -49,9 +48,6 @@
     ScriptExecutionContext* scriptExecutionContext() const final { return m_request.scriptExecutionContext(); }
 
     XMLHttpRequest& m_request;
-    bool m_lengthComputable { false };
-    unsigned long long m_loaded { 0 };
-    unsigned long long m_total { 0 };
 };
     
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to