Summary: | Fixed exception check handling below ScheduledAction::executeFunctionInContext() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||
Component: | WebCore JavaScript | Assignee: | Michael Saboff <msaboff> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | mark.lam, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Michael Saboff
2020-07-24 18:08:55 PDT
Created attachment 405209 [details]
Patch
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. 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. Committed r264876: <https://trac.webkit.org/changeset/264876> |