1
0
mirror of https://github.com/dart-lang/sdk synced 2024-07-03 00:08:46 +00:00

[html] Make cancel() always return synchronously

Before Null Safety, `_EventStreamSubscription.cancel()` used a trick
to run with synchronous timing even though it was typed to return a
`Future`. During the migration of the SDK to support Null Safety it
kept the synchronous timing in weak mode, but was changed to
asynchronous in sound mode so that the behavior matched the method
signature. In hindsight, changing the timing when opting into Null
Safety is problematic:

* A shared package has no control over what mode it runs in. Libraries
  may be opted in and run their tests with sound null safety but the
  apps they are used in could still be running in weak mode. This
  results in library unit tests that behave differently than the
  production app that deploys the code.

* This codepath can be triggered by EventTarget.dispatchEvent() from
  dart:html which should have synchronous timings for the event
  listeners before returning to the calling code. The asynchronous
  timing when running with sound null safety is inconsistent with the
  browser API.

This change reverses that migration decision and keeps the synchronous
timing in both modes. To support this in sound mode it returns a
special future value that is internal to the SDK and known to be used
for synchronous timing.

This change also removes the workaround introduced in DDC to avoid
warning/failing when `_EventStreamSubscription.cancel()` returned
null and the extra warnings/errors features were enabled in weak
null safety mode.

Change-Id: I6b08a2ada5b10120bea787ad59d1d58e6e181de5
Fixes: https://github.com/dart-lang/sdk/issues/44157
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175323
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
This commit is contained in:
Nicholas Shahan 2020-12-09 01:44:38 +00:00 committed by commit-bot@chromium.org
parent 87d5894a5a
commit 4c2edfd5b9
4 changed files with 12 additions and 35 deletions

View File

@ -30,6 +30,14 @@ opted out of null safety by adding `// @dart=2.9` to the beginning of the file.
* `LinkedList` made it explicit that elements are compared by identity,
and updated `contains` to take advantage of this.
#### `dart:html`
* `EventStreamSubscription.cancel` has been updated to retain its synchronous
timing when running in both sound and unsound null safety modes. See issue
[#44157][] for more details.
[#44157]: https://github.com/dart-lang/sdk/issues/44157
### Dart VM
* **Breaking Change** [#42312][]: `Dart_WeakPersistentHandle`s will no longer

View File

@ -5530,27 +5530,6 @@ class ProgramCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
js_ast.Expression visitAsExpression(AsExpression node) {
var fromExpr = node.operand;
var jsFrom = _visitExpression(fromExpr);
// The `_EventStreamSubscription.cancel()` method dart:html returns null in
// weak mode. This causes unwanted warnings/failures when you turn on the
// weak mode warnings/errors so we remove these specific runtime casts.
// TODO(44157) Remove this workaround once it returns a consistent type.
if (_isWebLibrary(currentLibraryUri) && node.parent is ReturnStatement) {
var parent = node.parent;
while (parent != null && parent is! FunctionNode) {
parent = parent?.parent;
}
parent = parent?.parent;
if (parent is Procedure) {
if (parent.enclosingClass != null &&
parent.enclosingClass.name == '_EventStreamSubscription' &&
parent.name.name == 'cancel') {
// Ignore these casts and just emit the expression.
return jsFrom;
}
}
}
var to = node.type;
var from = fromExpr.getStaticType(_staticTypeContext);

View File

@ -37285,18 +37285,13 @@ class _EventStreamSubscription<T extends Event> extends StreamSubscription<T> {
}
Future cancel() {
// Check for strong mode. This function can no longer return null in strong
// mode, so only return null in weak mode to preserve synchronous timing.
// See issue 41653 for more details.
dynamic emptyFuture =
typeAcceptsNull<Event>() ? null : Future<void>.value();
if (_canceled) return emptyFuture as Future;
if (_canceled) return nullFuture;
_unlisten();
// Clear out the target to indicate this is complete.
_target = null;
_onData = null;
return emptyFuture as Future;
return nullFuture;
}
bool get _canceled => _target == null;

View File

@ -248,18 +248,13 @@ class _EventStreamSubscription<T extends Event> extends StreamSubscription<T> {
}
Future cancel() {
// Check for strong mode. This function can no longer return null in strong
// mode, so only return null in weak mode to preserve synchronous timing.
// See issue 41653 for more details.
dynamic emptyFuture =
typeAcceptsNull<Event>() ? null : Future<void>.value();
if (_canceled) return emptyFuture as Future;
if (_canceled) return nullFuture;
_unlisten();
// Clear out the target to indicate this is complete.
_target = null;
_onData = null;
return emptyFuture as Future;
return nullFuture;
}
bool get _canceled => _target == null;