Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Top-level await integration #4352

Merged
merged 4 commits into from
Dec 10, 2020
+30 −49
Merged

Top-level await integration #4352

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb9e34a
Top-level await integration
littledan Feb 10, 2019
d7d0993
Fix promise return value
domenic Oct 12, 2020
d4c51ce
Tweaks
domenic Oct 12, 2020
714a7e7
Fix review comments.
Ms2ger Nov 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 31 additions & 43 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2653,6 +2653,8 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
sequence of Unicode scalar values</dfn></li>
<li><dfn data-x-href="https://heycam.github.io/webidl/#dfn-overload-resolution-algorithm">overload resolution algorithm</dfn></li>
<li><dfn data-x="idl-exposed" data-x-href="https://heycam.github.io/webidl/#dfn-exposed">exposed</dfn></li>
<li><dfn data-x-href="https://heycam.github.io/webidl/#a-promise-rejected-with">a promise rejected with</dfn></li>
<li><dfn data-x-href="https://heycam.github.io/webidl/#upon-rejection">upon rejection</dfn></li>
<li><dfn data-x="LegacyFactoryFunction" data-x-href="https://heycam.github.io/webidl/#LegacyFactoryFunction"><code>[LegacyFactoryFunction]</code></dfn></li>
<li><dfn data-x="LegacyLenientThis" data-x-href="https://heycam.github.io/webidl/#LegacyLenientThis"><code>[LegacyLenientThis]</code></dfn></li>
<li><dfn data-x="LegacyNullToEmptyString" data-x-href="https://heycam.github.io/webidl/#LegacyNullToEmptyString"><code>[LegacyNullToEmptyString]</code></dfn></li>
Expand Down Expand Up @@ -2866,6 +2868,9 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute

<p>Users agents that support JavaScript must also implement <cite>ECMAScript
Internationalization API</cite>. <ref spec=JSINTL></p>

<p>User agents that support JavaScript must also implement the <cite>top-level await</cite>
Ms2ger marked this conversation as resolved.
Show resolved Hide resolved
proposal. <ref spec=JSTLA></p>
</dd>


Expand Down Expand Up @@ -90353,11 +90358,9 @@ document.querySelector("button").addEventListener("click", bound);
<h5 id="calling-scripts">Calling scripts</h5>

<p>To <dfn export>run a classic script</dfn> given a <span>classic script</span> <var>script</var>
and an optional <var>rethrow errors</var> boolean:</p>
and an optional boolean <var>rethrow errors</var> (default false):</p>

<ol>
<li><p>If <var>rethrow errors</var> is not given, let it be false.</p></li>

<li><p>Let <var>settings</var> be the <span>settings object</span> of <var>script</var>.</p></li>

<li><p><span>Check if we can run script</span> with <var>settings</var>. If this returns "do
Expand Down Expand Up @@ -90434,25 +90437,21 @@ document.querySelector("button").addEventListener("click", bound);
</p></li>
</ol>

<p>To <dfn export>run a module script</dfn> given a <span>module script</span> <var>script</var>,
with an optional <var>rethrow errors</var> boolean:</p>
<p>To <dfn export>run a module script</dfn> given a <span>module script</span> <var>script</var>
and an optional boolean <var>preventErrorReporting</var> (default false):</p>

<ol>
<li><p>If <var>rethrow errors</var> is not given, let it be false.</p></li>

<li><p>Let <var>settings</var> be the <span>settings object</span> of <var>script</var>.</p></li>

<li><p><span>Check if we can run script</span> with <var>settings</var>. If this returns "do
not run" then return <span>NormalCompletion</span>(empty).</p></li>
not run", then return a promise resolved with undefined.</p></li>

<li><p><span>Prepare to run script</span> given <var>settings</var>.</p></li>

<li><p>Let <var>evaluationStatus</var> be null.</p></li>
Ms2ger marked this conversation as resolved.
Show resolved Hide resolved

<li><p>If <var>script</var>'s <span data-x="concept-script-error-to-rethrow">error to
rethrow</span> is not null, then set <var>evaluationStatus</var> to Completion { [[Type]]: throw,
[[Value]]: <var>script</var>'s <span data-x="concept-script-error-to-rethrow">error to
rethrow</span>, [[Target]]: empty }.</p></li>
rethrow</span> is not null, then let <var>evaluationPromise</var> be <span>a promise rejected
with</span> <var>script</var>'s <span data-x="concept-script-error-to-rethrow">error to
rethrow</span>.</p></li>

<li>
<p>Otherwise:</p>
Expand All @@ -90462,35 +90461,26 @@ document.querySelector("button").addEventListener("click", bound);
data-x="concept-script-record">record</span>.</p>

<li>
<p>Set <var>evaluationStatus</var> to <var>record</var>.<span
<p>Let <var>evaluationPromise</var> be <var>record</var>.<span
data-x="js-Evaluate">Evaluate</span>().</p>

<p class="note">This step will recursively evaluate all of the module's dependencies.</p>

<p>If <span data-x="js-Evaluate">Evaluate</span> fails to complete as a result of the user agent
<span data-x="abort a running script">aborting the running script</span>, then set
<var>evaluationStatus</var> to Completion { [[Type]]: throw, [[Value]]: a new
<span>"<code>QuotaExceededError</code>"</span> <code>DOMException</code>, [[Target]]: empty
}.</p>
<span data-x="abort a running script">aborting the running script</span>, then let
<var>evaluationPromise</var> be <span>a promise rejected with</span> a new
<span>"<code>QuotaExceededError</code>"</span> <code>DOMException</code>.</p>
</li>
</ol>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't the user agent abort a module evaluation after the first await? Unless I'm missing something, in that case evaluationPromise would never resolve.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, very interesting. So an instance of the case we're wondering about is

<script type="module">
try {
  await import("./bad.mjs");
} catch (e) {
  // e should ideally be a "QuotaExceededError" DOMException
}
</script>
// bad.mjs
await 1;
while (true) { }
await 2;

Any ideas what implementations are planning to do, @codehag and @camillobruni?

Copy link
Contributor

@syg syg Dec 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find! I tested in Chrome that an infinite looping module can hang the renderer when dynamically imported. The slow script killer doesn't kill dynamic imports correctly, even without top-level await. That is, the following also hangs:

<script>
(async function() {
  await import("./bad.mjs");
} catch (e) {
  // e should ideally be a "QuotaExceededError" DOMException
}
})();
</script>
// bad.mjs
while (true) { }

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=1157321

Copy link

@codehag codehag Dec 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that firefox has the same behavior as what @syg described for chrome above: both the test cases result in the tab hanging.

We do show a user prompt however, about a slow script and let the user respond. When the user responds on the second use case, the script is stopped and the page becomes usable again. Crashes with TLA though.

The bug tracking this on the firefox side is https://bugzilla.mozilla.org/show_bug.cgi?id=1681664

cc @annevk

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright. A simpler example without dynamic import is the following:

<script>
window.onerror = event => {
  console.log(event.error); // should ideally log a "QuotaExceededError" DOMException
};
</script>

<script type="module">
await 1;
while (true) { }
</script>

I'll try and work on some spec text giving UAs license to terminate top-level-await using scripts in the same way they are currently given license to terminate other scripts, and I'll be sure it also covers dynamic import(). I might merge this first if the fix seems especially separable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking like more of a mess than I thought. The script-killing section of the spec isn't very well defined or aligned with UAs; e.g. as far as I can tell from Chrome it kills the entire process after prompting, which is not really allowed by the current spec text. And we're not sure if any browser actually bothers with the "QuotaExceededError" DOMException business.

So I'll merge this as-is and file a followup bug.

</li>

<li>
<p>If <var>evaluationStatus</var> is an <span>abrupt completion</span>, then:</p>

<ol>
<li><p>If <var>rethrow errors</var> is true, rethrow the exception given by
<var>evaluationStatus</var>.[[Value]].</p></li>

<li><p>Otherwise, <span>report the exception</span> given by
<var>evaluationStatus</var>.[[Value]] for <var>script</var>.</p></li>
</ol>
</li>
<li><p>If <var>preventErrorReporting</var> is false, then <span>upon rejection</span> of
<var>evaluationPromise</var> with <var>reason</var>, <span>report the exception</span> given by
<var>reason</var> for <var>script</var>.</p></li>

<li><p><span>Clean up after running script</span> with <var>settings</var>.</p></li>

<li><p>Return <var>evaluationStatus</var>.</p></li>
<li><p>Return <var>evaluationPromise</var>.</p></li>
</ol>

<p>The steps to <dfn>check if we can run script</dfn> with an <span>environment settings
Expand Down Expand Up @@ -91245,26 +91235,21 @@ import "https://example.com/foo/../module2.mjs";</code></pre>
<p>If <var>result</var> is null, then:</p>

<ol>
<li><p>Let <var>completion</var> be Completion { [[Type]]: throw, [[Value]]: a new
<code>TypeError</code>, [[Target]]: empty }.</p></li>
<li><p>Let <var>promise</var> be <span>a promise rejected with</span> a new
<code>TypeError</code>.</p></li>
Ms2ger marked this conversation as resolved.
Show resolved Hide resolved

<li><p>Perform <span>FinishDynamicImport</span>(<var>referencingScriptOrModule</var>,
<var>specifier</var>, <var>promiseCapability</var>, <var>completion</var>).</p></li>
<var>specifier</var>, <var>promiseCapability</var>, <var>promise</var>).</p></li>

<li><p>Return.</p></li>
<li><p>Return undefined.</p></li>
</ol>
</li>

<li><p><span data-x="run a module script">Run the module script</span> <var>result</var>, with
the rethrow errors boolean set to true.</p></li>
<li><p>Let <var>promise</var> be the result of <span data-x="run a module script">running a
module script</span> given <var>result</var> and true.</p></li>

<li><p>If running the module script throws an exception, then perform
<span>FinishDynamicImport</span>(<var>referencingScriptOrModule</var>, <var>specifier</var>,
<var>promiseCapability</var>, the thrown exception completion).</p></li>

<li><p>Otherwise, perform
<span>FinishDynamicImport</span>(<var>referencingScriptOrModule</var>, <var>specifier</var>,
<var>promiseCapability</var>, <span>NormalCompletion</span>(undefined)).</p></li>
<li><p>Perform <span>FinishDynamicImport</span>(<var>referencingScriptOrModule</var>,
<var>specifier</var>, <var>promiseCapability</var>, <var>promise</var>).</p></li>

<li><p>Return undefined.</p></li>
</ol>
Expand Down Expand Up @@ -123109,6 +123094,9 @@ INSERT INTERFACES HERE
<dt id="refsJSINTL">[JSINTL]</dt>
<dd><cite><a href="https://tc39.es/ecma402/">ECMAScript Internationalization API Specification</a></cite>. Ecma International.</dd>

<dt id="refsJSTLA">[JSTLA]</dt>
<dd><cite><a href="https://tc39.es/proposal-top-level-await/">Top Level Await</a></cite>. Ecma International.</dd>
Ms2ger marked this conversation as resolved.
Show resolved Hide resolved

<dt id="refsJSON">[JSON]</dt>
<dd><cite><a href="https://tools.ietf.org/html/rfc7159">The JavaScript Object Notation (JSON) Data Interchange Format</a></cite>, T. Bray. IETF.</dd>

Expand Down