Optimize dartium dart:html bindings so real world application performance is acceptable. Improves d…

sdk/lib/html/html_common/conversions_dartium.dart

Index: sdk/lib/html/html_common/conversions_dartium.dart
diff --git a/sdk/lib/html/html_common/conversions_dartium.dart b/sdk/lib/html/html_common/conversions_dartium.dart
index 66299a05389fd4c7b79a0ef086ac052eccdd82ec..2802518bcf2cdac6fda6554d2c0e4ac914b375fd 100644
--- a/sdk/lib/html/html_common/conversions_dartium.dart
+++ b/sdk/lib/html/html_common/conversions_dartium.dart
@@ -140,17 +140,23 @@ Future convertNativePromiseToDartFuture(js.JsObject promise) {
 }
 
 convertDartToNative_DateTime(DateTime date) {
-  return new js.JsObject(js.context["Date"], [date.millisecondsSinceEpoch]);
+  return date; // TODO(jacobr): I don't get why this is needed. new js.JsObject(js.context["Date"], [date.millisecondsSinceEpoch]);

[Alan Knight, 2016/03/25 21:39:09]

There will be explicit calls to it from conversions in generator.py. Maybe those
could be removed?

[Jacob, 2016/03/30 00:19:00]

Removed. This is already done in Dartium C++

 }
 
 /// Creates a Dart Rectangle from a Javascript object with properties
-/// left, top, width and height. Used internally in Dartium.
-Rectangle make_dart_rectangle(r) =>
-    r == null ? null : new Rectangle(
-    js.JsNative.getProperty(r, 'left'),
-    js.JsNative.getProperty(r, 'top'),
-    js.JsNative.getProperty(r, 'width'),
-    js.JsNative.getProperty(r, 'height'));
+/// left, top, width and height or a 4 element array of integers. Used internally in Dartium.
+Rectangle make_dart_rectangle(r) {
+  if (r == null) return null;
+  if (r is List) {
+    return new Rectangle(r[0], r[1], r[2], r[3]);
+  }
+
+  return new Rectangle(
+      js.JsNative.getProperty(r, 'left'),
+      js.JsNative.getProperty(r, 'top'),
+      js.JsNative.getProperty(r, 'width'),
+      js.JsNative.getProperty(r, 'height'));
+}
 
 // Converts a flat Dart map into a JavaScript object with properties this is
 // is the Dartium only version it uses dart:js.
@@ -158,7 +164,7 @@ Rectangle make_dart_rectangle(r) =>
 // code in html_common and be more general.
 convertDartToNative_Dictionary(Map dict) {
   if (dict == null) return null;
-  var jsObject = new js.JsObject(js.JsNative.getProperty(js.context, 'Object'));
+  var jsObject = new js.JsObject(js.context['Object']);
   dict.forEach((String key, value) {
     if (value is List) {
       var jsArray = new js.JsArray();
@@ -205,61 +211,33 @@ List convertDartToNative_StringArray(List<String> input) => input;
 // Converts a Dart list into a JsArray. For the Dartium version only.
 convertDartToNative_List(List input) => new js.JsArray()..addAll(input);
 
-/// Find the underlying JS object for a dart:html Dart object.
-unwrap_jso(dartClass_instance) => js.unwrap_jso(dartClass_instance);
-
-// Flag to disable JS interop asserts.  Setting to false will speed up the
-// wrap_jso calls.
-bool interop_checks = false;
-
-/// Wrap a JS object with an instance of the matching dart:html class. Used only in Dartium.
-wrap_jso(jsObject) {
+// Incredibly slow implementation to lookup the runtime type for an object.
+// Fortunately, performance doesn't matter much as the results are cached
+// as long as the object being looked up has a valid prototype.
+// TODO(jacobr): we should track the # of lookups to ensure that things aren't
+// going off the rails due to objects with null prototypes, etc.
+Type lookupType(js.JsObject jsObject) {

[Alan Knight, 2016/03/25 21:39:09]

Will probably break if we have JS objects that share a prototype but map to
different things in Dart. But so does everything else.  See dartbug.com/23144

[Jacob, 2016/03/30 00:19:00]

Yep. But there is no cost of failing in the same places dart2js would fail.

   try {
-    if (jsObject is! js.JsObject || jsObject == null) {
-      // JS Interop converted the object to a Dart class e.g., Uint8ClampedList.
-      // or it's a simple type.
-      return jsObject;
-    }
-
-    var wrapper = js.getDartHtmlWrapperFor(jsObject);
-    // if we have a wrapper return the Dart instance.
-    if (wrapper != null) {
-      return wrapper;
-    }
-
-    if (jsObject is js.JsArray) {
-      wrapper = new js.JSArray.create(jsObject);
-      js.setDartHtmlWrapperFor(jsObject, wrapper);
-      return wrapper;
-    }
-    if (jsObject is js.JsFunction) {
-      wrapper = new js.JSFunction.create(jsObject);
-      js.setDartHtmlWrapperFor(jsObject, wrapper);
-      return wrapper;
-    }
-
-    // Try the most general type conversions on it.
-    // TODO(alanknight): We may be able to do better. This maintains identity,
-    // which is useful, but expensive. And if we nest something that only
-    // this conversion handles, how does that work? e.g. a list of maps of elements.
-    var converted = convertNativeToDart_SerializedScriptValue(jsObject);
-    if (!identical(converted, jsObject)) {
-      return converted;
-    }
+  // TODO(jacobr): add static methods that return the runtime type of the patch
+  // class so that this code works as expected.
+  if (jsObject is js.JsArray) {
+    return js.JSArray.instanceRuntimeType;
+  }
+  if (jsObject is js.JsFunction) {
+    return js.JSFunction.instanceRuntimeType;
+  }
 
     var constructor = js.JsNative.getProperty(jsObject, 'constructor');
     if (constructor == null) {
       // Perfectly valid case for JavaScript objects where __proto__ has
       // intentionally been set to null.
-      js.setDartHtmlWrapperFor(jsObject, new js.JSObject.create(jsObject));
-      return jsObject;
+      // We should track and warn about this case as peformance will be poor.
+      return js.JSObject.instanceRuntimeType;
     }
     var jsTypeName = js.JsNative.getProperty(constructor, 'name');
     if (jsTypeName is! String || jsTypeName.length == 0) {
       // Not an html type.
-      wrapper = new js.JSObject.create(jsObject);
-      js.setDartHtmlWrapperFor(jsObject, wrapper);
-      return wrapper;
+      return js.JSObject.instanceRuntimeType;
     }
 
     var dartClass_instance;
@@ -286,16 +264,18 @@ wrap_jso(jsObject) {
       if (func == null) {
         // Start walking the prototype chain looking for a JS class.
         var prototype = jsObject['__proto__'];
-        var keepWalking = true;
-        while (keepWalking && prototype.hasProperty('__proto__')) {
+        while (prototype != null && prototype.hasProperty('__proto__')) {
           prototype = prototype['__proto__'];
-          if (prototype != null && prototype is Element &&
-              prototype.blink_jsObject != null) {
-            // We're a Dart class that's pointing to a JS class.
-            var blinkJso = prototype.blink_jsObject;
-            jsTypeName = blinkJso['constructor']['name'];
+          // XXX FIX this... we are hosed if we hit a not JS class
+          // so this makes little sense.
+          if (prototype is! js.JsObject)  break;
+
+          // We're a Dart class that's pointing to a JS class.
+          var constructor =  prototype['constructor'];
+          if (constructor is js.JsObject) {
+            jsTypeName = constructor['name'];
             func = getHtmlCreateFunction(jsTypeName);
-            keepWalking = func == null;
+            if (func != null) break;
           }
         }
       }
@@ -303,186 +283,70 @@ wrap_jso(jsObject) {
 
     // Can we construct a Dart class?
     if (func != null) {
-      dartClass_instance = func();
-
-      // Wrap our Dart instance in both directions.
-      dartClass_instance.blink_jsObject = jsObject;
-      js.setDartHtmlWrapperFor(jsObject, dartClass_instance);
-    }
-
-    // TODO(jacobr): cache that this is not a dart:html JS class.
-    return dartClass_instance;
-  } catch (e, stacktrace) {
-    if (interop_checks) {
-      if (e is DebugAssertException) window.console
-          .log("${e.message}\n ${stacktrace}");
-      else window.console.log("${stacktrace}");
-    }
-  }
-
-  return null;
-}
-
-/**
- * Create Dart class that maps to the JS Type, add the JsObject as an expando
- * on the Dart class and return the created Dart class.
- */
-wrap_jso_no_SerializedScriptvalue(jsObject) {
-  try {
-    if (jsObject is! js.JsObject || jsObject == null) {
-      // JS Interop converted the object to a Dart class e.g., Uint8ClampedList.
-      // or it's a simple type.
-      return jsObject;
-    }
-
-    // TODO(alanknight): With upgraded custom elements this causes a failure because
-    // we need a new wrapper after the type changes. We could possibly invalidate this
-    // if the constructor name didn't match?
-    var wrapper = js.getDartHtmlWrapperFor(jsObject);
-    if (wrapper != null) {
-      return wrapper;
-    }
-
-    if (jsObject is js.JsArray) {
-      wrapper = new js.JSArray.create(jsObject);
-      js.setDartHtmlWrapperFor(jsObject, wrapper);
-      return wrapper;
-    }
-    if (jsObject is js.JsFunction) {
-      wrapper = new js.JSFunction.create(jsObject);
-      js.setDartHtmlWrapperFor(jsObject, wrapper);
-      return wrapper;
-    }
-
-    var constructor = js.JsNative.getProperty(jsObject, 'constructor');
-    if (constructor == null) {
-      // Perfectly valid case for JavaScript objects where __proto__ has
-      // intentionally been set to null.
-      js.setDartHtmlWrapperFor(jsObject, new js.JSObject.create(jsObject));
-      return jsObject;
-    }
-    var jsTypeName = js.JsNative.getProperty(constructor, 'name');
-    if (jsTypeName is! String || jsTypeName.length == 0) {
-      // Not an html type.
-      wrapper = new js.JSObject.create(jsObject);
-      js.setDartHtmlWrapperFor(jsObject, wrapper);
-      return wrapper;
-    }
-
-    var func = getHtmlCreateFunction(jsTypeName);
-    if (func != null) {
-      var dartClass_instance = func();
-      dartClass_instance.blink_jsObject = jsObject;
-      js.setDartHtmlWrapperFor(jsObject, dartClass_instance);
-      return dartClass_instance;
+      var ret = func().runtimeType;
+      return ret;
     }
-    wrapper = new js.JSObject.create(jsObject);
-    js.setDartHtmlWrapperFor(jsObject, wrapper);
-    return wrapper;
   } catch (e, stacktrace) {
-    if (interop_checks) {
-      if (e is DebugAssertException) window.console
-          .log("${e.message}\n ${stacktrace}");
-      else window.console.log("${stacktrace}");
-    }
-  }
-
-  return null;
-}
-
-/**
- * Create Dart class that maps to the JS Type that is the JS type being
- * extended using JS interop createCallback (we need the base type of the
- * custom element) not the Dart created constructor.
- */
-wrap_jso_custom_element(jsObject) {
-  try {
-    if (jsObject is! js.JsObject) {
-      // JS Interop converted the object to a Dart class e.g., Uint8ClampedList.
-      return jsObject;
-    }
-
-    // Find out what object we're extending.
-    var objectName = jsObject.toString();
-    // Expect to see something like '[object HTMLElement]'.
-    if (!objectName.startsWith('[object ')) {
-      return jsObject;
-    }
-
-    var extendsClass = objectName.substring(8, objectName.length - 1);
-    var func = getHtmlCreateFunction(extendsClass);
-    if (interop_checks)
-      debug_or_assert("func != null name = ${extendsClass}", func != null);
-    var dartClass_instance = func();
-    dartClass_instance.blink_jsObject = jsObject;
-    return dartClass_instance;
-  } catch(e, stacktrace){
-    if (interop_checks) {
-      if (e is DebugAssertException)
-        window.console.log("${e.message}\n ${stacktrace}");
-      else
-        window.console.log("${stacktrace}");
-    }
-
-    // Problem?
-    return null;
+    if (e is DebugAssertException) print("${e.message}\n ${stacktrace}");
+    else print("${stacktrace}");
   }
+  return js.JSObject.instanceRuntimeType;
 }
 
-getCustomElementEntry(element) {
+CustomElementEntry getCustomElementEntry(element) {
   var hasAttribute = false;
 
   var jsObject;
-  var tag = "";
-  var runtimeType = element.runtimeType;
-  if (runtimeType == HtmlElement) {
-    tag = element.localName;
-  } else if (runtimeType == TemplateElement) {
-    // Data binding with a Dart class.
-    tag = element.attributes['is'];
-  } else if (runtimeType == js.JsObject) {
-    // It's a Polymer core element (written in JS).
-    // Make sure it's an element anything else we can ignore.
-    if (element.hasProperty('nodeType') && element['nodeType'] == 1) {
-      if (js.JsNative.callMethod(element, 'hasAttribute', ['is'])) {
-        hasAttribute = true;
-        // It's data binding use the is attribute.
-        tag = js.JsNative.callMethod(element, 'getAttribute', ['is']);
-      } else {
-        // It's a custom element we want the local name.
-        tag = element['localName'];
-      }
+  var tag;
+
+  // It's a Polymer core element (written in JS).
+  // Make sure it's an element anything else we can ignore.
+  if (element.hasProperty('nodeType') && element['nodeType'] == 1) {
+    if (js.JsNative.callMethod(element, 'hasAttribute', ['is'])) {
+      hasAttribute = true;
+      // It's data binding use the is attribute.
+      tag = js.JsNative.callMethod(element, 'getAttribute', ['is']);
+    } else {
+      // It's a custom element we want the local name.
+      tag = js.JsNative.getProperty(element, 'localName');
     }
-  } else {
-    throw new UnsupportedError(
-        'Element is incorrect type. Got ${runtimeType}, expected HtmlElement/HtmlTemplate/JsObject.');
   }
 
-  var entry = _knownCustomElements[tag];
-  if (entry != null) {
-    // If there's an 'is' attribute then check if the extends tag registered
-    // matches the tag if so then return the entry that's registered for this
-    // extendsTag or if there's no 'is' tag then return the entry found.
-    if ((hasAttribute && entry['extends'] == tag) || !hasAttribute) {
-      return entry;
+  // TODO: handle template element case? I think it is handled by is, etc.
+  if (tag != null) {
+    CustomElementEntry entry = _knownCustomElements[tag];
+    if (entry != null) {
+      // If there's an 'is' attribute then check if the extends tag registered
+      // matches the tag if so then return the entry that's registered for this
+      // extendsTag or if there's no 'is' tag then return the entry found.
+      if ((hasAttribute && entry.extendTag == tag) || !hasAttribute) {
+        return entry;
+      }
     }
   }
 
   return null;
 }
 
+class CustomElementEntry {
+  CustomElementEntry({this.type, this.extendTag});
+
+  final Type type;
+  final String extendTag;
+}
+
 // List of known tagName to DartClass for custom elements, used for upgrade.
-var _knownCustomElements = new Map<String, Map<Type, String>>();
+var _knownCustomElements = new Map<String, CustomElementEntry>();
 
 void addCustomElementType(String tagName, Type dartClass, [String extendTag]) {
   _knownCustomElements[tagName] =
-      {'type': dartClass, 'extends': extendTag != null ? extendTag : "" };
+      new CustomElementEntry(type: dartClass, extendTag: extendTag != null ? extendTag : "");
 }
 
 Type getCustomElementType(object) {
   var entry = getCustomElementEntry(object);
   if (entry != null) {
-    return entry['type'];
+    return entry.type;
   }
   return null;
 }