[XHR] Accept data on didReceiveData call even if responseType is set to Blob

Source/core/xml/XMLHttpRequest.cpp

Index: Source/core/xml/XMLHttpRequest.cpp
diff --git a/Source/core/xml/XMLHttpRequest.cpp b/Source/core/xml/XMLHttpRequest.cpp
index 7315dd9326803e09632506d68e222b40475c8194..68bd3a1d698f7ea889af1cdd92de56f4bee4ab03 100644
--- a/Source/core/xml/XMLHttpRequest.cpp
+++ b/Source/core/xml/XMLHttpRequest.cpp
@@ -146,7 +146,8 @@ XMLHttpRequest::XMLHttpRequest(ExecutionContext* context, PassRefPtr<SecurityOri
     , m_timeoutMilliseconds(0)
     , m_loaderIdentifier(0)
     , m_state(UNSENT)
-    , m_downloadedBlobLength(0)
+    , m_downloadingToFile(false)
+    , m_lengthDownloadedToFile(0)
     , m_receivedLength(0)
     , m_lastSendLineNumber(0)
     , m_exceptionCode(0)
@@ -261,29 +262,43 @@ Document* XMLHttpRequest::responseXML(ExceptionState& exceptionState)
 Blob* XMLHttpRequest::responseBlob()
 {
     ASSERT(m_responseTypeCode == ResponseTypeBlob);
-    ASSERT(!m_binaryResponseBuilder.get());
 
     // We always return null before DONE.
     if (m_error || m_state != DONE)
         return 0;
 
     if (!m_responseBlob) {
-        // When responseType is set to "blob", we redirect the downloaded data
-        // to a file-handle directly in the browser process. We get the
-        // file-path from the ResourceResponse directly instead of copying the
-        // bytes between the browser and the renderer.
         OwnPtr<BlobData> blobData = BlobData::create();
-        String filePath = m_response.downloadedFilePath();
-        // If we errored out or got no data, we still return a blob, just an
-        // empty one.
-        if (!filePath.isEmpty() && m_downloadedBlobLength) {
-            blobData->appendFile(filePath);
-            // FIXME: finalResponseMIMETypeWithFallback() defaults to text/xml
-            // which may be incorrect. Replace it with finalResponseMIMEType()
-            // after compatibility investigation.
-            blobData->setContentType(finalResponseMIMETypeWithFallback());
+        if (m_downloadingToFile) {
+            ASSERT(!m_binaryResponseBuilder.get());
+
+            // When responseType is set to "blob", we redirect the downloaded data

[yhirano, 2014/09/10 10:56:38]

Please wrap comments in 80 columns.

[tyoshino (SeeGerritForStatus), 2014/09/10 12:19:29]

Done.

+            // to a file-handle directly in the browser process. We get the
+            // file-path from the ResourceResponse directly instead of copying the
+            // bytes between the browser and the renderer.
+            String filePath = m_response.downloadedFilePath();
+            // If we errored out or got no data, we still return a blob, just an
+            // empty one.
+            if (!filePath.isEmpty() && m_lengthDownloadedToFile) {
+                blobData->appendFile(filePath);
+                // FIXME: finalResponseMIMETypeWithFallback() defaults to text/xml
+                // which may be incorrect. Replace it with finalResponseMIMEType()
+                // after compatibility investigation.
+                blobData->setContentType(finalResponseMIMETypeWithFallback());
+            }
+            m_responseBlob = Blob::create(BlobDataHandle::create(blobData.release(), m_lengthDownloadedToFile));
+        } else {
+            size_t size = 0;
+            if (m_binaryResponseBuilder.get() && m_binaryResponseBuilder->size() > 0) {
+                RefPtr<RawData> rawData = RawData::create();
+                size = m_binaryResponseBuilder->size();
+                rawData->mutableData()->append(m_binaryResponseBuilder->data(), size);
+                blobData->appendData(rawData, 0, BlobDataItem::toEndOfFile);
+                blobData->setContentType(finalResponseMIMETypeWithFallback());
+                m_binaryResponseBuilder.clear();
+            }
+            m_responseBlob = Blob::create(BlobDataHandle::create(blobData.release(), size));
         }
-        m_responseBlob = Blob::create(BlobDataHandle::create(blobData.release(), m_downloadedBlobLength));
     }
 
     return m_responseBlob.get();
@@ -854,8 +869,11 @@ void XMLHttpRequest::createRequest(PassRefPtr<FormData> httpBody, ExceptionState
     // When responseType is set to "blob", we redirect the downloaded data to a
     // file-handle directly.
     if (responseTypeCode() == ResponseTypeBlob) {
+        m_downloadingToFile = true;
         request.setDownloadToFile(true);
         resourceLoaderOptions.dataBufferingPolicy = DoNotBufferData;
+    } else {
+        m_downloadingToFile = false;
     }
 
     m_exceptionCode = 0;
@@ -982,7 +1000,9 @@ void XMLHttpRequest::clearResponse()
     m_responseDocument = nullptr;
 
     m_responseBlob = nullptr;
-    m_downloadedBlobLength = 0;
+
+    m_downloadingToFile = false;
+    m_lengthDownloadedToFile = 0;
 
     m_responseLegacyStream = nullptr;
     m_responseStream = nullptr;
@@ -1419,7 +1439,7 @@ PassOwnPtr<TextResourceDecoder> XMLHttpRequest::createDecoder() const
 
 void XMLHttpRequest::didReceiveData(const char* data, int len)
 {
-    ASSERT(m_responseTypeCode != ResponseTypeBlob);
+    ASSERT(!m_downloadingToFile);
 
     if (m_error)
         return;
@@ -1440,7 +1460,7 @@ void XMLHttpRequest::didReceiveData(const char* data, int len)
             m_decoder = createDecoder();
 
         m_responseText = m_responseText.concatenateWith(m_decoder->decode(data, len));
-    } else if (m_responseTypeCode == ResponseTypeArrayBuffer) {
+    } else if (m_responseTypeCode == ResponseTypeArrayBuffer || m_responseTypeCode == ResponseTypeBlob) {
         // Buffer binary data.
         if (!m_binaryResponseBuilder)
             m_binaryResponseBuilder = SharedBuffer::create();
@@ -1465,7 +1485,7 @@ void XMLHttpRequest::didReceiveData(const char* data, int len)
 
 void XMLHttpRequest::didDownloadData(int dataLength)
 {
-    ASSERT(m_responseTypeCode == ResponseTypeBlob);
+    ASSERT(m_downloadingToFile);
 
     if (m_error)

[yhirano, 2014/09/10 10:56:38]

I'm not sure when the condition holds, but Shouldn't we omit the above assertion
when m_error is true?

[tyoshino (SeeGerritForStatus), 2014/09/10 12:19:29]

m_responseTypeCode check was working. So m_downloadingToFile should also work.

RedirectToFileResourceHandler is set up only when downloadToFile is specified.
There's no path (even error handling) where didDownloadData can be called
regardless of downloadToFile option.

[yhirano, 2014/09/10 12:29:26]

m_downloadingToFile is cleared in clearResponse while m_responseTypeCode isn't.
Hence the first statement is not generally true.

[tyoshino (SeeGerritForStatus), 2014/09/10 12:48:29]

Oh, good point! clearResponse() is basically preceded by internalAbort(). In
handleDidCancel() it isn't but in this case the loader is already stopped. So I
think we don't need to be worried about didDownloadData being called after we
reset m_downloadingToFile.

But I'm fine with putting this after m_error check. Done.

         return;
@@ -1481,7 +1501,7 @@ void XMLHttpRequest::didDownloadData(int dataLength)
     if (m_error)
         return;
 
-    m_downloadedBlobLength += dataLength;
+    m_lengthDownloadedToFile += dataLength;
 
     trackProgress(dataLength);
 }