Commit 467f2d99 authored by Adam Rice's avatar Adam Rice Committed by Commit Bot

Resolve ScriptPromiseResolver when Body methods fail

When blink::BodyBuffer::StartLoading failed , the ScriptPromiseResolver was left
unresolved, resulting in a DCHECK when it was destroyed. Resolve it explicitly
in this case, to avoid the DCHECK and retain the invariant.

BUG=893015

Change-Id: Ib437986c6dc0d8b40e8ed0040f8796387d94feb6
Reviewed-on: https://chromium-review.googlesource.com/c/1278613Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599589}
parent 977f98b4
......@@ -1448,3 +1448,19 @@ crbug.com/878650 [ Release ] inspector-protocol/heap-profiler/heap-snapshot-with
crbug.com/878995 [ Win ] external/wpt/async-local-storage/key-types.tentative.https.html [ Slow ]
crbug.com/888225 external/wpt/svg/idlharness.window.html [ Slow ]
crbug.com/893015 [ Linux ] http/tests/fetch/chromium/response-array-buffer-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] http/tests/fetch/chromium/response-blob-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] http/tests/fetch/chromium/response-form-data-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] http/tests/fetch/chromium/response-json-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] http/tests/fetch/chromium/response-text-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] virtual/outofblink-cors/http/tests/fetch/chromium/response-array-buffer-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] virtual/outofblink-cors/http/tests/fetch/chromium/response-blob-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] virtual/outofblink-cors/http/tests/fetch/chromium/response-form-data-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] virtual/outofblink-cors/http/tests/fetch/chromium/response-json-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] virtual/outofblink-cors/http/tests/fetch/chromium/response-text-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] virtual/outofblink-cors-ns/http/tests/fetch/chromium/response-array-buffer-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] virtual/outofblink-cors-ns/http/tests/fetch/chromium/response-blob-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] virtual/outofblink-cors-ns/http/tests/fetch/chromium/response-form-data-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] virtual/outofblink-cors-ns/http/tests/fetch/chromium/response-json-gc-crash.html [ Slow ]
crbug.com/893015 [ Linux ] virtual/outofblink-cors-ns/http/tests/fetch/chromium/response-text-gc-crash.html [ Slow ]
// Test that Response methods which return a Promise don't crash after garbage
// collection. Crashes on failure. This function is extremely slow on MSAN
// builds. Each call should be a separate HTML file and added to SlowTests.
// See https://bugs.chromium.org/p/chromium/issues/detail?id=829790#c5
function testResponseMethod(methodName) {
promise_test(async () => {
const rs = new ReadableStream();
const response = new Response(rs);
try {
// This throws an exception in debug builds but not in release builds.
// If the process doesn't crash, the test passed.
fillStackAndRun(() =>
Response.prototype[methodName].call(response).catch(() => {}),
784);
} catch (e) {
}
await asyncGC();
}, `stack overflow in response.${methodName}() should not crash the browser`);
}
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/js-test-resources/gc.js"></script>
<script src="resources/stack-overflow.js"></script>
<script src="resources/response-method-gc-crash.js"></script>
<script>
// See https://bugs.chromium.org/p/chromium/issues/detail?id=829790#c5
testResponseMethod('arrayBuffer');
</script>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/js-test-resources/gc.js"></script>
<script src="resources/stack-overflow.js"></script>
<script src="resources/response-method-gc-crash.js"></script>
<script>
// See https://bugs.chromium.org/p/chromium/issues/detail?id=829790#c5
testResponseMethod('blob');
</script>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/js-test-resources/gc.js"></script>
<script src="resources/stack-overflow.js"></script>
<script src="resources/response-method-gc-crash.js"></script>
<script>
// See https://bugs.chromium.org/p/chromium/issues/detail?id=829790#c5
testResponseMethod('formData');
</script>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/js-test-resources/gc.js"></script>
<script src="resources/stack-overflow.js"></script>
<script src="resources/response-method-gc-crash.js"></script>
<script>
// See https://bugs.chromium.org/p/chromium/issues/detail?id=829790#c5
testResponseMethod('json');
</script>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/js-test-resources/gc.js"></script>
<script src="resources/stack-overflow.js"></script>
<script src="resources/response-method-gc-crash.js"></script>
<script>
// See https://bugs.chromium.org/p/chromium/issues/detail?id=829790#c5
testResponseMethod('text');
</script>
......@@ -154,8 +154,11 @@ ScriptPromise Body::arrayBuffer(ScriptState* script_state,
BodyBuffer()->StartLoading(FetchDataLoader::CreateLoaderAsArrayBuffer(),
new BodyArrayBufferConsumer(resolver),
exception_state);
if (exception_state.HadException())
if (exception_state.HadException()) {
// Need to resolve the ScriptPromiseResolver to avoid a DCHECK().
resolver->Resolve();
return ScriptPromise();
}
} else {
resolver->Resolve(DOMArrayBuffer::Create(0u, 1));
}
......@@ -178,8 +181,11 @@ ScriptPromise Body::blob(ScriptState* script_state,
BodyBuffer()->StartLoading(
FetchDataLoader::CreateLoaderAsBlobHandle(MimeType()),
new BodyBlobConsumer(resolver), exception_state);
if (exception_state.HadException())
if (exception_state.HadException()) {
// Need to resolve the ScriptPromiseResolver to avoid a DCHECK().
resolver->Resolve();
return ScriptPromise();
}
} else {
std::unique_ptr<BlobData> blob_data = BlobData::Create();
blob_data->SetContentType(MimeType());
......@@ -211,8 +217,11 @@ ScriptPromise Body::formData(ScriptState* script_state,
body_buffer->StartLoading(
FetchDataLoader::CreateLoaderAsFormData(boundary),
new BodyFormDataConsumer(resolver), exception_state);
if (exception_state.HadException())
if (exception_state.HadException()) {
// Need to resolve the ScriptPromiseResolver to avoid a DCHECK().
resolver->Resolve();
return ScriptPromise();
}
return promise;
}
} else if (parsedType == "application/x-www-form-urlencoded") {
......@@ -220,8 +229,11 @@ ScriptPromise Body::formData(ScriptState* script_state,
BodyBuffer()->StartLoading(FetchDataLoader::CreateLoaderAsString(),
new BodyFormDataConsumer(resolver),
exception_state);
if (exception_state.HadException())
if (exception_state.HadException()) {
// Need to resolve the ScriptPromiseResolver to avoid a DCHECK().
resolver->Resolve();
return ScriptPromise();
}
} else {
resolver->Resolve(FormData::Create());
}
......@@ -231,8 +243,11 @@ ScriptPromise Body::formData(ScriptState* script_state,
BodyBuffer()->StartLoading(FetchDataLoader::CreateLoaderAsFailure(),
new BodyFormDataConsumer(resolver),
exception_state);
if (exception_state.HadException())
if (exception_state.HadException()) {
// Need to resolve the ScriptPromiseResolver to avoid a DCHECK().
resolver->Resolve();
return ScriptPromise();
}
return promise;
}
}
......@@ -257,8 +272,11 @@ ScriptPromise Body::json(ScriptState* script_state,
if (BodyBuffer()) {
BodyBuffer()->StartLoading(FetchDataLoader::CreateLoaderAsString(),
new BodyJsonConsumer(resolver), exception_state);
if (exception_state.HadException())
if (exception_state.HadException()) {
// Need to resolve the ScriptPromiseResolver to avoid a DCHECK().
resolver->Resolve();
return ScriptPromise();
}
} else {
resolver->Reject(V8ThrowException::CreateSyntaxError(
script_state->GetIsolate(), "Unexpected end of input"));
......@@ -281,8 +299,11 @@ ScriptPromise Body::text(ScriptState* script_state,
if (BodyBuffer()) {
BodyBuffer()->StartLoading(FetchDataLoader::CreateLoaderAsString(),
new BodyTextConsumer(resolver), exception_state);
if (exception_state.HadException())
if (exception_state.HadException()) {
// Need to resolve the ScriptPromiseResolver to avoid a DCHECK().
resolver->Resolve();
return ScriptPromise();
}
} else {
resolver->Resolve(String());
}
......
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