Chrome 18 fails html5test.com XHR Blob response test
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 3 Mar 2012 00:57:10 +0000 (00:57 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 3 Mar 2012 00:57:10 +0000 (00:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=76760

Reviewed by Adam Barth.

Source/WebCore:

Most of the code was already there, this just fixes the FIXME
which was causing this feature not to work.  Chromium already
had this #ifdef enabled, but other ports (at least Mac) do not.

Test: fast/files/xhr-response-blob.html

* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::didFinishLoading):

LayoutTests:

* fast/files/script-tests/xhr-response-blob.js: Added.
(xhr.onreadystatechange):
* fast/files/xhr-response-blob-expected.txt: Added.
* fast/files/xhr-response-blob.html: Added.
* platform/chromium/fast/files/xhr-response-blob-expected.txt: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@109635 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/files/script-tests/xhr-response-blob.js [new file with mode: 0644]
LayoutTests/fast/files/xhr-response-blob-expected.txt [new file with mode: 0644]
LayoutTests/fast/files/xhr-response-blob.html [new file with mode: 0644]
LayoutTests/platform/chromium/fast/files/xhr-response-blob-expected.txt [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/xml/XMLHttpRequest.cpp
Source/WebCore/xml/XMLHttpRequest.h

index 6d11e8b..32e301c 100644 (file)
@@ -1,3 +1,16 @@
+2012-03-01  Eric Seidel  <eric@webkit.org>
+
+        Chrome 18 fails html5test.com XHR Blob response test
+        https://bugs.webkit.org/show_bug.cgi?id=76760
+
+        Reviewed by Adam Barth.
+
+        * fast/files/script-tests/xhr-response-blob.js: Added.
+        (xhr.onreadystatechange):
+        * fast/files/xhr-response-blob-expected.txt: Added.
+        * fast/files/xhr-response-blob.html: Added.
+        * platform/chromium/fast/files/xhr-response-blob-expected.txt: Added.
+
 2012-03-02  Adam Klein  <adamk@chromium.org>
 
         Unreviewed test expectations cleanup.
diff --git a/LayoutTests/fast/files/script-tests/xhr-response-blob.js b/LayoutTests/fast/files/script-tests/xhr-response-blob.js
new file mode 100644 (file)
index 0000000..1b093e4
--- /dev/null
@@ -0,0 +1,30 @@
+description("Test that XHR.responseType = 'blob' gives you back a Blob.");
+
+if (window.layoutTestController)
+    layoutTestController.waitUntilDone();
+
+function testBlob(blobURL, blobType, doneFunction) {
+    window.xhr = new XMLHttpRequest();
+    xhr.open("GET", blobURL);
+    xhr.responseType = "blob";
+    shouldBeEqualToString("xhr.responseType", "blob");
+    xhr.send();
+    xhr.onreadystatechange = function() {
+        if (xhr.readyState != 4) {
+            shouldBeNull("xhr.response");
+            return;
+        }
+        shouldBeTrue("xhr.response instanceof Blob");
+        shouldBeEqualToString("xhr.response.type", blobType);
+        doneFunction();
+    }
+}
+
+testBlob("script-tests/xhr-response-blob.js", "text/javascript", function() {
+    testBlob("resources/does_not_exist.txt", "", function() {
+        testBlob("resources/empty-file", "", function() {
+            if (window.layoutTestController)
+                layoutTestController.notifyDone();
+        })
+    })
+});
diff --git a/LayoutTests/fast/files/xhr-response-blob-expected.txt b/LayoutTests/fast/files/xhr-response-blob-expected.txt
new file mode 100644 (file)
index 0000000..3921ccc
--- /dev/null
@@ -0,0 +1,51 @@
+Test that XHR.responseType = 'blob' gives you back a Blob.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+FAIL xhr.responseType should be blob. Was .
+PASS successfullyParsed is true
+
+TEST COMPLETE
+FAIL xhr.response should be null (of type object). Was  (of type string).
+FAIL xhr.response should be null (of type object). Was description("Test that XHR.responseType = 'blob' gives you back a Blob.");
+
+if (window.layoutTestController)
+    layoutTestController.waitUntilDone();
+
+function testBlob(blobURL, blobType, doneFunction) {
+    window.xhr = new XMLHttpRequest();
+    xhr.open("GET", blobURL);
+    xhr.responseType = "blob";
+    shouldBeEqualToString("xhr.responseType", "blob");
+    xhr.send();
+    xhr.onreadystatechange = function() {
+        if (xhr.readyState != 4) {
+            shouldBeNull("xhr.response");
+            return;
+        }
+        shouldBeTrue("xhr.response instanceof Blob");
+        shouldBeEqualToString("xhr.response.type", blobType);
+        doneFunction();
+    }
+}
+
+testBlob("script-tests/xhr-response-blob.js", "text/javascript", function() {
+    testBlob("resources/does_not_exist.txt", "", function() {
+        testBlob("resources/empty-file", "", function() {
+            if (window.layoutTestController)
+                layoutTestController.notifyDone();
+        })
+    })
+});
+ (of type string).
+FAIL xhr.response instanceof Blob should be true. Was false.
+FAIL xhr.response.type should be text/javascript (of type string). Was undefined (of type undefined).
+FAIL xhr.responseType should be blob. Was .
+FAIL xhr.response instanceof Blob should be true. Was false.
+FAIL xhr.response.type should be  (of type string). Was undefined (of type undefined).
+FAIL xhr.responseType should be blob. Was .
+FAIL xhr.response should be null (of type object). Was  (of type string).
+FAIL xhr.response instanceof Blob should be true. Was false.
+FAIL xhr.response.type should be  (of type string). Was undefined (of type undefined).
+
diff --git a/LayoutTests/fast/files/xhr-response-blob.html b/LayoutTests/fast/files/xhr-response-blob.html
new file mode 100644 (file)
index 0000000..48e5ec9
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/xhr-response-blob.js"></script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/platform/chromium/fast/files/xhr-response-blob-expected.txt b/LayoutTests/platform/chromium/fast/files/xhr-response-blob-expected.txt
new file mode 100644 (file)
index 0000000..c265f65
--- /dev/null
@@ -0,0 +1,21 @@
+Test that XHR.responseType = 'blob' gives you back a Blob.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS xhr.responseType is "blob"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS xhr.response is null
+PASS xhr.response is null
+PASS xhr.response instanceof Blob is true
+PASS xhr.response.type is "text/javascript"
+PASS xhr.responseType is "blob"
+PASS xhr.response instanceof Blob is true
+PASS xhr.response.type is ""
+PASS xhr.responseType is "blob"
+PASS xhr.response is null
+PASS xhr.response instanceof Blob is true
+PASS xhr.response.type is ""
+
index 01dcc4a..5673138 100644 (file)
@@ -1,3 +1,19 @@
+2012-03-01  Eric Seidel  <eric@webkit.org>
+
+        Chrome 18 fails html5test.com XHR Blob response test
+        https://bugs.webkit.org/show_bug.cgi?id=76760
+
+        Reviewed by Adam Barth.
+
+        Most of the code was already there, this just fixes the FIXME
+        which was causing this feature not to work.  Chromium already
+        had this #ifdef enabled, but other ports (at least Mac) do not.
+
+        Test: fast/files/xhr-response-blob.html
+
+        * xml/XMLHttpRequest.cpp:
+        (WebCore::XMLHttpRequest::didFinishLoading):
+
 2012-03-02  Kentaro Hara  <haraken@chromium.org>
 
         Revert Worker-related APIs from DOMWindowWorker.idl back to DOMWindow.idl
index 7ed8212..57e73ce 100644 (file)
@@ -23,6 +23,7 @@
 #include "XMLHttpRequest.h"
 
 #include "Blob.h"
+#include "BlobData.h"
 #include "ContentSecurityPolicy.h"
 #include "CrossOriginAccessControl.h"
 #include "DOMFormData.h"
@@ -262,12 +263,37 @@ Document* XMLHttpRequest::responseXML(ExceptionCode& ec)
 }
 
 #if ENABLE(XHR_RESPONSE_BLOB)
-Blob* XMLHttpRequest::responseBlob(ExceptionCode& ec) const
+Blob* XMLHttpRequest::responseBlob(ExceptionCode& ec)
 {
     if (m_responseTypeCode != ResponseTypeBlob) {
         ec = INVALID_STATE_ERR;
         return 0;
     }
+    // We always return null before DONE.
+    if (m_state != DONE)
+        return 0;
+
+    if (!m_responseBlob.get()) {
+        // FIXME: This causes two (or more) unnecessary copies of the data.
+        // Chromium stores blob data in the browser process, so we're pulling the data
+        // from the network only to copy it into the renderer to copy it back to the browser.
+        // Ideally we'd get the blob/file-handle from the ResourceResponse directly
+        // instead of copying the bytes. Embedders who store blob data in the
+        // same process as WebCore would at least to teach BlobData to take
+        // a SharedBuffer, even if they don't get the Blob from the network layer directly.
+        OwnPtr<BlobData> blobData = BlobData::create();
+        // If we errored out or got no data, we still return a blob, just an empty one.
+        if (m_binaryResponseBuilder.get()) {
+            RefPtr<RawData> rawData = RawData::create();
+            size_t size = m_binaryResponseBuilder->size();
+            rawData->mutableData()->append(m_binaryResponseBuilder->data(), size);
+            blobData->appendData(rawData, 0, BlobDataItem::toEndOfFile);
+            blobData->setContentType(responseMIMEType()); // responseMIMEType defaults to text/xml which may be incorrect.
+            m_binaryResponseBuilder.clear();
+        }
+        m_responseBlob = Blob::create(blobData.release(), m_binaryResponseBuilder.get() ? m_binaryResponseBuilder->size() : 0);
+    }
+
     return m_responseBlob.get();
 }
 #endif
@@ -287,10 +313,7 @@ ArrayBuffer* XMLHttpRequest::responseArrayBuffer(ExceptionCode& ec)
         m_binaryResponseBuilder.clear();
     }
 
-    if (m_responseArrayBuffer.get())
-        return m_responseArrayBuffer.get();
-
-    return 0;
+    return m_responseArrayBuffer.get();
 }
 
 void XMLHttpRequest::setResponseType(const String& responseType, ExceptionCode& ec)
@@ -1033,10 +1056,6 @@ void XMLHttpRequest::didFinishLoading(unsigned long identifier, double)
 
     m_responseBuilder.shrinkToFit();
 
-#if ENABLE(XHR_RESPONSE_BLOB)
-    // FIXME: Set m_responseBlob to something here in the ResponseTypeBlob case.
-#endif
-
     InspectorInstrumentation::resourceRetrievedByXMLHttpRequest(scriptExecutionContext(), identifier, m_responseBuilder.toStringPreserveCapacity(), m_url, m_lastSendURL, m_lastSendLineNumber);
 
     bool hadLoader = m_loader;
@@ -1106,7 +1125,11 @@ void XMLHttpRequest::didReceiveData(const char* data, int len)
 
     if (useDecoder)
         m_responseBuilder.append(m_decoder->decode(data, len));
-    else if (m_responseTypeCode == ResponseTypeArrayBuffer) {
+    else if (m_responseTypeCode == ResponseTypeArrayBuffer
+#if ENABLE(XHR_RESPONSE_BLOB)
+             || m_responseTypeCode == ResponseTypeBlob
+#endif
+             ) {
         // Buffer binary data.
         if (!m_binaryResponseBuilder)
             m_binaryResponseBuilder = SharedBuffer::create();
index 26ee65c..1373fe7 100644 (file)
@@ -106,7 +106,7 @@ public:
     Document* responseXML(ExceptionCode&);
     Document* optionalResponseXML() const { return m_responseDocument.get(); }
 #if ENABLE(XHR_RESPONSE_BLOB)
-    Blob* responseBlob(ExceptionCode&) const;
+    Blob* responseBlob(ExceptionCode&);
     Blob* optionalResponseBlob() const { return m_responseBlob.get(); }
 #endif