This occurs after a given DeferredPromise has had its context destroyed in Document::~Document.
Created attachment 447487 [details] Patch
<rdar://problem/83069008>
Comment on attachment 447487 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447487&action=review > Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp:47 > + if (shouldIgnoreRequestToFulfill()) Per the name shouldIgnoreRequestToFulfill() is about ignoring requests to fulfill (meaning resolve or reject) the promise. Using is as a condition in the promise() getter doesn't seem like the appropriate approach.
Comment on attachment 447487 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447487&action=review Seems like this needs a test to land? >> Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp:47 >> + if (shouldIgnoreRequestToFulfill()) > > Per the name shouldIgnoreRequestToFulfill() is about ignoring requests to fulfill (meaning resolve or reject) the promise. Using is as a condition in the promise() getter doesn't seem like the appropriate approach. Maybe you can use the isEmpty() check instead?
Created attachment 448855 [details] Patch
(In reply to Chris Dumez from comment #4) > Comment on attachment 447487 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=447487&action=review > > Seems like this needs a test to land? > > >> Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp:47 > >> + if (shouldIgnoreRequestToFulfill()) > > > > Per the name shouldIgnoreRequestToFulfill() is about ignoring requests to fulfill (meaning resolve or reject) the promise. Using is as a condition in the promise() getter doesn't seem like the appropriate approach. > > Maybe you can use the isEmpty() check instead? Thank you for the feedback, I have updated the change to use isEmpty() instead. I have also attached a test case which catches the issue.
Created attachment 448858 [details] Patch
Comment on attachment 448858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448858&action=review > Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp:47 > + if (isEmpty()) Other functions in this class use shouldIgnoreRequestToFulfill rather than calling isEmpty directly. Did you consider that?
(In reply to Darin Adler from comment #8) > Comment on attachment 448858 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=448858&action=review > > > Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp:47 > > + if (isEmpty()) > > Other functions in this class use shouldIgnoreRequestToFulfill rather than > calling isEmpty directly. Did you consider that? Yes, the original proposal used shouldIgnoreRequestToFulfill(), but I have updated to isEmpty() per https://bugs.webkit.org/show_bug.cgi?id=234447#c3.
I see now: sorry for not reviewing the comments before reviewing.
(In reply to Darin Adler from comment #10) > I see now: sorry for not reviewing the comments before reviewing. No problem. Thank you for the review!
Created attachment 448880 [details] Patch
Created attachment 448959 [details] Patch
Update TestExpectations to skip the test on iOS (as well as windows) as pointer-lock, which this test relies on, is not supported there.
Comment on attachment 448959 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448959&action=review > LayoutTests/platform/ios/TestExpectations:181 > +fast/js-promise/js-promise-invalid-context-access.html [ Skip ] Seems really unfortunate that this test depends on pointer-lock.
Committed r287993 (246020@main): <https://commits.webkit.org/246020@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 448959 [details].