Re: [whatwg/dom] Add AbortSignal.any() (PR #1152)

@domenic approved this pull request.

Normatively LGTM, a few suggestions for making things a bit cleaner.

>  them. For instance, if the operation has already completed.
 
-<p>To <dfn export for=AbortSignal>signal abort</dfn>, given an {{AbortSignal}} object
-<var>signal</var> and an optional <var>reason</var>, run these steps:
+<p>To <dfn for=AbortSignal>signal abort</dfn>, given an {{AbortSignal}} object <var>signal</var> and
+an optional <var>reason</var>, run these steps:

Nit: while here, you could remove `, run these steps`, as is the modern style. And also remove it from the below newly-added algorithm.

>  
- <li><p>If <var>parentSignal</var> is [=AbortSignal/aborted=], then
- <a for=AbortSignal>signal abort</a> on <var>followingSignal</var> with <var>parentSignal</var>'s
- [=AbortSignal/abort reason=].
+  <p class=XXX>As of the time of this writing Web IDL does not yet define any default behavior;
+  see <a href="https://github.com/whatwg/webidl/issues/135">whatwg/webidl#135</a>.

It might be better just to make the realm argument mandatory. For the `AbortSignal.any` caller, the correct realm is the current Realm.

> @@ -1828,6 +1837,17 @@ JavaScript value. It is undefined unless specified otherwise.
 <a for=/>set</a> of algorithms which are to be executed when it is [=AbortSignal/aborted=]. Unless
 specified otherwise, its value is the empty set.
 
+<p>An {{AbortSignal}} object has a <dfn for="AbortSignal">composite</dfn> (a boolean), which is
+initially false.
+
+<p>An {{AbortSignal}} object has associated <dfn for=AbortSignal>source signals</dfn>, which is a
+<a for=/>set</a> of weak references to {{AbortSignal}} objects that the object is dependent on for
+its [=AbortSignal/aborted=] state. Unless specified otherwise, its value is the empty set.
+
+<p>An {{AbortSignal}} object has associated <dfn for=AbortSignal>dependent signals</dfn>, which is a
+<a for=/>set</a> of weak references to {{AbortSignal}} objects that are dependent on it for their
+[=AbortSignal/aborted=] state. Unless specified otherwise, its value is the empty set.
+

This section has gotten a little disorganized. E.g. add/remove are no longer next to "abort algorithms"; "is aborted" is in the middle of a bunch of field definitions; most field definitions use "unless specified otherwise", but one uses "initially".

Suggested ordering:

- Fields first. I like "initially" better than "unless specified otherwise".
- `<hr>`
- API definitions
- `<hr>`
- Supporting algorithms, including add/remove and "is aborted".

> @@ -1828,6 +1837,17 @@ JavaScript value. It is undefined unless specified otherwise.
 <a for=/>set</a> of algorithms which are to be executed when it is [=AbortSignal/aborted=]. Unless
 specified otherwise, its value is the empty set.
 
+<p>An {{AbortSignal}} object has a <dfn for="AbortSignal">composite</dfn> (a boolean), which is
+initially false.
+
+<p>An {{AbortSignal}} object has associated <dfn for=AbortSignal>source signals</dfn>, which is a
+<a for=/>set</a> of weak references to {{AbortSignal}} objects that the object is dependent on for
+its [=AbortSignal/aborted=] state. Unless specified otherwise, its value is the empty set.
+
+<p>An {{AbortSignal}} object has associated <dfn for=AbortSignal>dependent signals</dfn>, which is a
+<a for=/>set</a> of weak references to {{AbortSignal}} objects that are dependent on it for their
+[=AbortSignal/aborted=] state. Unless specified otherwise, its value is the empty set.

I think these should be "weak sets" instead of "sets of weak references". The difference is not behaviorally significant, but I think you'd need more verbiage to rigorously manage a set of weak references. E.g., instead of "Append _sourceSignal_ to ..." you'd need "Append a weak reference to _sourceSignal_ to...". And you'd need to dereference and null-check and so on.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/pull/1152#pullrequestreview-1368463480
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/dom/pull/1152/review/1368463480@github.com>