Commit 2fa238e3 authored by Rayan Kanso's avatar Rayan Kanso Committed by Commit Bot

[Background Fetch] Fix match() crash.

Calling `match` was causing crashes in debug mode due to accessing
base::Optional<>::value without initialization. Calling match with
an unmatched request was crashing due to hitting a DCHECK. This returns
undefined now as per the spec.

Bug: 896768
Change-Id: I5d82e68ddca157a240ceaaec8990d2553366fbbb
Reviewed-on: https://chromium-review.googlesource.com/c/1289269
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601541}
parent 155c1f2b
...@@ -13,5 +13,7 @@ FAIL Fetches can have requests with a body promise_test: Unhandled rejection wit ...@@ -13,5 +13,7 @@ FAIL Fetches can have requests with a body promise_test: Unhandled rejection wit
PASS recordsAvailable is false after onbackgroundfetchsuccess finishes execution. PASS recordsAvailable is false after onbackgroundfetchsuccess finishes execution.
PASS Using Background Fetch to fetch a non-existent resource should fail. PASS Using Background Fetch to fetch a non-existent resource should fail.
PASS Fetches with mixed content should fail. PASS Fetches with mixed content should fail.
PASS Matching to a single request should work
PASS Matching to a non-existing request should work
Harness: the test ran to completion. Harness: the test ran to completion.
...@@ -282,3 +282,41 @@ backgroundFetchTest(async (test, backgroundFetch) => { ...@@ -282,3 +282,41 @@ backgroundFetchTest(async (test, backgroundFetch) => {
assert_equals(nullResponse, null); assert_equals(nullResponse, null);
}, 'Fetches with mixed content should fail.'); }, 'Fetches with mixed content should fail.');
backgroundFetchTest(async (test, backgroundFetch) => {
const registrationId = 'matchexistingrequest';
const registration =
await backgroundFetch.fetch(registrationId, 'resources/feature-name.txt');
assert_equals(registration.id, registrationId);
const {type, eventRegistration, results} = await getMessageFromServiceWorker();
assert_equals('backgroundfetchsuccess', type);
assert_equals(results.length, 1);
assert_equals(eventRegistration.id, registration.id);
assert_equals(eventRegistration.result, 'success');
assert_equals(eventRegistration.failureReason, '');
assert_true(results[0].url.includes('resources/feature-name.txt'));
assert_equals(results[0].status, 200);
assert_equals(results[0].text, 'Background Fetch');
}, 'Matching to a single request should work');
backgroundFetchTest(async (test, backgroundFetch) => {
const registrationId = 'matchmissingrequest';
const registration =
await backgroundFetch.fetch(registrationId, 'resources/feature-name.txt');
assert_equals(registration.id, registrationId);
const {type, eventRegistration, results} = await getMessageFromServiceWorker();
assert_equals('backgroundfetchsuccess', type);
assert_equals(results.length, 0);
assert_equals(eventRegistration.id, registration.id);
assert_equals(eventRegistration.result, 'success');
assert_equals(eventRegistration.failureReason, '');
}, 'Matching to a non-existing request should work');
\ No newline at end of file
...@@ -13,10 +13,33 @@ async function getFetchResult(record) { ...@@ -13,10 +13,33 @@ async function getFetchResult(record) {
} }
function handleBackgroundFetchEvent(event) { function handleBackgroundFetchEvent(event) {
let matchFunction = null;
switch (event.registration.id) {
case 'matchexistingrequest':
matchFunction = event.registration.match.bind(
event.registration, '/background-fetch/resources/feature-name.txt');
break;
case 'matchmissingrequest':
matchFunction = event.registration.match.bind(
event.registration, '/background-fetch/resources/missing.txt');
break;
default:
matchFunction = event.registration.matchAll.bind(event.registration);
break;
}
event.waitUntil( event.waitUntil(
event.registration.matchAll() matchFunction()
// Format `match(All)?` function results.
.then(records => {
if (!records) return []; // Nothing was matched.
if (!records.map) return [records]; // One entry was returned.
return records; // Already in a list.
})
// Extract responses.
.then(records => .then(records =>
Promise.all(records.map(record => getFetchResult(record)))) Promise.all(records.map(record => getFetchResult(record))))
// Clone registration and send message.
.then(results => { .then(results => {
const registrationCopy = cloneRegistration(event.registration); const registrationCopy = cloneRegistration(event.registration);
sendMessageToDocument( sendMessageToDocument(
......
...@@ -203,25 +203,27 @@ ScriptPromise BackgroundFetchRegistration::MatchImpl( ...@@ -203,25 +203,27 @@ ScriptPromise BackgroundFetchRegistration::MatchImpl(
ScriptPromise promise = resolver->Promise(); ScriptPromise promise = resolver->Promise();
// Convert |request| to WebServiceWorkerRequest. // Convert |request| to WebServiceWorkerRequest.
base::Optional<WebServiceWorkerRequest> request_to_match; base::Optional<WebServiceWorkerRequest> optional_request;
if (request.has_value()) { if (request.has_value()) {
WebServiceWorkerRequest request_to_match;
if (request->IsRequest()) { if (request->IsRequest()) {
request->GetAsRequest()->PopulateWebServiceWorkerRequest( request->GetAsRequest()->PopulateWebServiceWorkerRequest(
request_to_match.value()); request_to_match);
} else { } else {
Request* new_request = Request::Create( Request* new_request = Request::Create(
script_state, request->GetAsUSVString(), exception_state); script_state, request->GetAsUSVString(), exception_state);
if (exception_state.HadException()) if (exception_state.HadException())
return ScriptPromise(); return ScriptPromise();
new_request->PopulateWebServiceWorkerRequest(request_to_match.value()); new_request->PopulateWebServiceWorkerRequest(request_to_match);
} }
optional_request = request_to_match;
} }
DCHECK(registration_); DCHECK(registration_);
BackgroundFetchBridge::From(registration_) BackgroundFetchBridge::From(registration_)
->MatchRequests( ->MatchRequests(
developer_id_, unique_id_, request_to_match, developer_id_, unique_id_, optional_request,
std::move(cache_query_params), match_all, std::move(cache_query_params), match_all,
WTF::Bind(&BackgroundFetchRegistration::DidGetMatchingRequests, WTF::Bind(&BackgroundFetchRegistration::DidGetMatchingRequests,
WrapPersistent(this), WrapPersistent(resolver), match_all)); WrapPersistent(this), WrapPersistent(resolver), match_all));
...@@ -254,6 +256,11 @@ void BackgroundFetchRegistration::DidGetMatchingRequests( ...@@ -254,6 +256,11 @@ void BackgroundFetchRegistration::DidGetMatchingRequests(
} }
if (!return_all) { if (!return_all) {
if (settled_fetches.IsEmpty()) {
// Nothing was matched. Resolve with `undefined`.
resolver->Resolve();
return;
}
DCHECK_EQ(settled_fetches.size(), 1u); DCHECK_EQ(settled_fetches.size(), 1u);
DCHECK_EQ(to_return.size(), 1u); DCHECK_EQ(to_return.size(), 1u);
resolver->Resolve(to_return[0]); resolver->Resolve(to_return[0]);
......
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