Skip to content

Commit

Permalink
imported/w3c/web-platform-tests/background-fetch/abort.https.window.h…
Browse files Browse the repository at this point in the history
…tml is flaky.

https://bugs.webkit.org/show_bug.cgi?id=253314
<rdar://problem/106195333>

Reviewed by Chris Dumez.

The test was flakily crashing in debug bots when trying to resolve the promise of BackgroundFetchRegistration::matchAll.
The issue is that the toJS call can fail due to an exception.  When this happens, it returns an empty value that is
used to resolve the promise.

To prevent this, we add a check in DeferredPromise::callFunction to handle uncaught exceptions.  Also fix all exception
check validation failures under imported/w3c/web-platform-tests/background-fetch.

* Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp:
(WebCore::DeferredPromise::callFunction):
(WebCore::DeferredPromise::whenSettled):
(WebCore::DeferredPromise::reject):
(WebCore::rejectPromiseWithExceptionIfAny):
(WebCore::DeferredPromise::handleTerminationExceptionIfNeeded):
* Source/WebCore/bindings/js/JSDOMPromiseDeferred.h:
(WebCore::DeferredPromise::resolve):
(WebCore::DeferredPromise::resolveWithNewlyCreated):
(WebCore::DeferredPromise::resolveCallbackValueWithNewlyCreated):
(WebCore::DeferredPromise::reject):
(WebCore::DeferredPromise::resolveWithCallback):
(WebCore::DeferredPromise::rejectWithCallback):
(WebCore::DeferredPromise::needsAbort const):
* Source/WebCore/bindings/js/JSMicrotaskCallback.h:
(WebCore::JSMicrotaskCallback::call):
* Source/WebCore/dom/BroadcastChannel.cpp:
(WebCore::BroadcastChannel::dispatchMessage):
* Source/WebCore/dom/MessagePort.cpp:
(WebCore::MessagePort::dispatchMessages):
* Source/WebCore/dom/Microtasks.cpp:
(WebCore::MicrotaskQueue::performMicrotaskCheckpoint):
* Source/WebCore/page/LocalDOMWindow.cpp:
(WebCore::LocalDOMWindow::processPostMessage):
* Source/WebCore/workers/WorkerMessagingProxy.cpp:
(WebCore::WorkerMessagingProxy::postMessageToWorkerObject):
(WebCore::WorkerMessagingProxy::postMessageToWorkerGlobalScope):
* Source/WebCore/workers/WorkerRunLoop.cpp:
(WebCore::WorkerDedicatedRunLoop::Task::performTask):
* Source/WebCore/workers/service/ServiceWorkerContainer.cpp:
(WebCore::ServiceWorkerContainer::jobResolvedWithRegistration):
(WebCore::ServiceWorkerContainer::postMessage):

Canonical link: https://commits.webkit.org/264489@main
  • Loading branch information
Mark Lam committed May 24, 2023
1 parent f06fb53 commit 37cf833
Show file tree
Hide file tree
Showing 10 changed files with 180 additions and 50 deletions.
35 changes: 27 additions & 8 deletions Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2013-2021 Apple Inc. All rights reserved.
* Copyright (C) 2013-2023 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -38,6 +38,7 @@
#include <JavaScriptCore/JSONObject.h>
#include <JavaScriptCore/JSPromiseConstructor.h>
#include <JavaScriptCore/Strong.h>
#include <wtf/Scope.h>

namespace WebCore {
using namespace JSC;
Expand All @@ -56,6 +57,14 @@ void DeferredPromise::callFunction(JSGlobalObject& lexicalGlobalObject, ResolveM
if (shouldIgnoreRequestToFulfill())
return;

JSC::VM& vm = lexicalGlobalObject.vm();
auto scope = DECLARE_CATCH_SCOPE(vm);

auto handleExceptionIfNeeded = makeScopeExit([&] {
if (UNLIKELY(scope.exception()))
handleUncaughtException(scope, *jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject));
});

if (activeDOMObjectsAreSuspended()) {
JSC::Strong<JSC::Unknown, ShouldStrongDestructorGrabLock::Yes> strongResolution(lexicalGlobalObject.vm(), resolution);
ASSERT(scriptExecutionContext()->eventLoop().isSuspended());
Expand Down Expand Up @@ -100,7 +109,14 @@ void DeferredPromise::whenSettled(Function<void()>&& callback)
return;
}

DOMPromise::whenPromiseIsSettled(globalObject(), deferred(), WTFMove(callback));
{
auto* globalObject = this->globalObject();
auto& vm = globalObject->vm();
JSC::JSLockHolder locker(vm);
auto scope = DECLARE_CATCH_SCOPE(vm);
DOMPromise::whenPromiseIsSettled(globalObject, deferred(), WTFMove(callback));
DEFERRED_PROMISE_HANDLE_AND_RETURN_IF_EXCEPTION(scope, globalObject);
}
}

void DeferredPromise::reject(RejectAsHandled rejectAsHandled)
Expand Down Expand Up @@ -144,10 +160,10 @@ void DeferredPromise::reject(Exception exception, RejectAsHandled rejectAsHandle
EXCEPTION_ASSERT(scope.exception());
auto error = scope.exception()->value();
bool isTerminating = handleTerminationExceptionIfNeeded(scope, lexicalGlobalObject);
scope.clearException();

if (!isTerminating)
if (!isTerminating) {
scope.clearException();
reject<IDLAny>(error, rejectAsHandled);
}
return;
}

Expand Down Expand Up @@ -179,10 +195,10 @@ void DeferredPromise::reject(ExceptionCode ec, const String& message, RejectAsHa
EXCEPTION_ASSERT(scope.exception());
auto error = scope.exception()->value();
bool isTerminating = handleTerminationExceptionIfNeeded(scope, lexicalGlobalObject);
scope.clearException();

if (!isTerminating)
if (!isTerminating) {
scope.clearException();
reject<IDLAny>(error, rejectAsHandled);
}
return;
}

Expand Down Expand Up @@ -214,6 +230,8 @@ void rejectPromiseWithExceptionIfAny(JSC::JSGlobalObject& lexicalGlobalObject, J
UNUSED_PARAM(lexicalGlobalObject);
if (LIKELY(!catchScope.exception()))
return;
if (catchScope.vm().hasPendingTerminationException())
return;

JSValue error = catchScope.exception()->value();
catchScope.clearException();
Expand Down Expand Up @@ -284,6 +302,7 @@ bool DeferredPromise::handleTerminationExceptionIfNeeded(CatchScope& scope, JSDO
bool terminatorCausedException = vm.isTerminationException(exception);
if (terminatorCausedException || (scriptController && scriptController->isTerminatingExecution())) {
scriptController->forbidExecution();
m_needsAbort = true;
return true;
}
}
Expand Down
61 changes: 44 additions & 17 deletions Source/WebCore/bindings/js/JSDOMPromiseDeferred.h
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2013-2021 Apple Inc. All rights reserved.
* Copyright (C) 2013-2023 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -37,6 +37,14 @@ namespace WebCore {
class JSLocalDOMWindow;
enum class RejectAsHandled : bool { No, Yes };

#define DEFERRED_PROMISE_HANDLE_AND_RETURN_IF_EXCEPTION(scope, globalObject) do { \
if (UNLIKELY(scope.exception())) { \
handleUncaughtException(scope, *jsCast<JSDOMGlobalObject*>(globalObject)); \
return; \
} \
} while (false)


class DeferredPromise : public DOMGuarded<JSC::JSPromise> {
public:
enum class Mode {
Expand Down Expand Up @@ -66,8 +74,13 @@ class DeferredPromise : public DOMGuarded<JSC::JSPromise> {
ASSERT(deferred());
ASSERT(globalObject());
JSC::JSGlobalObject* lexicalGlobalObject = globalObject();
JSC::JSLockHolder locker(lexicalGlobalObject);
resolve(*lexicalGlobalObject, toJS<IDLType>(*lexicalGlobalObject, *globalObject(), std::forward<typename IDLType::ParameterType>(value)));
auto& vm = lexicalGlobalObject->vm();
JSC::JSLockHolder locker(vm);
auto scope = DECLARE_CATCH_SCOPE(vm);
auto jsValue = toJS<IDLType>(*lexicalGlobalObject, *globalObject(), std::forward<typename IDLType::ParameterType>(value));
DEFERRED_PROMISE_HANDLE_AND_RETURN_IF_EXCEPTION(scope, lexicalGlobalObject);
resolve(*lexicalGlobalObject, jsValue);
DEFERRED_PROMISE_HANDLE_AND_RETURN_IF_EXCEPTION(scope, lexicalGlobalObject);
}

void resolveWithJSValue(JSC::JSValue resolution)
Expand Down Expand Up @@ -103,8 +116,12 @@ class DeferredPromise : public DOMGuarded<JSC::JSPromise> {
ASSERT(deferred());
ASSERT(globalObject());
JSC::JSGlobalObject* lexicalGlobalObject = globalObject();
JSC::JSLockHolder locker(lexicalGlobalObject);
resolve(*lexicalGlobalObject, toJSNewlyCreated<IDLType>(*lexicalGlobalObject, *globalObject(), std::forward<typename IDLType::ParameterType>(value)));
auto& vm = lexicalGlobalObject->vm();
JSC::JSLockHolder locker(vm);
auto scope = DECLARE_CATCH_SCOPE(vm);
auto jsValue = toJSNewlyCreated<IDLType>(*lexicalGlobalObject, *globalObject(), std::forward<typename IDLType::ParameterType>(value));
DEFERRED_PROMISE_HANDLE_AND_RETURN_IF_EXCEPTION(scope, lexicalGlobalObject);
resolve(*lexicalGlobalObject, jsValue);
}

template<class IDLType>
Expand All @@ -116,8 +133,12 @@ class DeferredPromise : public DOMGuarded<JSC::JSPromise> {
ASSERT(deferred());
ASSERT(globalObject());
auto* lexicalGlobalObject = globalObject();
JSC::JSLockHolder locker(lexicalGlobalObject);
resolve(*lexicalGlobalObject, toJSNewlyCreated<IDLType>(*lexicalGlobalObject, *globalObject(), createValue(*globalObject()->scriptExecutionContext())));
auto& vm = lexicalGlobalObject->vm();
JSC::JSLockHolder locker(vm);
auto scope = DECLARE_CATCH_SCOPE(vm);
auto jsValue = toJSNewlyCreated<IDLType>(*lexicalGlobalObject, *globalObject(), createValue(*globalObject()->scriptExecutionContext()));
DEFERRED_PROMISE_HANDLE_AND_RETURN_IF_EXCEPTION(scope, lexicalGlobalObject);
resolve(*lexicalGlobalObject, jsValue);
}

template<class IDLType>
Expand All @@ -129,8 +150,12 @@ class DeferredPromise : public DOMGuarded<JSC::JSPromise> {
ASSERT(deferred());
ASSERT(globalObject());
JSC::JSGlobalObject* lexicalGlobalObject = globalObject();
JSC::JSLockHolder locker(lexicalGlobalObject);
reject(*lexicalGlobalObject, toJS<IDLType>(*lexicalGlobalObject, *globalObject(), std::forward<typename IDLType::ParameterType>(value)), rejectAsHandled);
auto& vm = lexicalGlobalObject->vm();
JSC::JSLockHolder locker(vm);
auto scope = DECLARE_CATCH_SCOPE(vm);
auto jsValue = toJS<IDLType>(*lexicalGlobalObject, *globalObject(), std::forward<typename IDLType::ParameterType>(value));
DEFERRED_PROMISE_HANDLE_AND_RETURN_IF_EXCEPTION(scope, lexicalGlobalObject);
reject(*lexicalGlobalObject, jsValue, rejectAsHandled);
}

void reject(RejectAsHandled = RejectAsHandled::No);
Expand All @@ -148,12 +173,12 @@ class DeferredPromise : public DOMGuarded<JSC::JSPromise> {
ASSERT(deferred());
ASSERT(globalObject());
auto* lexicalGlobalObject = globalObject();
JSC::VM& vm = lexicalGlobalObject->vm();
auto& vm = lexicalGlobalObject->vm();
JSC::JSLockHolder locker(vm);
auto scope = DECLARE_CATCH_SCOPE(vm);
resolve(*lexicalGlobalObject, callback(*globalObject()));
if (UNLIKELY(scope.exception()))
handleUncaughtException(scope, *lexicalGlobalObject);
auto jsValue = callback(*globalObject());
DEFERRED_PROMISE_HANDLE_AND_RETURN_IF_EXCEPTION(scope, lexicalGlobalObject);
resolve(*lexicalGlobalObject, jsValue);
}

template<typename Callback>
Expand All @@ -168,14 +193,15 @@ class DeferredPromise : public DOMGuarded<JSC::JSPromise> {
JSC::VM& vm = lexicalGlobalObject->vm();
JSC::JSLockHolder locker(vm);
auto scope = DECLARE_CATCH_SCOPE(vm);
reject(*lexicalGlobalObject, callback(*globalObject()), rejectAsHandled);
if (UNLIKELY(scope.exception()))
handleUncaughtException(scope, *lexicalGlobalObject);
auto jsValue = callback(*globalObject());
DEFERRED_PROMISE_HANDLE_AND_RETURN_IF_EXCEPTION(scope, lexicalGlobalObject);
reject(*lexicalGlobalObject, jsValue, rejectAsHandled);
}

JSC::JSValue promise() const;

void whenSettled(Function<void()>&&);
bool needsAbort() const { return m_needsAbort; }

private:
DeferredPromise(JSDOMGlobalObject& globalObject, JSC::JSPromise& deferred, Mode mode)
Expand All @@ -198,9 +224,10 @@ class DeferredPromise : public DOMGuarded<JSC::JSPromise> {
}

bool handleTerminationExceptionIfNeeded(JSC::CatchScope&, JSDOMGlobalObject& lexicalGlobalObject);
void handleUncaughtException(JSC::CatchScope&, JSDOMGlobalObject& lexicalGlobalObject);
WEBCORE_EXPORT void handleUncaughtException(JSC::CatchScope&, JSDOMGlobalObject& lexicalGlobalObject);

Mode m_mode;
bool m_needsAbort { false };
};

class DOMPromiseDeferredBase {
Expand Down
4 changes: 1 addition & 3 deletions Source/WebCore/bindings/js/JSMicrotaskCallback.h
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2018 Yusuke Suzuki <utatane.tea@gmail.com>.
* Copyright (C) 2021 Apple Inc. All rights reserved.
* Copyright (C) 2021-2023 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -43,9 +43,7 @@ class JSMicrotaskCallback : public RefCounted<JSMicrotaskCallback> {
auto protectedThis { Ref { *this } };
JSC::VM& vm = m_globalObject->vm();
JSC::JSLockHolder lock(vm);
auto scope = DECLARE_CATCH_SCOPE(vm);
JSExecState::runTask(m_globalObject.get(), m_task);
scope.assertNoExceptionExceptTermination();
}

private:
Expand Down
8 changes: 8 additions & 0 deletions Source/WebCore/dom/BroadcastChannel.cpp
Expand Up @@ -248,7 +248,15 @@ void BroadcastChannel::dispatchMessage(Ref<SerializedScriptValue>&& message)
if (!globalObject)
return;

auto& vm = globalObject->vm();
auto scope = DECLARE_CATCH_SCOPE(vm);
auto event = MessageEvent::create(*globalObject, WTFMove(message), scriptExecutionContext()->securityOrigin()->toString());
if (UNLIKELY(scope.exception())) {
// Currently, we assume that the only way we can get here is if we have a termination.
RELEASE_ASSERT(vm.hasPendingTerminationException());
return;
}

dispatchEvent(event.event);
});
}
Expand Down
12 changes: 10 additions & 2 deletions Source/WebCore/dom/MessagePort.cpp
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2008 Apple Inc. All Rights Reserved.
* Copyright (C) 2008-2023 Apple Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Expand Down Expand Up @@ -278,6 +278,9 @@ void MessagePort::dispatchMessages()
return;

ASSERT(context->isContextThread());
auto* globalObject = context->globalObject();
auto& vm = globalObject->vm();
auto scope = DECLARE_CATCH_SCOPE(vm);

bool contextIsWorker = is<WorkerGlobalScope>(*context);
for (auto& message : messages) {
Expand All @@ -286,7 +289,12 @@ void MessagePort::dispatchMessages()
return;

auto ports = MessagePort::entanglePorts(*context, WTFMove(message.transferredPorts));
auto event = MessageEvent::create(*context->globalObject(), message.message.releaseNonNull(), { }, { }, { }, WTFMove(ports));
auto event = MessageEvent::create(*globalObject, message.message.releaseNonNull(), { }, { }, { }, WTFMove(ports));
if (UNLIKELY(scope.exception())) {
// Currently, we assume that the only way we can get here is if we have a termination.
RELEASE_ASSERT(vm.hasPendingTerminationException());
return;
}

// Per specification, each MessagePort object has a task source called the port message queue.
queueTaskKeepingObjectAlive(*this, TaskSource::PostedMessageQueue, [this, event = WTFMove(event)] {
Expand Down
46 changes: 31 additions & 15 deletions Source/WebCore/dom/Microtasks.cpp
@@ -1,6 +1,7 @@
/*
* Copyright (C) 2014 Yoav Weiss (yoav@yoav.ws)
* Copyright (C) 2015 Akamai Technologies Inc. All rights reserved.
* Copyright (C) 2023 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
Expand Down Expand Up @@ -52,43 +53,58 @@ void MicrotaskQueue::performMicrotaskCheckpoint()
return;

SetForScope change(m_performingMicrotaskCheckpoint, true);
JSC::JSLockHolder locker(vm());
VM& vm = this->vm();
JSC::JSLockHolder locker(vm);
auto catchScope = DECLARE_CATCH_SCOPE(vm);

Vector<std::unique_ptr<EventLoopTask>> toKeep;
while (!m_microtaskQueue.isEmpty()) {
while (!m_microtaskQueue.isEmpty() && !vm.executionForbidden()) {
Vector<std::unique_ptr<EventLoopTask>> queue = WTFMove(m_microtaskQueue);
for (auto& task : queue) {
auto* group = task->group();
if (!group || group->isStoppedPermanently())
continue;
if (group->isSuspended())
toKeep.append(WTFMove(task));
else
else {
task->execute();
if (UNLIKELY(!catchScope.clearExceptionExceptTermination()))
break; // Encountered termination.
}
}
}

vm().finalizeSynchronousJSExecution();
vm.finalizeSynchronousJSExecution();
m_microtaskQueue = WTFMove(toKeep);

auto checkpointTasks = std::exchange(m_checkpointTasks, { });
for (auto& checkpointTask : checkpointTasks) {
auto* group = checkpointTask->group();
if (!group || group->isStoppedPermanently())
continue;
if (!vm.executionForbidden()) {
auto checkpointTasks = std::exchange(m_checkpointTasks, { });
for (auto& checkpointTask : checkpointTasks) {
auto* group = checkpointTask->group();
if (!group || group->isStoppedPermanently())
continue;

if (group->isSuspended()) {
m_checkpointTasks.append(WTFMove(checkpointTask));
continue;
}
if (group->isSuspended()) {
m_checkpointTasks.append(WTFMove(checkpointTask));
continue;
}

checkpointTask->execute();
checkpointTask->execute();
if (UNLIKELY(!catchScope.clearExceptionExceptTermination()))
break; // Encountered termination.
}
}

// https://html.spec.whatwg.org/multipage/webappapis.html#perform-a-microtask-checkpoint (step 4).
m_eventLoop->forEachAssociatedContext([](auto& context) {
auto* vmPtr = &vm;
m_eventLoop->forEachAssociatedContext([vmPtr](auto& context) {
auto& vm = *vmPtr;
if (UNLIKELY(vm.executionForbidden()))
return;
auto catchScope = DECLARE_CATCH_SCOPE(vm);
if (auto* tracker = context.rejectedPromiseTracker())
tracker->processQueueSoon();
catchScope.clearExceptionExceptTermination();
});

// FIXME: We should cleanup Indexed Database transactions as per:
Expand Down
11 changes: 10 additions & 1 deletion Source/WebCore/page/LocalDOMWindow.cpp
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2006-2021 Apple Inc. All rights reserved.
* Copyright (C) 2006-2023 Apple Inc. All rights reserved.
* Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
*
* Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -933,11 +933,20 @@ void LocalDOMWindow::processPostMessage(JSC::JSGlobalObject& lexicalGlobalObject
if (!globalObject)
return;

auto& vm = globalObject->vm();
auto scope = DECLARE_CATCH_SCOPE(vm);

UserGestureIndicator userGestureIndicator(userGestureToForward);
InspectorInstrumentation::willDispatchPostMessage(frame, postMessageIdentifier);

auto ports = MessagePort::entanglePorts(*document(), WTFMove(message.transferredPorts));
auto event = MessageEvent::create(*globalObject, message.message.releaseNonNull(), sourceOrigin, { }, incumbentWindowProxy ? std::make_optional(MessageEventSource(WTFMove(incumbentWindowProxy))) : std::nullopt, WTFMove(ports));
if (UNLIKELY(scope.exception())) {
// Currently, we assume that the only way we can get here is if we have a termination.
RELEASE_ASSERT(vm.hasPendingTerminationException());
return;
}

dispatchEvent(event.event);

InspectorInstrumentation::didDispatchPostMessage(frame, postMessageIdentifier);
Expand Down

0 comments on commit 37cf833

Please sign in to comment.