Bug 214774 – Fixed exception check handling below ScheduledAction::executeFunctionInContext()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Michael Saboff]

The simulated exception code found some issues in ScheduledAction::executeFunctionInContext() causing us not not properly account for exceptions thrown and caught there.

<rdar://problem/65925857>

[Michael Saboff]

Created attachment 405209 [details]
Patch

[Mark Lam]

Comment on attachment 405209 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405209&action=review

r=me with fixes.

> Source/WebCore/bindings/js/ScheduledAction.cpp:122
> +    EXCEPTION_ASSERT(catchScope.exception() == exception);

This should be `EXCEPTION_ASSERT(!catchScope.exception());` because by the time profiledCall() returns, it should have cleared the exception and return it via the NakedPtr.

> LayoutTests/js/dom/scheduled-action-exception-checks.html:1
> +<script>

Take a look at LayoutTests/js/dom/missing-exception-check-below-queueMicrotask.html and due the same with the stuff surrounding the script: they pass the needed JSC option and also make the test output cleaner.  Please rebase the expected results after you do that.

[Michael Saboff]

Created attachment 405215 [details]
Patch for Landing

(In reply to Mark Lam from comment #2)
> Comment on attachment 405209 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=405209&action=review
> 
> r=me with fixes.
> 
> > Source/WebCore/bindings/js/ScheduledAction.cpp:122
> > +    EXCEPTION_ASSERT(catchScope.exception() == exception);
> 
> This should be `EXCEPTION_ASSERT(!catchScope.exception());` because by the
> time profiledCall() returns, it should have cleared the exception and return
> it via the NakedPtr.
> 
> > LayoutTests/js/dom/scheduled-action-exception-checks.html:1
> > +<script>
> 
> Take a look at
> LayoutTests/js/dom/missing-exception-check-below-queueMicrotask.html and due
> the same with the stuff surrounding the script: they pass the needed JSC
> option and also make the test output cleaner.  Please rebase the expected
> results after you do that.

Done.

[Michael Saboff]

Committed r264876: <https://trac.webkit.org/changeset/264876>