Don't zone-register async callbacks for every await call in the VM.

Don't zone-register async callbacks for every await call in the VM.

This has the nice side-effect that the stack_trace package provides nicer chains.
Fixes issue 23394.

R=hausner@google.com

Committed: https://github.com/dart-lang/sdk/commit/4dae43d63727268f2e0cc45d6020246eb49177b8
Reverted: https://github.com/dart-lang/sdk/commit/cb4f9fc925c5a92c6cf756751e38296383f66507

Committed: https://github.com/dart-lang/sdk/commit/debf1d6f7c2f97831cda1f6fa18093f76ed0545c

[Lasse Reichstein Nielsen, 2015-08-06 14:19:55]

lrn@google.com changed reviewers:
+ lrn@google.com

[Lasse Reichstein Nielsen, 2015-08-06 14:19:58]

Optimization comments.

https://chromiumcodereview.appspot.com/1274133002/diff/1/runtime/lib/async_pa...
File runtime/lib/async_patch.dart (right):

https://chromiumcodereview.appspot.com/1274133002/diff/1/runtime/lib/async_pa...
runtime/lib/async_patch.dart:22: var future, Function thenCallback, Function
errorCallback) {
remove the "var", rename "future" to "result" (it might not be a future).

https://chromiumcodereview.appspot.com/1274133002/diff/1/runtime/lib/async_pa...
runtime/lib/async_patch.dart:25: }
  if (result is! Future) {
    result = new _Future().._setValue(result);  // avoid more tests. 
  } else if (result is! _Future) {
    return result.then(thenCallback, onError: errorCallback);
  }
  return result._thenNoZoneRegistration(thenCallback, errorCallback);
}

This avoids one extra test when we already know that we just created a _Future.

[floitsch, 2015-08-11 15:27:49]

floitsch@google.com changed reviewers:
+ hausner@google.com

[floitsch, 2015-08-11 15:27:50]

floitsch@google.com changed required reviewers:
+ hausner@google.com

[floitsch, 2015-08-11 15:27:50]

I ended up moving the other async helper to the async-patch, too. This way they
are grouped.

https://chromiumcodereview.appspot.com/1274133002/diff/1/runtime/lib/async_pa...
File runtime/lib/async_patch.dart (right):

https://chromiumcodereview.appspot.com/1274133002/diff/1/runtime/lib/async_pa...
runtime/lib/async_patch.dart:22: var future, Function thenCallback, Function
errorCallback) {
On 2015/08/06 14:19:58, Lasse Reichstein Nielsen wrote:
> remove the "var", rename "future" to "result" (it might not be a future).

changed to 'expression'. "Result" would be the result of this function.

https://chromiumcodereview.appspot.com/1274133002/diff/1/runtime/lib/async_pa...
runtime/lib/async_patch.dart:25: }
On 2015/08/06 14:19:58, Lasse Reichstein Nielsen wrote:
>   if (result is! Future) {
>     result = new _Future().._setValue(result);  // avoid more tests. 
>   } else if (result is! _Future) {
>     return result.then(thenCallback, onError: errorCallback);
>   }
>   return result._thenNoZoneRegistration(thenCallback, errorCallback);
> }
> 
> This avoids one extra test when we already know that we just created a
_Future.

Done.

[floitsch, 2015-08-11 15:28:37]

https://chromiumcodereview.appspot.com/1274133002/diff/1/runtime/lib/async_pa...
File runtime/lib/async_patch.dart (right):

https://chromiumcodereview.appspot.com/1274133002/diff/1/runtime/lib/async_pa...
runtime/lib/async_patch.dart:22: var future, Function thenCallback, Function
errorCallback) {
On 2015/08/11 15:27:50, floitsch wrote:
> On 2015/08/06 14:19:58, Lasse Reichstein Nielsen wrote:
> > remove the "var", rename "future" to "result" (it might not be a future).
> 
> changed to 'expression'. "Result" would be the result of this function.

Actually changed to "object".

[hausner, 2015-08-11 21:22:07]

LGTM.

Thank you for the comments and explanations in async_patch.dart.

https://chromiumcodereview.appspot.com/1274133002/diff/100001/runtime/lib/asy...
File runtime/lib/async_patch.dart (right):

https://chromiumcodereview.appspot.com/1274133002/diff/100001/runtime/lib/asy...
runtime/lib/async_patch.dart:22: // Note, that the contination accepts up to
three arguments. If the current
continuation

https://chromiumcodereview.appspot.com/1274133002/diff/100001/tests/lib/async...
File tests/lib/async/async_await_zones_test.dart (right):

https://chromiumcodereview.appspot.com/1274133002/diff/100001/tests/lib/async...
tests/lib/async/async_await_zones_test.dart:8: 
Maybe a sentence or two to document what this test does?

https://chromiumcodereview.appspot.com/1274133002/diff/100001/tests/lib/async...
tests/lib/async/async_await_zones_test.dart:28: await(null);
Curious: why use await like a function call here? Why not await null; ?

[floitsch, 2015-08-12 15:03:14]

https://codereview.chromium.org/1274133002/diff/100001/runtime/lib/async_patc...
File runtime/lib/async_patch.dart (right):

https://codereview.chromium.org/1274133002/diff/100001/runtime/lib/async_patc...
runtime/lib/async_patch.dart:22: // Note, that the contination accepts up to
three arguments. If the current
On 2015/08/11 21:22:07, hausner wrote:
> continuation

Done.

https://codereview.chromium.org/1274133002/diff/100001/tests/lib/async/async_...
File tests/lib/async/async_await_zones_test.dart (right):

https://codereview.chromium.org/1274133002/diff/100001/tests/lib/async/async_...
tests/lib/async/async_await_zones_test.dart:8: 
On 2015/08/11 21:22:07, hausner wrote:
> Maybe a sentence or two to document what this test does?

Done.

https://codereview.chromium.org/1274133002/diff/100001/tests/lib/async/async_...
tests/lib/async/async_await_zones_test.dart:28: await(null);
On 2015/08/11 21:22:07, hausner wrote:
> Curious: why use await like a function call here? Why not await null; ?

My mistake...
done.

[floitsch, 2015-08-17 13:58:23]

Committed patchset #7 (id:120001) manually as
4dae43d63727268f2e0cc45d6020246eb49177b8 (presubmit successful).

[floitsch, 2015-08-17 14:48:51]

Had to revert.
Reopening. Patchset 8 is a fresh upload after the revert.

[floitsch, 2015-08-17 17:13:50]

I had to readd the async_op variable to the asynchronous function. The
observatory uses it to determine if a function is asynchronous or not.

PTAL.

[rmacnak, 2015-08-17 17:23:53]

On 2015/08/17 17:13:50, floitsch wrote:
> I had to readd the async_op variable to the asynchronous function. The
> observatory uses it to determine if a function is asynchronous or not.
> 
> PTAL.

Not quite - the async_op variable is used by the implementation of async
stepping. It needs to contain the closure that will be activated to resume the
async function.

[hausner, 2015-08-17 17:25:50]

LGTM.

[floitsch, 2015-08-17 17:26:54]

Committed patchset #9 (id:160001) manually as
debf1d6f7c2f97831cda1f6fa18093f76ed0545c (presubmit successful).