Isolates in frog - tweaks in existing js code to make things run

frog/value.dart

Index: frog/value.dart
diff --git a/frog/value.dart b/frog/value.dart
index bb5ff4ce54ef2c3ecc63d4f970173e077ef6ba76..9d638d23050872ac1d129e02bcdf21e5fb17d5f8 100644
--- a/frog/value.dart
+++ b/frog/value.dart
@@ -195,8 +195,9 @@ class Value {
 
   /**
    * Assign or convert this value to another type.
-   * This is used for converting between function types, and inserting type
-   * checks when --enable_type_checks is enabled.
+   * This is used for converting between function types, inserting type
+   * checks when --enable_type_checks is enabled, and wrapping callback
+   * functions passed to the dom so we can restore their isolate context.
    */
   // WARNING: this needs to be kept in sync with needsConversion above.
   Value convertTo(MethodGenerator context, Type toType, Node node,
@@ -215,7 +216,11 @@ class Value {
       var myCall = type.getCallMethod();
       if (myCall == null || myCall.parameters.length != arity) {
         final stub = world.functionType.getCallStub(new Arguments.bare(arity));
-        return new Value(toType, 'to\$${stub.name}($code)');
+        var val = new Value(toType, 'to\$${stub.name}($code)');
+        return _isDomCallback(toType) && !_isDomCallback(type) ?

[Siggi Cherem (dart-lang), 2011/11/08 02:06:26]

BTW, this was a cool trick that John came up with. but I'm not very happy having
this as part of the compiler.

We have to first reconsider the design issue of whether or not we allow multiple
isolates to have access to the DOM, and if we do, is there a way to handle at
the library level?

[Jennifer Messerly, 2011/11/08 05:04:35]

It'd be worth putting the "we have to first reconsider..." as a TODO.

In general I don't think there's anything wrong with isolates & the DOM being a
bit special. What would be better though is if we had a generic way of marking
function-typed parameters as "native callbacks". Then the "conversion" only
applies to those parameters. (Converting arguments when going from
dynamic->static is a common trick in many systems. Usually you specify it with
some sort of metadata, e.g. the C# [MarshalAs] attribute)

I wonder if "typedefs" allow "native" in the grammar? That might give us the
hook we need to put it in the definition of EventListener & friends.

[jimhug, 2011/11/08 15:39:01]

I'm not sure about the right long-term solution, but like this as a hack for
now.  +1 to the suggestion to add a TODO with the contents of your comment
above.
On 2011/11/08 05:04:35, John Messerly wrote:

[Siggi Cherem (dart-lang), 2011/11/08 17:56:42]

Done

+            val._wrapDomCallback(toType, arity) : val;
+      } else if (_isDomCallback(toType) && !_isDomCallback(type)) {
+        return _wrapDomCallback(toType, arity);
       }
     }
 
@@ -273,6 +278,15 @@ class Value {
     }
   }
 
+  bool _isDomCallback(toType) {

[jimhug, 2011/11/08 15:39:01]

This could use a doc comment and possibly a TODO as well to highlight the
deliberate choice to do something a little weird for a compiler - in return for
great benefit as a web language.

[Siggi Cherem (dart-lang), 2011/11/08 17:56:42]

Done.

+    return (toType.definition is FunctionTypeDefinition
+        && toType.library == world.dom);
+  }
+
+  Value _wrapDomCallback(Type toType, int arity) {
+    return new Value(toType, '\$wrap_call\$$arity($code)');
+  }
+
   /**
    * Generates a run time type assertion for the given value. This works like
    * [instanceOf], but it allows null since Dart types are nullable.