Bug 214774

Summary: Fixed exception check handling below ScheduledAction::executeFunctionInContext()
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: WebCore JavaScriptAssignee: 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 Flags
Patch
mark.lam: review+
Patch for Landing mark.lam: review+

Michael Saboff
Reported 2020-07-24 18:08:55 PDT
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>
Attachments
Patch (3.90 KB, patch)
2020-07-24 18:15 PDT, Michael Saboff 		mark.lam: review+
DetailsFormatted DiffDiff
Patch for Landing (3.97 KB, patch)
2020-07-24 18:55 PDT, Michael Saboff 		mark.lam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Michael Saboff
Comment 1 2020-07-24 18:15:23 PDT
Created attachment 405209 [details] Patch
Mark Lam
Comment 2 2020-07-24 18:25:21 PDT
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
Comment 3 2020-07-24 18:55:47 PDT
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
Comment 4 2020-07-24 19:09:21 PDT
Committed r264876: <https://trac.webkit.org/changeset/264876>
Note You need to log in before you can comment on or make changes to this bug.