Skip to content

Commit

Permalink
[camera] Fix exception in registerWith (flutter#6009)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@stuartmorgan @mauricioluz
stuartmorgan authored and mauricioluz committed Jan 26, 2023
1 parent 87bfc20 commit 4637a13
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 54 deletions.
4 changes: 4 additions & 0 deletions packages/camera/camera_android/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
38 changes: 18 additions & 20 deletions packages/camera/camera_android/lib/src/android_camera.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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;
Expand Down Expand Up @@ -192,7 +194,7 @@ class AndroidCamera extends CameraPlatform {

@override
Stream<DeviceOrientationChangedEvent> onDeviceOrientationChanged() {
return deviceEventStreamController.stream
return _deviceEventStreamController.stream
.whereType<DeviceOrientationChangedEvent>();
}

Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera_android/pubspec.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
32 changes: 26 additions & 6 deletions packages/camera/camera_android/test/android_camera_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions packages/camera/camera_avfoundation/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
38 changes: 18 additions & 20 deletions packages/camera/camera_avfoundation/lib/src/avfoundation_camera.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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;
Expand Down Expand Up @@ -192,7 +194,7 @@ class AVFoundationCamera extends CameraPlatform {

@override
Stream<DeviceOrientationChangedEvent> onDeviceOrientationChanged() {
return deviceEventStreamController.stream
return _deviceEventStreamController.stream
.whereType<DeviceOrientationChangedEvent>();
}

Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera_avfoundation/pubspec.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
32 changes: 26 additions & 6 deletions packages/camera/camera_avfoundation/test/avfoundation_camera_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 4637a13

Please sign in to comment.