Diff
Modified: trunk/LayoutTests/ChangeLog (206953 => 206954)
--- trunk/LayoutTests/ChangeLog 2016-10-08 13:09:30 UTC (rev 206953)
+++ trunk/LayoutTests/ChangeLog 2016-10-08 14:49:47 UTC (rev 206954)
@@ -1,3 +1,12 @@
+2016-10-08 Youenn Fablet <[email protected]>
+
+ [Fetch API] Request constructor should provide exception messages
+ https://bugs.webkit.org/show_bug.cgi?id=162382
+
+ Reviewed by Darin Adler.
+
+ * fetch/fetch-url-serialization-expected.txt: Rebasing test expectation.
+
2016-10-07 Chris Dumez <[email protected]>
window.navigator.language incorrectly returns all lowercase string
Modified: trunk/LayoutTests/fetch/fetch-url-serialization-expected.txt (206953 => 206954)
--- trunk/LayoutTests/fetch/fetch-url-serialization-expected.txt 2016-10-08 13:09:30 UTC (rev 206953)
+++ trunk/LayoutTests/fetch/fetch-url-serialization-expected.txt 2016-10-08 14:49:47 UTC (rev 206954)
@@ -97,7 +97,7 @@
FAIL Testing Request url ' File:c|////foo\bar.html' with base 'file:///tmp/mock/path' assert_equals: expected "file:///c:////foo/bar.html" but got "file:///tmp/mock/c|////foo/bar.html"
FAIL Testing Request url 'C|/foo/bar' with base 'file:///tmp/mock/path' assert_equals: expected "file:///C:/foo/bar" but got "file:///tmp/mock/C|/foo/bar"
FAIL Testing Request url '/C|\foo\bar' with base 'file:///tmp/mock/path' assert_equals: expected "file:///C:/foo/bar" but got "file:///C|/foo/bar"
-FAIL Testing Request url '//C|/foo/bar' with base 'file:///tmp/mock/path' Type error
+FAIL Testing Request url '//C|/foo/bar' with base 'file:///tmp/mock/path' URL is not valid or contains user credentials.
PASS Testing Request url '//server/file' with base 'file:///tmp/mock/path'
PASS Testing Request url '\\server\file' with base 'file:///tmp/mock/path'
PASS Testing Request url '/\server/file' with base 'file:///tmp/mock/path'
Modified: trunk/Source/WebCore/ChangeLog (206953 => 206954)
--- trunk/Source/WebCore/ChangeLog 2016-10-08 13:09:30 UTC (rev 206953)
+++ trunk/Source/WebCore/ChangeLog 2016-10-08 14:49:47 UTC (rev 206954)
@@ -1,5 +1,42 @@
2016-10-08 Youenn Fablet <[email protected]>
+ [Fetch API] Request constructor should provide exception messages
+ https://bugs.webkit.org/show_bug.cgi?id=162382
+
+ Reviewed by Darin Adler.
+
+ No change of behavior, except that exceptions now have error messages.
+
+ Added support of exception messages to ExceptionOr.
+ Making use of ExceptionOr for Request constructor parameter checking.
+
+ * Modules/fetch/FetchRequest.cpp:
+ (WebCore::setReferrerPolicy):
+ (WebCore::setMode):
+ (WebCore::setCredentials):
+ (WebCore::setCache):
+ (WebCore::setRedirect):
+ (WebCore::setMethod):
+ (WebCore::setReferrer):
+ (WebCore::buildOptions):
+ (WebCore::FetchRequest::initializeOptions):
+ (WebCore::FetchRequest::initializeWith):
+ * Modules/fetch/FetchRequest.h:
+ * Modules/fetch/FetchRequest.idl:
+ * bindings/js/JSDOMBinding.cpp:
+ (WebCore::setDOMException):
+ * bindings/js/JSDOMBinding.h:
+ (WebCore::toJS):
+ (WebCore::toJSNewlyCreated):
+ * dom/Exception.h:
+ (WebCore::Exception::code):
+ (WebCore::Exception::message):
+ (WebCore::Exception::Exception):
+ * dom/ExceptionOr.h:
+ (WebCore::ExceptionOr<ReturnType>::exceptionMessage):
+
+2016-10-08 Youenn Fablet <[email protected]>
+
Refactor binding generated casted-this checks
https://bugs.webkit.org/show_bug.cgi?id=162677
Modified: trunk/Source/WebCore/Modules/fetch/FetchRequest.cpp (206953 => 206954)
--- trunk/Source/WebCore/Modules/fetch/FetchRequest.cpp 2016-10-08 13:09:30 UTC (rev 206953)
+++ trunk/Source/WebCore/Modules/fetch/FetchRequest.cpp 2016-10-08 14:49:47 UTC (rev 206954)
@@ -39,7 +39,7 @@
namespace WebCore {
-static bool setReferrerPolicy(FetchOptions& options, const String& referrerPolicy)
+static Optional<Exception> setReferrerPolicy(FetchOptions& options, const String& referrerPolicy)
{
if (referrerPolicy.isEmpty())
options.referrerPolicy = FetchOptions::ReferrerPolicy::EmptyString;
@@ -54,11 +54,11 @@
else if (referrerPolicy == "unsafe-url")
options.referrerPolicy = FetchOptions::ReferrerPolicy::UnsafeUrl;
else
- return false;
- return true;
+ return Exception { TypeError, ASCIILiteral("Bad referrer policy value.") };
+ return Nullopt;
}
-static bool setMode(FetchOptions& options, const String& mode)
+static Optional<Exception> setMode(FetchOptions& options, const String& mode)
{
if (mode == "navigate")
options.mode = FetchOptions::Mode::Navigate;
@@ -69,11 +69,11 @@
else if (mode == "cors")
options.mode = FetchOptions::Mode::Cors;
else
- return false;
- return true;
+ return Exception { TypeError, ASCIILiteral("Bad fetch mode value.") };
+ return Nullopt;
}
-static bool setCredentials(FetchOptions& options, const String& credentials)
+static Optional<Exception> setCredentials(FetchOptions& options, const String& credentials)
{
if (credentials == "omit")
options.credentials = FetchOptions::Credentials::Omit;
@@ -82,11 +82,11 @@
else if (credentials == "include")
options.credentials = FetchOptions::Credentials::Include;
else
- return false;
- return true;
+ return Exception { TypeError, ASCIILiteral("Bad credentials mode value.") };
+ return Nullopt;
}
-static bool setCache(FetchOptions& options, const String& cache)
+static Optional<Exception> setCache(FetchOptions& options, const String& cache)
{
if (cache == "default")
options.cache = FetchOptions::Cache::Default;
@@ -99,11 +99,11 @@
else if (cache == "force-cache")
options.cache = FetchOptions::Cache::ForceCache;
else
- return false;
- return true;
+ return Exception { TypeError, ASCIILiteral("Bad cache mode value.") };
+ return Nullopt;
}
-static bool setRedirect(FetchOptions& options, const String& redirect)
+static Optional<Exception> setRedirect(FetchOptions& options, const String& redirect)
{
if (redirect == "follow")
options.redirect = FetchOptions::Redirect::Follow;
@@ -112,85 +112,103 @@
else if (redirect == "manual")
options.redirect = FetchOptions::Redirect::Manual;
else
- return false;
- return true;
+ return Exception { TypeError, ASCIILiteral("Bad redirect mode value.") };
+ return Nullopt;
}
-static bool setMethod(ResourceRequest& request, const String& initMethod)
+static Optional<Exception> setMethod(ResourceRequest& request, const String& initMethod)
{
if (!isValidHTTPToken(initMethod))
- return false;
+ return Exception { TypeError, ASCIILiteral("Method is not a valid HTTP token.") };
String method = initMethod.convertToASCIIUppercase();
if (method == "CONNECT" || method == "TRACE" || method == "TRACK")
- return false;
+ return Exception { TypeError, ASCIILiteral("Method is forbidden.") };
request.setHTTPMethod((method == "DELETE" || method == "GET" || method == "HEAD" || method == "OPTIONS" || method == "POST" || method == "PUT") ? method : initMethod);
- return true;
+ return Nullopt;
}
-static bool setReferrer(FetchRequest::InternalRequest& request, ScriptExecutionContext& context, const Dictionary& init)
+static Optional<Exception> setReferrer(FetchRequest::InternalRequest& request, ScriptExecutionContext& context, const Dictionary& init)
{
String referrer;
if (!init.get("referrer", referrer))
- return true;
+ return Nullopt;
if (referrer.isEmpty()) {
request.referrer = ASCIILiteral("no-referrer");
- return true;
+ return Nullopt;
}
// FIXME: Tighten the URL parsing algorithm according https://url.spec.whatwg.org/#concept-url-parser.
URL referrerURL = context.completeURL(referrer);
if (!referrerURL.isValid())
- return false;
+ return Exception { TypeError, ASCIILiteral("Referrer is not a valid URL.") };
if (referrerURL.protocolIs("about") && referrerURL.path() == "client") {
request.referrer = ASCIILiteral("client");
- return true;
+ return Nullopt;
}
if (!(context.securityOrigin() && context.securityOrigin()->canRequest(referrerURL)))
- return false;
+ return Exception { TypeError, ASCIILiteral("Referrer is not same-origin.") };
request.referrer = referrerURL.string();
- return true;
+ return Nullopt;
}
-static bool buildOptions(FetchRequest::InternalRequest& request, ScriptExecutionContext& context, const Dictionary& init)
+static Optional<Exception> buildOptions(FetchRequest::InternalRequest& request, ScriptExecutionContext& context, const Dictionary& init)
{
JSC::JSValue window;
if (init.get("window", window)) {
if (!window.isNull())
- return false;
+ return Exception { TypeError, ASCIILiteral("Window can only be null.") };
}
- if (!setReferrer(request, context, init))
- return false;
+ auto exception = setReferrer(request, context, init);
+ if (exception)
+ return exception;
String value;
- if (init.get("referrerPolicy", value) && !setReferrerPolicy(request.options, value))
- return false;
+ if (init.get("referrerPolicy", value)) {
+ exception = setReferrerPolicy(request.options, value);
+ if (exception)
+ return exception;
+ }
- if (init.get("mode", value) && !setMode(request.options, value))
- return false;
+ if (init.get("mode", value)) {
+ exception = setMode(request.options, value);
+ if (exception)
+ return exception;
+ }
if (request.options.mode == FetchOptions::Mode::Navigate)
- return false;
+ return Exception { TypeError, ASCIILiteral("Request constructor does not accept navigate fetch mode.") };
- if (init.get("credentials", value) && !setCredentials(request.options, value))
- return false;
+ if (init.get("credentials", value)) {
+ exception = setCredentials(request.options, value);
+ if (exception)
+ return exception;
+ }
- if (init.get("cache", value) && !setCache(request.options, value))
- return false;
+ if (init.get("cache", value)) {
+ exception = setCache(request.options, value);
+ if (exception)
+ return exception;
+ }
- if (init.get("redirect", value) && !setRedirect(request.options, value))
- return false;
+ if (init.get("redirect", value)) {
+ exception = setRedirect(request.options, value);
+ if (exception)
+ return exception;
+ }
init.get("integrity", request.integrity);
- if (init.get("method", value) && !setMethod(request.request, value))
- return false;
-
- return true;
+ if (init.get("method", value)) {
+ exception = setMethod(request.request, value);
+ if (exception)
+ return exception;
+ }
+ return Nullopt;
}
static bool methodCanHaveBody(const FetchRequest::InternalRequest& internalRequest)
@@ -198,37 +216,32 @@
return internalRequest.request.httpMethod() != "GET" && internalRequest.request.httpMethod() != "HEAD";
}
-void FetchRequest::initializeOptions(const Dictionary& init, ExceptionCode& ec)
+ExceptionOr<Ref<FetchHeaders>> FetchRequest::initializeOptions(const Dictionary& init)
{
ASSERT(scriptExecutionContext());
- if (!buildOptions(m_internalRequest, *scriptExecutionContext(), init)) {
- ec = TypeError;
- return;
- }
+ auto exception = buildOptions(m_internalRequest, *scriptExecutionContext(), init);
+ if (exception)
+ return WTFMove(exception.value());
+
if (m_internalRequest.options.mode == FetchOptions::Mode::NoCors) {
const String& method = m_internalRequest.request.httpMethod();
- if (method != "GET" && method != "POST" && method != "HEAD") {
- ec = TypeError;
- return;
- }
- if (!m_internalRequest.integrity.isEmpty()) {
- ec = TypeError;
- return;
- }
+ if (method != "GET" && method != "POST" && method != "HEAD")
+ return Exception { TypeError, ASCIILiteral("Method must be GET, POST or HEAD in no-cors mode.") };
+ if (!m_internalRequest.integrity.isEmpty())
+ return Exception { TypeError, ASCIILiteral("There cannot be an integrity in no-cors mode.") };
m_headers->setGuard(FetchHeaders::Guard::RequestNoCors);
}
+ return m_headers.copyRef();
}
-FetchHeaders* FetchRequest::initializeWith(const String& url, const Dictionary& init, ExceptionCode& ec)
+ExceptionOr<Ref<FetchHeaders>> FetchRequest::initializeWith(const String& url, const Dictionary& init)
{
ASSERT(scriptExecutionContext());
// FIXME: Tighten the URL parsing algorithm according https://url.spec.whatwg.org/#concept-url-parser.
URL requestURL = scriptExecutionContext()->completeURL(url);
- if (!requestURL.isValid() || !requestURL.user().isEmpty() || !requestURL.pass().isEmpty()) {
- ec = TypeError;
- return nullptr;
- }
+ if (!requestURL.isValid() || !requestURL.user().isEmpty() || !requestURL.pass().isEmpty())
+ return Exception { TypeError, ASCIILiteral("URL is not valid or contains user credentials.") };
m_internalRequest.options.mode = Mode::Cors;
m_internalRequest.options.credentials = Credentials::Omit;
@@ -235,21 +248,17 @@
m_internalRequest.referrer = ASCIILiteral("client");
m_internalRequest.request.setURL(requestURL);
- initializeOptions(init, ec);
- return m_headers.ptr();
+ return initializeOptions(init);
}
-FetchHeaders* FetchRequest::initializeWith(FetchRequest& input, const Dictionary& init, ExceptionCode& ec)
+ExceptionOr<Ref<FetchHeaders>> FetchRequest::initializeWith(FetchRequest& input, const Dictionary& init)
{
- if (input.isDisturbedOrLocked()) {
- ec = TypeError;
- return nullptr;
- }
+ if (input.isDisturbedOrLocked())
+ return Exception {TypeError, ASCIILiteral("Request input is disturbed or locked.") };
m_internalRequest = input.m_internalRequest;
- initializeOptions(init, ec);
- return m_headers.ptr();
+ return initializeOptions(init);
}
void FetchRequest::setBody(JSC::ExecState& execState, JSC::JSValue body, FetchRequest* request, ExceptionCode& ec)
Modified: trunk/Source/WebCore/Modules/fetch/FetchRequest.h (206953 => 206954)
--- trunk/Source/WebCore/Modules/fetch/FetchRequest.h 2016-10-08 13:09:30 UTC (rev 206953)
+++ trunk/Source/WebCore/Modules/fetch/FetchRequest.h 2016-10-08 14:49:47 UTC (rev 206954)
@@ -31,9 +31,11 @@
#if ENABLE(FETCH_API)
+#include "ExceptionOr.h"
#include "FetchBodyOwner.h"
#include "FetchOptions.h"
#include "ResourceRequest.h"
+#include <wtf/Optional.h>
namespace WebCore {
@@ -46,8 +48,8 @@
public:
static Ref<FetchRequest> create(ScriptExecutionContext& context) { return adoptRef(*new FetchRequest(context, Nullopt, FetchHeaders::create(FetchHeaders::Guard::Request), { })); }
- FetchHeaders* initializeWith(FetchRequest&, const Dictionary&, ExceptionCode&);
- FetchHeaders* initializeWith(const String&, const Dictionary&, ExceptionCode&);
+ ExceptionOr<Ref<FetchHeaders>> initializeWith(FetchRequest&, const Dictionary&);
+ ExceptionOr<Ref<FetchHeaders>> initializeWith(const String&, const Dictionary&);
void setBody(JSC::ExecState&, JSC::JSValue, FetchRequest*, ExceptionCode&);
const String& method() const { return m_internalRequest.request.httpMethod(); }
@@ -96,7 +98,7 @@
private:
FetchRequest(ScriptExecutionContext&, Optional<FetchBody>&&, Ref<FetchHeaders>&&, InternalRequest&&);
- void initializeOptions(const Dictionary&, ExceptionCode&);
+ ExceptionOr<Ref<FetchHeaders>> initializeOptions(const Dictionary&);
// ActiveDOMObject API.
const char* activeDOMObjectName() const final;
Modified: trunk/Source/WebCore/Modules/fetch/FetchRequest.idl (206953 => 206954)
--- trunk/Source/WebCore/Modules/fetch/FetchRequest.idl 2016-10-08 13:09:30 UTC (rev 206953)
+++ trunk/Source/WebCore/Modules/fetch/FetchRequest.idl 2016-10-08 14:49:47 UTC (rev 206954)
@@ -62,8 +62,8 @@
[NewObject, CallWith=ScriptExecutionContext, MayThrowLegacyException] FetchRequest clone();
- [PrivateIdentifier, MayThrowLegacyException] FetchHeaders initializeWith(FetchRequest input, Dictionary init);
- [PrivateIdentifier, MayThrowLegacyException] FetchHeaders initializeWith(DOMString input, Dictionary init);
+ [PrivateIdentifier, NewObject] FetchHeaders initializeWith(FetchRequest input, Dictionary init);
+ [PrivateIdentifier, NewObject] FetchHeaders initializeWith(DOMString input, Dictionary init);
[PrivateIdentifier, MayThrowLegacyException, CallWith=ScriptState] void setBody(any body, FetchRequest? request);
};
FetchRequest implements FetchBody;
Modified: trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp (206953 => 206954)
--- trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp 2016-10-08 13:09:30 UTC (rev 206953)
+++ trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp 2016-10-08 14:49:47 UTC (rev 206954)
@@ -376,6 +376,17 @@
throwDOMException(exec, scope, ec);
}
+void setDOMException(ExecState* exec, ExceptionCode ec, const String& message)
+{
+ VM& vm = exec->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
+ if (!ec || scope.exception())
+ return;
+
+ throwException(exec, scope, createDOMException(exec, ec, message));
+}
+
void setDOMException(ExecState* exec, const ExceptionCodeWithMessage& ec)
{
VM& vm = exec->vm();
Modified: trunk/Source/WebCore/bindings/js/JSDOMBinding.h (206953 => 206954)
--- trunk/Source/WebCore/bindings/js/JSDOMBinding.h 2016-10-08 13:09:30 UTC (rev 206953)
+++ trunk/Source/WebCore/bindings/js/JSDOMBinding.h 2016-10-08 14:49:47 UTC (rev 206954)
@@ -187,6 +187,7 @@
// Convert a DOM implementation exception code into a _javascript_ exception in the execution state.
WEBCORE_EXPORT void setDOMException(JSC::ExecState*, ExceptionCode);
+void setDOMException(JSC::ExecState*, ExceptionCode, const String&);
void setDOMException(JSC::ExecState*, const ExceptionCodeWithMessage&);
WEBCORE_EXPORT void setDOMExceptionSlow(JSC::ExecState*, JSC::ThrowScope&, ExceptionCode);
@@ -946,7 +947,7 @@
template<typename T> inline JSC::JSValue toJS(JSC::ExecState* state, JSDOMGlobalObject* globalObject, ExceptionOr<T>&& value)
{
if (UNLIKELY(value.hasException())) {
- setDOMException(state, value.exceptionCode());
+ setDOMException(state, value.exceptionCode(), value.exceptionMessage());
return JSC::jsUndefined();
}
return toJS(state, globalObject, value.takeReturnValue());
@@ -956,7 +957,7 @@
{
// FIXME: It's really annoying to have two of these functions. Should find a way to combine toJS and toJSNewlyCreated.
if (UNLIKELY(value.hasException())) {
- setDOMException(state, value.exceptionCode());
+ setDOMException(state, value.exceptionCode(), value.exceptionMessage());
return JSC::jsUndefined();
}
return toJSNewlyCreated(state, globalObject, value.takeReturnValue());
Modified: trunk/Source/WebCore/dom/Exception.h (206953 => 206954)
--- trunk/Source/WebCore/dom/Exception.h 2016-10-08 13:09:30 UTC (rev 206953)
+++ trunk/Source/WebCore/dom/Exception.h 2016-10-08 14:49:47 UTC (rev 206954)
@@ -26,6 +26,8 @@
#pragma once
+#include <wtf/text/WTFString.h>
+
namespace WebCore {
using ExceptionCode = int;
@@ -33,11 +35,14 @@
class Exception {
public:
explicit Exception(ExceptionCode);
+ explicit Exception(ExceptionCode, const String&);
- ExceptionCode code() const;
+ ExceptionCode code() const { return m_code; }
+ const String& message() const { return m_message; }
private:
ExceptionCode m_code;
+ String m_message;
};
inline Exception::Exception(ExceptionCode code)
@@ -45,9 +50,10 @@
{
}
-inline ExceptionCode Exception::code() const
+inline Exception::Exception(ExceptionCode code, const String& message)
+ : m_code(code)
+ , m_message(message)
{
- return m_code;
}
}
Modified: trunk/Source/WebCore/dom/ExceptionOr.h (206953 => 206954)
--- trunk/Source/WebCore/dom/ExceptionOr.h 2016-10-08 13:09:30 UTC (rev 206953)
+++ trunk/Source/WebCore/dom/ExceptionOr.h 2016-10-08 14:49:47 UTC (rev 206954)
@@ -38,6 +38,7 @@
bool hasException() const;
ExceptionCode exceptionCode() const;
+ const String& exceptionMessage() const;
ReturnType&& takeReturnValue();
private:
@@ -64,6 +65,11 @@
return std::experimental::get<Exception>(m_value).code();
}
+template<typename ReturnType> inline const String& ExceptionOr<ReturnType>::exceptionMessage() const
+{
+ return std::experimental::get<Exception>(m_value).message();
+}
+
template<typename ReturnType> inline ReturnType&& ExceptionOr<ReturnType>::takeReturnValue()
{
return std::experimental::get<ReturnType>(WTFMove(m_value));
