[camera_android] Convert Dart to native calls to use Pigeon

[yaakovschectman]

Add structured types and convert implementations to use typesafe method calls for platform methods from Dart to native side.

Android analog of #6601

Part of flutter/flutter#117905

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[stuartmorgan-g]

I haven't had time for a full review yet, but posting initial comments on the Pigeon API definition now since I had time to start looking this morning. Feel free to wait until I've had time to do a full review if you'd rather, but I thought I'd post this now in case you wanted to adjust the use of async in the meantime.

[stuartmorgan-g]

Sorry for the delay in getting to the full review. Thanks for all the cleanup where async results were being passed down into Camera.java but didn't actually need to be to make even more methods sync!

[stuartmorgan-g]

This seems really bad; it looks like all the code using this has handleException as the only thing in catch, which means that if we get here we never call result and instead just have an unhandled native exception and no response.

Could you file an issue about this, and leave a TODO here referencing it (something like "Re-evaluate the way exceptions are handled here; see issue-link-here")? We shouldn't make that behavioral change in this PR in case there are issues that would require undoing it, but I really don't understand why this is throwing instead of calling result.error.

[stuartmorgan-g]

Sorry, I realized looking at this again today that I was wrong about this; this is—non-obviously—a regression in this PR. With method channels, on Android, the engine call to onMethodCall is wrapped in an exception handler (specifically a RuntimeException handler, thus the current structure of this code). So the old code would correctly report these exceptions back to Dart, but this version won't because Pigeon doesn't try/catch async methods.

So we can close flutter/flutter#157021, and this code needs to be updated here. To be fully behavior-preserving, handleException should look like this:

// The code below exactly preserves the format of the native exceptions generated by pre-Pigeon
// code. Since `camera` currently leaks the raw platform exceptions to clients, there may be client
// code that relies on specific string values here, so these should not be changed. See
// https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#platform-exception-handling
// for longer-term solutions to this.
if (exception instanceof CameraAccessException) {
  result.error(new FlutterError("CameraAccess", exception.getMessage(), null));
  return;
}
result.error(new FlutterError("error", exception.getMessage(), null, Log.getStackTraceString(exception)));

Apologies for missing this before; I'm still more used to the Obj-C method channel behaviors that the Java method channels.

[stuartmorgan-g]

Sorry, I realized looking at this again today that I was wrong about this; this is—non-obviously—a regression in this PR. With method channels, on Android, the engine call to onMethodCall is wrapped in an exception handler (specifically a RuntimeException handler, thus the current structure of this code). So the old code would correctly report these exceptions back to Dart, but this version won't because Pigeon doesn't try/catch async methods.

So we can close flutter/flutter#157021, and this code needs to be updated here. To be fully behavior-preserving, handleException should look like this:

// The code below exactly preserves the format of the native exceptions generated by pre-Pigeon
// code. Since `camera` currently leaks the raw platform exceptions to clients, there may be client
// code that relies on specific string values here, so these should not be changed. See
// https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#platform-exception-handling
// for longer-term solutions to this.
if (exception instanceof CameraAccessException) {
  result.error(new FlutterError("CameraAccess", exception.getMessage(), null));
  return;
}
result.error(new FlutterError("error", exception.getMessage(), null, Log.getStackTraceString(exception)));

Apologies for missing this before; I'm still more used to the Obj-C method channel behaviors that the Java method channels.

[stuartmorgan-g]

I missed this, but here and below you are catching Exception but reporting it as a CameraAccessException. I don't think you need these two handlers at all since you aren't doing anything specific with the exceptions, and these are sync methods, so the generic Pigeon-level handler should be fine.

[stuartmorgan-g]

Same with stopImageStream.

[yaakovschectman]

stopImageStream calls startPreview, which can throw two types of non-runtime Exception, so it needs to be wrapped in a try/catch to compile. I'm not sure what you're referring to by "and below", but I made the change to this method and incorporated the feedback from your other comment.

[bparrishMines]

LGTM but @stuartmorgan can provide the final LGTM

[stuartmorgan-g]

LGTM!