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

Jump to bottom

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 •
edited
Loading

As identified in #1152 it would be good to no longer do this. https://dontcallmedom.github.io/webdex/s.html#signal%20abort%40%40AbortSignal%40dfn 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
Merged
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
@shaseley
Add AbortSignal.any()
6c02df2 
- 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.
This was referenced May 23, 2023
Merged
Closed
Merged
Merged
Merged
annevk pushed a commit to w3c/ServiceWorker that referenced this issue May 31, 2023
@shaseley
Use an AbortController instead of signaling abort on AbortSignal
fa114cd 
Needed for whatwg/dom#1194.
aarongable pushed a commit to chromium/chromium that referenced this issue Jun 22, 2023
@shaseley
AbortSignal: Don't call SignalAbort directly in navigation_api
d894ea5 
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 <shaseley@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1161349}
aarongable pushed a commit to chromium/chromium that referenced this issue Jun 22, 2023
@shaseley
AbortSignal: Guard AbortSignal::SignalAbort with a pass key
4486ffc 
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 <shaseley@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1161467}
@shaseley shaseley mentioned this issue Jun 30, 2023
Open
@shaseley shaseley mentioned this issue Sep 26, 2023
Merged
annevk pushed a commit that referenced this issue Sep 27, 2023
@shaseley
Meta: stop exporting AbortSignal/signal abort
337fdc8 
Helps with #1194.
@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
Assignees
No one assigned
Labels
clarification Standard could be clearer topic: aborting AbortController and AbortSignal
Milestone
No milestone
Development

No branches or pull requests
2 participants
@annevk @shaseley