Expose Zones.

Expose Zones.

BUG= http://dartbug.com/11279
R=lrn@google.com

Committed: https://code.google.com/p/dart/source/detail?r=27802

Commit also includes CL https://codereview.chromium.org/23450047/ ("Run runAsync callbacks of futures in their zone").

[kevmoo-old, 2013-09-18 20:37:18]

Early DBC. Curious.

https://codereview.chromium.org/23875032/diff/6001/sdk/lib/async/zone.dart
File sdk/lib/async/zone.dart (right):

https://codereview.chromium.org/23875032/diff/6001/sdk/lib/async/zone.dart#ne...
sdk/lib/async/zone.dart:685: */
@deprecated?

[floitsch, 2013-09-20 12:48:48]

https://codereview.chromium.org/23875032/diff/6001/sdk/lib/async/zone.dart
File sdk/lib/async/zone.dart (right):

https://codereview.chromium.org/23875032/diff/6001/sdk/lib/async/zone.dart#ne...
sdk/lib/async/zone.dart:685: */
On 2013/09/18 20:37:18, kevmoo wrote:
> @deprecated?

Done.

[floitsch, 2013-09-20 13:16:48]

@nweiz: I had to disable the stack trace test.

[Lasse Reichstein Nielsen, 2013-09-23 14:24:12]

LGTM

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/_collection_dev/i...
File sdk/lib/_collection_dev/iterable.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/_collection_dev/i...
sdk/lib/_collection_dev/iterable.dart:23: // TODO(floitsch): also used in
dart:async until end of september for
Capitalize September.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/async.dart
File sdk/lib/async/async.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/async.dart#...
sdk/lib/async/async.dart:21: import "dart:_collection-dev" show deprecated;
Have we told the editor people to not treat annotations badly?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/future.dart
File sdk/lib/async/future.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/future.dart...
sdk/lib/async/future.dart:198: handleError(error) {
Without the "void" this looks too much like a function call.
How about having "dynamic" as return type (because neither void nor Null seems
to typecheck. At least Null probably should).

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/future_impl...
File sdk/lib/async/future_impl.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/future_impl...
sdk/lib/async/future_impl.dart:160: : _onValueCallback =
Zone._current.registerCallback1(onValueCallback),
This uses Zone._current here because that is the zone being captured above in
the _zone variable, right?

It may be more readable if _zone is initialized explicitly in the constructors.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_cont...
File sdk/lib/async/stream_controller.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_cont...
sdk/lib/async/stream_controller.dart:582:
Zone.current.handleUncaughtError(_asyncError(e, s));
For unhandled errors, I wouldn't mind having an UnhandledAsyncError class
implementing Error which stores the stacktrace explicitly and returns it from
its Error.stackTrace getter.

The sooner we get away from attached stack traces, the better.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_impl...
File sdk/lib/async/stream_impl.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_impl...
sdk/lib/async/stream_impl.dart:326: _zone.runGuarded1(_onData, data);
Is that runGuarded*digit one*? It's very hard to read (I read it as "lower case
l").
Zone method names don't have to be short, so how about runGuardedUnary?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_impl...
sdk/lib/async/stream_impl.dart:341: _zone.runGuarded1(_onError, error);
Ditto.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_impl...
sdk/lib/async/stream_impl.dart:767: _zone.run1(_onCancelHandler, new
_BroadcastSubscriptionWrapper(this));
runUnary?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/timer.dart
File sdk/lib/async/timer.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/timer.dart#...
sdk/lib/async/timer.dart:70: duration, Zone.current.bindCallback1(callback,
runGuarded: true));
bindCallbackUnary?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart
File sdk/lib/async/zone.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:31: * This class provides a description for a forked
zone.
Description seems like a passive thing, not an active way to change the new
zone.
How about "specification" or "options"?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:43: *     bubbled down).
Perplexing, but I feel like bubbles should go up.
Would it change the meaning if you wrote "bubbled up" instead?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:45: * Handlers can either intercept the request (by
simply not calling the
intercept -> stop propagating
It always intercept the request. It may or may not propagate the request to the
parent, and it may or may not change the arguments on the way.

The DOM equivalent to not calling the parent zone is "event.stopPropagation()".

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:50: const factory ZoneDescription({
Document constructor. Just something along the lines of "Creates a description
with the provided handlers".

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:69: factory ZoneDescription.from(ZoneDescription other,
{
Documentation, something like: Creates description similar to [other], but
overridden with provided handlers.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:154: * This class allows to delegate callbacks to a
parent zone.
allows to delegate -> delegates
Or either "allows delegating callbacks" or "allows callbacks to be delegated" if
"allow" needs to be there.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:165: Zone get zone;
Can you avoid making the zone public? That would avoid calling the parent zone
directly.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:311: parent = parent.parent;
If you hit the root zone here, is that also a _CustomizedZone?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:389: final _CustomizedZone parent;
Again, are all zones '_CustomizedZone's?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:453: Zone fork([Map zoneValues, ZoneDescription
description]) {
Is there a reason for putting zoneValues first.
I'd expect the description to be more important, and therefore go first, but if
you expect the most common use of fork is adding values, putting that first
makes sense too.

Named arguments perhaps, if ordering isn't trivial?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:488: _scheduleAsyncCallback(() {
Not scheduleMicrotask?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:489: print("Uncaught Error: ${error}");
How about creating an UncaughtAsyncError object and just throwing that. It has
stackTrace (since it's an Error).
Give it a toString and we can print it directly. (Should Error.toString include
the stackTrace?)

I don't like printing the error here as well as throwing it. It should be
printed by the top-level loop. Can we avoid it?

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:549: Map<Symbol, dynamic> zoneValues, ZoneDescription
description) {
BTW, could we allow anything as keys, instead of just symbols.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:551: if (description is! _ZoneDescription) {
"else if", since that won't be the case after the assignment above.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:567: const _ROOT_DESCRIPTION =  const ZoneDescription(
double-space between "=" and "const".

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:579: const _ROOT_ZONE = const _CustomizedZone(null,
_ROOT_DESCRIPTION, const {});
Make the last argument "const <Symbol,dynamic>{}" ?

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/catch_err...
File tests/lib/async/catch_errors_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/catch_err...
tests/lib/async/catch_errors_test.dart:14: catchErrors(() {
catchErrors uses runZonedExperimental.

Could it be written with non-deprecated methods?

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
File tests/lib/async/zone_create_periodic_timer_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
tests/lib/async/zone_create_periodic_timer_test.dart:14: // gives that one to
handleUncaughtError.
Comment seems unrelated to test.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
tests/lib/async/zone_create_periodic_timer_test.dart:20: Duration period,
f(Timer timer)) {
Indent to after '('.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
File tests/lib/async/zone_create_timer2_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
tests/lib/async/zone_create_timer2_test.dart:14: // gives that one to
handleUncaughtError.
Wrong comment.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
File tests/lib/async/zone_create_timer_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
tests/lib/async/zone_create_timer_test.dart:14: // gives that one to
handleUncaughtError.
Bad comment

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_debu...
File tests/lib/async/zone_debug_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_debu...
tests/lib/async/zone_debug_test.dart:11: int stackTrace = 0;
Name stackTraceCount would be easier to understand.
If that is what it is, otherwise a comment would be great.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_fork...
File tests/lib/async/zone_fork_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_fork...
tests/lib/async/zone_fork_test.dart:14: // gives that one to
handleUncaughtError.
Bad comment?

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_regi...
File tests/lib/async/zone_register_callback1_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_regi...
tests/lib/async/zone_register_callback1_test.dart:34: // It is legal to invoke
the callback in a different zone. This is, of course,
I generally prefer "allowed" or "valid" to "legal" - reserving "legal" for
actual laws.

[floitsch, 2013-09-23 17:12:06]

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/_collection_dev/i...
File sdk/lib/_collection_dev/iterable.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/_collection_dev/i...
sdk/lib/_collection_dev/iterable.dart:23: // TODO(floitsch): also used in
dart:async until end of september for
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Capitalize September.

Done.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/async.dart
File sdk/lib/async/async.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/async.dart#...
sdk/lib/async/async.dart:21: import "dart:_collection-dev" show deprecated;
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Have we told the editor people to not treat annotations badly?

Currently it still works. It doesn't look like a high priority to change the
editor to look for specific values in the annotation.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/future.dart
File sdk/lib/async/future.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/future.dart...
sdk/lib/async/future.dart:198: handleError(error) {
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Without the "void" this looks too much like a function call.
> How about having "dynamic" as return type (because neither void nor Null seems
> to typecheck. At least Null probably should).
> 
> 
It sticks nicely out in the editor. So syntax highlighting makes a huge
difference.
"void" would actually work again (I had the FutureImpl temporarily changed but
needed to revert the change because too many users were depending on it).
Added "dynamic", but if you prefer I could go back to "void".

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/future_impl...
File sdk/lib/async/future_impl.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/future_impl...
sdk/lib/async/future_impl.dart:160: : _onValueCallback =
Zone._current.registerCallback1(onValueCallback),
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> This uses Zone._current here because that is the zone being captured above in
> the _zone variable, right?
> 
> It may be more readable if _zone is initialized explicitly in the
constructors.

done.
And made it Zone.current instead of Zone._current.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_cont...
File sdk/lib/async/stream_controller.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_cont...
sdk/lib/async/stream_controller.dart:582:
Zone.current.handleUncaughtError(_asyncError(e, s));
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> For unhandled errors, I wouldn't mind having an UnhandledAsyncError class
> implementing Error which stores the stacktrace explicitly and returns it from
> its Error.stackTrace getter.
> 
> The sooner we get away from attached stack traces, the better.

Let's discuss this in person tomorrow.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_impl...
File sdk/lib/async/stream_impl.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_impl...
sdk/lib/async/stream_impl.dart:326: _zone.runGuarded1(_onData, data);
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Is that runGuarded*digit one*? It's very hard to read (I read it as "lower
case
> l").
> Zone method names don't have to be short, so how about runGuardedUnary?

Done.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_impl...
sdk/lib/async/stream_impl.dart:341: _zone.runGuarded1(_onError, error);
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Ditto.

Done.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/stream_impl...
sdk/lib/async/stream_impl.dart:767: _zone.run1(_onCancelHandler, new
_BroadcastSubscriptionWrapper(this));
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> runUnary?

Done.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/timer.dart
File sdk/lib/async/timer.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/timer.dart#...
sdk/lib/async/timer.dart:70: duration, Zone.current.bindCallback1(callback,
runGuarded: true));
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> bindCallbackUnary?

Done.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart
File sdk/lib/async/zone.dart (right):

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:31: * This class provides a description for a forked
zone.
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Description seems like a passive thing, not an active way to change the new
> zone.
> How about "specification" or "options"?

Went with specification

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:43: *     bubbled down).
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Perplexing, but I feel like bubbles should go up.
> Would it change the meaning if you wrote "bubbled up" instead?

I guess not. Probably more correct.
done.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:45: * Handlers can either intercept the request (by
simply not calling the
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> intercept -> stop propagating
> It always intercept the request. It may or may not propagate the request to
the
> parent, and it may or may not change the arguments on the way.
> 
> The DOM equivalent to not calling the parent zone is
"event.stopPropagation()".

Done.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:50: const factory ZoneDescription({
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Document constructor. Just something along the lines of "Creates a description
> with the provided handlers".

Done.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:69: factory ZoneDescription.from(ZoneDescription other,
{
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Documentation, something like: Creates description similar to [other], but
> overridden with provided handlers.

Done.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:154: * This class allows to delegate callbacks to a
parent zone.
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> allows to delegate -> delegates
> Or either "allows delegating callbacks" or "allows callbacks to be delegated"
if
> "allow" needs to be there.

"This class wraps zones for delegation".

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:165: Zone get zone;
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Can you avoid making the zone public? That would avoid calling the parent zone
> directly.

Done. But it won't change much. The Zone itself exposes a pointer to the parent.
So self.parent will work the same way.
That said: in some cases it makes sense to just call something in the parent
zone (like the actual handler).

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:311: parent = parent.parent;
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> If you hit the root zone here, is that also a _CustomizedZone?

Yes. Currently the root-zone is a customized zone with all handlers set.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:389: final _CustomizedZone parent;
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Again, are all zones '_CustomizedZone's?

Currently yes.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:453: Zone fork([Map zoneValues, ZoneDescription
description]) {
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Is there a reason for putting zoneValues first.
> I'd expect the description to be more important, and therefore go first, but
if
> you expect the most common use of fork is adding values, putting that first
> makes sense too.
> 
> Named arguments perhaps, if ordering isn't trivial?

Made them named.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:488: _scheduleAsyncCallback(() {
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Not scheduleMicrotask?

This is the internals. We can change it in another CL. This is the direct call
to the embedder's function.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:489: print("Uncaught Error: ${error}");
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> How about creating an UncaughtAsyncError object and just throwing that. It has
> stackTrace (since it's an Error).
> Give it a toString and we can print it directly. (Should Error.toString
include
> the stackTrace?)
> 
> I don't like printing the error here as well as throwing it. It should be
> printed by the top-level loop. Can we avoid it?

Ok for discussion in a different CL. (this is still the same behavior as
before.)

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:549: Map<Symbol, dynamic> zoneValues, ZoneDescription
description) {
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> BTW, could we allow anything as keys, instead of just symbols.

We could. Karl and I discussed different possibilities and decided that relaxing
is easy.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:551: if (description is! _ZoneDescription) {
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> "else if", since that won't be the case after the assignment above.

should still be the case, since ZoneDescription is _ZoneDescription

but making it an else-if.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:567: const _ROOT_DESCRIPTION =  const ZoneDescription(
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> double-space between "=" and "const".

Done.

https://codereview.chromium.org/23875032/diff/28001/sdk/lib/async/zone.dart#n...
sdk/lib/async/zone.dart:579: const _ROOT_ZONE = const _CustomizedZone(null,
_ROOT_DESCRIPTION, const {});
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Make the last argument "const <Symbol,dynamic>{}" ?

Done.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/catch_err...
File tests/lib/async/catch_errors_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/catch_err...
tests/lib/async/catch_errors_test.dart:14: catchErrors(() {
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> catchErrors uses runZonedExperimental.
> 
> Could it be written with non-deprecated methods?

Done.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
File tests/lib/async/zone_create_periodic_timer_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
tests/lib/async/zone_create_periodic_timer_test.dart:14: // gives that one to
handleUncaughtError.
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Comment seems unrelated to test.

Done.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
tests/lib/async/zone_create_periodic_timer_test.dart:20: Duration period,
f(Timer timer)) {
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Indent to after '('.

Done.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
File tests/lib/async/zone_create_timer2_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
tests/lib/async/zone_create_timer2_test.dart:14: // gives that one to
handleUncaughtError.
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Wrong comment.

Done.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
File tests/lib/async/zone_create_timer_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_crea...
tests/lib/async/zone_create_timer_test.dart:14: // gives that one to
handleUncaughtError.
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Bad comment

Done.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_debu...
File tests/lib/async/zone_debug_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_debu...
tests/lib/async/zone_debug_test.dart:11: int stackTrace = 0;
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Name stackTraceCount would be easier to understand.
> If that is what it is, otherwise a comment would be great.

It stands for normal stack traces. So "count" would be wrong.
Added comment.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_fork...
File tests/lib/async/zone_fork_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_fork...
tests/lib/async/zone_fork_test.dart:14: // gives that one to
handleUncaughtError.
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> Bad comment?

Done.

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_regi...
File tests/lib/async/zone_register_callback1_test.dart (right):

https://codereview.chromium.org/23875032/diff/28001/tests/lib/async/zone_regi...
tests/lib/async/zone_register_callback1_test.dart:34: // It is legal to invoke
the callback in a different zone. This is, of course,
On 2013/09/23 14:24:12, Lasse Reichstein Nielsen wrote:
> I generally prefer "allowed" or "valid" to "legal" - reserving "legal" for
> actual laws.

Done.

[floitsch, 2013-09-24 09:23:26]

Committed patchset #11 manually as r27802 (presubmit successful).