Skip to content

Commit

Permalink
Cherry-pick 265870.579@safari-7616-branch (587ed30). https://bugs.web…
Browse files Browse the repository at this point in the history
…kit.org/show_bug.cgi?id=261829

    Regression(265870.536@safari-7616-branch) Crashes under DeferredPromise::callFunction()
    https://bugs.webkit.org/show_bug.cgi?id=261829
    rdar://115712299

    Reviewed by Brent Fulgham and Mark Lam.

    The RELEASE_ASSERT() added by  Ryosuke in 265870.536@safari-7616-branch in DeferredPromise::callFunction()
    is hitting a lot in the wild, from various call sites. The assertion makes sure we're allowed to run
    script when resolving a promise (i.e. we're not in the middle of layout or style resolution).

    * LayoutTests/webaudio/promise-resolution-crash-expected.txt: Added.
    * LayoutTests/webaudio/promise-resolution-crash.html: Added.
    New test coverage.

    * Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp:
    (WebCore::DeferredPromise::callFunction):
    Drop the release assertion and instead schedule a task to resolve the promise
    asynchronously when we're not allowed to run script, similarly to what we were
    already doing when b/f cache suspended.

    Canonical link: https://commits.webkit.org/265870.579@safari-7616-branch
  • Loading branch information
cdumez authored and aperezdc committed Oct 24, 2023
1 parent 4660562 commit 325c110
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 4 deletions.
1 change: 1 addition & 0 deletions LayoutTests/webaudio/promise-resolution-crash-expected.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This test passes if it doesn't crash.
7 changes: 7 additions & 0 deletions LayoutTests/webaudio/promise-resolution-crash.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
This test passes if it doesn't crash.
<script>
if (window.testRunner)
testRunner.dumpAsText();

onbeforeunload = () => new AudioContext().close();
</script>
6 changes: 2 additions & 4 deletions Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ void DeferredPromise::callFunction(JSGlobalObject& lexicalGlobalObject, ResolveM
if (shouldIgnoreRequestToFulfill())
return;

RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(ScriptDisallowedScope::isScriptAllowedInMainThread());

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

Expand All @@ -68,9 +66,9 @@ void DeferredPromise::callFunction(JSGlobalObject& lexicalGlobalObject, ResolveM
handleUncaughtException(scope, *jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject));
});

if (activeDOMObjectsAreSuspended()) {
if (activeDOMObjectsAreSuspended() || !ScriptDisallowedScope::isScriptAllowedInMainThread()) {
JSC::Strong<JSC::Unknown, ShouldStrongDestructorGrabLock::Yes> strongResolution(lexicalGlobalObject.vm(), resolution);
ASSERT(scriptExecutionContext()->eventLoop().isSuspended());
ASSERT(!activeDOMObjectsAreSuspended() || scriptExecutionContext()->eventLoop().isSuspended());
scriptExecutionContext()->eventLoop().queueTask(TaskSource::Networking, [this, protectedThis = Ref { *this }, mode, strongResolution = WTFMove(strongResolution)]() mutable {
if (shouldIgnoreRequestToFulfill())
return;
Expand Down

0 comments on commit 325c110

Please sign in to comment.