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.

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

Stop exporting AbortSignal's signal abort #1194

Open
annevk opened this issue May 5, 2023 · 2 comments
Open

Stop exporting AbortSignal's signal abort #1194

annevk opened this issue May 5, 2023 · 2 comments
Labels
clarification Standard could be clearer topic: aborting AbortController and AbortSignal

Comments

@annevk
Copy link
Member

annevk commented May 5, 2023

As identified in #1152 it would be good to no longer do this. https://dontcallmedom.github.io/webdex/s.html#signal abort@@AbortSignal@dfn identifies specifications to update first.


Separately there's

(probably) Make sure algorithms are being removed when they will no longer have an effect, e.g. when the underlying operation is completed or failed.

which maybe requires its own issue. Not sure.

cc @shaseley

@annevk annevk added the topic: aborting AbortController and AbortSignal label May 5, 2023
@annevk annevk mentioned this issue May 5, 2023
4 tasks
@annevk annevk added the clarification Standard could be clearer label May 5, 2023
annevk pushed a commit that referenced this issue May 17, 2023
- This implements an optimization that puts all children on
  non-dependent signals (i.e., those associated with a controller).
  This allows "intermediate" nodes (e.g., B in A follows B follows C)
  to be garbage collected if they are being kept alive to propagate
  aborts.

- This removes the follow algorithm, so callsites will need to be
  updated.

- The "create a composite abort signal" algorithm takes an interface so
  that TaskSignal.any() can easily hook into it, but create a 
  TaskSignal.

- Some algorithms that invoke "follow" create an AbortSignal in a 
  particular realm. This enables doing that, but borrows some language 
  from elsewhere in the spec w.r.t. doing the default thing. Neither of
  the other two static members specify a realm.

Follow-up PRs:

- whatwg/fetch#1646
- w3c/ServiceWorker#1678
- whatwg/streams#1277

This also sets the stage to make AbortSignal's "signal abort" fully 
internal. #1194 tracks the remainder.

Tests: web-platform-tests/wpt#37434 and web-platform-tests/wpt#39785.

Fixes #920.
annevk pushed a commit to w3c/ServiceWorker that referenced this issue May 31, 2023
aarongable pushed a commit to chromium/chromium that referenced this issue Jun 22, 2023
Instead, create an AbortController per NavigateEvent and initialize the
event's signal to the controller's signal, using the controller to abort
the signal.

See also whatwg/dom#1194 and
WICG/navigation-api#265.

Bug: 1448352
Change-Id: I87d020ca7aab7396a7b7f2896bf886c4dd9e61b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/ /4562529
Commit-Queue: Scott Haseley <[email protected]>
Reviewed-by: Nate Chapin <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1161349}
aarongable pushed a commit to chromium/chromium that referenced this issue Jun 22, 2023
Directly aborting signals violates assumptions for composite signals,
and should be reserved for AbortController and AbortSignal. This CL
guards SignalAbort with a pass key.

See also whatwg/dom#1194.

Bug: 1448352
Low-Coverage-Reason: Follow() is covered by virtual tests
Change-Id: I02cec638d1c4c20c6d85830ea5eb26b5cf4fd251
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/ /4638358
Commit-Queue: Scott Haseley <[email protected]>
Reviewed-by: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1161467}
annevk pushed a commit that referenced this issue Sep 27, 2023
@annevk
Copy link
Member Author

annevk commented Sep 27, 2023

@shaseley I guess we'll keep this open for the (probably) item above?

@shaseley
Copy link
Contributor

That sounds good -- unless it's easier from your perspective to separate it. I'll try to at least look at the fetch case sometime this coming quarter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer topic: aborting AbortController and AbortSignal
Development

No branches or pull requests

2 participants