Commit 9a63b165 authored by Dominic Farolino's avatar Dominic Farolino Committed by Commit Bot

Fix AbortSignal conversion regression

By working around a bug in the IDL compiler when generating RequestInit
with said compiler, a regression was introduced that stopped TypeErrors
from being thrown when RequestInit's signal member does not implement
the AbortSignal interface. This CL fixes that regression and adds tests
for it.

R=kinuko@chromium.org, yoav@yoav.ws

Bug: 836873
Change-Id: I6ab7be2ee8356eb7a997b904d121a7061986096a
Reviewed-on: https://chromium-review.googlesource.com/1123771Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarAdam Rice <ricea@chromium.org>
Commit-Queue: Dominic Farolino <domfarolino@gmail.com>
Cr-Commit-Position: refs/heads/master@{#572449}
parent 6a85c30b
...@@ -677,8 +677,9 @@ test(() => { ...@@ -677,8 +677,9 @@ test(() => {
}, 'Used => clone'); }, 'Used => clone');
test(() => { test(() => {
// We implement RequestInit manually so we need to test the functionality // We used to implement RequestInit manually so we needed to test this
// here. // functionality here. We now generate RequestInit with the IDL compiler,
// but it's still good to keep these around.
function undefined_notpresent(property_name) { function undefined_notpresent(property_name) {
assert_not_equals(property_name, 'referrer', 'property_name'); assert_not_equals(property_name, 'referrer', 'property_name');
const request = new Request('/', {referrer: '/'}); const request = new Request('/', {referrer: '/'});
...@@ -709,8 +710,9 @@ test(() => { ...@@ -709,8 +710,9 @@ test(() => {
}, 'An undefined member should be treated as not-present'); }, 'An undefined member should be treated as not-present');
test(() => { test(() => {
// We implement RequestInit manually so we need to test the functionality // We used to implement RequestInit manually so we needed to test this
// here. // functionality here. We now generate RequestInit with the IDL compiler,
// but it's still good to keep these around.
const e = Error(); const e = Error();
assert_throws(e, () => { assert_throws(e, () => {
new Request('/', {get method() { throw e; }})}, 'method'); new Request('/', {get method() { throw e; }})}, 'method');
...@@ -742,6 +744,21 @@ test(() => { ...@@ -742,6 +744,21 @@ test(() => {
// new Request('/', {get window() { throw e; }})}, 'window'); // new Request('/', {get window() { throw e; }})}, 'window');
}, 'Getter exceptions should not be silently ignored'); }, 'Getter exceptions should not be silently ignored');
test(() => {
// At the time of writing this test, an `any` type is used to represent
// RequestInit's signal member instead of AbortSignal due to a bug in the IDL
// compiler; see https://crbug.com/855968. This test ensures that that we
// treat the `any` member exactly as an AbortSignal member would be treated.
const e = TypeError();
assert_throws(e, () => {
new Request('/', {signal: {}})}, 'signal');
assert_throws(e, () => {
new Request('/', {signal: new Request('/')})}, 'signal');
assert_throws(e, () => {
new Request('/', {signal: new Response('/')})}, 'signal');
}, 'TypeError should be thrown when RequestInit\'s signal member does not implement the AbortSignal interface');
promise_test(function() { promise_test(function() {
var headers = new Headers; var headers = new Headers;
headers.set('Content-Language', 'ja'); headers.set('Content-Language', 'ja');
......
...@@ -454,8 +454,17 @@ Request* Request::CreateRequestWithRequestOrString( ...@@ -454,8 +454,17 @@ Request* Request::CreateRequestWithRequestOrString(
v8::Local<v8::Value> init_signal = v8::Local<v8::Value> init_signal =
init.hasSignal() ? init.signal().V8Value() : v8::Local<v8::Value>(); init.hasSignal() ? init.signal().V8Value() : v8::Local<v8::Value>();
if (!init_signal.IsEmpty()) { if (!init_signal.IsEmpty()) {
signal = V8AbortSignal::ToImplWithTypeCheck(script_state->GetIsolate(), if (init_signal->IsNull()) {
init_signal); // This allows a null AbortSignal to override |input_request|'s, which
// would otherwise be used.
signal = nullptr;
} else {
signal = NativeValueTraits<AbortSignal>::NativeValue(
script_state->GetIsolate(), init_signal, exception_state);
if (exception_state.HadException()) {
return nullptr;
}
}
} }
// "Let |r| be a new Request object associated with |request| and a new // "Let |r| be a new Request object associated with |request| and a new
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment