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

Source/core/xml/XMLHttpRequest.cpp

 /*
  *  Copyright (C) 2004, 2006, 2008 Apple Inc. All rights reserved.
  *  Copyright (C) 2005-2007 Alexey Proskuryakov <ap@webkit.org>
  *  Copyright (C) 2007, 2008 Julien Chaffraix <jchaffraix@webkit.org>
  *  Copyright (C) 2008, 2011 Google Inc. All rights reserved.
  *  Copyright (C) 2012 Intel Corporation
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
  *  License as published by the Free Software Foundation; either
@@ skipping 128 matching lines @@
     xmlHttpRequest->suspendIfNeeded();
 
     return xmlHttpRequest.release();
 }
 
 XMLHttpRequest::XMLHttpRequest(ExecutionContext* context, PassRefPtr<SecurityOrigin> securityOrigin)
     : ActiveDOMObject(context)
     , 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)
     , m_progressEventThrottle(this)
     , m_responseTypeCode(ResponseTypeDefault)
     , m_securityOrigin(securityOrigin)
     , m_async(true)
     , m_includeCredentials(false)
     , m_parsedResponse(false)
     , m_error(false)
@@ skipping 94 matching lines @@
 
         m_parsedResponse = true;
     }
 
     return m_responseDocument.get();
 }
 
 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 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) {

[sof, 2014/09/10 14:21:39]

Can be shortened to 

  if (m_binaryResponseBuilder && m_binaryResponseBuilder->size())

[tyoshino (SeeGerritForStatus), 2014/09/11 04:30:57]

Done.

+                RefPtr<RawData> rawData = RawData::create();
+                size = m_binaryResponseBuilder->size();

[sof, 2014/09/10 14:21:39]

What happens if size_t is signed and MSB of the returned size here is set?

[sof, 2014/09/10 20:24:10]

Please disregard, a non-issue. But couldn't this buffer append be simplified to

  blobData->appendBytes(m_binaryResponseBuilder->data(),
m_binaryResponseBuilder->size());

i.e., there's now a bunch of manual uses of appendData(PassRefPtr<RawData>) in
the codebase. I think we should make appendBytes() public and use it instead.

Totally fine to handle that separately & as a follow-up, though.

[tyoshino (SeeGerritForStatus), 2014/09/11 04:30:57]

Sounds good. I'll do it

+                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();
 }
 
 ArrayBuffer* XMLHttpRequest::responseArrayBuffer()
 {
     ASSERT(m_responseTypeCode == ResponseTypeArrayBuffer);
 
     if (m_error || m_state != DONE)
@@ skipping 550 matching lines @@
 
     ResourceLoaderOptions resourceLoaderOptions;
     resourceLoaderOptions.allowCredentials = (m_sameOriginRequest || m_includeCredentials) ? AllowStoredCredentials : DoNotAllowStoredCredentials;
     resourceLoaderOptions.credentialsRequested = m_includeCredentials ? ClientRequestedCredentials : ClientDidNotRequestCredentials;
     resourceLoaderOptions.securityOrigin = securityOrigin();
     resourceLoaderOptions.mixedContentBlockingTreatment = RuntimeEnabledFeatures::laxMixedContentCheckingEnabled() ? TreatAsPassiveContent : TreatAsActiveContent;
 
     // When responseType is set to "blob", we redirect the downloaded data to a
     // file-handle directly.
     if (responseTypeCode() == ResponseTypeBlob) {
+        m_downloadingToFile = true;

[sof, 2014/09/10 14:21:39]

Move out of "if" as,

  m_downloadingToFile = responseTypeCode() == ResponseTypeBlob;
  if (m_downloadingToFile) { ... }

Goes better with the comment above.

[tyoshino (SeeGerritForStatus), 2014/09/11 04:30:57]

Nice! Done

         request.setDownloadToFile(true);
         resourceLoaderOptions.dataBufferingPolicy = DoNotBufferData;
+    } else {
+        m_downloadingToFile = false;
     }
 
     m_exceptionCode = 0;
     m_error = false;
 
     if (m_async) {
         if (m_upload)
             request.setReportUploadProgress(true);
 
         // ThreadableLoader::create can return null here, for example if we're no longer attached to a page.
@@ skipping 106 matching lines @@
     m_receivedLength = 0;
 
     m_response = ResourceResponse();
 
     m_responseText.clear();
 
     m_parsedResponse = false;
     m_responseDocument = nullptr;
 
     m_responseBlob = nullptr;
-    m_downloadedBlobLength = 0;
+
+    m_downloadingToFile = false;
+    m_lengthDownloadedToFile = 0;
 
     m_responseLegacyStream = nullptr;
     m_responseStream = nullptr;
 
     // These variables may referred by the response accessors. So, we can clear
     // this only when we clear the response holder variables above.
     m_binaryResponseBuilder.clear();
     m_responseArrayBuffer.clear();
 }
 
@@ skipping 416 matching lines @@
     }
 
     if (responseIsHTML())
         return TextResourceDecoder::create("text/html", "UTF-8");
 
     return TextResourceDecoder::create("text/plain", "UTF-8");
 }
 
 void XMLHttpRequest::didReceiveData(const char* data, int len)
 {
-    ASSERT(m_responseTypeCode != ResponseTypeBlob);
+    ASSERT(!m_downloadingToFile);
 
     if (m_error)
         return;
 
     if (m_state < HEADERS_RECEIVED)
         changeState(HEADERS_RECEIVED);
 
     if (!len)
         return;
 
     if (len == -1)
         len = strlen(data);
 
     if (m_responseTypeCode == ResponseTypeDocument && responseIsHTML()) {
         parseDocumentChunk(data, len);
     } else if (m_responseTypeCode == ResponseTypeDefault || m_responseTypeCode == ResponseTypeText || m_responseTypeCode == ResponseTypeJSON || m_responseTypeCode == ResponseTypeDocument) {
         if (!m_decoder)
             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();
         m_binaryResponseBuilder->append(data, len);
     } else if (m_responseTypeCode == ResponseTypeLegacyStream) {
         if (!m_responseLegacyStream)
             m_responseLegacyStream = Stream::create(executionContext(), responseType());
         m_responseLegacyStream->addData(data, len);
     } else if (m_responseTypeCode == ResponseTypeStream) {
         if (!m_responseStream) {
             m_responseStream = new ReadableStreamImpl<ReadableStreamChunkTypeTraits<ArrayBuffer> >(executionContext(), new ReadableStreamSource(this));
             m_responseStream->didSourceStart();
         }
         m_responseStream->enqueue(ArrayBuffer::create(data, len));
     }
 
     if (m_error)
         return;
 
     trackProgress(len);
 }
 
 void XMLHttpRequest::didDownloadData(int dataLength)
 {
-    ASSERT(m_responseTypeCode == ResponseTypeBlob);
-
     if (m_error)
         return;
 
+    ASSERT(m_downloadingToFile);
+
     if (m_state < HEADERS_RECEIVED)
         changeState(HEADERS_RECEIVED);
 
     if (!dataLength)
         return;
 
     // readystatechange event handler may do something to put this XHR in error
     // state. We need to check m_error again here.
     if (m_error)
         return;
 
-    m_downloadedBlobLength += dataLength;
+    m_lengthDownloadedToFile += dataLength;
 
     trackProgress(dataLength);
 }
 
 void XMLHttpRequest::handleDidTimeout()
 {
     WTF_LOG(Network, "XMLHttpRequest %p handleDidTimeout()", this);
 
     // Response is cleared next, save needed progress event data.
     long long expectedLength = m_response.expectedContentLength();
@@ skipping 52 matching lines @@
     visitor->trace(m_responseStream);
     visitor->trace(m_streamSource);
     visitor->trace(m_responseDocument);
     visitor->trace(m_responseDocumentParser);
     visitor->trace(m_progressEventThrottle);
     visitor->trace(m_upload);
     XMLHttpRequestEventTarget::trace(visitor);
 }
 
 } // namespace blink