[JSC] Remove JSPromiseDeferred
Created attachment 381884 [details] Patch
Created attachment 381888 [details] Patch
Created attachment 381890 [details] Patch
Created attachment 381891 [details] Patch
Created attachment 381892 [details] Patch
Created attachment 381893 [details] Patch
Created attachment 381894 [details] Patch
Comment on attachment 381894 [details] Patch Attachment 381894 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/13175853 New failing tests: js/dom/promise-rejection-event-should-follow-webidl-promise-conversion-rule.html
Created attachment 381903 [details] Archive of layout-test-results from ews212 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews212 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Created attachment 381984 [details] Patch
Created attachment 381990 [details] Patch
Created attachment 381993 [details] Patch
<rdar://problem/32757152>
Created attachment 381994 [details] Patch
Created attachment 381995 [details] Patch
Comment on attachment 381995 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381995&action=review > Source/JavaScriptCore/API/JSAPIGlobalObject.mm:175 > - return deferred->reject(globalObject, exception); > + promise->reject(globalObject, exception); > + scope.clearException(); > + return promise; Wait... does this mean if there's a stack overflow we just return a non-rejected promise? That seems wrong... Shouldn't we propagate that stack overflow to the caller?
Comment on attachment 381995 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381995&action=review >> Source/JavaScriptCore/API/JSAPIGlobalObject.mm:175 >> + return promise; > > Wait... does this mean if there's a stack overflow we just return a non-rejected promise? That seems wrong... Shouldn't we propagate that stack overflow to the caller? Currently, this patch keeps the original behavior. And I think this is not possible in practice to get stack-overflow here since module loader callbacks are driven by microtask callbacks. So this function would be typically called in the bottom of the stack. I'm planning to add a new C++ function `Promise::rejectedPromise` to offer rejected promise without involving any JS things. This is useful to return a rejected promise when we found `scope.exception()`. But I filed this in a separate bug for now to make this patch simple (just keeping the current behavior in this patch) https://bugs.webkit.org/show_bug.cgi?id=203402.
Comment on attachment 381995 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381995&action=review r=me but I think the functionality you marked as deprecated is public API so we can't get rid of it. Can we remove the deprecated API naming? > Source/JavaScriptCore/runtime/Completion.cpp:197 > scope.releaseAssertNoException(); This seems wrong. > Source/JavaScriptCore/runtime/JSPromise.h:67 > + // Deprecated APIs. Remove it once SPIs are removed. I think we have shipped this API, I don't think we can remove it.
Comment on attachment 381995 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381995&action=review Thanks! >> Source/JavaScriptCore/runtime/Completion.cpp:197 >> scope.releaseAssertNoException(); > > This seems wrong. Removed, nice! >> Source/JavaScriptCore/runtime/JSPromise.h:67 >> + // Deprecated APIs. Remove it once SPIs are removed. > > I think we have shipped this API, I don't think we can remove it. Yeah, we already shipped it. Remove this comment, and rename `deprecatedCreateDeferredData` to `createDeferredData`.
Committed r251691: <https://trac.webkit.org/changeset/251691>
*** Bug 201160 has been marked as a duplicate of this bug. ***