[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 flutter/flutter#106236
flutter · Jun 21, 2022 · e868ca2 · e868ca2
1 parent c3ba213
commit e868ca2
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 54 deletions.
diff --git 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
@@ -19,21 +19,19 @@ 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();
   }
 
   final Map<int, MethodChannel> _channels = <int, MethodChannel>{};
 
+  /// 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<DeviceEvent> deviceEventStreamController =
-      StreamController<DeviceEvent>.broadcast();
+  late final StreamController<DeviceEvent> _deviceEventStreamController =
+      _createDeviceEventStreamController();
+
+  StreamController<DeviceEvent> _createDeviceEventStreamController() {
+    // Set up the method handler lazily.
+    const MethodChannel channel = MethodChannel(deviceEventChannelName);
+    channel.setMethodCallHandler(_handleDeviceMethodCall);
+    return StreamController<DeviceEvent>.broadcast();
+  }
 
   // The stream to receive frames from the native code.
   StreamSubscription<dynamic>? _platformImageStreamSubscription;
@@ -192,7 +194,7 @@ class AndroidCamera extends CameraPlatform {
 
   @override
   Stream<DeviceOrientationChangedEvent> onDeviceOrientationChanged() {
-    return deviceEventStreamController.stream
+    return _deviceEventStreamController.stream
         .whereType<DeviceOrientationChangedEvent>();
   }
 
@@ -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<dynamic> handleDeviceMethodCall(MethodCall call) async {
+  Future<dynamic> _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
@@ -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
@@ -25,6 +25,24 @@ void main() {
     expect(CameraPlatform.instance, isA<AndroidCamera>());
   });
 
+  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',
+                <String, Object>{'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
@@ -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
@@ -19,21 +19,19 @@ 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();
   }
 
   final Map<int, MethodChannel> _channels = <int, MethodChannel>{};
 
+  /// 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<DeviceEvent> deviceEventStreamController =
-      StreamController<DeviceEvent>.broadcast();
+  late final StreamController<DeviceEvent> _deviceEventStreamController =
+      _createDeviceEventStreamController();
+
+  StreamController<DeviceEvent> _createDeviceEventStreamController() {
+    // Set up the method handler lazily.
+    const MethodChannel channel = MethodChannel(deviceEventChannelName);
+    channel.setMethodCallHandler(_handleDeviceMethodCall);
+    return StreamController<DeviceEvent>.broadcast();
+  }
 
   // The stream to receive frames from the native code.
   StreamSubscription<dynamic>? _platformImageStreamSubscription;
@@ -192,7 +194,7 @@ class AVFoundationCamera extends CameraPlatform {
 
   @override
   Stream<DeviceOrientationChangedEvent> onDeviceOrientationChanged() {
-    return deviceEventStreamController.stream
+    return _deviceEventStreamController.stream
         .whereType<DeviceOrientationChangedEvent>();
   }
 
@@ -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<dynamic> handleDeviceMethodCall(MethodCall call) async {
+  Future<dynamic> _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
@@ -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
@@ -25,6 +25,24 @@ void main() {
     expect(CameraPlatform.instance, isA<AVFoundationCamera>());
   });
 
+  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',
+                <String, Object>{'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);