From 4637a13212fa415fcf11907ac30c985f939841c6 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Tue, 21 Jun 2022 13:33:24 -0400 Subject: [PATCH] [camera] Fix exception in registerWith (#6009) Fixes a regression from an unintented change in behavior during the conversion to an in-app method channel for Android and iOS. Although the Dart code for their implementations is almost identical to the shared method channel version, the differences in initialization paths caused the platform versions to try to use the widget bindings before they had been set up: The constructor for a `dartPluginClass` is called during `registerWith`, which is before `main`, but the constructor for the default implementation isn't called until `CameraPlatform.instance` is called, since Dart automatically does lazy static class initializtion. To avoid the issue without forcing bindings to be initialized early, this makes setting up the platform channel listener lazily. Fixes https://github.com/flutter/flutter/issues/106236 --- packages/camera/camera_android/CHANGELOG.md | 4 ++ .../lib/src/android_camera.dart | 38 +++++++++---------- packages/camera/camera_android/pubspec.yaml | 2 +- .../test/android_camera_test.dart | 32 +++++++++++++--- .../camera/camera_avfoundation/CHANGELOG.md | 4 ++ .../lib/src/avfoundation_camera.dart | 38 +++++++++---------- .../camera/camera_avfoundation/pubspec.yaml | 2 +- .../test/avfoundation_camera_test.dart | 32 +++++++++++++--- 8 files changed, 98 insertions(+), 54 deletions(-) diff --git a/packages/camera/camera_android/CHANGELOG.md b/packages/camera/camera_android/CHANGELOG.md index eab03e81bef5..e9972aede28a 100644 --- a/packages/camera/camera_android/CHANGELOG.md +++ b/packages/camera/camera_android/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.9.8+2 + +* Fixes exception in registerWith caused by the switch to an in-package method channel. + ## 0.9.8+1 * Ignores deprecation warnings for upcoming styleFrom button API changes. diff --git a/packages/camera/camera_android/lib/src/android_camera.dart b/packages/camera/camera_android/lib/src/android_camera.dart index 5fb3443ff91e..db6264cae011 100644 --- a/packages/camera/camera_android/lib/src/android_camera.dart +++ b/packages/camera/camera_android/lib/src/android_camera.dart @@ -19,14 +19,6 @@ const MethodChannel _channel = /// The Android implementation of [CameraPlatform] that uses method channels. class AndroidCamera extends CameraPlatform { - /// Construct a new method channel camera instance. - AndroidCamera() { - const MethodChannel channel = - MethodChannel('plugins.flutter.io/camera_android/fromPlatform'); - channel.setMethodCallHandler( - (MethodCall call) => handleDeviceMethodCall(call)); - } - /// Registers this class as the default instance of [CameraPlatform]. static void registerWith() { CameraPlatform.instance = AndroidCamera(); @@ -34,6 +26,12 @@ class AndroidCamera extends CameraPlatform { final Map _channels = {}; + /// The name of the channel that device events from the platform side are + /// sent on. + @visibleForTesting + static const String deviceEventChannelName = + 'plugins.flutter.io/camera_android/fromPlatform'; + /// The controller we need to broadcast the different events coming /// from handleMethodCall, specific to camera events. /// @@ -50,11 +48,15 @@ class AndroidCamera extends CameraPlatform { /// /// It is a `broadcast` because multiple controllers will connect to /// different stream views of this Controller. - /// This is only exposed for test purposes. It shouldn't be used by clients of - /// the plugin as it may break or change at any time. - @visibleForTesting - final StreamController deviceEventStreamController = - StreamController.broadcast(); + late final StreamController _deviceEventStreamController = + _createDeviceEventStreamController(); + + StreamController _createDeviceEventStreamController() { + // Set up the method handler lazily. + const MethodChannel channel = MethodChannel(deviceEventChannelName); + channel.setMethodCallHandler(_handleDeviceMethodCall); + return StreamController.broadcast(); + } // The stream to receive frames from the native code. StreamSubscription? _platformImageStreamSubscription; @@ -192,7 +194,7 @@ class AndroidCamera extends CameraPlatform { @override Stream onDeviceOrientationChanged() { - return deviceEventStreamController.stream + return _deviceEventStreamController.stream .whereType(); } @@ -518,14 +520,10 @@ class AndroidCamera extends CameraPlatform { } /// Converts messages received from the native platform into device events. - /// - /// This is only exposed for test purposes. It shouldn't be used by clients of - /// the plugin as it may break or change at any time. - @visibleForTesting - Future handleDeviceMethodCall(MethodCall call) async { + Future _handleDeviceMethodCall(MethodCall call) async { switch (call.method) { case 'orientation_changed': - deviceEventStreamController.add(DeviceOrientationChangedEvent( + _deviceEventStreamController.add(DeviceOrientationChangedEvent( deserializeDeviceOrientation( call.arguments['orientation']! as String))); break; diff --git a/packages/camera/camera_android/pubspec.yaml b/packages/camera/camera_android/pubspec.yaml index eca2b53949e9..7d93ecb22e47 100644 --- a/packages/camera/camera_android/pubspec.yaml +++ b/packages/camera/camera_android/pubspec.yaml @@ -2,7 +2,7 @@ name: camera_android description: Android implementation of the camera plugin. repository: https://github.com/flutter/plugins/tree/main/packages/camera/camera_android issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22 -version: 0.9.8+1 +version: 0.9.8+2 environment: sdk: ">=2.14.0 <3.0.0" diff --git a/packages/camera/camera_android/test/android_camera_test.dart b/packages/camera/camera_android/test/android_camera_test.dart index 9674b0c5a420..ca1f245019a8 100644 --- a/packages/camera/camera_android/test/android_camera_test.dart +++ b/packages/camera/camera_android/test/android_camera_test.dart @@ -25,6 +25,24 @@ void main() { expect(CameraPlatform.instance, isA()); }); + test('registration does not set message handlers', () async { + AndroidCamera.registerWith(); + + // Setting up a handler requires bindings to be initialized, and since + // registerWith is called very early in initialization the bindings won't + // have been initialized. While registerWith could intialize them, that + // could slow down startup, so instead the handler should be set up lazily. + final ByteData? response = await TestDefaultBinaryMessengerBinding + .instance!.defaultBinaryMessenger + .handlePlatformMessage( + AndroidCamera.deviceEventChannelName, + const StandardMethodCodec().encodeMethodCall(const MethodCall( + 'orientation_changed', + {'orientation': 'portraitDown'})), + (ByteData? data) {}); + expect(response, null); + }); + group('Creation, Initialization & Disposal Tests', () { test('Should send creation data and receive back a camera id', () async { // Arrange @@ -402,12 +420,14 @@ void main() { // Emit test events const DeviceOrientationChangedEvent event = DeviceOrientationChangedEvent(DeviceOrientation.portraitUp); - await camera.handleDeviceMethodCall( - MethodCall('orientation_changed', event.toJson())); - await camera.handleDeviceMethodCall( - MethodCall('orientation_changed', event.toJson())); - await camera.handleDeviceMethodCall( - MethodCall('orientation_changed', event.toJson())); + for (int i = 0; i < 3; i++) { + await TestDefaultBinaryMessengerBinding.instance!.defaultBinaryMessenger + .handlePlatformMessage( + AndroidCamera.deviceEventChannelName, + const StandardMethodCodec().encodeMethodCall( + MethodCall('orientation_changed', event.toJson())), + null); + } // Assert expect(await streamQueue.next, event); diff --git a/packages/camera/camera_avfoundation/CHANGELOG.md b/packages/camera/camera_avfoundation/CHANGELOG.md index eab03e81bef5..e9972aede28a 100644 --- a/packages/camera/camera_avfoundation/CHANGELOG.md +++ b/packages/camera/camera_avfoundation/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.9.8+2 + +* Fixes exception in registerWith caused by the switch to an in-package method channel. + ## 0.9.8+1 * Ignores deprecation warnings for upcoming styleFrom button API changes. diff --git a/packages/camera/camera_avfoundation/lib/src/avfoundation_camera.dart b/packages/camera/camera_avfoundation/lib/src/avfoundation_camera.dart index 1bff9011586b..c75d41f47c81 100644 --- a/packages/camera/camera_avfoundation/lib/src/avfoundation_camera.dart +++ b/packages/camera/camera_avfoundation/lib/src/avfoundation_camera.dart @@ -19,14 +19,6 @@ const MethodChannel _channel = /// An iOS implementation of [CameraPlatform] based on AVFoundation. class AVFoundationCamera extends CameraPlatform { - /// Construct a new method channel camera instance. - AVFoundationCamera() { - const MethodChannel channel = - MethodChannel('plugins.flutter.io/camera_avfoundation/fromPlatform'); - channel.setMethodCallHandler( - (MethodCall call) => handleDeviceMethodCall(call)); - } - /// Registers this class as the default instance of [CameraPlatform]. static void registerWith() { CameraPlatform.instance = AVFoundationCamera(); @@ -34,6 +26,12 @@ class AVFoundationCamera extends CameraPlatform { final Map _channels = {}; + /// The name of the channel that device events from the platform side are + /// sent on. + @visibleForTesting + static const String deviceEventChannelName = + 'plugins.flutter.io/camera_avfoundation/fromPlatform'; + /// The controller we need to broadcast the different events coming /// from handleMethodCall, specific to camera events. /// @@ -50,11 +48,15 @@ class AVFoundationCamera extends CameraPlatform { /// /// It is a `broadcast` because multiple controllers will connect to /// different stream views of this Controller. - /// This is only exposed for test purposes. It shouldn't be used by clients of - /// the plugin as it may break or change at any time. - @visibleForTesting - final StreamController deviceEventStreamController = - StreamController.broadcast(); + late final StreamController _deviceEventStreamController = + _createDeviceEventStreamController(); + + StreamController _createDeviceEventStreamController() { + // Set up the method handler lazily. + const MethodChannel channel = MethodChannel(deviceEventChannelName); + channel.setMethodCallHandler(_handleDeviceMethodCall); + return StreamController.broadcast(); + } // The stream to receive frames from the native code. StreamSubscription? _platformImageStreamSubscription; @@ -192,7 +194,7 @@ class AVFoundationCamera extends CameraPlatform { @override Stream onDeviceOrientationChanged() { - return deviceEventStreamController.stream + return _deviceEventStreamController.stream .whereType(); } @@ -523,14 +525,10 @@ class AVFoundationCamera extends CameraPlatform { } /// Converts messages received from the native platform into device events. - /// - /// This is only exposed for test purposes. It shouldn't be used by clients of - /// the plugin as it may break or change at any time. - @visibleForTesting - Future handleDeviceMethodCall(MethodCall call) async { + Future _handleDeviceMethodCall(MethodCall call) async { switch (call.method) { case 'orientation_changed': - deviceEventStreamController.add(DeviceOrientationChangedEvent( + _deviceEventStreamController.add(DeviceOrientationChangedEvent( deserializeDeviceOrientation( call.arguments['orientation']! as String))); break; diff --git a/packages/camera/camera_avfoundation/pubspec.yaml b/packages/camera/camera_avfoundation/pubspec.yaml index f1a5edfb6c07..adb0bf8dbafb 100644 --- a/packages/camera/camera_avfoundation/pubspec.yaml +++ b/packages/camera/camera_avfoundation/pubspec.yaml @@ -2,7 +2,7 @@ name: camera_avfoundation description: iOS implementation of the camera plugin. repository: https://github.com/flutter/plugins/tree/main/packages/camera/camera_avfoundation issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22 -version: 0.9.8+1 +version: 0.9.8+2 environment: sdk: ">=2.14.0 <3.0.0" diff --git a/packages/camera/camera_avfoundation/test/avfoundation_camera_test.dart b/packages/camera/camera_avfoundation/test/avfoundation_camera_test.dart index 4b32d2e50b4a..67adcfab81f2 100644 --- a/packages/camera/camera_avfoundation/test/avfoundation_camera_test.dart +++ b/packages/camera/camera_avfoundation/test/avfoundation_camera_test.dart @@ -25,6 +25,24 @@ void main() { expect(CameraPlatform.instance, isA()); }); + test('registration does not set message handlers', () async { + AVFoundationCamera.registerWith(); + + // Setting up a handler requires bindings to be initialized, and since + // registerWith is called very early in initialization the bindings won't + // have been initialized. While registerWith could intialize them, that + // could slow down startup, so instead the handler should be set up lazily. + final ByteData? response = await TestDefaultBinaryMessengerBinding + .instance!.defaultBinaryMessenger + .handlePlatformMessage( + AVFoundationCamera.deviceEventChannelName, + const StandardMethodCodec().encodeMethodCall(const MethodCall( + 'orientation_changed', + {'orientation': 'portraitDown'})), + (ByteData? data) {}); + expect(response, null); + }); + group('Creation, Initialization & Disposal Tests', () { test('Should send creation data and receive back a camera id', () async { // Arrange @@ -402,12 +420,14 @@ void main() { // Emit test events const DeviceOrientationChangedEvent event = DeviceOrientationChangedEvent(DeviceOrientation.portraitUp); - await camera.handleDeviceMethodCall( - MethodCall('orientation_changed', event.toJson())); - await camera.handleDeviceMethodCall( - MethodCall('orientation_changed', event.toJson())); - await camera.handleDeviceMethodCall( - MethodCall('orientation_changed', event.toJson())); + for (int i = 0; i < 3; i++) { + await TestDefaultBinaryMessengerBinding.instance!.defaultBinaryMessenger + .handlePlatformMessage( + AVFoundationCamera.deviceEventChannelName, + const StandardMethodCodec().encodeMethodCall( + MethodCall('orientation_changed', event.toJson())), + null); + } // Assert expect(await streamQueue.next, event);