Fix behavior when typed JS interop getters are called as functions.

BUG=
R=sigmund@google.com, sra@google.com

Review URL: https://codereview.chromium.org/1431523002 .

pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart

@@ -30,7 +30,9 @@ import '../../constants/values.dart' show
 import '../../core_types.dart' show
     CoreClasses;
 import '../../dart_types.dart' show
-    DartType;
+    DartType,
+    FunctionType,
+    TypedefType;
 import '../../elements/elements.dart' show
     ClassElement,
     Element,
@@ -38,6 +40,7 @@ import '../../elements/elements.dart' show
     FieldElement,
     FunctionElement,
     FunctionSignature,
+    GetterElement,
     LibraryElement,
     MethodElement,
     Name,
@@ -407,10 +410,50 @@ class ProgramBuilder {
               }
             }
 
+            // Generating stubs for direct calls and stubs for call-through
+            // of getters that happen to be functions.
+            bool isFunctionLike = false;
+            FunctionType functionType = null;
+
             if (member.isFunction) {
+              FunctionElement fn = member;
+              functionType = fn.type;
+            } else if (member.isGetter) {
+              if (_compiler.trustTypeAnnotations) {
+                GetterElement getter = member;
+                DartType returnType = getter.type.returnType;
+                if (returnType.isFunctionType) {
+                  functionType = returnType;
+                } else if (returnType.treatAsDynamic ||
+                    _compiler.types.isSubtype(returnType,
+                        backend.coreTypes.functionType)) {
+                  if (returnType.isTypedef) {
+                    TypedefType typedef = returnType;
+                    // TODO(jacobr): can we just use typdef.unaliased instead?
+                    functionType = typedef.element.functionSignature.type;
+                  } else {
+                    // Other misc function type such as coreTypes.Function.
+                    // Allow any number of arguments.
+                    isFunctionLike = true;
+                  }
+                }
+              } else {
+                isFunctionLike = true;
+              }
+            } // TODO(jacobr): handle field elements.
+
+            if (isFunctionLike || functionType != null) {
+              int minArgs;
+              int maxArgs;
+              if (functionType != null) {
+                minArgs = functionType.parameterTypes.length;
+                maxArgs = minArgs + functionType.optionalParameterTypes.length;
+              } else {
+                minArgs = 0;
+                maxArgs = 32767;
+              }
               var selectors =
                   _compiler.codegenWorld.invocationsByName(member.name);
-              FunctionElement fn = member;
               // Named arguments are not yet supported. In the future we
               // may want to map named arguments to an object literal containing
               // all named arguments.
@@ -418,16 +461,23 @@ class ProgramBuilder {
                 for (var selector in selectors.keys) {
                   // Check whether the arity matches this member.
                   var argumentCount = selector.argumentCount;
-                  if (argumentCount > fn.parameters.length) break;
-                  if (argumentCount < fn.parameters.length &&
-                      !fn.parameters[argumentCount].isOptional) break;
+                  // JS interop does not support named arguments.
+                  if (selector.namedArgumentCount > 0) break;
+                  if (argumentCount < minArgs) break;
+                  if (argumentCount > maxArgs) break;
                   var stubName = namer.invocationName(selector);
                   if (!stubNames.add(stubName.key)) break;
-                  var candidateParameterNames =
-                      'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLOMOPQRSTUVWXYZ';
                   var parameters = new List<String>.generate(argumentCount,
-                      (i) => candidateParameterNames[i]);
+                      (i) => 'p$i');
 
+                  // We intentionally generate the same stub method for direct
+                  // calls and call-throughs of getters so that calling a
+                  // getter that returns a function behaves the same as calling
+                  // a method. This is helpful as many typed JavaScript APIs
+                  // specify member functions with getters that return
+                  // functions. The behavior of this solution matches JavaScript
+                  // behavior implicitly binding this only when JavaScript
+                  // would.
                   interceptorClass.callStubs.add(_buildStubMethod(
                       stubName,
                       js.js('function(receiver, #) { return receiver.#(#) }',

tests/html/html.status

@@ -19,9 +19,11 @@ js_test/JsArray: RuntimeError # Dartium JSInterop failure
 native_gc_test: Skip # Dartium JSInterop failure
 transferables_test: RuntimeError # Dartium JSInterop failure
 
+
 [ $compiler == none && ($runtime == drt || $runtime == dartium ) ]
 worker_api_test: Fail # Issue 10223
 resource_http_test: Fail # Issue 24203
+js_function_getter_trust_types_test: Skip # dartium doesn't support trust types.
 
 [ $compiler == none && $mode == debug && ($runtime == drt || $runtime == dartium ) ]
 datalistelement_test: Skip # Issue 20540
@@ -380,6 +382,8 @@ js_interop_1_test: Skip             # Test cannot run under CSP restrictions (ti
 js_test: Skip                       # Test cannot run under CSP restrictions (times out).
 js_array_test: Skip                 # Test cannot run under CSP restrictions.
 js_typed_interop_test: Skip         # Test cannot run under CSP restrictions.
+js_function_getter_test: Skip       # Test cannot run under CSP restrictions.
+js_function_getter_trust_types_test: Skip  # Test cannot run under CSP restrictions.
 js_dart_to_string_test: Skip        # Test cannot run under CSP restrictions.
 mirrors_js_typed_interop_test: Skip # Test cannot run under CSP restrictions.
 postmessage_structured_test: Skip   # Test cannot run under CSP restrictions (times out).
@@ -403,6 +407,7 @@ element_add_test: StaticWarning
 element_test: StaticWarning
 events_test: StaticWarning
 htmlelement_test: StaticWarning
+js_function_getter_trust_types_test: skip # dart2js specific flags.
 localstorage_test: StaticWarning
 mutationobserver_test: StaticWarning
 queryall_test: fail

tests/html/js_function_getter_test.dart

@@ -0,0 +1,109 @@
+// Copyright (c) 2015, 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.
+
+@JS()
+library js_function_getter_test;
+
+import 'dart:html';
+
+import 'package:js/js.dart';
+import 'package:unittest/unittest.dart';
+import 'package:unittest/html_config.dart';
+import 'package:unittest/html_individual_config.dart';
+
+_injectJs() {
+  document.body.append(new ScriptElement()
+    ..type = 'text/javascript'
+    ..innerHtml = r"""
+  var bar = { };
+
+  bar.instanceMember = function() {
+    if (this !== bar) {
+      throw 'Unexpected this!';
+    }
+    return arguments.length;
+  };
+
+  bar.staticMember = function() {
+    return arguments.length * 2;
+  };
+
+  bar.dynamicStatic = function() {
+    return arguments.length;
+  };
+
+  bar.add = function(a, b) {
+    return a + b;
+  };
+
+  var foo = { 'bar' : bar };
+""");
+}
+
+typedef int AddFn(int x, int y);
+
+@JS()
+abstract class Bar {
+  external Function get staticMember;
+  external Function get instanceMember;
+  external AddFn get add;
+  external get dynamicStatic;
+  external num get nonFunctionStatic;
+}
+
+@JS()
+abstract class Foo {
+  external Bar get bar;
+}
+
+@JS()
+external Foo get foo;
+
+main() {
+  _injectJs();
+
+  useHtmlIndividualConfiguration();
+
+  group('call getter as function', () {
+    test('member function', () {
+      expect(foo.bar.instanceMember(), equals(0));
+      expect(foo.bar.instanceMember(0), equals(1));
+      expect(foo.bar.instanceMember(0,0), equals(2));
+      expect(foo.bar.instanceMember(0,0,0,0,0,0), equals(6));
+      var instanceMember = foo.bar.instanceMember;
+      expect(() => instanceMember(), throws);
+      expect(() => instanceMember(0), throws);
+      expect(() => instanceMember(0,0), throws);
+      expect(() => instanceMember(0,0,0,0,0,0), throws);
+    });
+
+    test('static function', () {
+      expect(foo.bar.staticMember(), equals(0));
+      expect(foo.bar.staticMember(0), equals(2));
+      expect(foo.bar.staticMember(0,0), equals(4));
+      expect(foo.bar.staticMember(0,0,0,0,0,0), equals(12));
+      var staticMember = foo.bar.staticMember;
+      expect(staticMember(), equals(0));
+      expect(staticMember(0), equals(2));
+      expect(staticMember(0,0), equals(4));
+      expect(staticMember(0,0,0,0,0,0), equals(12));
+    });
+
+    test('static dynamicStatic', () {
+      expect(foo.bar.dynamicStatic(), equals(0));
+      expect(foo.bar.dynamicStatic(0), equals(1));
+      expect(foo.bar.dynamicStatic(0,0), equals(2));
+      expect(foo.bar.dynamicStatic(0,0,0,0,0,0), equals(6));
+      var dynamicStatic = foo.bar.dynamicStatic;
+      expect(dynamicStatic(), equals(0));
+      expect(dynamicStatic(0), equals(1));
+      expect(dynamicStatic(0,0), equals(2));
+      expect(dynamicStatic(0,0,0,0,0,0), equals(6));
+    });
+
+    test('typedef function', () {
+      expect(foo.bar.add(4,5), equals(9));
+    });
+  });
+}

tests/html/js_function_getter_trust_types_test.dart

@@ -0,0 +1,72 @@
+// Copyright (c) 2015, 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.
+
+// SharedOptions=--trust-type-annotations
+@JS()
+library js_function_getter_trust_types_test;
+
+import 'dart:html';
+
+import 'package:js/js.dart';
+import 'package:unittest/unittest.dart';
+import 'package:unittest/html_config.dart';
+import 'package:unittest/html_individual_config.dart';
+
+_injectJs() {
+  document.body.append(new ScriptElement()
+    ..type = 'text/javascript'
+    ..innerHtml = r"""
+  var bar = { };
+
+  bar.nonFunctionStatic = function() {
+    return arguments.length * 2;
+  };
+
+  bar.add = function(a, b) {
+    return a + b;
+  };
+
+  var foo = { 'bar' : bar };
+""");
+}
+
+typedef int AddFn(int x, int y);
+
+@JS()
+class NotAFn { }
+
+@JS()
+abstract class Bar {
+  external AddFn get add;
+  external NotAFn get nonFunctionStatic;
+}
+
+@JS()
+abstract class Foo {
+  external Bar get bar;
+}
+
+@JS()
+external Foo get foo;
+
+main() {
+  _injectJs();
+
+  useHtmlIndividualConfiguration();
+
+  group('trust types', () {
+    test('static nonFunctionStatic', () {
+      expect(() => foo.bar.nonFunctionStatic(), throws);
+      expect(() => foo.bar.nonFunctionStatic(0), throws);
+      expect(() => foo.bar.nonFunctionStatic(0,0), throws);
+      expect(() => foo.bar.nonFunctionStatic(0,0,0,0,0,0), throws);
+    });
+
+    test('typedef function', () {
+      expect(() => foo.bar.add(4), throws);
+      expect(() => foo.bar.add(4,5,10), throws);
+      expect(foo.bar.add(4,5), equals(9));
+    });
+  });
+}