Clean up the patch file for the isolate library by introducing a new builtin library: isolate_helpe…

Clean up the patch file for the isolate library by introducing a new builtin library: isolate_helper.dart.
Committed: https://code.google.com/p/dart/source/detail?r=16029

[ngeoffray, 2012-12-12 10:54:15]

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart (right):

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:1232: int
setTimeout(_TimeoutHandler handler, int timeout) {
This class does not have access to the native keyword anymore, so I'm
implementing by hand what the previous code would automatically generate. Don't
worry too much about it: I plan on removing this class in the isolate library
since it does not work with web workers.

[kasperl, 2012-12-12 11:24:00]

LGTM.

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart (right):

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:380: void
terminate();
So this is the only function that needs to be native? Can we handle that with JS
instead? Wouldn't it be fairly simple to get rid of these native classes and
just use JS code that operates on the DOM objects (Worker/Window) from the side?

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:1262: class
TimerImpl implements Timer {
You converted this to be a public class just because it is safe to do so -- or
because you need to access it from outside the library?

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:1266:
TimerImpl(int milliSeconds, void callback(Timer timer))
milliSeconds -> milliseconds (repeated)

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/isolate_patch.dart (right):

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/isolate_patch.dart:31: patch
factory Timer(int milliSeconds, void callback(Timer timer)) {
milliSeconds -> milliseconds (repeated)

[ngeoffray, 2012-12-12 11:27:59]

Thanks Kasper

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart (right):

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:380: void
terminate();
On 2012/12/12 11:24:00, kasperl wrote:
> So this is the only function that needs to be native? Can we handle that with
JS
> instead? Wouldn't it be fairly simple to get rid of these native classes and
> just use JS code that operates on the DOM objects (Worker/Window) from the
side?

Yes, that's what I plan on doing. Note that this class is dead code right now
because we don't support web workers.

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:1262: class
TimerImpl implements Timer {
On 2012/12/12 11:24:00, kasperl wrote:
> You converted this to be a public class just because it is safe to do so -- or
> because you need to access it from outside the library? 

I need to access it outside the library: in the isolate_patch.dart file. Since
no-one else can import this library (one can import it manually but it would be
a different library), making the class public is fine.

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:1266:
TimerImpl(int milliSeconds, void callback(Timer timer))
On 2012/12/12 11:24:00, kasperl wrote:
> milliSeconds -> milliseconds (repeated)

Done.

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/isolate_patch.dart (right):

https://codereview.chromium.org/11549017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/isolate_patch.dart:31: patch
factory Timer(int milliSeconds, void callback(Timer timer)) {
On 2012/12/12 11:24:00, kasperl wrote:
> milliSeconds -> milliseconds (repeated)

Done.

[ahe, 2012-12-14 08:49:15]

LGTM

https://codereview.chromium.org/11549017/diff/7002/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart (right):

https://codereview.chromium.org/11549017/diff/7002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:373: //
@Native("*Worker");
Add a comment explaining why you're commenting this out for now.

https://codereview.chromium.org/11549017/diff/7002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:1230: //
@Native("*DOMWindow");
Ditto.