Add dartExperimentalFixupGetTag for polymer experiments.

sdk/lib/_internal/lib/native_helper.dart

Index: sdk/lib/_internal/lib/native_helper.dart
diff --git a/sdk/lib/_internal/lib/native_helper.dart b/sdk/lib/_internal/lib/native_helper.dart
index 56c098f07f9473e33dc3e5b7b3d15504335ff2d2..4c66f658aaf7208225c04588f23036b0c8746433 100644
--- a/sdk/lib/_internal/lib/native_helper.dart
+++ b/sdk/lib/_internal/lib/native_helper.dart
@@ -140,9 +140,33 @@ newJsObject() {
 }
 
 /**
- * Returns the function to use to get the type name of an object.
+ * Cached value for the function to use to get the type name of an
+ * object.
+ */
+Function _getTypeNameOf;

[ahe, 2013/07/24 08:46:18]

I would prefer this:

final Function getTypeNameOf = getFunctionForTypeNameOf();

+
+/**
+ * Returns the type name of [obj].
+ */
+String getTypeNameOf(var obj) {
+  if (_getTypeNameOf == null) _getTypeNameOf = getFunctionForTypeNameOf();
+  return _getTypeNameOf(obj);
+}
+
+/**
+ * Returns the function to use to get the type name (i.e. dispatch tag) of an
+ * object.
  */
 Function getFunctionForTypeNameOf() {
+  var getTagFunction = _getFunctionForTypeNameOf();
+  if (JS('bool', 'typeof dartExperimentalFixupGetTag == "function"')) {

[ahe, 2013/07/24 08:46:18]

Which is better:

if (JS('bool', 'typeof dartExperimentalFixupGetTag == "function"')) {

or:

if (JS('String', 'typeof dartExperimentalFixupGetTag') == 'function') {

I'm asking because I don't know. If they are roughly the same, I think I would
prefer the latter as it has the least JS code.

[sra1, 2013/07/25 21:47:09]

I thing the first is better since it guarantees the generated code contains 

   typeof dartExperimentalFixupGetTag == "function"

which is compiled by V8 to a special test that is much more efficient than

   typeof dartExperimentalFixupGetTag

There is a small (but probably unrealistic) chance that giving the compiler
flexibility will let it generate code (e.g. via outlining) that is not
recognized  by the V8 as a function check.

+    return _applyExperimentalFixup(
+        JS('', 'dartExperimentalFixupGetTag'), getTagFunction);
+  }
+  return getTagFunction;
+}
+
+Function _getFunctionForTypeNameOf() {

[ahe, 2013/07/24 08:46:18]

Please don't make stuff private.  Library privacy is a broken concept, and it
doesn't convey the intent.

In this case, the method is internal and should only be called by
getFunctionTypeNameOf, but it might still be good to test it.  However, by
making it private you say: "this method is private to this library (so any
method in this library might want to call it) and it should never be tested".

I have a tendency to request unit tests of functions that are made private to
help programmers through the thought process of how library privacy simply
doesn't work.

If you need to hide something, you can create a separate library for it. 
However, this is mostly overkill.

In my opinion, this is a much better solution:

/// Do not call this directly, call [getFunctionForTypeNameOf] instead.
Function getFunctionForTypeNameOfImplementation() {

This:

* Clearly documents the intent.

* Helps your co-workers avoid accidentally calling this method from elsewhere in
this library.

* Allows the method to be tested.

I think it is perfectly fine to use private instance members to hide
implementation details in implementation classes when this protects your users
against accidentally using undocumented implementation details that could lead
to compatibility problems.  However, when your users are your co-workers, who
also work on the same library, you need to be clear about intent, and you need
to trust them to do the right thing.

[sra1, 2013/07/25 21:47:09]

Done.

   // If we're not in the browser, we're almost certainly running on v8.
   if (!identical(JS('String', 'typeof(navigator)'), 'object')) return typeNameInChrome;
 
@@ -167,21 +191,28 @@ Function getFunctionForTypeNameOf() {
   }
 }
 
+Function _applyExperimentalFixup(fixupJSFunction,

[ahe, 2013/07/24 08:46:18]

Please don't make stuff private.

[sra1, 2013/07/25 21:47:09]

Done.

+                                 Function originalGetTagFunction) {
+  // Since DART_CLOSURE_TO_JS works only for top level functions, store the
+  // closed over JavaScript function in a top level variable and use a top level
+  // function.  This is fine since we have only one instance of the 'closure'.
+  _getTagJSFunction = originalGetTagFunction;

[ahe, 2013/07/24 08:46:18]

I would prefer this:

var boundClosure =
    JS('', '#.bind(null, #)', DART_CLOSURE_TO_JS(_callGetTagJSFunction),
       originalGetTagFunction);

Or possibly:

var boundClosure =
    JS('', '(function() { return function(object) { return #(#, object) } })()',
       DART_CLOSURE_TO_JS(callGetTagJSFunction),
       originalGetTagFunction);

...

callGetTagJSFunction(tagFunction, object) => tagFunction(object);

+  var newGetTagJSFunction =
+      JS('', '#(#)',
+         fixupJSFunction, DART_CLOSURE_TO_JS(_callGetTagJSFunction));
 
-/**
- * Cached value for the function to use to get the type name of an
- * object.
- */
-Function _getTypeNameOf;
+  String newGetTagDartFunction(object) =>
+      JS('', '#(#)', newGetTagJSFunction, object);
 
-/**
- * Returns the type name of [obj].
- */
-String getTypeNameOf(var obj) {
-  if (_getTypeNameOf == null) _getTypeNameOf = getFunctionForTypeNameOf();
-  return _getTypeNameOf(obj);
+  return newGetTagDartFunction;
 }
 
+var _getTagJSFunction;

[ahe, 2013/07/24 08:46:18]

Please don't make stuff private.

[sra1, 2013/07/25 21:47:09]

Done.

+_callGetTagJSFunction(object) => _getTagJSFunction(object);

[ahe, 2013/07/24 08:46:18]

Please don't make stuff private.

[sra1, 2013/07/25 21:47:09]

Done.

+
+

[ahe, 2013/07/24 08:46:18]

Extra lines.

[sra1, 2013/07/25 21:47:09]

Done.

+
+
 String toStringForNativeObject(var obj) {
   String name = JS('String', '#', getTypeNameOf(obj));
   return 'Instance of $name';