Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1150)

Unified Diff: Source/core/xml/XMLHttpRequest.cpp

Issue 23444058: Use downloadToFile option when XHR downloads a Blob (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Rebase Created 7 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « Source/core/xml/XMLHttpRequest.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/xml/XMLHttpRequest.cpp
diff --git a/Source/core/xml/XMLHttpRequest.cpp b/Source/core/xml/XMLHttpRequest.cpp
index 5b8ff2000c4a3d67ff675e6e624a89d9f49ee020..3ab64a79f4c8063a88fa009f43a70f214857543a 100644
--- a/Source/core/xml/XMLHttpRequest.cpp
+++ b/Source/core/xml/XMLHttpRequest.cpp
@@ -173,6 +173,7 @@ XMLHttpRequest::XMLHttpRequest(ExecutionContext* context, PassRefPtr<SecurityOri
, m_timeoutMilliseconds(0)
, m_state(UNSENT)
, m_createdDocument(false)
+ , m_downloadedBlobLength(0)
, m_error(false)
, m_uploadEventsAllowed(true)
, m_uploadComplete(false)
@@ -276,31 +277,26 @@ Document* XMLHttpRequest::responseXML(ExceptionState& es)
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) {
- // 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.
+ // When "blob" is specified for the responseType attribute,
+ // 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();
+ const String& filePath = m_response.downloadedFilePath();
abarth-chromium 2013/11/05 07:01:51 const String& -> String String is a value type wi
yusukesuzuki 2013/11/05 11:04:00 That's reasonable. Done.
// If we errored out or got no data, we still return a blob, just an empty one.
- size_t size = 0;
- if (m_binaryResponseBuilder) {
- RefPtr<RawData> rawData = RawData::create();
- size = m_binaryResponseBuilder->size();
- rawData->mutableData()->append(m_binaryResponseBuilder->data(), size);
- blobData->appendData(rawData, 0, BlobDataItem::toEndOfFile);
+ if (!filePath.isEmpty() && m_downloadedBlobLength) {
yusukesuzuki 2013/10/24 15:22:41 In the previous patch, I used m_receivedLength as
+ blobData->appendFile(filePath);
blobData->setContentType(responseMIMEType()); // responseMIMEType defaults to text/xml which may be incorrect.
- 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();
@@ -412,6 +408,30 @@ XMLHttpRequestUpload* XMLHttpRequest::upload()
return m_upload.get();
}
+void XMLHttpRequest::trackProgress(int length)
+{
+ m_receivedLength += length;
+
+ 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.dispatchProgressEvent(lengthComputable, m_receivedLength, total);
+ }
+
+ if (m_state != LOADING) {
+ changeState(LOADING);
+ } else {
+ // Firefox calls readyStateChanged every time it receives data. Do
+ // the same to align with Firefox.
+ //
+ // FIXME: Make our implementation and the spec consistent. This
+ // behavior was needed when the progress event was not available.
+ dispatchReadyStateChangeEvent();
+ }
+}
+
void XMLHttpRequest::changeState(State newState)
{
if (m_state != newState) {
@@ -785,6 +805,12 @@ void XMLHttpRequest::createRequest(ExceptionState& es)
request.setHTTPMethod(m_method);
request.setTargetType(ResourceRequest::TargetIsXHR);
+ // When "blob" is specified for the responseType attribute,
+ // we redirect the downloaded data to a file-handle directly
+ // and get the file-path as the result.
+ if (responseTypeCode() == ResponseTypeBlob)
+ request.setDownloadToFile(true);
+
InspectorInstrumentation::willLoadXHR(executionContext(), this, m_method, m_url, m_async, m_requestEntityBody ? m_requestEntityBody->deepCopy() : 0, m_requestHeaders, m_includeCredentials);
if (m_requestEntityBody) {
@@ -810,6 +836,12 @@ void XMLHttpRequest::createRequest(ExceptionState& es)
options.mixedContentBlockingTreatment = TreatAsPassiveContent;
options.timeoutMilliseconds = m_timeoutMilliseconds;
+ // Since we redirect the downloaded data to a file-handle directly
+ // when "blob" is specified for the responseType attribute,
+ // buffering is not needed.
+ if (responseTypeCode() == ResponseTypeBlob)
+ options.dataBufferingPolicy = DoNotBufferData;
+
m_exceptionCode = 0;
m_error = false;
@@ -1256,6 +1288,8 @@ void XMLHttpRequest::didReceiveResponse(unsigned long identifier, const Resource
void XMLHttpRequest::didReceiveData(const char* data, int len)
{
+ ASSERT(m_responseTypeCode != ResponseTypeBlob);
+
if (m_error)
return;
@@ -1288,7 +1322,7 @@ void XMLHttpRequest::didReceiveData(const char* data, int len)
if (useDecoder) {
m_responseText = m_responseText.concatenateWith(m_decoder->decode(data, len));
- } else if (m_responseTypeCode == ResponseTypeArrayBuffer || m_responseTypeCode == ResponseTypeBlob) {
+ } else if (m_responseTypeCode == ResponseTypeArrayBuffer) {
// Buffer binary data.
if (!m_binaryResponseBuilder)
m_binaryResponseBuilder = SharedBuffer::create();
@@ -1302,26 +1336,27 @@ void XMLHttpRequest::didReceiveData(const char* data, int len)
if (m_error)
return;
- m_receivedLength += len;
+ trackProgress(len);
+}
- if (m_async) {
- long long expectedLength = m_response.expectedContentLength();
- bool lengthComputable = expectedLength > 0 && m_receivedLength <= expectedLength;
- unsigned long long total = lengthComputable ? expectedLength : 0;
+void XMLHttpRequest::didDownloadData(int dataLength)
+{
+ ASSERT(m_responseTypeCode == ResponseTypeBlob);
- m_progressEventThrottle.dispatchProgressEvent(lengthComputable, m_receivedLength, total);
- }
+ if (m_error)
+ return;
- if (m_state != LOADING) {
- changeState(LOADING);
- } else {
- // Firefox calls readyStateChanged every time it receives data. Do
- // the same to align with Firefox.
- //
- // FIXME: Make our implementation and the spec consistent. This
- // behavior was needed when the progress event was not available.
- dispatchReadyStateChangeEvent();
- }
+ if (m_state < HEADERS_RECEIVED)
+ changeState(HEADERS_RECEIVED);
+
+ if (!dataLength)
+ return;
+
+ if (m_error)
+ return;
abarth-chromium 2013/11/05 07:01:51 Can changeState set m_error? We just checked it a
yusukesuzuki 2013/11/05 11:04:00 Yeah, changeState can set m_error. changeState cau
+
+ m_downloadedBlobLength += dataLength;
+ trackProgress(dataLength);
}
void XMLHttpRequest::handleDidTimeout()
« no previous file with comments | « Source/core/xml/XMLHttpRequest.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698