[dart2wasm] Fix handling of generic callbacks in trampoline

Closes #54192

Callbacks were being casted to their static type before being
called. With generic callbacks, we do not know their exact type
at compile time, therefore cannot make that cast. Similarly, a
null check was being done if the static type of the parameter
was potentially non-nullable, whereas it should be done if it's
*not* potentially nullable.

Change-Id: I3d30e901f1ff2ae4c17887564f15368244baeb24
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/340390
Reviewed-by: Sigmund Cherem <[email protected]>

Author: srujzs

pkg/_js_interop_checks/lib/src/transformations/js_util_optimizer.dart

@@ -957,7 +957,7 @@ class ExtensionIndex {
     } else if (type is ExtensionType) {
       return isInteropExtensionType(type.extensionTypeDeclaration);
     } else if (type is TypeParameterType) {
-      return isStaticInteropType(type.bound);
+      return isStaticInteropType(type.nonTypeVariableBound);
     }
     return false;
   }

pkg/dart2wasm/lib/js/callback_specializer.dart

@@ -35,7 +35,7 @@ class CallbackSpecializer {
       if (_extensionIndex.isStaticInteropType(callbackParameterType) &&
           boxExternRef) {
         expression = _createJSValue(v);
-        if (callbackParameterType.isPotentiallyNonNullable) {
+        if (!callbackParameterType.isPotentiallyNullable) {
           expression = NullCheck(expression);
         }
       } else {
@@ -47,23 +47,24 @@ class CallbackSpecializer {
     return ReturnStatement(StaticInvocation(
         _util.jsifyTarget(function.returnType),
         Arguments([
-          FunctionInvocation(
-              FunctionAccessKind.FunctionType,
-              AsExpression(VariableGet(callbackVariable), function),
-              Arguments(callbackArguments),
-              functionType: function),
+          FunctionInvocation(FunctionAccessKind.FunctionType,
+              VariableGet(callbackVariable), Arguments(callbackArguments),
+              functionType: null),
         ])));
   }
 
-  /// Creates a callback trampoline for the given [function]. This callback
-  /// trampoline expects a Dart callback as its first argument, then an integer
-  /// value(double type) indicating the position of the last defined
-  /// argument(only for callbacks that take optional parameters), followed by
-  /// all of the arguments to the Dart callback as JS objects.  The trampoline
-  /// will `dartifyRaw` all incoming JS objects and then cast them to their
-  /// appropriate types, dispatch, and then `jsifyRaw` any returned value.
-  /// [_createFunctionTrampoline] Returns a [String] function name representing
-  /// the name of the wrapping function.
+  /// Creates a callback trampoline for the given [function].
+  ///
+  /// This callback trampoline expects a Dart callback as its first argument,
+  /// then an integer value(double type) indicating the position of the last
+  /// defined argument(only for callbacks that take optional parameters),
+  /// followed by all of the arguments to the Dart callback as JS objects. The
+  /// trampoline will `dartifyRaw` or box all incoming JS objects and then cast
+  /// them to their appropriate types, dispatch, and then `jsifyRaw` or box any
+  /// returned value.
+  ///
+  /// Returns a [String] function name representing the name of the wrapping
+  /// function.
   String _createFunctionTrampoline(Procedure node, FunctionType function,
       {required bool boxExternRef}) {
     // Create arguments for each positional parameter in the function. These

tests/lib/js/static_interop_test/js_function_conversions_test.dart

@@ -30,7 +30,7 @@ external JSBoolean boolF(JSBoolean b);
 external void voidF(JSString s);
 
 @JS('jsFunction')
-external JSAny? anyF(JSAny? n);
+external JSAny? anyF([JSAny? n]);
 
 @JS()
 external JSAny? callFunctionWithUndefined();
@@ -39,6 +39,7 @@ external JSAny? callFunctionWithUndefined();
 external JSAny? callFunctionWithJSNull();
 
 void main() {
+  // Test primitive conversions.
   jsFunction = ((String arg) => arg).toJS;
   expect(stringF('stringF'.toJS).toDart, 'stringF');
   Expect.throws(() => anyF(0.toJS));
@@ -70,6 +71,7 @@ void main() {
   jsFunction = ((String arg) => arg).toJS;
   voidF('voidF'.toJS);
 
+  // Test nullability with JS null and JS undefined.
   eval('''
     globalThis.callFunctionWithUndefined = function() {
       return globalThis.jsFunction(undefined);
@@ -108,4 +110,71 @@ void main() {
   expectNullFail(((String arg) => arg).toJS);
 
   expectNullFail(((JSString arg) => arg).toJS);
+
+  // Test conversions with allowed type parameters.
+  void setBoundAnyFunction<T extends JSAny?>() {
+    jsFunction = ((T t) => t).toJS;
+  }
+
+  final zero = 0.toJS;
+  final empty = ''.toJS;
+
+  setBoundAnyFunction();
+  expect(anyF(null), null);
+  expect(anyF(zero), zero);
+  setBoundAnyFunction<JSAny>();
+  // TODO(srujzs): The commented out null checks do not throw. There should be a
+  // check within the body of the callback that the parameter is the right
+  // generic type, but there isn't.
+  // Expect.throws(() => anyF());
+  // Expect.throws(() => anyF(null));
+  expect(anyF(zero), zero);
+  setBoundAnyFunction<JSNumber>();
+  // Expect.throws(() => anyF(null));
+  Expect.throws(() {
+    final any = anyF(empty);
+    // TODO(54179): Better way of writing this is to cast to JSNumber and
+    // convert, but currently that does not throw on dart2wasm.
+    if (!any.typeofEquals('number')) {
+      throw TypeError();
+    }
+  });
+  expect(anyF(zero), zero);
+
+  void setBoundNonNullAnyFunction<T extends JSAny>() {
+    jsFunction = ((T t) => t).toJS;
+  }
+
+  setBoundNonNullAnyFunction();
+  Expect.throws(() => anyF());
+  if (hasSoundNullSafety) Expect.throws(() => anyF(null));
+  expect(anyF(zero), zero);
+  setBoundNonNullAnyFunction<JSNumber>();
+  if (hasSoundNullSafety) Expect.throws(() => anyF(null));
+  Expect.throws(() {
+    final any = anyF(empty);
+    // TODO(54179): Better way of writing this is to cast to JSNumber and
+    // convert, but currently that does not throw on dart2wasm.
+    if (!any.typeofEquals('number')) {
+      throw TypeError();
+    }
+  });
+  expect(anyF(zero), zero);
+
+  void setBoundJSNumberFunction<T extends JSNumber>() {
+    jsFunction = ((T t) => t).toJS;
+  }
+
+  setBoundJSNumberFunction();
+  Expect.throws(() => anyF());
+  if (hasSoundNullSafety) Expect.throws(() => anyF(null));
+  Expect.throws(() {
+    final any = anyF(empty);
+    // TODO(54179): Better way of writing this is to cast to JSNumber and
+    // convert, but currently that does not throw on dart2wasm.
+    if (!any.typeofEquals('number')) {
+      throw TypeError();
+    }
+  });
+  expect(anyF(zero), zero);
 }