Make realAsyncZone run microtasks and timers in the correct zone.
#164982
Conversation
This file contains hidden or 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
Current implementation runs timers and microtask callbacks in the root zone. That assumes that the top-level `scheduleMicrotask` or `Timer` constructors have been used, which have so far wrapped the callback with `runCallbackGuarded` before calling the zone implementation. That means that doing `zone.scheduleMicrotask` directly would not ensure that the microtask was run in the correct zone. If a `run` handler throws, it wouldn't be caught. This change makes the `realAsyncZone` do whatever the root zone would do if its `ZoneDelegate` got called with the intended zone and arguments. That should be consistent with the current behavior, and be compatible with changes to the platform `Zone` behavior.
github-actions
bot
added
a: tests
Contributor
Author
|
Does seem that there are no golden issues. PTAL either this or #162731 to get the Dart SDK CL unblocked. |
Contributor
|
Since #162731 was a copy of this, and it was merged, I am going to close this PR since it looks like we're all set here. Thanks for contributing! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Current implementation runs timers and microtask callbacks in the root zone. That assumes that the top-level
scheduleMicrotaskorTimerconstructors have been used, which have so far wrapped the callback withrunCallbackGuardedbefore calling the zone implementation.That means that doing
zone.scheduleMicrotaskdirectly would not ensure that the microtask was run in the correct zone. If arunhandler throws, it wouldn't be caught.This change makes the
realAsyncZonedo whatever the root zone would do if itsZoneDelegategot called with the intended zone and arguments. That should be consistent with the current behavior, and be compatible with incoming bug-fixes to the platformZonebehavior.Prepares Flutter for landing https://dart-review.googlesource.com/c/sdk/+/406961
which is currently blocked (so this indirectly blocks fixing the dart-lang/sdk#59913).
Adds test to check that the callbacks do run in the expected zone. Otherwise all existing tests should keep running, and they should keep doing so when the Dart CL lands. Currently that CL only breaks one test, the
dev/automated_tests/test_smoke_test/fail_test_on_exception_after_test.darttest which threw the error-after-test in the root zone instead of the test zone. This change fixes that.(This is a copy of #162731 merged to head, to see if the goldens change is spurious. It seems so.)