Fix #835 (#1203)

* Fix some block ref counting tests

* Add a test that reproduces the bug.

* Generate the trampoline

* fixes

* Update json schema

* Fix NPE

* Fix swift test setup

* Make varName optional

pkgs/ffigen/CHANGELOG.md

@@ -3,17 +3,26 @@
 - __Breaking change__: Code-gen the ObjC `id` type to `ObjCObjectBase` rather
   than `NSObject`, since not all ObjC classes inherit from `NSObject`. Eg
   `NSProxy`.
+- __Breaking change__: Generate a native trampoline for each listener block, to
+  fix a ref counting bug: https://github.com/dart-lang/native/issues/835.
+    - If you have listener blocks affected by this ref count bug, a .m file will
+      be generated containing the trampoline. You must compile this .m file into
+      your package. If you already have a flutter plugin or build.dart, you can
+      simply add this generated file to that build.
+    - If you don't use listener blocks, you can ignore the .m file.
+    - You can choose where the generated .m file is placed with the
+      `output.objc-bindings` config option.
 - __Breaking change__: Native enums are now generated as real Dart enums, instead
   of abstract classes with integer constants. Native enum members with the same
   integer values are handled properly on the Dart side, and native functions
   that use enums in their signatures now accept the generated enums on the Dart
-  side, instead of integer values. 
-- Rename ObjC interface methods that clash with type names. Fixes
-  https://github.com/dart-lang/native/issues/1007.
+  side, instead of integer values.
 - __Breaking change__: Enum integer types are implementation-defined and not
   part of the ABI. Therefore FFIgen does a best-effort approach trying to mimic
   the most common compilers for the various OS and architecture combinations.
   To silence the warning set config `silence-enum-warning` to `true`.
+- Rename ObjC interface methods that clash with type names. Fixes
+  https://github.com/dart-lang/native/issues/1007.
 
 ## 12.0.0
 
@@ -26,7 +35,7 @@
 - __Breaking change__: Use `package:objective_c` in ObjC bindings.
   - ObjC packages will have a flutter dependency (until
     https://github.com/dart-lang/native/issues/1068 is fixed).
-  - Core classes such as `NSString` have been moved intpu `package:objective_c`.
+  - Core classes such as `NSString` have been moved into `package:objective_c`.
   - ObjC class methods don't need the ubiquitous `lib` argument anymore. In
     fact, ffigen won't even generate the native library class (unless it needs
     to bind top level functions without using `@Native`). It is still necessary

pkgs/ffigen/README.md

@@ -593,6 +593,28 @@ ffi-native:
 language: 'objc'
 ```
   </td>
+  </tr>
+  <tr>
+    <td>output -> objc-bindings</td>
+    <td>
+      Choose where the generated ObjC code (if any) is placed. The default path
+      is `'${output.bindings}.m'`, so if your Dart bindings are in
+      `generated_bindings.dart`, your ObjC code will be in
+      `generated_bindings.dart.m`.
+      <br><br>
+      This ObjC file will only be generated if it's needed. If it is generated,
+      it must be compiled into your package, as part of a flutter plugin or
+      build.dart script. If your package already has some sort of native build,
+      you can simply add this generated ObjC file to that build.
+    </td>
+    <td>
+
+```yaml
+output:
+  ...
+  objc-bindings: 'generated_bindings.m'
+```
+</td>
   </tr>
   <tr>
     <td>output -> symbol-file</td>

pkgs/ffigen/ffigen.schema.json

@@ -23,6 +23,9 @@
             "bindings": {
               "$ref": "#/$defs/filePath"
             },
+            "objc-bindings": {
+              "$ref": "#/$defs/filePath"
+            },
             "symbol-file": {
               "type": "object",
               "additionalProperties": false,

pkgs/ffigen/lib/src/code_generator/binding.dart

@@ -37,6 +37,9 @@ abstract class Binding {
   /// Note: This does not print the typedef dependencies.
   /// Must call [getTypedefDependencies] first.
   BindingString toBindingString(Writer w);
+
+  /// Returns the Objective C bindings, if any.
+  BindingString? toObjCBindingString(Writer w) => null;
 }
 
 /// Base class for bindings which look up symbols in dynamic library.

pkgs/ffigen/lib/src/code_generator/compound.dart

@@ -156,6 +156,9 @@ abstract class Compound extends BindingType {
   @override
   String getCType(Writer w) => name;
 
+  @override
+  String getNativeType({String varName = ''}) => '$originalName $varName';
+
   @override
   bool get sameFfiDartAndCType => true;
 }

pkgs/ffigen/lib/src/code_generator/enum_class.dart

@@ -255,6 +255,9 @@ class EnumClass extends BindingType {
   @override
   String getDartType(Writer w) => name;
 
+  @override
+  String getNativeType({String varName = ''}) => '$originalName $varName';
+
   @override
   bool get sameFfiDartAndCType => nativeType.sameFfiDartAndCType;
 

pkgs/ffigen/lib/src/code_generator/func_type.dart

@@ -68,6 +68,12 @@ class FunctionType extends Type {
   String getDartType(Writer w, {bool writeArgumentNames = true}) =>
       _getTypeImpl(writeArgumentNames, (Type t) => t.getDartType(w));
 
+  @override
+  String getNativeType({String varName = ''}) {
+    final arg = dartTypeParameters.map<String>((p) => p.type.getNativeType());
+    return '${returnType.getNativeType()} (*$varName)(${arg.join(', ')})';
+  }
+
   @override
   bool get sameFfiDartAndCType =>
       returnType.sameFfiDartAndCType &&
@@ -141,6 +147,10 @@ class NativeFunc extends Type {
   @override
   String getFfiDartType(Writer w) => getCType(w);
 
+  @override
+  String getNativeType({String varName = ''}) =>
+      _type.getNativeType(varName: varName);
+
   @override
   bool get sameFfiDartAndCType => true;
 

pkgs/ffigen/lib/src/code_generator/handle.dart

@@ -18,6 +18,11 @@ class HandleType extends Type {
   @override
   String getFfiDartType(Writer w) => 'Object';
 
+  // The real native type is Dart_Handle, but that would mean importing
+  // dart_api.h into the generated native code.
+  @override
+  String getNativeType({String varName = ''}) => 'void* $varName';
+
   @override
   bool get sameFfiDartAndCType => false;
 

pkgs/ffigen/lib/src/code_generator/imports.dart

@@ -40,9 +40,10 @@ class ImportedType extends Type {
   final LibraryImport libraryImport;
   final String cType;
   final String dartType;
+  final String nativeType;
   final String? defaultValue;
 
-  ImportedType(this.libraryImport, this.cType, this.dartType,
+  ImportedType(this.libraryImport, this.cType, this.dartType, this.nativeType,
       [this.defaultValue]);
 
   @override
@@ -54,6 +55,9 @@ class ImportedType extends Type {
   @override
   String getFfiDartType(Writer w) => cType == dartType ? getCType(w) : dartType;
 
+  @override
+  String getNativeType({String varName = ''}) => '$nativeType $varName';
+
   @override
   bool get sameFfiDartAndCType => cType == dartType;
 
@@ -93,27 +97,36 @@ final objcPkgImport = LibraryImport(
     importPathWhenImportedByPackageObjC: '../objective_c.dart');
 final self = LibraryImport('self', '');
 
-final voidType = ImportedType(ffiImport, 'Void', 'void');
-
-final unsignedCharType = ImportedType(ffiImport, 'UnsignedChar', 'int', '0');
-final signedCharType = ImportedType(ffiImport, 'SignedChar', 'int', '0');
-final charType = ImportedType(ffiImport, 'Char', 'int', '0');
-final unsignedShortType = ImportedType(ffiImport, 'UnsignedShort', 'int', '0');
-final shortType = ImportedType(ffiImport, 'Short', 'int', '0');
-final unsignedIntType = ImportedType(ffiImport, 'UnsignedInt', 'int', '0');
-final intType = ImportedType(ffiImport, 'Int', 'int', '0');
-final unsignedLongType = ImportedType(ffiImport, 'UnsignedLong', 'int', '0');
-final longType = ImportedType(ffiImport, 'Long', 'int', '0');
-final unsignedLongLongType =
-    ImportedType(ffiImport, 'UnsignedLongLong', 'int', '0');
-final longLongType = ImportedType(ffiImport, 'LongLong', 'int', '0');
-
-final floatType = ImportedType(ffiImport, 'Float', 'double', '0.0');
-final doubleType = ImportedType(ffiImport, 'Double', 'double', '0.0');
-
-final sizeType = ImportedType(ffiImport, 'Size', 'int', '0');
-final wCharType = ImportedType(ffiImport, 'WChar', 'int', '0');
-
-final objCObjectType = ImportedType(objcPkgImport, 'ObjCObject', 'ObjCObject');
-final objCSelType = ImportedType(objcPkgImport, 'ObjCSelector', 'ObjCSelector');
-final objCBlockType = ImportedType(objcPkgImport, 'ObjCBlock', 'ObjCBlock');
+final voidType = ImportedType(ffiImport, 'Void', 'void', 'void');
+
+final unsignedCharType =
+    ImportedType(ffiImport, 'UnsignedChar', 'int', 'unsigned char', '0');
+final signedCharType =
+    ImportedType(ffiImport, 'SignedChar', 'int', 'char', '0');
+final charType = ImportedType(ffiImport, 'Char', 'int', 'char', '0');
+final unsignedShortType =
+    ImportedType(ffiImport, 'UnsignedShort', 'int', 'unsigned short', '0');
+final shortType = ImportedType(ffiImport, 'Short', 'int', 'short', '0');
+final unsignedIntType =
+    ImportedType(ffiImport, 'UnsignedInt', 'int', 'unsigned', '0');
+final intType = ImportedType(ffiImport, 'Int', 'int', 'int', '0');
+final unsignedLongType =
+    ImportedType(ffiImport, 'UnsignedLong', 'int', 'unsigned long', '0');
+final longType = ImportedType(ffiImport, 'Long', 'int', 'long', '0');
+final unsignedLongLongType = ImportedType(
+    ffiImport, 'UnsignedLongLong', 'int', 'unsigned long long', '0');
+final longLongType =
+    ImportedType(ffiImport, 'LongLong', 'int', 'long long', '0');
+
+final floatType = ImportedType(ffiImport, 'Float', 'double', 'float', '0.0');
+final doubleType = ImportedType(ffiImport, 'Double', 'double', 'double', '0.0');
+
+final sizeType = ImportedType(ffiImport, 'Size', 'int', 'size_t', '0');
+final wCharType = ImportedType(ffiImport, 'WChar', 'int', 'wchar_t', '0');
+
+final objCObjectType =
+    ImportedType(objcPkgImport, 'ObjCObject', 'ObjCObject', 'void');
+final objCSelType =
+    ImportedType(objcPkgImport, 'ObjCSelector', 'ObjCSelector', 'void');
+final objCBlockType =
+    ImportedType(objcPkgImport, 'ObjCBlock', 'ObjCBlock', 'id');

pkgs/ffigen/lib/src/code_generator/library.dart

@@ -136,6 +136,25 @@ class Library {
     }
   }
 
+  /// Generates [file] with the Objective C code needed for the bindings, if
+  /// any.
+  ///
+  /// Returns whether bindings were generated.
+  bool generateObjCFile(File file) {
+    final bindings = writer.generateObjC();
+
+    if (bindings == null) {
+      // No ObjC code needed. If there's already a file (eg from an earlier
+      // run), delete it so it's not accidentally included in the build.
+      if (file.existsSync()) file.deleteSync();
+      return false;
+    }
+
+    if (!file.existsSync()) file.createSync(recursive: true);
+    file.writeAsStringSync(bindings);
+    return true;
+  }
+
   /// Generates [file] with symbol output yaml.
   void generateSymbolOutputFile(File file, String importPath) {
     if (!file.existsSync()) file.createSync(recursive: true);

pkgs/ffigen/lib/src/code_generator/native_type.dart

@@ -26,27 +26,31 @@ enum SupportedNativeType {
 /// Represents a primitive native type, such as float.
 class NativeType extends Type {
   static const _primitives = <SupportedNativeType, NativeType>{
-    SupportedNativeType.Void: NativeType._('Void', 'void', null),
-    SupportedNativeType.Char: NativeType._('Uint8', 'int', '0'),
-    SupportedNativeType.Int8: NativeType._('Int8', 'int', '0'),
-    SupportedNativeType.Int16: NativeType._('Int16', 'int', '0'),
-    SupportedNativeType.Int32: NativeType._('Int32', 'int', '0'),
-    SupportedNativeType.Int64: NativeType._('Int64', 'int', '0'),
-    SupportedNativeType.Uint8: NativeType._('Uint8', 'int', '0'),
-    SupportedNativeType.Uint16: NativeType._('Uint16', 'int', '0'),
-    SupportedNativeType.Uint32: NativeType._('Uint32', 'int', '0'),
-    SupportedNativeType.Uint64: NativeType._('Uint64', 'int', '0'),
-    SupportedNativeType.Float: NativeType._('Float', 'double', '0.0'),
-    SupportedNativeType.Double: NativeType._('Double', 'double', '0.0'),
-    SupportedNativeType.IntPtr: NativeType._('IntPtr', 'int', '0'),
-    SupportedNativeType.UintPtr: NativeType._('UintPtr', 'int', '0'),
+    SupportedNativeType.Void: NativeType._('Void', 'void', 'void', null),
+    SupportedNativeType.Char: NativeType._('Uint8', 'int', 'char', '0'),
+    SupportedNativeType.Int8: NativeType._('Int8', 'int', 'int8_t', '0'),
+    SupportedNativeType.Int16: NativeType._('Int16', 'int', 'int16_t', '0'),
+    SupportedNativeType.Int32: NativeType._('Int32', 'int', 'int32_t', '0'),
+    SupportedNativeType.Int64: NativeType._('Int64', 'int', 'int64_t', '0'),
+    SupportedNativeType.Uint8: NativeType._('Uint8', 'int', 'uint8_t', '0'),
+    SupportedNativeType.Uint16: NativeType._('Uint16', 'int', 'uint16_t', '0'),
+    SupportedNativeType.Uint32: NativeType._('Uint32', 'int', 'uint32_t', '0'),
+    SupportedNativeType.Uint64: NativeType._('Uint64', 'int', 'uint64_t', '0'),
+    SupportedNativeType.Float: NativeType._('Float', 'double', 'float', '0.0'),
+    SupportedNativeType.Double:
+        NativeType._('Double', 'double', 'double', '0.0'),
+    SupportedNativeType.IntPtr: NativeType._('IntPtr', 'int', 'intptr_t', '0'),
+    SupportedNativeType.UintPtr:
+        NativeType._('UintPtr', 'int', 'uintptr_t', '0'),
   };
 
   final String _cType;
   final String _dartType;
+  final String _nativeType;
   final String? _defaultValue;
 
-  const NativeType._(this._cType, this._dartType, this._defaultValue);
+  const NativeType._(
+      this._cType, this._dartType, this._nativeType, this._defaultValue);
 
   factory NativeType(SupportedNativeType type) => _primitives[type]!;
 
@@ -56,6 +60,9 @@ class NativeType extends Type {
   @override
   String getFfiDartType(Writer w) => _dartType;
 
+  @override
+  String getNativeType({String varName = ''}) => '$_nativeType $varName';
+
   @override
   bool get sameFfiDartAndCType => _cType == _dartType;
 
@@ -70,7 +77,7 @@ class NativeType extends Type {
 }
 
 class BooleanType extends NativeType {
-  const BooleanType._() : super._('Bool', 'bool', 'false');
+  const BooleanType._() : super._('Bool', 'bool', 'BOOL', 'false');
   static const _boolean = BooleanType._();
   factory BooleanType() => _boolean;