Introduce and use Zone:_enter and Zone:_leave, in Future.

sdk/lib/async/future_impl.dart

Index: sdk/lib/async/future_impl.dart
diff --git a/sdk/lib/async/future_impl.dart b/sdk/lib/async/future_impl.dart
index 114614cae7304526360fcc983819139d4c12d67a..119ac93a59bb21890cac81cb237bfdc4b06e5588 100644
--- a/sdk/lib/async/future_impl.dart
+++ b/sdk/lib/async/future_impl.dart
@@ -411,74 +411,87 @@ class _Future<T> implements Future<T> {
         _propagateMultipleListeners(source, listeners);
         return;
       }
-      if (hasError && !source._zone.inSameErrorZone(listener._zone)) {
+      Zone zone = listener._zone;
+      if (hasError && !source._zone.inSameErrorZone(zone)) {
         // Don't cross zone boundaries with errors.
         _AsyncError asyncError = source._error;
         source._zone.handleUncaughtError(
             asyncError.error, asyncError.stackTrace);
         return;
       }
-      if (!identical(Zone.current, listener._zone)) {
-        // Run the propagation in the listener's zone to avoid
-        // zone transitions. The idea is that many chained futures will
-        // be in the same zone.
-        listener._zone.run(() {
-          _propagateToListeners(source, listener);
-        });
-        return;
+      Zone old;
+      if (!identical(Zone.current, zone)) {
+        // Change zone if it's not current.
+        old = Zone._enter(zone);
       }
-
       // Do the actual propagation.
-      // TODO(floitsch): Do we need to go through the zone even if we
-      // don't have a callback to execute?
       bool listenerHasValue;
       var listenerValueOrError;
       // Set to true if a whenComplete needs to wait for a future.
       // The whenComplete action will resume the propagation by itself.
       bool isPropagationAborted = false;
-      // Even though we are already in the right zone (due to the optimization
-      // above), we still need to go through the zone. The overhead of
-      // executeCallback is however smaller when it is already in the correct
-      // zone.
-      // TODO(floitsch): only run callbacks in the zone, not the whole
-      // handling code.
-      listener._zone.run(() {
-        // TODO(floitsch): mark the listener as pending completion. Currently
-        // we can't do this, since the markPendingCompletion verifies that
-        // the future is not already marked (or chained).
-        try {
-          if (!hasError) {
-            var value = source._value;
-            if (listener._onValue != null) {
-              listenerValueOrError = listener._onValue(value);
-              listenerHasValue = true;
-            } else {
-              // Copy over the value from the source.
-              listenerValueOrError = value;
-              listenerHasValue = true;
-            }
-          } else {
-            _AsyncError asyncError = source._error;
-            _FutureErrorTest test = listener._errorTest;
-            bool matchesTest = true;
-            if (test != null) {
-              matchesTest = test(asyncError.error);
-            }
-            if (matchesTest && listener._onError != null) {
-              Function errorCallback = listener._onError;
-              listenerValueOrError = _invokeErrorHandler(errorCallback,
-                                                         asyncError.error,
-                                                         asyncError.stackTrace);
-              listenerHasValue = true;
-            } else {
-              // Copy over the error from the source.
-              listenerValueOrError = asyncError;
-              listenerHasValue = false;
-            }
+      // At this point we are in the right zone. Each callback is invoked
+      // through Zone.run* to be sure to invoke potential callbacks.
+      // TODO(floitsch): mark the listener as pending completion. Currently
+      // we can't do this, since the markPendingCompletion verifies that
+      // the future is not already marked (or chained).
+      bool handleValue() {

[floitsch, 2014/01/28 14:22:45]

add new line before "handleValue".

[Anders Johnsen, 2014/01/29 08:00:28]

Done.

+        var value = source._value;
+        listenerHasValue = true;
+        if (listener._onValue != null) {
+          try {
+            listenerValueOrError = zone.runUnary(listener._onValue, value);
+          } catch (e, s) {
+            listenerValueOrError = new _AsyncError(e, s);
+            listenerHasValue = false;
+            return false;
           }
-
-          if (listener._whenCompleteAction != null) {
-            var completeResult = listener._whenCompleteAction();
+        } else {
+          // Copy over the value from the source.
+          listenerValueOrError = value;
+        }
+        return true;
+      }
+      bool handleError() {

[floitsch, 2014/01/28 14:22:45]

new line before nested function.

[Anders Johnsen, 2014/01/29 08:00:28]

Done.

+        _AsyncError asyncError = source._error;
+        _FutureErrorTest test = listener._errorTest;
+        bool matchesTest = true;
+        if (test != null) {
+          try {
+            matchesTest = zone.runUnary(test, asyncError.error);
+          } catch (e, s) {
+            listenerValueOrError = identical(asyncError.error, e) ?
+                asyncError : new _AsyncError(e, s);
+            listenerHasValue = false;
+            return false;
+          }
+        }
+        if (matchesTest && listener._onError != null) {
+          Function errorCallback = listener._onError;
+          // TODO(ajohnsen): Use either runUnary or runBinary.

[floitsch, 2014/01/28 14:22:45]

if (errorCallback is ZoneBinaryCallback) {
  listenerValueOrError = zone.runBinary(errorCallback, asyncError.error,
asyncError.stackTrace);
} else {
  listenerValueOrError = zone.runUnary(errorCallback, asyncError.error);
}

[Anders Johnsen, 2014/01/29 08:00:28]

Done.

+          try {
+            listenerValueOrError = zone.run(
+                () => _invokeErrorHandler(errorCallback,
+                                          asyncError.error,
+                                          asyncError.stackTrace));
+          } catch (e, s) {
+            listenerValueOrError = identical(asyncError.error, e) ?
+                asyncError : new _AsyncError(e, s);
+            listenerHasValue = false;
+            return false;
+          }
+          listenerHasValue = true;
+        } else {
+          // Copy over the error from the source.
+          listenerValueOrError = asyncError;
+          listenerHasValue = false;
+        }
+        return true;
+      }
+      void handleWhenComplete() {

[floitsch, 2014/01/28 14:22:45]

New line before nested function.

[Anders Johnsen, 2014/01/29 08:00:28]

Done.

+        if (listener._whenCompleteAction != null) {
+          try {
+            var completeResult = zone.run(listener._whenCompleteAction);
             if (completeResult is Future) {
               listener._isChained = true;
               completeResult.then((ignored) {
@@ -496,18 +509,27 @@ class _Future<T> implements Future<T> {
               });
               isPropagationAborted = true;
             }
+          } catch (e, s) {

[floitsch, 2014/01/28 14:22:45]

The try/catch can just surround the whenCompleteAction callback.

[Anders Johnsen, 2014/01/29 08:00:28]

Done.

+            if (hasError && identical(source._error.error, e)) {
+              listenerValueOrError = source._error;
+            } else {
+              listenerValueOrError = new _AsyncError(e, s);
+            }
+            listenerHasValue = false;
           }
-        } catch (e, s) {
-          // Set the exception as error unless the error is the same as the
-          // original one.
-          if (hasError && identical(source._error.error, e)) {
-            listenerValueOrError = source._error;
-          } else {
-            listenerValueOrError = new _AsyncError(e, s);
-          }
-          listenerHasValue = false;
         }
-      });
+      }
+      if (!hasError) {

[floitsch, 2014/01/28 14:22:45]

I think there is no case where there is a value/error handler and at the same
time a complete-handler.
This can thus be simplified to:

if (hasError) {
  handleError();
} else {
  handleValue();
}
handleWhenComplete();

[Anders Johnsen, 2014/01/29 08:00:28]

Sadly, depending on if any error was cough, handleWhenComplete should not be
called.

+        if (handleValue()) {
+          handleWhenComplete();
+        }
+      } else {
+        if (handleError()) {
+          handleWhenComplete();
+        }
+      }
+      // If we changed zone, old will not be null.
+      if (old != null) Zone._leave(old);
       if (isPropagationAborted) return;
       // If the listener's value is a future we need to chain it.
       if (listenerHasValue && listenerValueOrError is Future) {