Diff
Modified: trunk/LayoutTests/ChangeLog (203493 => 203494)
--- trunk/LayoutTests/ChangeLog 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/LayoutTests/ChangeLog 2016-07-21 06:51:05 UTC (rev 203494)
@@ -1,3 +1,13 @@
+2016-07-20 Youenn Fablet <[email protected]>
+
+ [XHR] Cache response JS object in case of arraybuffer and blob response types
+ https://bugs.webkit.org/show_bug.cgi?id=128903
+
+ Reviewed by Alex Christensen.
+
+ * http/tests/xmlhttprequest/onabort-response-getters-expected.txt:
+ * http/tests/xmlhttprequest/onabort-response-getters.html:
+
2016-07-19 Filip Pizlo <[email protected]>
Switching on symbols should be fast
Modified: trunk/LayoutTests/http/tests/xmlhttprequest/onabort-response-getters-expected.txt (203493 => 203494)
--- trunk/LayoutTests/http/tests/xmlhttprequest/onabort-response-getters-expected.txt 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/onabort-response-getters-expected.txt 2016-07-21 06:51:05 UTC (rev 203494)
@@ -1,6 +1,12 @@
PASS getting arraybuffer response within abort event callback
+PASS getting arraybuffer response within abort event callback (aborting in loadend)
PASS getting blob response within abort event callback
+PASS getting blob response within abort event callback (aborting in loadend)
PASS getting json response within abort event callback
+PASS getting json response within abort event callback (aborting in loadend)
PASS getting document response within abort event callback
+PASS getting document response within abort event callback (aborting in loadend)
+PASS getting text response within abort event callback
+PASS getting text response within abort event callback (aborting in loadend)
Modified: trunk/LayoutTests/http/tests/xmlhttprequest/onabort-response-getters.html (203493 => 203494)
--- trunk/LayoutTests/http/tests/xmlhttprequest/onabort-response-getters.html 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/onabort-response-getters.html 2016-07-21 06:51:05 UTC (rev 203494)
@@ -7,6 +7,14 @@
<body>
<div id="log"></div>
<script>
+ function checkCachedResponse(client) {
+ assert_true(client.response === client.response);
+ if (client.responseType == "text")
+ assert_true(client.responseText === client.response);
+ if (client.responseType == "document")
+ assert_true(client.responseXML === client.response);
+ }
+
function runTest(name, fileName, mimeType, setupClient, checkResponse) {
var test = async_test(name)
test.step(function() {
@@ -28,6 +36,7 @@
if (client.readyState == 4) {
checkResponse(test, client)
client.isResponseChecked = true
+ checkCachedResponse(client);
}
})
@@ -38,10 +47,29 @@
client._onloadend_ = test.step_func(function () {
assert_true(client.hasAborted, "xhr should have aborted")
assert_true(client.isResponseChecked, "xhr response should have been checked")
+ checkCachedResponse(client);
test.done()
})
client.send(null)
})
+
+ var test2 = async_test(name + " (aborting in loadend)")
+ test2.step(function() {
+ var client = new XMLHttpRequest()
+ var url = "" + fileName + "&waitFor=1&mimeType=" + mimeType
+ client.open("GET", url, true)
+ setupClient(test2, client)
+
+ client._onloadend_ = test2.step_func(function () {
+ assert_true(client.response != null);
+ checkCachedResponse(client);
+ client.abort();
+ checkResponse(test2, client);
+ checkCachedResponse(client);
+ test2.done();
+ })
+ client.send(null)
+ })
}
runTest("getting arraybuffer response within abort event callback",
@@ -68,6 +96,12 @@
function(test, client) {assert_true(client.response == null, "document response must be empty")}
)
+ runTest("getting text response within abort event callback",
+ "resources/test.json","text/plain",
+ function(test, client) {client.responseType = "text"},
+ function(test, client) {assert_true(client.response == "", "text response must be an empty string")}
+ )
+
</script>
</body>
</html>
Modified: trunk/Source/WebCore/CMakeLists.txt (203493 => 203494)
--- trunk/Source/WebCore/CMakeLists.txt 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/Source/WebCore/CMakeLists.txt 2016-07-21 06:51:05 UTC (rev 203494)
@@ -3726,6 +3726,7 @@
${WEBCORE_DIR}/Modules/streams/StreamInternals.js
${WEBCORE_DIR}/Modules/streams/WritableStream.js
${WEBCORE_DIR}/Modules/streams/WritableStreamInternals.js
+ ${WEBCORE_DIR}/xml/XMLHttpRequest.js
)
set(BUILTINS_GENERATOR_SCRIPTS
Modified: trunk/Source/WebCore/ChangeLog (203493 => 203494)
--- trunk/Source/WebCore/ChangeLog 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/Source/WebCore/ChangeLog 2016-07-21 06:51:05 UTC (rev 203494)
@@ -1,5 +1,35 @@
2016-07-20 Youenn Fablet <[email protected]>
+ [XHR] Cache response JS object in case of arraybuffer and blob response types
+ https://bugs.webkit.org/show_bug.cgi?id=128903
+
+ Reviewed by Alex Christensen.
+
+ Covered by existing and modified tests.
+
+ Making response getter a JS builtin that caches response in @response private slot.
+ Handling invalidation of cached response with @responseCacheIsValid new private method.
+ Handling creation of cached response with @retrieveResponse new private method which reuses most of
+ JSXMLHttpRequest::response previous code.
+
+ Caching of responses is activated whenever load ended without any error for blob and arraybuffer response types.
+
+ Caching of response for document is also activated in case the response getter is used but not if responseXML getter is used.
+
+ * CMakeLists.txt: Adding XMLHttpRequest.js.
+ * DerivedSources.make: Ditto.
+ * bindings/js/JSXMLHttpRequestCustom.cpp:
+ (WebCore::JSXMLHttpRequest::retrieveResponse): Implements creation of to-be-cached response.
+ (WebCore::JSXMLHttpRequest::response): Deleted.
+ * bindings/js/WebCoreBuiltinNames.h: Adding new private names.
+ * xml/XMLHttpRequest.cpp:
+ (WebCore::XMLHttpRequest::didCacheResponse): Renamed from didCacheResponseJSON as all response types are now cached.
+ (WebCore::XMLHttpRequest::didCacheResponseJSON): Deleted.
+ * xml/XMLHttpRequest.h:
+ * xml/XMLHttpRequest.idl:
+
+2016-07-20 Youenn Fablet <[email protected]>
+
Remove crossOriginRequestPolicy from ThreadableLoaderOptions
https://bugs.webkit.org/show_bug.cgi?id=159417
Modified: trunk/Source/WebCore/DerivedSources.make (203493 => 203494)
--- trunk/Source/WebCore/DerivedSources.make 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/Source/WebCore/DerivedSources.make 2016-07-21 06:51:05 UTC (rev 203494)
@@ -1299,6 +1299,7 @@
$(WebCore)/Modules/streams/StreamInternals.js \
$(WebCore)/Modules/streams/WritableStream.js \
$(WebCore)/Modules/streams/WritableStreamInternals.js \
+ $(WebCore)/xml/XMLHttpRequest.js \
#
BUILTINS_GENERATOR_SCRIPTS = \
Modified: trunk/Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp (203493 => 203494)
--- trunk/Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp 2016-07-21 06:51:05 UTC (rev 203494)
@@ -63,12 +63,6 @@
if (Document* responseDocument = wrapped().optionalResponseXML())
visitor.addOpaqueRoot(responseDocument);
-
- if (ArrayBuffer* responseArrayBuffer = wrapped().optionalResponseArrayBuffer())
- visitor.addOpaqueRoot(responseArrayBuffer);
-
- if (Blob* responseBlob = wrapped().optionalResponseBlob())
- visitor.addOpaqueRoot(responseBlob);
}
class SendFunctor {
@@ -151,12 +145,8 @@
return jsOwnedStringOrNull(&state, text);
}
-JSValue JSXMLHttpRequest::response(ExecState& state) const
+JSValue JSXMLHttpRequest::retrieveResponse(ExecState& state)
{
- // FIXME: Use CachedAttribute for other types than JSON as well.
- if (m_response && wrapped().responseCacheIsValid())
- return m_response.get();
-
auto type = wrapped().responseType();
switch (type) {
@@ -170,42 +160,36 @@
if (!wrapped().doneWithoutErrors())
return jsNull();
+ JSValue value;
switch (type) {
case XMLHttpRequest::ResponseType::EmptyString:
case XMLHttpRequest::ResponseType::Text:
ASSERT_NOT_REACHED();
- break;
+ return jsUndefined();
case XMLHttpRequest::ResponseType::Json:
- {
- JSValue value = JSONParse(&state, wrapped().responseTextIgnoringResponseType());
- if (!value)
- value = jsNull();
- m_response.set(state.vm(), this, value);
- wrapped().didCacheResponseJSON();
- return value;
- }
+ value = JSONParse(&state, wrapped().responseTextIgnoringResponseType());
+ if (!value)
+ value = jsNull();
+ break;
- case XMLHttpRequest::ResponseType::Document:
- {
- ExceptionCode ec = 0;
- Document* document = wrapped().responseXML(ec);
- if (ec) {
- setDOMException(&state, ec);
- return jsUndefined();
- }
- return toJS(&state, globalObject(), document);
- }
-
+ case XMLHttpRequest::ResponseType::Document: {
+ ExceptionCode ec = 0;
+ auto document = wrapped().responseXML(ec);
+ ASSERT(!ec);
+ value = toJS(&state, globalObject(), document);
+ break;
+ }
case XMLHttpRequest::ResponseType::Blob:
- return toJS(&state, globalObject(), wrapped().responseBlob());
+ value = toJSNewlyCreated(&state, globalObject(), wrapped().createResponseBlob());
+ break;
case XMLHttpRequest::ResponseType::Arraybuffer:
- return toJS(&state, globalObject(), wrapped().responseArrayBuffer());
+ value = toJS(&state, globalObject(), wrapped().createResponseArrayBuffer());
+ break;
}
-
- ASSERT_NOT_REACHED();
- return jsUndefined();
+ wrapped().didCacheResponse();
+ return value;
}
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h (203493 => 203494)
--- trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h 2016-07-21 06:51:05 UTC (rev 203494)
@@ -62,6 +62,9 @@
macro(readRequests) \
macro(readyPromiseCapability) \
macro(removeTrack) \
+ macro(responseCacheIsValid) \
+ macro(retrieveResponse) \
+ macro(response) \
macro(setStatus) \
macro(state) \
macro(started) \
@@ -85,6 +88,7 @@
macro(ReadableStreamController) \
macro(RTCIceCandidate) \
macro(RTCSessionDescription) \
+ macro(XMLHttpRequest)
class WebCoreBuiltinNames {
public:
Modified: trunk/Source/WebCore/xml/XMLHttpRequest.cpp (203493 => 203494)
--- trunk/Source/WebCore/xml/XMLHttpRequest.cpp 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.cpp 2016-07-21 06:51:05 UTC (rev 203494)
@@ -171,9 +171,8 @@
return responseTextIgnoringResponseType();
}
-void XMLHttpRequest::didCacheResponseJSON()
+void XMLHttpRequest::didCacheResponse()
{
- ASSERT(m_responseType == ResponseType::Json);
ASSERT(doneWithoutErrors());
m_responseCacheIsValid = true;
m_responseBuilder.clear();
@@ -218,42 +217,30 @@
return m_responseDocument.get();
}
-Blob* XMLHttpRequest::responseBlob()
+Ref<Blob> XMLHttpRequest::createResponseBlob()
{
ASSERT(m_responseType == ResponseType::Blob);
ASSERT(doneWithoutErrors());
- if (!m_responseBlob) {
- if (m_binaryResponseBuilder) {
- // FIXME: We just received the data from NetworkProcess, and are sending it back. This is inefficient.
- Vector<uint8_t> data;
- data.append(m_binaryResponseBuilder->data(), m_binaryResponseBuilder->size());
- String normalizedContentType = Blob::normalizedContentType(responseMIMEType()); // responseMIMEType defaults to text/xml which may be incorrect.
- m_responseBlob = Blob::create(WTFMove(data), normalizedContentType);
- m_binaryResponseBuilder = nullptr;
- } else {
- // If we errored out or got no data, we still return a blob, just an empty one.
- m_responseBlob = Blob::create();
- }
- }
+ if (!m_binaryResponseBuilder)
+ return Blob::create();
- return m_responseBlob.get();
+ // FIXME: We just received the data from NetworkProcess, and are sending it back. This is inefficient.
+ Vector<uint8_t> data;
+ data.append(m_binaryResponseBuilder->data(), m_binaryResponseBuilder->size());
+ m_binaryResponseBuilder = nullptr;
+ String normalizedContentType = Blob::normalizedContentType(responseMIMEType()); // responseMIMEType defaults to text/xml which may be incorrect.
+ return Blob::create(WTFMove(data), normalizedContentType);
}
-ArrayBuffer* XMLHttpRequest::responseArrayBuffer()
+RefPtr<ArrayBuffer> XMLHttpRequest::createResponseArrayBuffer()
{
ASSERT(m_responseType == ResponseType::Arraybuffer);
ASSERT(doneWithoutErrors());
- if (!m_responseArrayBuffer) {
- if (m_binaryResponseBuilder)
- m_responseArrayBuffer = m_binaryResponseBuilder->createArrayBuffer();
- else
- m_responseArrayBuffer = ArrayBuffer::create(nullptr, 0);
- m_binaryResponseBuilder = nullptr;
- }
-
- return m_responseArrayBuffer.get();
+ auto result = m_binaryResponseBuilder ? m_binaryResponseBuilder->createArrayBuffer() : ArrayBuffer::create(nullptr, 0);
+ m_binaryResponseBuilder = nullptr;
+ return result;
}
void XMLHttpRequest::setTimeout(unsigned timeout, ExceptionCode& ec)
@@ -819,9 +806,7 @@
m_responseEncoding = String();
m_createdDocument = false;
m_responseDocument = nullptr;
- m_responseBlob = nullptr;
m_binaryResponseBuilder = nullptr;
- m_responseArrayBuffer = nullptr;
m_responseCacheIsValid = false;
}
Modified: trunk/Source/WebCore/xml/XMLHttpRequest.h (203493 => 203494)
--- trunk/Source/WebCore/xml/XMLHttpRequest.h 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.h 2016-07-21 06:51:05 UTC (rev 203494)
@@ -88,15 +88,18 @@
String responseText(ExceptionCode&);
String responseTextIgnoringResponseType() const { return m_responseBuilder.toStringPreserveCapacity(); }
String responseMIMEType() const;
+
+ Document* optionalResponseXML() const { return m_responseDocument.get(); }
Document* responseXML(ExceptionCode&);
- Document* optionalResponseXML() const { return m_responseDocument.get(); }
- Blob* responseBlob();
- Blob* optionalResponseBlob() const { return m_responseBlob.get(); }
+
+ Ref<Blob> createResponseBlob();
+ RefPtr<JSC::ArrayBuffer> createResponseArrayBuffer();
+
unsigned timeout() const { return m_timeoutMilliseconds; }
void setTimeout(unsigned timeout, ExceptionCode&);
bool responseCacheIsValid() const { return m_responseCacheIsValid; }
- void didCacheResponseJSON();
+ void didCacheResponse();
// Expose HTTP validation methods for other untrusted requests.
static bool isAllowedHTTPMethod(const String&);
@@ -109,10 +112,6 @@
String responseURL() const;
- // response attribute has custom getter.
- JSC::ArrayBuffer* responseArrayBuffer();
- JSC::ArrayBuffer* optionalResponseArrayBuffer() const { return m_responseArrayBuffer.get(); }
-
void setLastSendLineAndColumnNumber(unsigned lineNumber, unsigned columnNumber);
void setLastSendURL(const String& url) { m_lastSendURL = url; }
@@ -187,7 +186,6 @@
String m_mimeTypeOverride;
bool m_async { true };
bool m_includeCredentials { false };
- RefPtr<Blob> m_responseBlob;
RefPtr<ThreadableLoader> m_loader;
State m_state { UNSENT };
@@ -201,9 +199,8 @@
StringBuilder m_responseBuilder;
bool m_createdDocument { false };
RefPtr<Document> m_responseDocument;
-
+
RefPtr<SharedBuffer> m_binaryResponseBuilder;
- RefPtr<JSC::ArrayBuffer> m_responseArrayBuffer;
bool m_error { false };
Modified: trunk/Source/WebCore/xml/XMLHttpRequest.idl (203493 => 203494)
--- trunk/Source/WebCore/xml/XMLHttpRequest.idl 2016-07-21 06:41:43 UTC (rev 203493)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.idl 2016-07-21 06:51:05 UTC (rev 203494)
@@ -44,6 +44,8 @@
JSGenerateToNativeObject,
JSGenerateToJSObject,
ExportMacro=WEBCORE_EXPORT,
+ PublicIdentifier,
+ PrivateIdentifier,
] interface XMLHttpRequest : XMLHttpRequestEventTarget {
attribute EventHandler onreadystatechange;
@@ -78,12 +80,15 @@
[GetterRaisesException] readonly attribute Document responseXML;
[SetterRaisesException] attribute XMLHttpRequestResponseType responseType;
- [GetterRaisesException, CachedAttribute, CustomGetter] readonly attribute Object response;
+ [JSBuiltin] readonly attribute Object response;
readonly attribute unsigned short status;
readonly attribute DOMString statusText;
readonly attribute DOMString responseURL;
+ [PrivateIdentifier] boolean responseCacheIsValid();
+ [PrivateIdentifier, Custom] any retrieveResponse();
+
// Extension
[RaisesException] void overrideMimeType(DOMString override);
};
Added: trunk/Source/WebCore/xml/XMLHttpRequest.js (0 => 203494)
--- trunk/Source/WebCore/xml/XMLHttpRequest.js (rev 0)
+++ trunk/Source/WebCore/xml/XMLHttpRequest.js 2016-07-21 06:51:05 UTC (rev 203494)
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2016 Apple Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+function response()
+{
+ "use strict";
+
+ // FIXME: Add a helper routine for that kind of checks.
+ if (!(this instanceof @XMLHttpRequest))
+ throw new @TypeError("The XMLHttpRequest.response getter can only be used on instances of XMLHttpRequest");
+
+ if (@XMLHttpRequest.prototype.@responseCacheIsValid.@call(this))
+ return this.@response;
+
+ this.@response = @XMLHttpRequest.prototype.@retrieveResponse.@call(this);
+ return this.@response;
+}
