[jnigen] Few improvements in generated C code including exception support. (#87)

* C bindings no longer use fully qualified names. Instead, it uses number-renamed simple names. The classes are sorted in the summarizer to ensure the order is deterministic. This enables shorter symbol names and less verbose bindings.

* C bindings are formatted using same style as Dart SDK, using clang-format. If clang-format is not available, jnigen will issue a warning.
Closes: #84

* C bindings return `JniResult`. The older `Jni.checkException` stopgap is removed. It reduces one line per function binding in dart. Lastly exceptions will work properly on android
Closes: #56

Author: mahesh-hegde

.github/workflows/test-package.yml

@@ -79,6 +79,12 @@ jobs:
           distribution: 'zulu'
           java-version: '11'
           cache: maven
+      ## Committed bindings are formatted with clang-format.
+      ## So this is required to format generated bindings identically
+      - name: install clang tools
+        run: |
+          sudo apt-get update -y
+          sudo apt-get install -y clang-format
       - name: Install dependencies
         run: dart pub get
       - name: Run VM tests
@@ -126,7 +132,7 @@ jobs:
         with:
           distribution: 'zulu'
           java-version: '11'
-      - name: install clang tools
+      - name: install clang tools & CMake
         run: |
           sudo apt-get update -y
           sudo apt-get install -y clang-format build-essential cmake
@@ -143,6 +149,7 @@ jobs:
           dart run tool/generate_ide_files.dart
           ls src/compile_commands.json
 
+
   test_jni:
     runs-on: ubuntu-latest
     needs: [analyze_jni]
@@ -220,6 +227,11 @@ jobs:
         working-directory: pkg/jnigen
     steps:
       - uses: actions/checkout@v3
+      - name: Setup clang
+        uses: egor-tensin/setup-clang@v1
+        with:
+          version: latest
+          platform: x64
       - uses: subosito/flutter-action@v2
         with:
           channel: 'stable'
@@ -341,6 +353,10 @@ jobs:
           channel: 'stable'
           cache: true
           cache-key: 'flutter-:os:-:channel:-:version:-:arch:-:hash:'
+      - name: install clang tools
+        run: |
+          sudo apt-get update -y
+          sudo apt-get install -y clang-format
       - run: flutter pub get
       - run: flutter analyze
       - run: flutter build apk
@@ -369,7 +385,7 @@ jobs:
           java-version: '11'
       - run: |
           sudo apt-get update -y
-          sudo apt-get install -y ninja-build libgtk-3-dev
+          sudo apt-get install -y ninja-build libgtk-3-dev clang-format
       - run: flutter config --enable-linux-desktop
       - run: dart pub get
       - name: Generate bindings
@@ -378,7 +394,7 @@ jobs:
       - name: Compare generated bindings
         run: |
           diff -qr _c src/
-          diff -qr _dart lib/third_party
+          diff -qr _dart lib/src/third_party
       - name: Generate full bindings
         run: dart run jnigen --config jnigen.yaml --override classes="org.apache.pdfbox.pdmodel;org.apache.pdfbox.text"
       - name: Analyze generated bindings

pkgs/jni/lib/jni.dart

@@ -60,11 +60,9 @@
 library jni;
 
 export 'src/third_party/jni_bindings_generated.dart'
-    hide JniBindings, JniEnv, JniEnv1;
+    hide JniBindings, JniEnv, JniEnv1, JniExceptionDetails;
 export 'src/jni.dart' hide ProtectedJniExtensions;
 export 'src/jvalues.dart' hide JValueArgs, toJValues;
-export 'src/env_extensions.dart'
-    show StringMethodsForJni, CharPtrMethodsForJni, AdditionalEnvMethods;
 export 'src/jni_exceptions.dart';
 export 'src/jni_object.dart' hide JniReference;
 

pkgs/jni/lib/src/accessors.dart

@@ -9,13 +9,11 @@ import 'package:jni/src/jvalues.dart';
 
 import 'third_party/jni_bindings_generated.dart';
 import 'jni.dart';
-import 'env_extensions.dart';
-
-Pointer<GlobalJniEnv> env = Jni.env;
+import 'jni_exceptions.dart';
 
 void _check(JThrowable exception) {
   if (exception != nullptr) {
-    env.throwException(exception);
+    Jni.accessors.throwException(exception);
   }
 }
 
@@ -88,6 +86,21 @@ extension JniClassLookupResultMethods on JniClassLookupResult {
 }
 
 extension JniAccessorWrappers on Pointer<JniAccessors> {
+  /// Rethrows Java exception in Dart as [JniException].
+  ///
+  /// The original exception object is deleted by this method. The message
+  /// and Java stack trace are included in the exception.
+  void throwException(JThrowable exception) {
+    final details = getExceptionDetails(exception);
+    final env = Jni.env;
+    final message = env.asDartString(details.message);
+    final stacktrace = env.asDartString(details.stacktrace);
+    env.DeleteGlobalRef(exception);
+    env.DeleteGlobalRef(details.message);
+    env.DeleteGlobalRef(details.stacktrace);
+    throw JniException(message, stacktrace);
+  }
+
   // TODO(PR): How to name these methods? These only wrap toNativeChars()
   // so that generated bindings are less verbose.
   JClass getClassOf(String internalName) =>

pkgs/jni/lib/src/env_extensions.dart

@@ -9,7 +9,6 @@ import 'package:ffi/ffi.dart';
 import 'package:path/path.dart';
 
 import 'third_party/jni_bindings_generated.dart';
-import 'env_extensions.dart';
 import 'jvalues.dart';
 import 'jni_exceptions.dart';
 import 'jni_object.dart';
@@ -256,3 +255,54 @@ extension ProtectedJniExtensions on Jni {
     return lookup;
   }
 }
+
+extension AdditionalEnvMethods on Pointer<GlobalJniEnv> {
+  /// Convenience method for converting a [JString]
+  /// to dart string.
+  /// if [deleteOriginal] is specified, jstring passed will be deleted using
+  /// DeleteLocalRef.
+  String asDartString(JString jstring, {bool deleteOriginal = false}) {
+    if (jstring == nullptr) {
+      throw NullJniStringException();
+    }
+    final chars = GetStringUTFChars(jstring, nullptr);
+    if (chars == nullptr) {
+      throw InvalidJniStringException(jstring);
+    }
+    final result = chars.cast<Utf8>().toDartString();
+    ReleaseStringUTFChars(jstring, chars);
+    if (deleteOriginal) {
+      DeleteGlobalRef(jstring);
+    }
+    return result;
+  }
+
+  /// Return a new [JString] from contents of [s].
+  JString asJString(String s) => using((arena) {
+        final utf = s.toNativeUtf8().cast<Char>();
+        final result = NewStringUTF(utf);
+        malloc.free(utf);
+        return result;
+      });
+
+  /// Deletes all references in [refs].
+  void deleteAllRefs(List<JObject> refs) {
+    for (final ref in refs) {
+      DeleteGlobalRef(ref);
+    }
+  }
+}
+
+extension StringMethodsForJni on String {
+  /// Returns a Utf-8 encoded Pointer<Char> with contents same as this string.
+  Pointer<Char> toNativeChars([Allocator allocator = malloc]) {
+    return toNativeUtf8(allocator: allocator).cast<Char>();
+  }
+}
+
+extension CharPtrMethodsForJni on Pointer<Char> {
+  /// Same as calling `cast<Utf8>` followed by `toDartString`.
+  String toDartString() {
+    return cast<Utf8>().toDartString();
+  }
+}

pkgs/jni/lib/src/jni.dart

@@ -22,6 +22,14 @@ class NullJniStringException implements Exception {
   String toString() => 'toDartString called on null JniString reference';
 }
 
+class InvalidJniStringException implements Exception {
+  Pointer<Void> reference;
+  InvalidJniStringException(this.reference);
+  @override
+  String toString() => 'Not a valid Java String: '
+      '0x${reference.address.toRadixString(16)}';
+}
+
 class DoubleFreeException implements Exception {
   dynamic object;
   Pointer<Void> ptr;
@@ -69,15 +77,16 @@ class InvalidCallTypeException implements Exception {
 }
 
 class JniException implements Exception {
-  /// Exception object pointer from JNI.
-  final JObject err;
+  /// Error message from Java exception.
+  final String message;
 
-  /// brief description, usually initialized with error message from Java.
-  final String msg;
-  JniException(this.err, this.msg);
+  /// Stack trace from Java.
+  final String stackTrace;
+  JniException(this.message, this.stackTrace);
 
   @override
-  String toString() => msg;
+  String toString() => 'Exception in Java code called through JNI: '
+      '$message\n\n$stackTrace\n';
 }
 
 class HelperNotFoundException implements Exception {

pkgs/jni/lib/src/jni_exceptions.dart

@@ -9,7 +9,6 @@ import 'package:ffi/ffi.dart';
 import 'third_party/jni_bindings_generated.dart';
 import 'jni_exceptions.dart';
 import 'jni.dart';
-import 'env_extensions.dart';
 import 'accessors.dart';
 import 'jvalues.dart';
 
@@ -86,7 +85,7 @@ Pointer<T> _getID<T extends NativeType>(
   final result = using(
       (arena) => f(ptr, name.toNativeChars(arena), sig.toNativeChars(arena)));
   if (result.exception != nullptr) {
-    env.throwException(result.exception);
+    _accessors.throwException(result.exception);
   }
   return result.id.cast<T>();
 }
@@ -213,7 +212,9 @@ class JniObject extends JniReference {
   JniClass getClass() {
     _ensureNotDeleted();
     final classRef = _env.GetObjectClass(reference);
-    if (classRef == nullptr) _env.checkException();
+    if (classRef == nullptr) {
+      _accessors.throwException(_env.ExceptionOccurred());
+    }
     return JniClass.fromRef(classRef);
   }
 
@@ -425,7 +426,7 @@ class JniString extends JniObject {
         final chars = s.toNativeUtf8(allocator: arena).cast<Char>();
         final jstr = _env.NewStringUTF(chars);
         if (jstr == nullptr) {
-          _env.checkException();
+          throw 'Fatal: cannot convert string to Java string: $s';
         }
         return jstr;
       });

pkgs/jni/lib/src/jni_object.dart

@@ -8,7 +8,6 @@ import 'package:ffi/ffi.dart';
 import 'third_party/jni_bindings_generated.dart';
 import 'jni.dart';
 import 'jni_object.dart';
-import 'env_extensions.dart';
 
 void _fillJValue(Pointer<JValue> pos, dynamic arg) {
   if (arg is JniObject) {