from(): Close synchronous iterators properly
#192
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Jan 28, 2025
This CL adds and supplements a few tests:
1. First we modify the existing "subscribe with aborted signal" tests.
Specifically, we expand their assertions to not only assert that
`next()` isn't ever called, but make more assertions about the
iterator protocol getter and function invocations in general.
2. Second, we modify the test that asserts `next()` is not called
when you subscribe with an unaborted signal, but that signal gets
aborted while the iterator protocol methods are called during
subscription of the Observable. We expand the assertions in the
same way as (1), and combine the two separate tests into one that
covers both sync and async iterators, also to match (1).
3. Finally, this CL adds a sync iterable version of the test added in
https://crrev.com/c/6199630. The test scenario is: you subscribe to
a sync iterable with an unaborted signal that gets aborted while
obtaining the iterator (just like (2)), BUT while getting the
iterator, an error is thrown. The tests asserts that the error is
reported to the global before we consult the aborted signal and
stop the subscription process. This ensures that the exception is
not swallowed, but is appropriately surfaced, even though the
subscription is aborted.
This corresponds with the spec PR:
WICG/observable#192.
R=masonf
Bug: 363015168
Change-Id: Ida605c49a2d73cd407a9dc3c392d6b2f338855be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6202182
Commit-Queue: Dominic Farolino <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1412315}
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Jan 28, 2025
This CL adds and supplements a few tests:
1. First we modify the existing "subscribe with aborted signal" tests.
Specifically, we expand their assertions to not only assert that
`next()` isn't ever called, but make more assertions about the
iterator protocol getter and function invocations in general.
2. Second, we modify the test that asserts `next()` is not called
when you subscribe with an unaborted signal, but that signal gets
aborted while the iterator protocol methods are called during
subscription of the Observable. We expand the assertions in the
same way as (1), and combine the two separate tests into one that
covers both sync and async iterators, also to match (1).
3. Finally, this CL adds a sync iterable version of the test added in
https://crrev.com/c/6199630. The test scenario is: you subscribe to
a sync iterable with an unaborted signal that gets aborted while
obtaining the iterator (just like (2)), BUT while getting the
iterator, an error is thrown. The tests asserts that the error is
reported to the global before we consult the aborted signal and
stop the subscription process. This ensures that the exception is
not swallowed, but is appropriately surfaced, even though the
subscription is aborted.
This corresponds with the spec PR:
WICG/observable#192.
R=masonf
Bug: 363015168
Change-Id: Ida605c49a2d73cd407a9dc3c392d6b2f338855be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6202182
Commit-Queue: Dominic Farolino <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1412315}
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Jan 28, 2025
This CL adds and supplements a few tests:
1. First we modify the existing "subscribe with aborted signal" tests.
Specifically, we expand their assertions to not only assert that
`next()` isn't ever called, but make more assertions about the
iterator protocol getter and function invocations in general.
2. Second, we modify the test that asserts `next()` is not called
when you subscribe with an unaborted signal, but that signal gets
aborted while the iterator protocol methods are called during
subscription of the Observable. We expand the assertions in the
same way as (1), and combine the two separate tests into one that
covers both sync and async iterators, also to match (1).
3. Finally, this CL adds a sync iterable version of the test added in
https://crrev.com/c/6199630. The test scenario is: you subscribe to
a sync iterable with an unaborted signal that gets aborted while
obtaining the iterator (just like (2)), BUT while getting the
iterator, an error is thrown. The tests asserts that the error is
reported to the global before we consult the aborted signal and
stop the subscription process. This ensures that the exception is
not swallowed, but is appropriately surfaced, even though the
subscription is aborted.
This corresponds with the spec PR:
WICG/observable#192.
R=masonf
Bug: 363015168
Change-Id: Ida605c49a2d73cd407a9dc3c392d6b2f338855be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6202182
Commit-Queue: Dominic Farolino <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1412315}
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Jan 31, 2025
… a=testonly
Automatic update from web-platform-tests
DOM: Add more Observable iterable tests
This CL adds and supplements a few tests:
1. First we modify the existing "subscribe with aborted signal" tests.
Specifically, we expand their assertions to not only assert that
`next()` isn't ever called, but make more assertions about the
iterator protocol getter and function invocations in general.
2. Second, we modify the test that asserts `next()` is not called
when you subscribe with an unaborted signal, but that signal gets
aborted while the iterator protocol methods are called during
subscription of the Observable. We expand the assertions in the
same way as (1), and combine the two separate tests into one that
covers both sync and async iterators, also to match (1).
3. Finally, this CL adds a sync iterable version of the test added in
https://crrev.com/c/6199630. The test scenario is: you subscribe to
a sync iterable with an unaborted signal that gets aborted while
obtaining the iterator (just like (2)), BUT while getting the
iterator, an error is thrown. The tests asserts that the error is
reported to the global before we consult the aborted signal and
stop the subscription process. This ensures that the exception is
not swallowed, but is appropriately surfaced, even though the
subscription is aborted.
This corresponds with the spec PR:
WICG/observable#192.
R=masonf
Bug: 363015168
Change-Id: Ida605c49a2d73cd407a9dc3c392d6b2f338855be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6202182
Commit-Queue: Dominic Farolino <[email protected]>
Reviewed-by: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1412315}
--
wpt-commits: ea15691f277a5e965d90f9c0167638559ff62f0d
wpt-pr: 50338
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes
Observable.from()'s synchronous iteration to respond to aborted signals properly, and close the iterator accordingly. Upon subscription, this PR adds checks to see whether the subscriber's signal is aborted:Subscriber#next(), but before the next iteration starts. This is consistent with the Chromium implementation and tests.Furthermore, after the two checks, the subscription algorithm to run the IteratorRecord's "return" function is also added to the signal. This is also consistent with the Chromium implementation and tested by the above tests.
Preview | Diff