Editorial: rigorize tasks and task queuing #4465

domenic · 2019-03-29T18:54:42Z

This introduces tasks as a proper struct, with well-defined fields, and
makes "queue a task" and "queue a microtask" into more well-defined
algorithms for setting up and queuing those tasks. It centralizes and
calls out the still-somewhat-vague parts in the "implied event loop" and
"implied document" definitions.

Other minor cleanups and clarifications:

Fixes "performing a microtask checkpoint" to be a boolean; previously
it was referred to as a flag but set to true or false.
Fixes Script event-loop: ignoring tasks for documents that are not fully active #4242 by making it clearer how tasks are chosen by the event
loop.
Adds more guidance text around how task sources and task queues
interact.
Introduces a new subsection "Queuing tasks" to contain the
task-queuing definitions, which were previously spread out through
"Definitions" (for non-microtask tasks) and "Processing model" (for
microtasks).
Centralizes all properties of the event loop into "Definitions". They
were previously spread out throughout "Definitions" and "Processing
model".

/cc @littledan, although this is probably only tangentially related to stuff you've been thinking about.

Interesting things to note about the task struct is how rarely some of its fields are used:

type is not really used. I guess I will try to remove it in a second commit which we can squash.
source is only used because spin the event loop can suspend a task and then we need to put the continuation back onto the same task source

/index.html ( diff )
/webappapis.html ( diff )

This introduces tasks as a proper struct, with well-defined fields, and makes "queue a task" and "queue a microtask" into more well-defined algorithms for setting up and queuing those tasks. It centralizes and calls out the still-somewhat-vague parts in the "implied event loop" and "implied document" definitions. Other minor cleanups and clarifications: * Fixes "performing a microtask checkpoint" to be a boolean; previously it was referred to as a flag but set to true or false. * Fixes #4242 by making it clearer how tasks are chosen by the event loop. * Adds more guidance text around how task sources and task queues interact. * Introduces a new subsection "Queuing tasks" to contain the task-queuing definitions, which were previously spread out through "Definitions" (for non-microtask tasks) and "Processing model" (for microtasks). * Centralizes all properties of the event loop into "Definitions". They were previously spread out throughout "Definitions" and "Processing model".

source

annevk · 2019-04-05T12:24:40Z

source

+   <dd>A <code>Document</code> associated with the task, or null for tasks that are not in a
+   <span>window event loop</span>.</dd>
+
+   <dt>A <dfn data-export="">script evaluation environment settings object set</dfn></dt>


Why is only this field exported?

This one is referenced from the long tasks spec. None of the others were exported at this time, and probably should not be.

annevk · 2019-04-05T12:33:52Z

source

+
+  <p>To <dfn data-export="">queue a task</dfn> on a <span>task source</span> <var>source</var>,
+  which performs a series of steps <var>steps</var>, optionally given an event loop <var>event
+  loop</var>:</p>


This requires rephrasing almost all callers, right?

To introduce a "on the X task source"? Yes, that's the intent, is to require being explicit there. Or did you mean something else?

Well, that this PR doesn't change any callers.

I thought we could do that as a follow-up, to avoid this growing into a mega-patch. Do you think it's necessary to do that in the same patch?

Not necessarily, if it'll happen.

Filed #4506 to track. Do you think it's worth linking to in the meantime, from the spec?

annevk · 2019-04-05T12:46:39Z

source

-   below.</p></li>
+   on one of the <span>event loop</span>'s <span data-x="task queue">task queues</span>. In the case
+   of a <span>window event loop</span>, the task selected must have a <span
+   data-x="concept-task-document">document</span> that is <span>fully active</span>. The user agent


I don't think this has quite the same meaning as the old text. This is a requirement on the selected task, whereas before it was a requirement on how to select.

What would the new text allow/disallow that the old text did not?

The new text is unclear to me. I select the oldest task. It violates the must, now what?

You're not allowed to do things that violate the must, so you don't do that.

Can you suggest alternate phrasing that fixes the concerns in #4242? This was my best attempt.

Maybe we can make it an algorithm instead?

Let oldest task be the oldest task.
While true:

If oldest task's document is null or is fully active, then break.

Set oldest task to the task following oldest task.

- Task queues are sets, not lists, and not queues. Explains why. - Microtask queue can be a queue. - Explain that not just source, but also document and script evaluation environment settings object set, are irrelevant in most cases for microtask. - Fix oldestTask selection algorithm to be more precise, by introducing a "runnable" task concept. - Use more Infra

annevk

This looks pretty great to me. Perhaps we should find one additional reviewer though given the complexity of the algorithm. Alternatively I could give it a fresh look next week to see if it still makes sense.

annevk · 2019-04-05T16:53:30Z

source

-  <dl>
+  <p class="note"><span data-x="task queue">Task queues</span> are <span data-x="set">sets</span>,
+  not <span data-x="queue">queues</span>, because <a href="#step1">step one of the event loop
+  processing model</a> grabs the first <span data-x="concept-task-runnable"><em>runnable</em></span>


s/grabs/removes/?

jakearchibald · 2019-04-17T18:39:07Z

source

+  event loop</span>. This is the only case in which the <span
+  data-x="concept-task-source">source</span>, <span data-x="concept-task-document">document</span>,
+  and <span>script evaluation environment settings object set</span> of the microtask are consulted;
+  they are ignored by the <span>perform a microtask checkpoint</span> algorithm.</p>


Interesting! This is news to me. I thought we processed the microtask queue after looking for an item in the task queues (whether a task is found or not).

jakearchibald · 2019-04-17T18:40:45Z

source

+   <li><p>If <var>event loop</var> is not a <span>window event loop</span>, then return
+   null.</p></li>
+
+   <li><p>If the task is being queued in the context of an element, then return the element's


What does "in the context of an element" mean? Does it mean it looks inside the task to see what the algorithm acts upon?

It's pretty fuzzy, to be honest. This is just porting the preexisting vague language.

jakearchibald · 2019-04-17T18:46:36Z

source

-   queue</span>. If there is no task to select, then jump to the <i>microtasks</i> step
-   below.</p></li>
+   <li id="step1"><p>Let <var>taskQueue</var> be one of the <span>event loop</span>'s <span
+   data-x="task queue">task queues</span>, chosen in a user-agent-defined manner, with the


Could it end up picking the microtask task queue at this point?

The "microtask queue" is not a "task queue"; all "task queues" are implementation-defined. I will add a note making that explicit.

domenic · 2019-04-26T19:53:07Z

If @annevk or @jakearchibald wants to take another look at my latest commit, I'd love to commit this :).

annevk

I'm still happy with this.

domenic requested a review from annevk March 29, 2019 18:54

Make microtask a derived, colloquial concept instead of first-class

7b897d6

domenic added clarification Standard could be clearer topic: event loop labels Mar 29, 2019

annevk reviewed Apr 5, 2019

View reviewed changes

domenic mentioned this pull request Apr 5, 2019

Update all "queue a task" invocations to explicitly specify their task source #4506

Open

annevk approved these changes Apr 5, 2019

View reviewed changes

annevk mentioned this pull request Apr 5, 2019

Error not surfaced properly tobie/pr-preview#30

Closed

domenic mentioned this pull request Apr 12, 2019

Issues with "complete portal activation" WICG/portals#69

Closed

jakearchibald reviewed Apr 17, 2019

View reviewed changes

domenic mentioned this pull request Apr 26, 2019

Describe integration with the WeakRefs proposal #4571

Merged

Clarify microtask queue is not a task queue

0707a26

annevk approved these changes Apr 30, 2019

View reviewed changes

domenic merged commit f4e2907 into master Apr 30, 2019

domenic deleted the domenic/clarify-event-loop branch April 30, 2019 16:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editorial: rigorize tasks and task queuing #4465

Editorial: rigorize tasks and task queuing #4465

domenic commented Mar 29, 2019 •

edited by pr-preview bot

Loading

annevk Apr 5, 2019

domenic Apr 5, 2019

annevk Apr 5, 2019

domenic Apr 5, 2019

annevk Apr 5, 2019

domenic Apr 5, 2019

annevk Apr 5, 2019

domenic Apr 5, 2019

annevk Apr 5, 2019

domenic Apr 5, 2019

annevk Apr 5, 2019

domenic Apr 5, 2019

annevk Apr 5, 2019

annevk left a comment

annevk Apr 5, 2019

jakearchibald Apr 17, 2019

jakearchibald Apr 17, 2019

domenic Apr 26, 2019

jakearchibald Apr 17, 2019

domenic Apr 26, 2019

domenic commented Apr 26, 2019

annevk left a comment

Editorial: rigorize tasks and task queuing #4465

Editorial: rigorize tasks and task queuing #4465

Conversation

domenic commented Mar 29, 2019 • edited by pr-preview bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic commented Apr 26, 2019

annevk left a comment

Choose a reason for hiding this comment

domenic commented Mar 29, 2019 •

edited by pr-preview bot

Loading