Add Zone support for Timers.

Add Zone support for Timers.

R=lrn@google.com

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

[Lasse Reichstein Nielsen, 2013-05-24 09:29:36]

lgtm

[floitsch, 2013-05-28 19:35:46]

Fixed a bug, commented tests, added tests, and adapted dart2js implementation.
PTAL.

[Lasse Reichstein Nielsen, 2013-05-29 08:04:59]

lgtm

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

https://codereview.chromium.org/15764003/diff/4001/sdk/lib/async/future_impl....
sdk/lib/async/future_impl.dart:142: throw new UnsupportedError(
Then just return true.
You never know if someone later wants to do some checking, and calls the
function.

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

https://codereview.chromium.org/15764003/diff/4001/sdk/lib/async/zone.dart#ne...
sdk/lib/async/zone.dart:68: void executeGuardedCallback(void fun());
Name annoys me.
It's not the callback which has the property of being guarded.
It is the execution.
So I'd prefer either 
  guardedExecuteCallback 
or 
  executeCallbackGuarded
Probably just being silly.

https://codereview.chromium.org/15764003/diff/4001/sdk/lib/async/zone.dart#ne...
sdk/lib/async/zone.dart:74: void executePeriodicCallback(void fun());
execueCallbackPeriodically?

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors13_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors13_test.dart:15: // necessary, but it definitely
doesn't hurt.
A "not sure" in a unit test is worrying.
Can we find out?

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors28_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors28_test.dart:11: // time out if the callbacks are
not invoked.
The VM won't time out. It will hang, and the test suite will time out the test. 
Or the VM will be timed out/preempted.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors28_test.dart:18: if (counter++ == 5) timer.cancel();
Please move 
 counter++ 
to a line of its own.
Only very idiomatic cases of side effects in tests are acceptable (and that
would be pre-increment).

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors2_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors2_test.dart:22: Expect.fail("Unexped callback");
Unexpected.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors3_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors3_test.dart:29: Expect.fail("Unexped callback");
Unexpected.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors4_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors4_test.dart:23: "catch error entry",
Remove space between [ and ".

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors5_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors5_test.dart:24: Expect.listEquals([
Remove space between [ and ".

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors7_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors7_test.dart:26: "catch error entry",
Indentation looks iffy (indent by 6, not 2 or 4).
I can see the argument, but perhaps move the [ to the next line to get the
indent by 4.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors8_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors8_test.dart:39: Expect.listEquals([
Remove space between [ and ".

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors9_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors9_test.dart:22: Expect.fail("Unexped callback");
Unexpected.

[floitsch, 2013-05-29 15:21:29]

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

https://codereview.chromium.org/15764003/diff/4001/sdk/lib/async/future_impl....
sdk/lib/async/future_impl.dart:142: throw new UnsupportedError(
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> Then just return true.
> You never know if someone later wants to do some checking, and calls the
> function.
> 

Actually it is not necessarily `true`. It depends who is calling. Currently
nobody calls, so anything would be valid. Any other call would depend. -> I
prefer leaving as is.
Removed last line of comment.

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

https://codereview.chromium.org/15764003/diff/4001/sdk/lib/async/zone.dart#ne...
sdk/lib/async/zone.dart:68: void executeGuardedCallback(void fun());
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> Name annoys me.
> It's not the callback which has the property of being guarded.
> It is the execution.
> So I'd prefer either 
>   guardedExecuteCallback 
> or 
>   executeCallbackGuarded
> Probably just being silly.

changed to executeCallbackGuarded

https://codereview.chromium.org/15764003/diff/4001/sdk/lib/async/zone.dart#ne...
sdk/lib/async/zone.dart:74: void executePeriodicCallback(void fun());
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> execueCallbackPeriodically?

no. The callback is not executed periodically.
it's a periodic callback we are executing (which is why we don't decrement the
counter).

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors13_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors13_test.dart:15: // necessary, but it definitely
doesn't hurt.
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> A "not sure" in a unit test is worrying.
> Can we find out?

Changed comment.
Still necessary, since we still use Timers for async scheduling.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors28_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors28_test.dart:11: // time out if the callbacks are
not invoked.
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> The VM won't time out. It will hang, and the test suite will time out the
test. 
> Or the VM will be timed out/preempted.

Done.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors28_test.dart:18: if (counter++ == 5) timer.cancel();
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> Please move 
>  counter++ 
> to a line of its own.
> Only very idiomatic cases of side effects in tests are acceptable (and that
> would be pre-increment).

Done.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors2_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors2_test.dart:22: Expect.fail("Unexped callback");
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> Unexpected.

Done.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors3_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors3_test.dart:29: Expect.fail("Unexped callback");
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> Unexpected.

Done.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors4_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors4_test.dart:23: "catch error entry",
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> Remove space between [ and ".

I prefer if I can cut/copy lines but ok...

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors5_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors5_test.dart:24: Expect.listEquals([
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> Remove space between [ and ".

Done.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors7_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors7_test.dart:26: "catch error entry",
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> Indentation looks iffy (indent by 6, not 2 or 4).
> I can see the argument, but perhaps move the [ to the next line to get the
> indent by 4.

Done.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors8_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors8_test.dart:39: Expect.listEquals([
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> Remove space between [ and ".

Done.

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
File tests/lib/async/catch_errors9_test.dart (right):

https://codereview.chromium.org/15764003/diff/4001/tests/lib/async/catch_erro...
tests/lib/async/catch_errors9_test.dart:22: Expect.fail("Unexped callback");
On 2013/05/29 08:04:59, Lasse Reichstein Nielsen wrote:
> Unexpected.

Done.

[floitsch, 2013-06-13 12:45:53]

PTAL.

[Lasse Reichstein Nielsen, 2013-06-13 12:48:42]

lgtm

[floitsch, 2013-06-14 16:28:04]

Committed patchset #8 manually as r24035 (presubmit successful).

[floitsch, 2013-06-14 16:31:29]

Forgot to CC dart2js and VM for library changes.
If you have comments I will address them in a separate CL (or, in the worst
case, revert this CL).

[srdjan, 2013-06-14 17:06:30]

Thanks for CC-ing, LGTM

[ngeoffray, 2013-06-17 06:52:31]

dart2js changes LGTM