Revert "Make nullFuture be per-zone."

This reverts commit 4d750a8.

Reason for revert: breaks google3 (b/236665701)

Original change's description:
> Make `nullFuture` be per-zone.
>
> We introduced a `nullFuture` during the null-safety migration where
> we changed some methods to no longer allow returning `null`,
> and they therefore had to return a `Future`.
> That affected timing, because returning `null` was processed
> synchronously, and that change in timing made some tests fail.
> Rather that fix the fragile tests, we made the function return
> a recognizable future, a canonical `Future<Null>.value(null)`,
> and then recognized it and took a synchronous path for it.
>
> That caused other issues, because the future was created in the
> root zone. (Well, originally, it was created in the first zone
> which needed one, that was worse. Now it's created in the root zone.)
> Some code tries to contain asynchrony inside a custom zone, and
> then the get a `nullFuture` and calls `then` on it, and that
> schedules a microtask in the root zone.
> (It should probably have used the listener's zone, and not store
> a zone in the future at all, but that's how it was first done,
> and now people rely on that behavior too.)
>
> This change creates a `null` future *per zone* (lazily initialized
> when asked for). That should be sufficient because the code recognizing
> a returned `null` future is generally running in the same zone,
> but if any other code gets the `nullFuture`, it will be in the
> expected zone for where it was requested.
>
> This is a reland of commit a247b15
>
> Change-Id: Ia113756de1f6d50af4b1abfec219d6b4dcd5d59b
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249488
> Reviewed-by: Nate Bosch <[email protected]>
> Commit-Queue: Lasse Nielsen <[email protected]>

[email protected],[email protected]

Change-Id: I02d62d58bae33d6a606a80eb3eee2e8e721a8e20
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249620
Commit-Queue: Emmanuel Pellereau <[email protected]>
Reviewed-by: Emmanuel Pellereau <[email protected]>
Reviewed-by: Nate Bosch <[email protected]>

Author: emmanuel-p

pkg/front_end/testcases/nnbd/issue42546.dart.weak.outline.expect

@@ -28,19 +28,19 @@ static method main() → dynamic
 
 
 Extra constant evaluation status:
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:814:13 -> SymbolConstant(#catchError)
-Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:814:13 -> ListConstant(const <Type*>[])
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:814:13 -> SymbolConstant(#test)
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:865:13 -> SymbolConstant(#whenComplete)
-Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:865:13 -> ListConstant(const <Type*>[])
-Evaluated: MapLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:865:13 -> MapConstant(const <Symbol*, dynamic>{})
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:909:13 -> SymbolConstant(#timeout)
-Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:909:13 -> ListConstant(const <Type*>[])
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:909:13 -> SymbolConstant(#onTimeout)
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:763:13 -> SymbolConstant(#then)
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:763:13 -> SymbolConstant(#onError)
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:874:13 -> SymbolConstant(#asStream)
-Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:874:13 -> ListConstant(const <Type*>[])
-Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:874:13 -> ListConstant(const <dynamic>[])
-Evaluated: MapLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:874:13 -> MapConstant(const <Symbol*, dynamic>{})
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:821:13 -> SymbolConstant(#catchError)
+Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:821:13 -> ListConstant(const <Type*>[])
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:821:13 -> SymbolConstant(#test)
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:872:13 -> SymbolConstant(#whenComplete)
+Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:872:13 -> ListConstant(const <Type*>[])
+Evaluated: MapLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:872:13 -> MapConstant(const <Symbol*, dynamic>{})
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:916:13 -> SymbolConstant(#timeout)
+Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:916:13 -> ListConstant(const <Type*>[])
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:916:13 -> SymbolConstant(#onTimeout)
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:770:13 -> SymbolConstant(#then)
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:770:13 -> SymbolConstant(#onError)
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:881:13 -> SymbolConstant(#asStream)
+Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:881:13 -> ListConstant(const <Type*>[])
+Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:881:13 -> ListConstant(const <dynamic>[])
+Evaluated: MapLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:881:13 -> MapConstant(const <Symbol*, dynamic>{})
 Extra constant evaluation: evaluated: 61, effectively constant: 15

sdk/lib/async/async.dart

@@ -110,6 +110,7 @@ import "dart:_internal"
         checkNotNullable,
         EmptyIterator,
         IterableElementError,
+        nullFuture,
         printToZone,
         printToConsole,
         Since,

sdk/lib/async/future.dart

@@ -223,6 +223,13 @@ abstract class FutureOr<T> {
 /// it's very clearly documented.
 @pragma("wasm:entry-point")
 abstract class Future<T> {
+  /// A `Future<Null>` completed with `null`.
+  ///
+  /// Currently shared with `dart:internal`.
+  /// If that future can be removed, then change this back to
+  /// `_Future<Null>.zoneValue(null, _rootZone);`
+  static final _Future<Null> _nullFuture = nullFuture as _Future<Null>;
+
   /// A `Future<bool>` completed with `false`.
   static final _Future<bool> _falseFuture =
       new _Future<bool>.zoneValue(false, _rootZone);

sdk/lib/async/stream.dart

@@ -515,7 +515,7 @@ abstract class Stream<T> {
       controller
         ..onCancel = () {
           timer.cancel();
-          return Zone._current._nullFuture;
+          return Future._nullFuture;
         }
         ..onPause = () {
           watch.stop();

sdk/lib/async/stream_controller.dart

@@ -588,10 +588,7 @@ abstract class _StreamController<T> implements _StreamControllerBase<T> {
   Future<void> get done => _ensureDoneFuture();
 
   Future<void> _ensureDoneFuture() =>
-      _doneFuture ??
-      (_isCanceled
-          ? Zone._current._nullFuture as Future<void>
-          : _doneFuture = _Future<void>());
+      _doneFuture ??= _isCanceled ? Future._nullFuture : _Future<void>();
 
   /// Send or enqueue a data event.
   void add(T value) {
@@ -922,7 +919,7 @@ class _AddStreamState<T> {
     var cancel = addSubscription.cancel();
     if (cancel == null) {
       addStreamFuture._asyncComplete(null);
-      return Zone._current._nullFuture;
+      return Future._nullFuture;
     }
     return cancel.whenComplete(() {
       addStreamFuture._asyncComplete(null);

sdk/lib/async/stream_impl.dart

@@ -197,7 +197,7 @@ class _BufferingStreamSubscription<T>
     if (!_isCanceled) {
       _cancel();
     }
-    return _cancelFuture ?? Zone._current._nullFuture;
+    return _cancelFuture ?? Future._nullFuture;
   }
 
   Future<E> asFuture<E>([E? futureValue]) {
@@ -217,7 +217,7 @@ class _BufferingStreamSubscription<T>
     };
     _onError = (Object error, StackTrace stackTrace) {
       Future cancelFuture = cancel();
-      if (!identical(Zone._current._nullFuture, cancelFuture)) {
+      if (!identical(cancelFuture, Future._nullFuture)) {
         cancelFuture.whenComplete(() {
           result._completeError(error, stackTrace);
         });
@@ -297,7 +297,7 @@ class _BufferingStreamSubscription<T>
   // Hooks called when the input is paused, unpaused or canceled.
   // These must not throw. If overwritten to call user code, include suitable
   // try/catch wrapping and send any errors to
-  // [Zone._current.handleUncaughtError].
+  // [_Zone.current.handleUncaughtError].
   void _onPause() {
     assert(_isInputPaused);
   }
@@ -352,6 +352,7 @@ class _BufferingStreamSubscription<T>
       // future to finish we must not report the error.
       if (_isCanceled && !_waitsForCancel) return;
       _state |= _STATE_IN_CALLBACK;
+      // TODO(floitsch): this dynamic should be 'void'.
       var onError = _onError;
       if (onError is void Function(Object, StackTrace)) {
         _zone.runBinaryGuarded<Object, StackTrace>(onError, error, stackTrace);
@@ -366,7 +367,7 @@ class _BufferingStreamSubscription<T>
       _cancel();
       var cancelFuture = _cancelFuture;
       if (cancelFuture != null &&
-          !identical(Zone._current._nullFuture, cancelFuture)) {
+          !identical(cancelFuture, Future._nullFuture)) {
         cancelFuture.whenComplete(sendError);
       } else {
         sendError();
@@ -395,8 +396,7 @@ class _BufferingStreamSubscription<T>
     _cancel();
     _state |= _STATE_WAIT_FOR_CANCEL;
     var cancelFuture = _cancelFuture;
-    if (cancelFuture != null &&
-        !identical(Zone._current._nullFuture, cancelFuture)) {
+    if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
       cancelFuture.whenComplete(sendDone);
     } else {
       sendDone();
@@ -672,7 +672,7 @@ class _DoneStreamSubscription<T> implements StreamSubscription<T> {
     }
   }
 
-  Future cancel() => Zone._current._nullFuture;
+  Future cancel() => Future._nullFuture;
 
   Future<E> asFuture<E>([E? futureValue]) {
     E resultValue;
@@ -819,7 +819,7 @@ class _BroadcastSubscriptionWrapper<T> implements StreamSubscription<T> {
 
   Future cancel() {
     _stream._cancelSubscription();
-    return Zone._current._nullFuture;
+    return Future._nullFuture;
   }
 
   bool get isPaused {
@@ -963,7 +963,7 @@ class _StreamIterator<T> implements StreamIterator<T> {
       }
       return subscription.cancel();
     }
-    return Zone._current._nullFuture;
+    return Future._nullFuture;
   }
 
   void _onData(T data) {

sdk/lib/async/stream_pipe.dart

@@ -26,8 +26,7 @@ _runUserCode<T>(T userCode(), onSuccess(T value),
 void _cancelAndError(StreamSubscription subscription, _Future future,
     Object error, StackTrace stackTrace) {
   var cancelFuture = subscription.cancel();
-  if (cancelFuture != null &&
-      !identical(Zone._current._nullFuture, cancelFuture)) {
+  if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
     cancelFuture.whenComplete(() => future._completeError(error, stackTrace));
   } else {
     future._completeError(error, stackTrace);
@@ -56,8 +55,7 @@ void Function(Object error, StackTrace stackTrace) _cancelAndErrorClosure(
   before completing with a value. */
 void _cancelAndValue(StreamSubscription subscription, _Future future, value) {
   var cancelFuture = subscription.cancel();
-  if (cancelFuture != null &&
-      !identical(Zone._current._nullFuture, cancelFuture)) {
+  if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
     cancelFuture.whenComplete(() => future._complete(value));
   } else {
     future._complete(value);

sdk/lib/async/zone.dart

@@ -1078,28 +1078,6 @@ abstract class _Zone implements Zone {
           implZone, e, identical(error, e) ? stackTrace : s);
     }
   }
-
-  /// A reusable `null`-valued future per zone used by `dart:async`.
-  ///
-  /// **DO NOT USE.**
-  ///
-  /// This future is used in situations where a future is expected,
-  /// but no asynchronous computation actually happens,
-  /// like cancelling a stream from a controller with no `onCancel` callback.
-  /// *Some code depends on recognizing this future in order to react
-  /// synchronously.*
-  /// It does so to avoid changing event interleaving during the null safety
-  /// migration where, for example, the [StreamSubscription.cancel] method
-  /// stopped being able to return `null`.
-  /// The code that would be broken by such a timing change is fragile,
-  /// but we are not able to simply change it.
-  /// For better or worse, code depends on the precise timing
-  /// that our libraries have so far exhibited.
-  ///
-  /// This future will be removed again if we can ever do so.
-  /// Do not use it for anything other than preserving timing
-  /// during the null safety migration.
-  _Future<Null> get _nullFuture;
 }
 
 class _CustomZone extends _Zone {
@@ -1127,9 +1105,6 @@ class _CustomZone extends _Zone {
   /// The parent zone.
   final _Zone parent;
 
-  /// Cached value for [_nullFuture];
-  _Future<Null>? _nullFutureCache;
-
   /// The zone's scoped value declaration map.
   ///
   /// This is always a [HashMap].
@@ -1221,18 +1196,6 @@ class _CustomZone extends _Zone {
   /// parent's error-zone.
   Zone get errorZone => _handleUncaughtError.zone;
 
-  _Future<Null> get _nullFuture {
-    _Future<Null>? result = _nullFutureCache;
-    if (result != null) return result;
-    // We only care about the zone of the null future
-    // because of the zone it schedules microtasks in.
-    var microtaskZone = _scheduleMicrotask.zone;
-    if (!identical(microtaskZone, this)) {
-      return _nullFutureCache = microtaskZone._nullFuture;
-    }
-    return _nullFutureCache = _Future<Null>.value(null);
-  }
-
   void runGuarded(void f()) {
     try {
       run(f);
@@ -1546,8 +1509,6 @@ Zone _rootFork(Zone? self, ZoneDelegate? parent, Zone zone,
 }
 
 class _RootZone extends _Zone {
-  static final _nullFutureCache = _Future<Null>.zoneValue(null, _rootZone);
-
   const _RootZone();
 
   _ZoneFunction<RunHandler> get _run =>
@@ -1586,8 +1547,6 @@ class _RootZone extends _Zone {
   // The parent zone.
   _Zone? get parent => null;
 
-  _Future<Null> get _nullFuture => _nullFutureCache;
-
   /// The zone's scoped value declaration map.
   ///
   /// This is always a [HashMap].

sdk/lib/html/dart2js/html_dart2js.dart

@@ -37302,13 +37302,13 @@ class _EventStreamSubscription<T extends Event> extends StreamSubscription<T> {
   }
 
   Future cancel() {
-    if (!_canceled) {
-      _unlisten();
-      // Clear out the target to indicate this is complete.
-      _target = null;
-      _onData = null;
-    }
-    return Future<void>.value(null);
+    if (_canceled) return nullFuture;
+
+    _unlisten();
+    // Clear out the target to indicate this is complete.
+    _target = null;
+    _onData = null;
+    return nullFuture;
   }
 
   bool get _canceled => _target == null;

sdk/lib/internal/internal.dart

@@ -136,7 +136,27 @@ int parseHexByte(String source, int index) {
   return digit1 * 16 + digit2 - (digit2 & 256);
 }
 
-final Expando<Future<Null>> _nullFutures = Expando<Future<Null>>();
+/// A reusable `null`-valued future used by `dart:async`.
+///
+/// **DO NOT USE.**
+///
+/// This future is used in situations where a future is expected,
+/// but no asynchronous computation actually happens,
+/// like cancelling a stream from a controller with no `onCancel` callback.
+/// *Some code depends on recognizing this future in order to react
+/// synchronously.*
+/// It does so to avoid changing event interleaving during the null safety
+/// migration where, for example, the [StreamSubscription.cancel] method
+/// stopped being able to return `null`.
+/// The code that would be broken by such a timing change is fragile,
+/// but we are not able to simply change it.
+/// For better or worse, code depends on the precise timing that our libraries
+/// have so far exhibited.
+///
+/// This future will be removed again if we can ever do so.
+/// Do not use it for anything other than preserving timing
+/// during the null safety migration.
+final Future<Null> nullFuture = Zone.root.run(() => Future<Null>.value(null));
 
 /// A default hash function used by the platform in various places.
 ///

tests/lib/async/null_future_zone_test.dart

@@ -13,32 +13,21 @@ main() {
     Expect.isFalse(await it.moveNext());
 
     late Future nullFuture;
+    late Future falseFuture;
 
-    bool nullFutureZoneUsed = false;
     runZoned(() {
       nullFuture = (new StreamController()..stream.listen(null).cancel()).done;
+      falseFuture = it.moveNext();
     }, zoneSpecification: new ZoneSpecification(scheduleMicrotask:
         (Zone self, ZoneDelegate parent, Zone zone, void f()) {
-      Expect.identical(zone, self);
-      nullFutureZoneUsed = true;
-      parent.scheduleMicrotask(zone, f);
+      Expect.fail("Should not be called");
     }));
 
     nullFuture.then((value) {
       Expect.isNull(value);
-      Expect.isTrue(nullFutureZoneUsed);
       asyncEnd();
     });
 
-    late Future falseFuture;
-
-    runZoned(() {
-      falseFuture = it.moveNext();
-    }, zoneSpecification: new ZoneSpecification(scheduleMicrotask:
-        (Zone self, ZoneDelegate parent, Zone zone, void f()) {
-      Expect.fail("Should not be called");
-    }));
-
     falseFuture.then((value) {
       Expect.isFalse(value);
       asyncEnd();

tests/lib_2/async/null_future_zone_test.dart

@@ -15,32 +15,21 @@ main() {
     Expect.isFalse(await it.moveNext());
 
     Future nullFuture;
+    Future falseFuture;
 
-    bool nullFutureZoneUsed = false;
     runZoned(() {
       nullFuture = (new StreamController()..stream.listen(null).cancel()).done;
+      falseFuture = it.moveNext();
     }, zoneSpecification: new ZoneSpecification(scheduleMicrotask:
         (Zone self, ZoneDelegate parent, Zone zone, void f()) {
-      Expect.identical(zone, self);
-      nullFutureZoneUsed = true;
-      parent.scheduleMicrotask(zone, f);
+      Expect.fail("Should not be called");
     }));
 
     nullFuture.then((value) {
       Expect.isNull(value);
-      Expect.isTrue(nullFutureZoneUsed);
       asyncEnd();
     });
 
-    Future falseFuture;
-
-    runZoned(() {
-      falseFuture = it.moveNext();
-    }, zoneSpecification: new ZoneSpecification(scheduleMicrotask:
-        (Zone self, ZoneDelegate parent, Zone zone, void f()) {
-      Expect.fail("Should not be called");
-    }));
-
     falseFuture.then((value) {
       Expect.isFalse(value);
       asyncEnd();