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

frog/value.dart

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * Represents a meta-value for code generation.
  */
 class Value {
   /** The [Type] of the [Value]. */
   Type type;
@@ skipping 177 matching lines @@
         fromType = world.objectType;
       }
       bool bothNum = type.isNum && toType.isNum;
       return fromType.isSubtypeOf(toType) || bothNum;
     }
     return false;
   }
 
   /**
    * 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,
       [bool isDynamic=false]) {
 
     // Issue type warnings unless we are processing a dynamic operation.
     bool checked = !isDynamic;
 
     var callMethod = toType.getCallMethod();
     if (callMethod != null) {
       if (checked && !toType.isAssignable(type)) {
         convertWarning(toType, node);
       }
 
       int arity = callMethod.parameters.length;
       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);
       }
     }
 
     // Don't add runtime asserts unless we have type checks turned on.
     if (!options.enableTypeChecks) {
       return this;
     }
 
     // If we're assigning from a var, pretend it's Object for the purpose of
     // runtime checks.
@@ skipping 37 matching lines @@
     } else {
       // TODO(jmesserly): this is hacky.
       if (code.startsWith('\$notnull_bool')) {
         return this;
       } else {
         return new Value(world.boolType, '\$notnull_bool($code)');
       }
     }
   }
 
+  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.
    * Also it will throw a TypeError if it gets the wrong type.
    */
   Value _typeAssert(MethodGenerator context, Type toType, Node node) {
     if (toType is ParameterType) {
       ParameterType p = toType;
       toType = p.extendsType;
     }
@@ skipping 312 matching lines @@
       return 1;
     } else if (name != null && other.name == null) {
       return -1;
     } else if (name != null) {
       return name.compareTo(other.name);
     } else {
       return field.name.compareTo(other.field.name);
     }
   }
 }