Native interop rewrite#13462
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Removed placeholder for diagram and extra line breaks.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive documentation and guides for direct native interop in Flutter, including new pages on using jnigen for Android, ffigen for iOS, and a comparison of platform integration options. The review feedback identifies several critical issues in the code snippets, such as a guaranteed runtime crash in the iOS FFI example due to incorrect string casting, missing configuration classes in the jnigen setup causing compilation failures, multiple JNI local reference memory leaks in the Android permissions example, duplicated Groovy dependencies, and syntax errors from missing closing braces. Addressing these issues will ensure the code examples are safe, correct, and idiomatic.
| classes: [ | ||
| // provided by Android OS | ||
| 'android.os.Bundle', | ||
| 'android.content.Intent', | ||
| 'android.content.Context' | ||
| ], |
There was a problem hiding this comment.
The custom SecondActivity Kotlin class is missing from the classes list in tool/jnigen.dart. Without adding it here, jnigen will not generate the Dart bindings for SecondActivity, causing the Dart compilation to fail on line 125 when referencing native.SecondActivity.type.jClass.
| classes: [ | |
| // provided by Android OS | |
| 'android.os.Bundle', | |
| 'android.content.Intent', | |
| 'android.content.Context' | |
| ], | |
| classes: [ | |
| // provided by Android OS | |
| 'android.os.Bundle', | |
| 'android.content.Intent', | |
| 'android.content.Context', | |
| 'com.example.android_launch_activity.SecondActivity' | |
| ], |
|
|
||
| // Context.fromReference ensures we get Android Context object | ||
| // rather than the default `JObject` | ||
| var context = native.Context.fromReference(Jni.androidApplicationContext.reference); |
There was a problem hiding this comment.
Instead of manually accessing the low-level .reference property and calling fromReference, you can use the more idiomatic .as() extension method on JObject to cast the context to native.Context. This matches the modern style used in jnigen.md.
| var context = native.Context.fromReference(Jni.androidApplicationContext.reference); | |
| var context = Jni.androidApplicationContext.as(native.Context.type); |
| int checkAndRequestPermission( | ||
| JObject context, | ||
| String permission, | ||
| Function callback, | ||
| ) { | ||
| // Do I have permission? | ||
| if (ContextCompat.checkSelfPermission(context, permission.toJString()) == | ||
| PackageManager.PERMISSION_GRANTED) { | ||
| callback(); | ||
| } else if (ActivityCompat.shouldShowRequestPermissionRationale( | ||
| Jni.androidActivity(PlatformDispatcher.instance.engineId!), | ||
| permission.toJString(), | ||
| ) == | ||
| true) { | ||
| // Has the user denied the permission before? | ||
| // Give a reason why I need the permission | ||
| // and allow a re-request | ||
| print("I should ask for permission"); | ||
| // TODO Flow to show UI to reshow perms dialog | ||
| return -2; | ||
| } else { | ||
| // Ask for permission | ||
| ActivityCompat.requestPermissions( | ||
| Jni.androidActivity(PlatformDispatcher.instance.engineId!), | ||
| JArray.of(JString.type, [permission.toJString()]), | ||
| 0, | ||
| ); | ||
| } | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
In checkAndRequestPermission, permission.toJString() is called up to three times, creating multiple JNI local references that are never released. It is much more efficient and leak-safe to convert the string once, use the variable, and release it in a finally block.
int checkAndRequestPermission(
JObject context,
String permission,
Function callback,
) {
final jPermission = permission.toJString();
try {
// Do I have permission?
if (ContextCompat.checkSelfPermission(context, jPermission) ==
PackageManager.PERMISSION_GRANTED) {
callback();
} else if (ActivityCompat.shouldShowRequestPermissionRationale(
Jni.androidActivity(PlatformDispatcher.instance.engineId!),
jPermission,
) ==
true) {
// Has the user denied the permission before?
// Give a reason why I need the permission
// and allow a re-request
print("I should ask for permission");
// TODO Flow to show UI to reshow perms dialog
return -2;
} else {
// Ask for permission
ActivityCompat.requestPermissions(
Jni.androidActivity(PlatformDispatcher.instance.engineId!),
JArray.of(JString.type, [jPermission]),
0,
);
}
return 0;
} finally {
jPermission.release();
}
}|
@jwill this is draft (v2) of the Interop rewrite. I still need to test the ffigen tutorial for iOS. I could use some extra attention on the table to help devs choose an Interop solution: https://github.com/flutter/website/pull/13462/changes#diff-39b4cf53fbefc7c1e7d95424eaea65acb9a711a8a0473ed816ff4474123220a6R15 |
|
Visit the preview URL for this PR (updated for commit b52b94c): https://flutter-docs-prod--pr13462-native-interop-rewrite-je6br604.web.app |
|
I'm iterating on the ffigen tutorial but I can't get the generated code to work with the example code. I might have to rework this tutorial to use a much simpler example. |
|
Thanks for closing that old PR!!! Once the code changes have been made and approved (by both Gemini and a DRE). I'm good! |
b52b94c to
ecc2329
Compare
|
Staged preview of the updated docs.flutter.dev site (updated for commit 52f63ad): https://flutter-docs-prod--docs-pr13462-native-interop-rewrite-wju7kso6.web.app |
Remove duplicated dependencies block Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Staged preview of the updated flutter.dev site (updated for commit 52f63ad): https://flutter-dev-230821--www-pr13462-native-interop-rewrite-h2p0zkrz.web.app |
Fixed link that was preventing dash_site from checking the links.
Added redirect for the "call-jetpack-apis" page to "jnigen" and verified with check links.
It is based on the ios calendar demo in flutter/demos
sfshaza2
left a comment
There was a problem hiding this comment.
In general, this PR:
- Doesn't use semantic line breaks (many lines break 80-char limit)
- Uses first person (don't use "we" or "our" or "I", use "you" or "your", etc)
- Fails to use sentence casing in several places
- Uses "will" (future tense) unnecessarily
All of these are listed in the Developer Documentation Style Guidelines.
Also, this PR fails to use our URL macros.
| On the Flutter side, you will need to create Dart bindings for various Android APIs | ||
| and call them using Dart code. This will involve setting up `jni`, `jnigen`, | ||
| and a configuration file describing the generated output. |
There was a problem hiding this comment.
| On the Flutter side, you will need to create Dart bindings for various Android APIs | |
| and call them using Dart code. This will involve setting up `jni`, `jnigen`, | |
| and a configuration file describing the generated output. | |
| On the Flutter side, you need to create Dart bindings for various Android APIs | |
| and call them using Dart code. This involves setting up `jni`, `jnigen`, | |
| and a configuration file describing the generated output. |
|
|
||
| The [code][] for this version is available on Github. | ||
|
|
||
| [code]:https://github.com/flutter/demos/tree/main/native_interop_demos/android_launch_activity |
There was a problem hiding this comment.
| [code]:https://github.com/flutter/demos/tree/main/native_interop_demos/android_launch_activity | |
| [code]: {{site.github}}/flutter/demos/tree/main/native_interop_demos/android_launch_activity |
|
|
||
| ## Overview of launching Activities using Method Channels | ||
|
|
||
| On the Flutter side, you will need to create a new |
There was a problem hiding this comment.
| On the Flutter side, you will need to create a new | |
| On the Flutter side, you need to create a new |
| On the Flutter side, you will need to create a new | ||
| platform method channel and call its `invokeMethod` method. | ||
|
|
||
| On the Android side, you will need to register a matching native `MethodChannel` |
There was a problem hiding this comment.
| On the Android side, you will need to register a matching native `MethodChannel` | |
| On the Android side, you need to register a matching native `MethodChannel` |
| ### On the command line | ||
|
|
||
| On the commandline, add `jnigen` as a development dependency | ||
| and `jni` as a runtime dependency. `jnigen` will be used to |
There was a problem hiding this comment.
| and `jni` as a runtime dependency. `jnigen` will be used to | |
| and `jni` as a runtime dependency. `jnigen` is used to |
|
|
||
| ## Conclusion | ||
|
|
||
| You have successfully built a Flutter application that interacts directly with iOS APIs! By using `ffigen` and `objective_c`, you can skip writing manual message channels and focus on building high quality Dart code. |
There was a problem hiding this comment.
| You have successfully built a Flutter application that interacts directly with iOS APIs! By using `ffigen` and `objective_c`, you can skip writing manual message channels and focus on building high quality Dart code. | |
| You have successfully built a Flutter application that interacts directly with iOS APIs! | |
| By using `ffigen` and `objective_c`, you can skip writing manual message channels | |
| and focus on building high quality Dart code. |
|
|
||
| ## Plugins | ||
|
|
||
| We recommend checking [pub.dev](https://pub.dev) first |
There was a problem hiding this comment.
| We recommend checking [pub.dev](https://pub.dev) first | |
| We recommend checking [pub.dev]({{site.pub}}) first |
|
|
||
| ## Custom solutions | ||
|
|
||
| If a package does not support your desired feature on pub.dev, |
There was a problem hiding this comment.
| If a package does not support your desired feature on pub.dev, | |
| If a package does not support your desired feature on `pub.dev`, |
| and executes native code on the UI thread. | ||
|
|
||
| To create compilation bindings automatically, | ||
| use generator tools like [`ffigen`](https://pub.dev/packages/ffigen) |
There was a problem hiding this comment.
| use generator tools like [`ffigen`](https://pub.dev/packages/ffigen) | |
| use generator tools like [`ffigen`]({{site.pub-pkg}}/ffigen) |
| To create compilation bindings automatically, | ||
| use generator tools like [`ffigen`](https://pub.dev/packages/ffigen) | ||
| (for C, Objective-C, and Swift) | ||
| or [`jnigen`](https://pub.dev/packages/jnigen) (for Java and Kotlin on Android). |
There was a problem hiding this comment.
| or [`jnigen`](https://pub.dev/packages/jnigen) (for Java and Kotlin on Android). | |
| or [`jnigen`]({{site.pub-pkg}}/jnigen) (for Java and Kotlin on Android). |
Updates #13115