WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
203400
[JSC] Remove JSPromiseDeferred
https://bugs.webkit.org/show_bug.cgi?id=203400
Summary
[JSC] Remove JSPromiseDeferred
Yusuke Suzuki
Reported
2019-10-24 23:13:46 PDT
[JSC] Remove JSPromiseDeferred
Attachments
Patch
(100.81 KB, patch)
2019-10-24 23:15 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(105.78 KB, patch)
2019-10-24 23:55 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(140.75 KB, patch)
2019-10-25 00:24 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(163.75 KB, patch)
2019-10-25 00:34 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(164.26 KB, patch)
2019-10-25 00:37 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(110.95 KB, patch)
2019-10-25 01:12 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(168.03 KB, patch)
2019-10-25 01:14 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews212 for win-future
(13.91 MB, application/zip)
2019-10-25 03:31 PDT
,
EWS Watchlist
no flags
Details
Patch
(169.31 KB, patch)
2019-10-25 18:30 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(169.27 KB, patch)
2019-10-25 20:17 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(172.98 KB, patch)
2019-10-25 21:13 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(169.17 KB, patch)
2019-10-25 21:23 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(169.07 KB, patch)
2019-10-25 21:40 PDT
,
Yusuke Suzuki
keith_miller
: review+
Details
Formatted Diff
Diff
Show Obsolete
(11)
View All
Add attachment
proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1
2019-10-24 23:15:34 PDT
Created
attachment 381884
[details]
Patch
Yusuke Suzuki
Comment 2
2019-10-24 23:55:14 PDT
Created
attachment 381888
[details]
Patch
Yusuke Suzuki
Comment 3
2019-10-25 00:24:07 PDT
Created
attachment 381890
[details]
Patch
Yusuke Suzuki
Comment 4
2019-10-25 00:34:06 PDT
Created
attachment 381891
[details]
Patch
Yusuke Suzuki
Comment 5
2019-10-25 00:37:35 PDT
Created
attachment 381892
[details]
Patch
Yusuke Suzuki
Comment 6
2019-10-25 01:12:28 PDT
Created
attachment 381893
[details]
Patch
Yusuke Suzuki
Comment 7
2019-10-25 01:14:00 PDT
Created
attachment 381894
[details]
Patch
EWS Watchlist
Comment 8
2019-10-25 03:31:28 PDT
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
EWS Watchlist
Comment 9
2019-10-25 03:31:30 PDT
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
Yusuke Suzuki
Comment 10
2019-10-25 18:30:53 PDT
Created
attachment 381984
[details]
Patch
Yusuke Suzuki
Comment 11
2019-10-25 20:17:26 PDT
Created
attachment 381990
[details]
Patch
Yusuke Suzuki
Comment 12
2019-10-25 21:13:19 PDT
Created
attachment 381993
[details]
Patch
Yusuke Suzuki
Comment 13
2019-10-25 21:16:46 PDT
<
rdar://problem/32757152
>
Yusuke Suzuki
Comment 14
2019-10-25 21:23:34 PDT
Created
attachment 381994
[details]
Patch
Yusuke Suzuki
Comment 15
2019-10-25 21:40:22 PDT
Created
attachment 381995
[details]
Patch
Keith Miller
Comment 16
2019-10-28 11:49:08 PDT
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?
Yusuke Suzuki
Comment 17
2019-10-28 13:02:44 PDT
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
.
Keith Miller
Comment 18
2019-10-28 16:58:55 PDT
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.
Yusuke Suzuki
Comment 19
2019-10-28 18:34:48 PDT
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`.
Yusuke Suzuki
Comment 20
2019-10-28 18:37:19 PDT
Committed
r251691
: <
https://trac.webkit.org/changeset/251691
>
Yusuke Suzuki
Comment 21
2019-10-29 20:13:33 PDT
***
Bug 201160
has been marked as a duplicate of this bug. ***
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug