Implement spawnDomFunction

Implement spawnDomFunction

Removed the spawnDomIsolate in favor of spawnDomFunction.  The signature of this function matches spawnFunction.

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

[Anton Muhin, 2012-08-03 13:16:00]

Why we're dropping window argument?

https://chromiumcodereview.appspot.com/10837088/diff/7001/tests/html/dom_isol...
File tests/html/dom_isolates_test.dart (right):

https://chromiumcodereview.appspot.com/10837088/diff/7001/tests/html/dom_isol...
tests/html/dom_isolates_test.dart:58: // test('Masked local function', () {
I'd rather guard it with if (false)

[vsm, 2012-08-03 15:20:44]

Re window argument to spawnDomFunction, I was mirroring the spawnFunction API.
- Do we have a good use case for spawning on a different window?
- What's the security model?

Thoughts?

https://chromiumcodereview.appspot.com/10837088/diff/7001/tests/html/dom_isol...
File tests/html/dom_isolates_test.dart (right):

https://chromiumcodereview.appspot.com/10837088/diff/7001/tests/html/dom_isol...
tests/html/dom_isolates_test.dart:58: // test('Masked local function', () {
On 2012/08/03 13:16:00, Anton Mukhin wrote:
> I'd rather guard it with if (false)

Done.

[Siggi Cherem (dart-lang), 2012-08-03 17:30:57]

lgtm 

About window: one option we have discussed was to make 'spawnDomFunction' a
method in window, rather than passing window as an argument. Then the isolate
would have access to the window that receives the call (e.g. this would also be
the same way to spawn an isolate in an iframe window.) We haven't come up with
good use cases yet, so I'm happy with the current API for now.

https://chromiumcodereview.appspot.com/10837088/diff/10001/lib/dom/src/native...
File lib/dom/src/native_DOMImplementation.dart (right):

https://chromiumcodereview.appspot.com/10837088/diff/10001/lib/dom/src/native...
lib/dom/src/native_DOMImplementation.dart:35: static SendPort
spawnDomFunctionImpl(Function topLevelFunction) native "Utils_spawnDomFunction";
yay!

https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
File tests/html/dom_isolates_test.dart (right):

https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
tests/html/dom_isolates_test.dart:46: });
one more case to test would be a static method in a class (which is valid, just
like top-level functions). AFAIK, that doesn't work today in dart2js.

[Anton Muhin, 2012-08-07 12:55:29]

Regarding window argument.

I don't think I am aware of any use cases, but you may want run it by Dart
community---there were some people who played w/ Dartium DOM isolate API.

https://chromiumcodereview.appspot.com/10837088/diff/10001/lib/dom/src/native...
File lib/dom/src/native_DOMPublic.dart (right):

https://chromiumcodereview.appspot.com/10837088/diff/10001/lib/dom/src/native...
lib/dom/src/native_DOMPublic.dart:6: spawnDomFunction(Function topLevelFunction)
{
nit: arrow syntax, please

https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
File tests/html/dom_isolates_test.dart (right):

https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
tests/html/dom_isolates_test.dart:12: replyTo.send(42);
why it's 42?  Intent of window.location.toString() (which should have been
'${window.location}' was to check DOM API is working in child isolate.

https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
tests/html/dom_isolates_test.dart:41: var sendPort =
spawnDomFunction(isolateMainTrampoline);
do you need sendPort at all? 
spawnDomFunction(isolateMainTrampoline).call('check')... ?  Ditto for test
above.

[vsm, 2012-08-08 16:59:36]

On 2012/08/07 12:55:29, Anton Mukhin wrote:
> Regarding window argument.

BTW, what are your thoughts on security here?  I didn't see anything like SOP
checks.  We shouldn't allow executing code in an arbitrary Window.  We've
eliminated SOP checks for the most part in favor of a more restrictive mode.  Do
we want SOP here?

> 
> I don't think I am aware of any use cases, but you may want run it by Dart
> community---there were some people who played w/ Dartium DOM isolate API.
> 
>
https://chromiumcodereview.appspot.com/10837088/diff/10001/lib/dom/src/native...
> File lib/dom/src/native_DOMPublic.dart (right):
> 
>
https://chromiumcodereview.appspot.com/10837088/diff/10001/lib/dom/src/native...
> lib/dom/src/native_DOMPublic.dart:6: spawnDomFunction(Function
topLevelFunction)
> {
> nit: arrow syntax, please
> 
>
https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
> File tests/html/dom_isolates_test.dart (right):
> 
>
https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
> tests/html/dom_isolates_test.dart:12: replyTo.send(42);
> why it's 42?  Intent of window.location.toString() (which should have been
> '${window.location}' was to check DOM API is working in child isolate.
> 
>
https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
> tests/html/dom_isolates_test.dart:41: var sendPort =
> spawnDomFunction(isolateMainTrampoline);
> do you need sendPort at all? 
> spawnDomFunction(isolateMainTrampoline).call('check')... ?  Ditto for test
> above.

[Anton Muhin, 2012-08-09 16:39:56]

lgtm w/ my comments addressed or answered :)

Regarding security model.  SOP looks like a reasonable default.

I wonder if there are chances to circumvent it via script injection though.

[vsm, 2012-08-10 00:06:43]

thanks, landing.

https://chromiumcodereview.appspot.com/10837088/diff/10001/lib/dom/src/native...
File lib/dom/src/native_DOMPublic.dart (right):

https://chromiumcodereview.appspot.com/10837088/diff/10001/lib/dom/src/native...
lib/dom/src/native_DOMPublic.dart:6: spawnDomFunction(Function topLevelFunction)
{
On 2012/08/07 12:55:29, Anton Muhin wrote:
> nit: arrow syntax, please

Done.

https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
File tests/html/dom_isolates_test.dart (right):

https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
tests/html/dom_isolates_test.dart:12: replyTo.send(42);
Good point.  I changed it because it's no longer 'about:blank'.  I'll keep and
compare with the main isolate's window.location instead.

On 2012/08/07 12:55:29, Anton Muhin wrote:
> why it's 42?  Intent of window.location.toString() (which should have been
> '${window.location}' was to check DOM API is working in child isolate.

https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
tests/html/dom_isolates_test.dart:41: var sendPort =
spawnDomFunction(isolateMainTrampoline);
On 2012/08/07 12:55:29, Anton Muhin wrote:
> do you need sendPort at all? 
> spawnDomFunction(isolateMainTrampoline).call('check')... ?  Ditto for test
> above.

Done.

https://chromiumcodereview.appspot.com/10837088/diff/10001/tests/html/dom_iso...
tests/html/dom_isolates_test.dart:46: });
I'll add in a separate CL.

On 2012/08/03 17:30:57, sigmund wrote:
> one more case to test would be a static method in a class (which is valid,
just
> like top-level functions). AFAIK, that doesn't work today in dart2js.