Changeset 243551 in webkit


Ignore:
Timestamp:
Mar 27, 2019 11:10:51 AM (5 years ago)
Author:
Chris Dumez
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

Location:
trunk
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/http/tests/xmlhttprequest/upload-onabort-progressevent-attributes.html

    r161668 r243551  
    2727}
    2828
     29function onErrorProgressEvent(e)
     30{
     31    if (e.lengthComputable)
     32        fail("Event " + e.type + " lengthComputable is true");
     33    if (e.total != 0 || e.loaded != 0)
     34        fail("Event " + e.type + " total/loaded values not matching: "
     35            + "(" + e.loaded + " / " + e.total + "), expected (0 / 0)");
     36}
     37
    2938function onUnexpectedProgressEvent(e)
    3039{
    … …  
    5160    req.upload.onerror = onUnexpectedProgressEvent;
    5261    req.upload.onload = onUnexpectedProgressEvent;
    53     req.upload.onabort = onProgressEvent;
     62    req.upload.onabort = onErrorProgressEvent;
    5463    req.upload.onprogress = function(e) {
    5564        onProgressEvent(e);
    … …  
    5766    }
    5867    req.upload.onloadend = function(e) {
    59         onProgressEvent(e);
     68        onErrorProgressEvent(e);
    6069        completeTest();
    6170    }

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r243437 r243551  
     12019-03-27  Chris Dumez  <cdumez@apple.com>
     2
     3        XMLHttpRequestUpload's loadstart event not correct initialized
     4        https://bugs.webkit.org/show_bug.cgi?id=196174
     5        <rdar://problem/49191412>
     6
     7        Reviewed by Alex Christensen.
     8
     9        * web-platform-tests/xhr/event-error-order.sub.html:
     10        Update test after https://github.com/web-platform-tests/wpt/pull/13365
     11
     12        * web-platform-tests/xhr/abort-during-upload-expected.txt:
     13        * web-platform-tests/xhr/event-error-order.sub-expected.txt:
     14        * web-platform-tests/xhr/event-loadstart-upload-expected.txt:
     15        * web-platform-tests/xhr/event-timeout-order-expected.txt:
     16        * web-platform-tests/xhr/send-response-event-order-expected.txt:
     17        Rebaseline several WPT tests that are now passing.
     18
    1192019-03-25  Javier Fernandez  <jfernandez@igalia.com>
    220

  • trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/abort-during-upload-expected.txt

    r235354 r243551  
    11
    2 FAIL XMLHttpRequest: abort() while sending data assert_equals: expected "upload.loadstart(0,9999,true)" but got "upload.loadstart(0,0,false)"
     2PASS XMLHttpRequest: abort() while sending data
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt

    r235354 r243551  
    11Blocked access to external URL http://nonexistent.localhost:8800/
    22
    3 FAIL XMLHttpRequest: event - error (order of events) assert_equals: expected "upload.loadstart(0,12,true)" but got "upload.loadstart(0,0,false)"
     3PASS XMLHttpRequest: event - error (order of events)
    44

  • trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub.html

    r235354 r243551  
    2323                test.step(function() {
    2424                    // no progress events due to CORS failure
    25                     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)"]);
     25                    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)"]);
    2626                    test.done();
    2727                });

  • trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-loadstart-upload-expected.txt

    r235354 r243551  
    11
    2 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
     2PASS XMLHttpRequest: The send() method: Fire a progress event named loadstart on upload object (synchronous flag is unset)
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/event-timeout-order-expected.txt

    r235354 r243551  
    11
    2 FAIL XMLHttpRequest: event - timeout (order of events) assert_equals: expected "upload.loadstart(0,12,true)" but got "upload.loadstart(0,0,false)"
     2PASS XMLHttpRequest: event - timeout (order of events)
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/xhr/send-response-event-order-expected.txt

    r235354 r243551  
    11
    2 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)"
     2PASS XMLHttpRequest: The send() method: event order when synchronous flag is unset
    33

  • trunk/LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt

    r235354 r243551  
    11
    2 FAIL XMLHttpRequest: event - error (order of events) assert_equals: expected "upload.loadstart(0,12,true)" but got "upload.loadstart(0,0,false)"
     2PASS XMLHttpRequest: event - error (order of events)
    33

  • trunk/Source/WebCore/ChangeLog

    r243550 r243551  
     12019-03-27  Chris Dumez  <cdumez@apple.com>
     2
     3        XMLHttpRequestUpload's loadstart event not correct initialized
     4        https://bugs.webkit.org/show_bug.cgi?id=196174
     5        <rdar://problem/49191412>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Align progress event firing with the XHR specification.
     10
     11        No new tests, rebaselined existing tests.
     12
     13        * xml/XMLHttpRequest.cpp:
     14        (WebCore::XMLHttpRequest::createRequest):
     15        As per [1], the loadstart event fired on the XMLHttpRequestUpload object should use
     16        loaded=0 and total=`req’s body’s total bytes`.
     17        [1] https://xhr.spec.whatwg.org/#the-send()-method (step 11.2.)
     18
     19        (WebCore::XMLHttpRequest::didSendData):
     20        As per [2], the progress / load / loadend should use loaded=transmitted and total=length.
     21        [2] https://xhr.spec.whatwg.org/#ref-for-process-request-end-of-body (steps 5, 6 and 7)
     22
     23        (WebCore::XMLHttpRequest::didReceiveData):
     24        As per [3], we should fire the readystatechange event *before* the progress event.
     25        This is covered by web-platform-tests/xhr/send-response-event-order.htm which was failing
     26        differently after the other changes in this patch.
     27        [3] https://xhr.spec.whatwg.org/#ref-for-process-response (steps 9.4 and 9.5)
     28
     29        (WebCore::XMLHttpRequest::dispatchErrorEvents):
     30        As per [4], in case of an error, we should fire the provided 'event' and 'loadend' with
     31        loaded=0 and total=0.
     32        [4] https://xhr.spec.whatwg.org/#request-error-steps (steps 7 and 8)
     33
     34        * xml/XMLHttpRequestUpload.cpp:
     35        (WebCore::XMLHttpRequestUpload::dispatchProgressEvent):
     36        * xml/XMLHttpRequestUpload.h:
     37        Simplify XMLHttpRequestUpload. It no longer needs to store loaded / total as data
     38        members now that they are always passed by the call site. lengthComputable is set
     39        to !!total as [5] says to set it to true if length/total is not 0.
     40        [5] https://xhr.spec.whatwg.org/#concept-event-fire-progress
     41
    1422019-03-27  Simon Fraser  <simon.fraser@apple.com>
    243

  • trunk/Source/WebCore/xml/XMLHttpRequest.cpp

    r242284 r243551  
    627627        m_progressEventThrottle.dispatchProgressEvent(eventNames().loadstartEvent);
    628628        if (!m_uploadComplete && m_uploadListenerFlag)
    629             m_upload->dispatchProgressEvent(eventNames().loadstartEvent);
     629            m_upload->dispatchProgressEvent(eventNames().loadstartEvent, 0, request.httpBody()->lengthInBytes());
    630630
    631631        if (readyState() != OPENED || !m_sendFlag || m_loader)
    … …  
    715715{
    716716    m_response = ResourceResponse();
     717    m_didReceiveResponseHandler = nullptr;
    717718    clearResponseBuffers();
    718719}
    … …  
    946947        return;
    947948
     949    if (m_response.isNull()) {
     950        // We should not notify of progress until we've received a response from the server.
     951        m_didReceiveResponseHandler = [this, bytesSent, totalBytesToBeSent] {
     952            didSendData(bytesSent, totalBytesToBeSent);
     953        };
     954        return;
     955    }
     956
    948957    if (m_uploadListenerFlag)
    949         m_upload->dispatchThrottledProgressEvent(true, bytesSent, totalBytesToBeSent);
     958        m_upload->dispatchProgressEvent(eventNames().progressEvent, bytesSent, totalBytesToBeSent);
    950959
    951960    if (bytesSent == totalBytesToBeSent && !m_uploadComplete) {
    952961        m_uploadComplete = true;
    953962        if (m_uploadListenerFlag) {
    954             m_upload->dispatchProgressEvent(eventNames().loadEvent);
    955             m_upload->dispatchProgressEvent(eventNames().loadendEvent);
     963            m_upload->dispatchProgressEvent(eventNames().loadEvent, bytesSent, totalBytesToBeSent);
     964            m_upload->dispatchProgressEvent(eventNames().loadendEvent, bytesSent, totalBytesToBeSent);
    956965        }
    957966    }
    … …  
    961970{
    962971    m_response = response;
     972    if (auto didReceiveResponseHandler = WTFMove(m_didReceiveResponseHandler))
     973        didReceiveResponseHandler();
    963974}
    964975
    … …  
    10481059        m_receivedLength += len;
    10491060
     1061        if (readyState() != LOADING)
     1062            changeState(LOADING);
     1063        else {
     1064            // Firefox calls readyStateChanged every time it receives data, 4449442
     1065            callReadyStateChangeListener();
     1066        }
     1067
    10501068        if (m_async) {
    10511069            long long expectedLength = m_response.expectedContentLength();
    … …  
    10541072            m_progressEventThrottle.dispatchThrottledProgressEvent(lengthComputable, m_receivedLength, total);
    10551073        }
    1056 
    1057         if (readyState() != LOADING)
    1058             changeState(LOADING);
    1059         else
    1060             // Firefox calls readyStateChanged every time it receives data, 4449442
    1061             callReadyStateChangeListener();
    10621074    }
    10631075}
    … …  
    10681080        m_uploadComplete = true;
    10691081        if (m_upload && m_uploadListenerFlag) {
    1070             m_upload->dispatchProgressEvent(type);
    1071             m_upload->dispatchProgressEvent(eventNames().loadendEvent);
     1082            m_upload->dispatchProgressEvent(type, 0, 0);
     1083            m_upload->dispatchProgressEvent(eventNames().loadendEvent, 0, 0);
    10721084        }
    10731085    }

  • trunk/Source/WebCore/xml/XMLHttpRequest.h

    r239427 r243551  
    219219
    220220    ResourceResponse m_response;
     221    Function<void()> m_didReceiveResponseHandler;
    221222
    222223    RefPtr<TextResourceDecoder> m_decoder;

  • trunk/Source/WebCore/xml/XMLHttpRequestUpload.cpp

    r231488 r243551  
    3939}
    4040
    41 void XMLHttpRequestUpload::dispatchThrottledProgressEvent(bool lengthComputable, unsigned long long loaded, unsigned long long total)
    42 {
    43     m_lengthComputable = lengthComputable;
    44     m_loaded = loaded;
    45     m_total = total;
    46 
    47     dispatchEvent(XMLHttpRequestProgressEvent::create(eventNames().progressEvent, lengthComputable, loaded, total));
    48 }
    49 
    50 void XMLHttpRequestUpload::dispatchProgressEvent(const AtomicString& type)
     41void XMLHttpRequestUpload::dispatchProgressEvent(const AtomicString& type, unsigned long long loaded, unsigned long long total)
    5142{
    5243    ASSERT(type == eventNames().loadstartEvent || type == eventNames().progressEvent || type == eventNames().loadEvent || type == eventNames().loadendEvent || type == eventNames().abortEvent || type == eventNames().errorEvent || type == eventNames().timeoutEvent);
    5344
    54     if (type == eventNames().loadstartEvent) {
    55         m_lengthComputable = false;
    56         m_loaded = 0;
    57         m_total = 0;
    58     }
    59 
    60     dispatchEvent(XMLHttpRequestProgressEvent::create(type, m_lengthComputable, m_loaded, m_total));
     45    // https://xhr.spec.whatwg.org/#firing-events-using-the-progressevent-interface
     46    dispatchEvent(XMLHttpRequestProgressEvent::create(type, !!total, loaded, total));
    6147}
    6248

  • trunk/Source/WebCore/xml/XMLHttpRequestUpload.h

    r231488 r243551  
    3939    void deref() { m_request.deref(); }
    4040
    41     void dispatchThrottledProgressEvent(bool lengthComputable, unsigned long long loaded, unsigned long long total);
    42     void dispatchProgressEvent(const AtomicString& type);
     41    void dispatchProgressEvent(const AtomicString& type, unsigned long long loaded, unsigned long long total);
    4342
    4443private:
    … …  
    5049
    5150    XMLHttpRequest& m_request;
    52     bool m_lengthComputable { false };
    53     unsigned long long m_loaded { 0 };
    54     unsigned long long m_total { 0 };
    5551};
    5652   
Note: See TracChangeset for help on using the changeset viewer.