Commit 88878f4f authored by Rayan Kanso's avatar Rayan Kanso Committed by Commit Bot

[Background Fetch] Reject call to updateUI if event is inactive.

Spec:
https://wicg.github.io/background-fetch/#background-fetch-update-ui-event-update-ui

Bug: 901909
Change-Id: Ia319cd03e41e65c5a6a1f5e61cde6f4ae7705a32
Reviewed-on: https://chromium-review.googlesource.com/c/1320330
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606118}
parent 5568b9ca
...@@ -248,8 +248,6 @@ void BackgroundFetchContext::UpdateUI( ...@@ -248,8 +248,6 @@ void BackgroundFetchContext::UpdateUI(
blink::mojom::BackgroundFetchService::UpdateUICallback callback) { blink::mojom::BackgroundFetchService::UpdateUICallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
// TODO(crbug.com/901909): This call should reject if the event
// is out of scope.
delegate_proxy_.UpdateUI(registration_id.unique_id(), title, icon); delegate_proxy_.UpdateUI(registration_id.unique_id(), title, icon);
std::move(callback).Run(blink::mojom::BackgroundFetchError::NONE); std::move(callback).Run(blink::mojom::BackgroundFetchError::NONE);
} }
......
importScripts('/resources/testharness.js');
importScripts('sw-helpers.js'); importScripts('sw-helpers.js');
async function updateUI(event) { async function updateUI(event) {
...@@ -13,10 +14,20 @@ async function updateUI(event) { ...@@ -13,10 +14,20 @@ async function updateUI(event) {
return Promise.all(updateParams.map(param => event.updateUI(param))) return Promise.all(updateParams.map(param => event.updateUI(param)))
.then(() => 'update success') .then(() => 'update success')
.catch(e => e.message); .catch(e => e.name);
} }
self.addEventListener('backgroundfetchsuccess', event => { self.addEventListener('backgroundfetchsuccess', event => {
if (event.registration.id === 'update-inactive') {
// Post an async task before calling updateUI from the inactive event.
// Any async behaviour outside `waitUntil` should mark the event as
// inactive, and subsequent calls to `updateUI` should fail.
new Promise(r => step_timeout(r, 0))
.then(() => event.updateUI({ title: 'New title' }))
.catch(e => sendMessageToDocument({ update: e.name }));
return;
}
event.waitUntil(updateUI(event) event.waitUntil(updateUI(event)
.then(update => sendMessageToDocument({ type: event.type, update }))) .then(update => sendMessageToDocument({ update })));
}); });
...@@ -27,6 +27,17 @@ backgroundFetchTest(async (test, backgroundFetch) => { ...@@ -27,6 +27,17 @@ backgroundFetchTest(async (test, backgroundFetch) => {
assert_equals(registration.id, registrationId); assert_equals(registration.id, registrationId);
const message = await getMessageFromServiceWorker(); const message = await getMessageFromServiceWorker();
assert_equals(message.update, 'updateUI may only be called once.'); assert_equals(message.update, 'InvalidStateError');
}, 'Background Fetch updateUI called twice fails', swName); }, 'Background Fetch updateUI called twice fails', swName);
backgroundFetchTest(async (test, backgroundFetch) => {
const registrationId = 'update-inactive';
const registration =
await backgroundFetch.fetch(registrationId, 'resources/feature-name.txt');
assert_equals(registration.id, registrationId);
const message = await getMessageFromServiceWorker();
assert_equals(message.update, 'InvalidStateError');
}, 'Background Fetch updateUI fails when event is not active', swName);
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "third_party/blink/renderer/modules/background_fetch/background_fetch_registration.h" #include "third_party/blink/renderer/modules/background_fetch/background_fetch_registration.h"
#include "third_party/blink/renderer/modules/background_fetch/background_fetch_ui_options.h" #include "third_party/blink/renderer/modules/background_fetch/background_fetch_ui_options.h"
#include "third_party/blink/renderer/modules/event_interface_modules_names.h" #include "third_party/blink/renderer/modules/event_interface_modules_names.h"
#include "third_party/blink/renderer/modules/service_worker/wait_until_observer.h"
#include "third_party/blink/renderer/platform/bindings/script_state.h" #include "third_party/blink/renderer/platform/bindings/script_state.h"
namespace blink { namespace blink {
...@@ -43,12 +44,19 @@ void BackgroundFetchUpdateUIEvent::Trace(blink::Visitor* visitor) { ...@@ -43,12 +44,19 @@ void BackgroundFetchUpdateUIEvent::Trace(blink::Visitor* visitor) {
ScriptPromise BackgroundFetchUpdateUIEvent::updateUI( ScriptPromise BackgroundFetchUpdateUIEvent::updateUI(
ScriptState* script_state, ScriptState* script_state,
const BackgroundFetchUIOptions* ui_options) { const BackgroundFetchUIOptions* ui_options) {
if (observer_ && !observer_->IsEventActive(script_state)) {
// Return a rejected promise as the event is no longer active.
return ScriptPromise::RejectWithDOMException(
script_state,
DOMException::Create(DOMExceptionCode::kInvalidStateError,
"ExtendableEvent is no longer active."));
}
if (update_ui_called_) { if (update_ui_called_) {
// Return a rejected promise as this method should only be called once. // Return a rejected promise as this method should only be called once.
return ScriptPromise::Reject( return ScriptPromise::RejectWithDOMException(
script_state, script_state,
V8ThrowException::CreateTypeError(script_state->GetIsolate(), DOMException::Create(DOMExceptionCode::kInvalidStateError,
"updateUI may only be called once.")); "updateUI may only be called once."));
} }
update_ui_called_ = true; update_ui_called_ = true;
......
...@@ -137,34 +137,13 @@ void WaitUntilObserver::WaitUntil(ScriptState* script_state, ...@@ -137,34 +137,13 @@ void WaitUntilObserver::WaitUntil(ScriptState* script_state,
ExceptionState& exception_state, ExceptionState& exception_state,
PromiseSettledCallback on_promise_fulfilled, PromiseSettledCallback on_promise_fulfilled,
PromiseSettledCallback on_promise_rejected) { PromiseSettledCallback on_promise_rejected) {
if (pending_promises_ == 0) { DCHECK_NE(event_dispatch_state_, EventDispatchState::kInitial);
switch (event_dispatch_state_) {
case EventDispatchState::kInitial: if (!IsEventActive(script_state)) {
NOTREACHED(); exception_state.ThrowDOMException(
return; DOMExceptionCode::kInvalidStateError,
case EventDispatchState::kDispatching: "The event handler is already finished and no extend lifetime "
if (!v8::MicrotasksScope::IsRunningMicrotasks( "promises are outstanding.");
script_state->GetIsolate())) {
break;
}
// didDispatchEvent() is called after both the event handler
// execution finished and microtasks queued by the event handler execution
// finished, it's hard to get the precise time point between the 2
// execution phases.
// So even in EventDispatchState::kDispatching state at this time point,
// running microtask indicates that event handler execution has actually
// finished, in such case if there aren't any outstanding extend lifetime
// promises, we should throw here.
FALLTHROUGH;
case EventDispatchState::kDispatched:
case EventDispatchState::kFailed:
exception_state.ThrowDOMException(
DOMExceptionCode::kInvalidStateError,
"The event handler is already finished "
"and no extend lifetime promises are "
"outstanding.");
return;
}
} }
if (!execution_context_) if (!execution_context_)
...@@ -194,6 +173,30 @@ void WaitUntilObserver::WaitUntil(ScriptState* script_state, ...@@ -194,6 +173,30 @@ void WaitUntilObserver::WaitUntil(ScriptState* script_state,
std::move(on_promise_rejected))); std::move(on_promise_rejected)));
} }
bool WaitUntilObserver::IsEventActive(ScriptState* script_state) const {
if (pending_promises_ > 0)
return true;
switch (event_dispatch_state_) {
case EventDispatchState::kDispatching:
// DidDispatchEvent() is called after both the event handler
// execution finished and microtasks queued by the event handler execution
// finished, it's hard to get the precise time point between the 2
// execution phases.
// So even in EventDispatchState::kDispatching state at this time point,
// running microtask indicates that event handler execution has actually
// finished, in such case if there aren't any outstanding extend lifetime
// promises.
return !v8::MicrotasksScope::IsRunningMicrotasks(
script_state->GetIsolate());
case EventDispatchState::kInitial:
case EventDispatchState::kDispatched:
case EventDispatchState::kFailed:
return false;
}
NOTREACHED();
}
WaitUntilObserver::WaitUntilObserver(ExecutionContext* context, WaitUntilObserver::WaitUntilObserver(ExecutionContext* context,
EventType type, EventType type,
int event_id) int event_id)
...@@ -243,7 +246,7 @@ void WaitUntilObserver::MaybeCompleteEvent() { ...@@ -243,7 +246,7 @@ void WaitUntilObserver::MaybeCompleteEvent() {
// event. // event.
break; break;
case EventDispatchState::kFailed: case EventDispatchState::kFailed:
// Dispatch had some error, complete the event immediatelly. // Dispatch had some error, complete the event immediately.
break; break;
} }
......
...@@ -69,6 +69,10 @@ class MODULES_EXPORT WaitUntilObserver final ...@@ -69,6 +69,10 @@ class MODULES_EXPORT WaitUntilObserver final
PromiseSettledCallback on_promise_fulfilled = PromiseSettledCallback(), PromiseSettledCallback on_promise_fulfilled = PromiseSettledCallback(),
PromiseSettledCallback on_promise_rejected = PromiseSettledCallback()); PromiseSettledCallback on_promise_rejected = PromiseSettledCallback());
// Whether the associated event is active.
// https://w3c.github.io/ServiceWorker/#extendableevent-active.
bool IsEventActive(ScriptState* script_state) const;
virtual void Trace(blink::Visitor*); virtual void Trace(blink::Visitor*);
private: private:
......
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