Unreviewed, rolling out r243551.

Seems to have broken file uploads to SoundCloud Reverted changeset: "XMLHttpRequestUpload's loadstart event not correct initialized" https://bugs.webkit.org/show_bug.cgi?id=196174 https://trac.webkit.org/changeset/243551 git-svn-id: http://svn.webkit.org/repository/webkit/trunk@243756 268f45cc-cd09-0410-ab3c-d52691b4dbfc
caiolima · Apr 2, 2019 · 1702b14 · 1702b14
1 parent 0f8bd18
commit 1702b14
Show file tree

Hide file tree

Showing 15 changed files with 82 additions and 47 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2019-04-02  Chris Dumez  <cdumez@apple.com>
+
+        Unreviewed, rolling out r243551.
+
+        Seems to have broken file uploads to SoundCloud
+
+        Reverted changeset:
+
+        "XMLHttpRequestUpload's loadstart event not correct
+        initialized"
+        https://bugs.webkit.org/show_bug.cgi?id=196174
+        https://trac.webkit.org/changeset/243551
+
 2019-04-02  Justin Fan  <justin_fan@apple.com>
 
         [Web GPU] Implement blend states and color write mask for  GPUColorStateDescriptor

diff --git a/LayoutTests/http/tests/xmlhttprequest/upload-onabort-progressevent-attributes.html b/LayoutTests/http/tests/xmlhttprequest/upload-onabort-progressevent-attributes.html
@@ -26,15 +26,6 @@
             + "(" + 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);
@@ -59,13 +50,13 @@
     var req = new XMLHttpRequest();
     req.upload.onerror = onUnexpectedProgressEvent;
     req.upload.onload = onUnexpectedProgressEvent;
-    req.upload.onabort = onErrorProgressEvent;
+    req.upload.onabort = onProgressEvent;
     req.upload.onprogress = function(e) {
         onProgressEvent(e);
         req.abort();
     }
     req.upload.onloadend = function(e) {
-        onErrorProgressEvent(e);
+        onProgressEvent(e);
         completeTest();
     }
 

diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
@@ -1,3 +1,16 @@
+2019-04-02  Chris Dumez  <cdumez@apple.com>
+
+        Unreviewed, rolling out r243551.
+
+        Seems to have broken file uploads to SoundCloud
+
+        Reverted changeset:
+
+        "XMLHttpRequestUpload's loadstart event not correct
+        initialized"
+        https://bugs.webkit.org/show_bug.cgi?id=196174
+        https://trac.webkit.org/changeset/243551
+
 2019-04-01  Chris Dumez  <cdumez@apple.com>
 
         Attr nodes are not cloned properly

diff --git a/LayoutTests/imported/w3c/web-platform-tests/xhr/abort-during-upload-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/xhr/abort-during-upload-expected.txt
@@ -1,3 +1,3 @@
 
-PASS XMLHttpRequest: abort() while sending data 
+FAIL XMLHttpRequest: abort() while sending data assert_equals: expected "upload.loadstart(0,9999,true)" but got "upload.loadstart(0,0,false)"
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt
@@ -1,4 +1,4 @@
 Blocked access to external URL http://nonexistent.localhost:8800/
 
-PASS XMLHttpRequest: event - error (order of events) 
+FAIL XMLHttpRequest: event - error (order of events) assert_equals: expected "upload.loadstart(0,12,true)" but got "upload.loadstart(0,0,false)"
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub.html b/LayoutTests/imported/w3c/web-platform-tests/xhr/event-error-order.sub.html
@@ -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)", 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)", 2, 4, "upload.error(0,0,false)", "upload.loadend(0,0,false)", "error(0,0,false)", "loadend(0,0,false)"]);
                     test.done();
                 });
             });

diff --git a/LayoutTests/imported/w3c/web-platform-tests/xhr/event-loadstart-upload-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/xhr/event-loadstart-upload-expected.txt
@@ -1,3 +1,3 @@
 
-PASS XMLHttpRequest: The send() method: Fire a progress event named loadstart on upload object (synchronous flag is unset) 
+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
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/xhr/event-timeout-order-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/xhr/event-timeout-order-expected.txt
@@ -1,3 +1,3 @@
 
-PASS XMLHttpRequest: event - timeout (order of events) 
+FAIL XMLHttpRequest: event - timeout (order of events) assert_equals: expected "upload.loadstart(0,12,true)" but got "upload.loadstart(0,0,false)"
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/xhr/send-response-event-order-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/xhr/send-response-event-order-expected.txt
@@ -1,3 +1,3 @@
 
-PASS XMLHttpRequest: The send() method: event order when synchronous flag is unset 
+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)"
 
diff --git a/...s/platform/mac-wk1/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt b/...s/platform/mac-wk1/imported/w3c/web-platform-tests/xhr/event-error-order.sub-expected.txt
@@ -1,3 +1,3 @@
 
-PASS XMLHttpRequest: event - error (order of events) 
+FAIL XMLHttpRequest: event - error (order of events) assert_equals: expected "upload.loadstart(0,12,true)" but got "upload.loadstart(0,0,false)"
 
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2019-04-02  Chris Dumez  <cdumez@apple.com>
+
+        Unreviewed, rolling out r243551.
+
+        Seems to have broken file uploads to SoundCloud
+
+        Reverted changeset:
+
+        "XMLHttpRequestUpload's loadstart event not correct
+        initialized"
+        https://bugs.webkit.org/show_bug.cgi?id=196174
+        https://trac.webkit.org/changeset/243551
+
 2019-04-02  Justin Fan  <justin_fan@apple.com>
 
         [Web GPU] Implement blend states and color write mask for GPUColorStateDescriptor

diff --git a/Source/WebCore/xml/XMLHttpRequest.cpp b/Source/WebCore/xml/XMLHttpRequest.cpp
@@ -626,7 +626,7 @@ ExceptionOr<void> XMLHttpRequest::createRequest()
     if (m_async) {
         m_progressEventThrottle.dispatchProgressEvent(eventNames().loadstartEvent);
         if (!m_uploadComplete && m_uploadListenerFlag)
-            m_upload->dispatchProgressEvent(eventNames().loadstartEvent, 0, request.httpBody()->lengthInBytes());
+            m_upload->dispatchProgressEvent(eventNames().loadstartEvent);
 
         if (readyState() != OPENED || !m_sendFlag || m_loader)
             return { };
@@ -714,7 +714,6 @@ bool XMLHttpRequest::internalAbort()
 void XMLHttpRequest::clearResponse()
 {
     m_response = ResourceResponse();
-    m_didReceiveResponseHandler = nullptr;
     clearResponseBuffers();
 }
 
@@ -946,31 +945,21 @@ void XMLHttpRequest::didSendData(unsigned long long bytesSent, unsigned long lon
     if (!m_upload)
         return;
 
-    if (m_response.isNull()) {
-        // We should not notify of progress until we've received a response from the server.
-        m_didReceiveResponseHandler = [this, bytesSent, totalBytesToBeSent] {
-            didSendData(bytesSent, totalBytesToBeSent);
-        };
-        return;
-    }
-
     if (m_uploadListenerFlag)
-        m_upload->dispatchProgressEvent(eventNames().progressEvent, bytesSent, totalBytesToBeSent);
+        m_upload->dispatchThrottledProgressEvent(true, bytesSent, totalBytesToBeSent);
 
     if (bytesSent == totalBytesToBeSent && !m_uploadComplete) {
         m_uploadComplete = true;
         if (m_uploadListenerFlag) {
-            m_upload->dispatchProgressEvent(eventNames().loadEvent, bytesSent, totalBytesToBeSent);
-            m_upload->dispatchProgressEvent(eventNames().loadendEvent, bytesSent, totalBytesToBeSent);
+            m_upload->dispatchProgressEvent(eventNames().loadEvent);
+            m_upload->dispatchProgressEvent(eventNames().loadendEvent);
         }
     }
 }
 
 void XMLHttpRequest::didReceiveResponse(unsigned long, const ResourceResponse& response)
 {
     m_response = response;
-    if (auto didReceiveResponseHandler = WTFMove(m_didReceiveResponseHandler))
-        didReceiveResponseHandler();
 }
 
 static inline bool shouldDecodeResponse(XMLHttpRequest::ResponseType type)
@@ -1058,19 +1047,18 @@ void XMLHttpRequest::didReceiveData(const char* data, int len)
     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;
             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();
     }
 }
 
@@ -1079,8 +1067,8 @@ void XMLHttpRequest::dispatchErrorEvents(const AtomicString& type)
     if (!m_uploadComplete) {
         m_uploadComplete = true;
         if (m_upload && m_uploadListenerFlag) {
-            m_upload->dispatchProgressEvent(type, 0, 0);
-            m_upload->dispatchProgressEvent(eventNames().loadendEvent, 0, 0);
+            m_upload->dispatchProgressEvent(type);
+            m_upload->dispatchProgressEvent(eventNames().loadendEvent);
         }
     }
     m_progressEventThrottle.dispatchProgressEvent(type);

diff --git a/Source/WebCore/xml/XMLHttpRequest.h b/Source/WebCore/xml/XMLHttpRequest.h
@@ -218,7 +218,6 @@ class XMLHttpRequest final : public ActiveDOMObject, public RefCounted<XMLHttpRe
     String m_responseEncoding;
 
     ResourceResponse m_response;
-    Function<void()> m_didReceiveResponseHandler;
 
     RefPtr<TextResourceDecoder> m_decoder;
 

diff --git a/Source/WebCore/xml/XMLHttpRequestUpload.cpp b/Source/WebCore/xml/XMLHttpRequestUpload.cpp
@@ -38,12 +38,26 @@ XMLHttpRequestUpload::XMLHttpRequestUpload(XMLHttpRequest& request)
 {
 }
 
-void XMLHttpRequestUpload::dispatchProgressEvent(const AtomicString& type, unsigned long long loaded, unsigned long long total)
+void XMLHttpRequestUpload::dispatchThrottledProgressEvent(bool lengthComputable, 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);
 
-    // https://xhr.spec.whatwg.org/#firing-events-using-the-progressevent-interface
-    dispatchEvent(XMLHttpRequestProgressEvent::create(type, !!total, loaded, total));
+    if (type == eventNames().loadstartEvent) {
+        m_lengthComputable = false;
+        m_loaded = 0;
+        m_total = 0;
+    }
+
+    dispatchEvent(XMLHttpRequestProgressEvent::create(type, m_lengthComputable, m_loaded, m_total));
 }
 
 } // namespace WebCore
diff --git a/Source/WebCore/xml/XMLHttpRequestUpload.h b/Source/WebCore/xml/XMLHttpRequestUpload.h
@@ -38,7 +38,8 @@ class XMLHttpRequestUpload final : public XMLHttpRequestEventTarget {
     void ref() { m_request.ref(); }
     void deref() { m_request.deref(); }
 
-    void dispatchProgressEvent(const AtomicString& type, unsigned long long loaded, unsigned long long total);
+    void dispatchThrottledProgressEvent(bool lengthComputable, unsigned long long loaded, unsigned long long total);
+    void dispatchProgressEvent(const AtomicString& type);
 
 private:
     void refEventTarget() final { ref(); }
@@ -48,6 +49,9 @@ class XMLHttpRequestUpload final : public XMLHttpRequestEventTarget {
     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