Bug 202484 - Implement the Top-level await proposal
Summary: Implement the Top-level await proposal
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Enhancement
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-02 11:09 PDT by Myles Borins
Modified: 2021-06-21 12:08 PDT (History)
22 users (show)

See Also:


Attachments
WIP (11.26 KB, patch)
2021-02-01 15:48 PST, Keith Miller
no flags Details | Formatted Diff | Diff
WIP (63.52 KB, patch)
2021-02-21 13:56 PST, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (81.38 KB, patch)
2021-02-21 15:34 PST, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (80.16 KB, patch)
2021-02-21 15:44 PST, Keith Miller
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (80.93 KB, patch)
2021-02-21 15:53 PST, Keith Miller
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (82.05 KB, patch)
2021-02-21 16:02 PST, Keith Miller
no flags Details | Formatted Diff | Diff
Patch (101.97 KB, patch)
2021-02-21 16:05 PST, Keith Miller
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (93.58 KB, patch)
2021-02-21 16:36 PST, Keith Miller
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles Borins 2019-10-02 11:09:36 PDT
The proposal is currently Stage-3 in the TC39 process which means it is ready for engines to implement.

https://github.com/tc39/proposal-top-level-await

I'm the champion from the TC39 committee so please feel free to ask me any questions you might have.
Comment 1 Radar WebKit Bug Importer 2019-10-04 17:08:27 PDT
<rdar://problem/55999996>
Comment 2 Keith Miller 2021-02-01 15:48:35 PST
Created attachment 418936 [details]
WIP
Comment 3 Keith Miller 2021-02-21 13:56:42 PST
Created attachment 421151 [details]
WIP
Comment 4 Keith Miller 2021-02-21 15:34:43 PST
Created attachment 421154 [details]
Patch
Comment 5 Keith Miller 2021-02-21 15:44:05 PST
Created attachment 421155 [details]
Patch
Comment 6 Keith Miller 2021-02-21 15:53:44 PST
Created attachment 421156 [details]
Patch
Comment 7 Keith Miller 2021-02-21 16:02:44 PST
Created attachment 421157 [details]
Patch
Comment 8 Keith Miller 2021-02-21 16:05:41 PST
Created attachment 421158 [details]
Patch
Comment 9 Yusuke Suzuki 2021-02-21 16:13:36 PST
Comment on attachment 421156 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=421156&action=review

Please check WebCore side too since now Workers support modules?

> Source/JavaScriptCore/API/JSAPIGlobalObject.mm:261
> +    // FIXME: We should update the delegate callbacks for async modules.

Can you file a bug and paste URL?

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:958
> +        for (unsigned i = 0; i <= JSGenerator::Argument::NumberOfArguments; ++i)

Why isn't it `< jsgenerator::Argument::NumberOfArguments`? If it is because |this| parameter, I think,

initializeNextParameter(); // |this|
for (unsigned i = 0; i < JSGenerator::Argument::NumberOfArguments; ++i)
    initializeNextParameter(); 

would be better.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:3747
> +    // RELEASE_ASSERT(m_scopeNode->isFunctionNode());

Why is  it necessary? If this comment-out is right, can you remove this assert?

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:5066
> +    // FIXME: It seems like this will create a lot of constants if there are many yield points. Maybe we should op_inc the old state.

Can you file a bug and paste URL?
Comment 10 Keith Miller 2021-02-21 16:32:51 PST
Comment on attachment 421156 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=421156&action=review

>> Source/JavaScriptCore/API/JSAPIGlobalObject.mm:261
>> +    // FIXME: We should update the delegate callbacks for async modules.
> 
> Can you file a bug and paste URL?

Done.

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:958
>> +        for (unsigned i = 0; i <= JSGenerator::Argument::NumberOfArguments; ++i)
> 
> Why isn't it `< jsgenerator::Argument::NumberOfArguments`? If it is because |this| parameter, I think,
> 
> initializeNextParameter(); // |this|
> for (unsigned i = 0; i < JSGenerator::Argument::NumberOfArguments; ++i)
>     initializeNextParameter(); 
> 
> would be better.

Yeah, that's the reason. I'll change.

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:3747
>> +    // RELEASE_ASSERT(m_scopeNode->isFunctionNode());
> 
> Why is  it necessary? If this comment-out is right, can you remove this assert?

Yeah missed that during cleanup. Deleted.

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:5066
>> +    // FIXME: It seems like this will create a lot of constants if there are many yield points. Maybe we should op_inc the old state.
> 
> Can you file a bug and paste URL?

Done.
Comment 11 Keith Miller 2021-02-21 16:36:10 PST
Created attachment 421159 [details]
Patch for landing
Comment 12 Keith Miller 2021-02-21 16:41:55 PST
Landed in r273225
Comment 13 Aakash Jain 2021-02-22 04:27:32 PST
Seems like there was a follow-up fix: https://trac.webkit.org/r273226 (mentioning here just for reference)
Comment 14 Aakash Jain 2021-02-22 04:33:17 PST
(In reply to Keith Miller from comment #12)
> Landed in r273225
This broke 5 layout-tests on mac and ios, as also indicated by ews. Should have waited for EWS to complete.

- imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/evaluation-order-4-tla.html
- imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/document-write/module-tla-delayed.html
- imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/document-write/module-tla-import.html
- imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/document-write/module-tla-promise.html
- imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/document-write/module-tla-immediate-promise.html
Comment 15 Keith Miller 2021-02-22 10:26:23 PST
(In reply to Aakash Jain from comment #14)
> (In reply to Keith Miller from comment #12)
> > Landed in r273225
> This broke 5 layout-tests on mac and ios, as also indicated by ews. Should
> have waited for EWS to complete.
> 
> -
> imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-
> element/module/evaluation-order-4-tla.html
> -
> imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/
> document-write/module-tla-delayed.html
> -
> imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/
> document-write/module-tla-import.html
> -
> imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/
> document-write/module-tla-promise.html
> -
> imported/w3c/web-platform-tests/html/webappapis/dynamic-markup-insertion/
> document-write/module-tla-immediate-promise.html

Weird that those didn't show up for me locally. They just need to be rebaselined.
Comment 16 Ryan Haddad 2021-02-22 10:41:39 PST
(In reply to Aakash Jain from comment #14)
> (In reply to Keith Miller from comment #12)
> > Landed in r273225
> This broke 5 layout-tests on mac and ios, as also indicated by ews. Should
> have waited for EWS to complete.
Rebaselined these tests in r273261