From 741d98bacab5096e892d5942430836224575f15a Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 29 Sep 2023 10:36:48 -0400 Subject: [PATCH 1/2] [camera] Remove `@throw` from iOS implementation Using `@throw` in iOS code violates the style guide, so it shouldn't be done in the plugin as mechanism for communicating errors. More importantly, `NSError` is not intended to be used with `@throw`/`@catch`, and is causing issues when compiled with the iOS 17 SDK. This removes all use of `@throw`, and all `@catch (NSError* e)`, in favor of other methods of communicating errors. It explicitly does not try to fix all the other strange things about this code (having an `NSError` out-param in an init method, using Cocoa and NSURL error domains and codes for some reason), and instead preserves existing behavior as much as possible. In practice, none of these codepaths should ever actually happen (they indicate programming errors within the plugin, not unexpected runtime behavior), and all of this code will go away when converting to Pigeon anyway, so there's not much value in trying to unwind this structure further. Fixes https://github.com/flutter/flutter/issues/135195 --- .../camera/camera_avfoundation/CHANGELOG.md | 4 + .../ios/RunnerTests/CameraPropertiesTests.m | 14 +-- .../ios/Classes/CameraProperties.h | 12 +++ .../ios/Classes/CameraProperties.m | 63 +++---------- .../camera_avfoundation/ios/Classes/FLTCam.m | 91 ++++++++++++------- .../camera/camera_avfoundation/pubspec.yaml | 2 +- 6 files changed, 94 insertions(+), 92 deletions(-) diff --git a/packages/camera/camera_avfoundation/CHANGELOG.md b/packages/camera/camera_avfoundation/CHANGELOG.md index 011d9c59dc0..8f221842e40 100644 --- a/packages/camera/camera_avfoundation/CHANGELOG.md +++ b/packages/camera/camera_avfoundation/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.9.13+6 + +* Fixes incorrect use of `NSError` that could cause crashes on launch. + ## 0.9.13+5 * Ignores audio samples until the first video sample arrives. diff --git a/packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraPropertiesTests.m b/packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraPropertiesTests.m index 18c01e59990..95203bfa9e9 100644 --- a/packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraPropertiesTests.m +++ b/packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraPropertiesTests.m @@ -19,7 +19,7 @@ - (void)testFLTGetFLTFlashModeForString { XCTAssertEqual(FLTFlashModeAuto, FLTGetFLTFlashModeForString(@"auto")); XCTAssertEqual(FLTFlashModeAlways, FLTGetFLTFlashModeForString(@"always")); XCTAssertEqual(FLTFlashModeTorch, FLTGetFLTFlashModeForString(@"torch")); - XCTAssertThrows(FLTGetFLTFlashModeForString(@"unkwown")); + XCTAssertEqual(FLTFlashModeInvalid, FLTGetFLTFlashModeForString(@"unknown")); } - (void)testFLTGetAVCaptureFlashModeForFLTFlashMode { @@ -34,13 +34,13 @@ - (void)testFLTGetAVCaptureFlashModeForFLTFlashMode { - (void)testFLTGetStringForFLTExposureMode { XCTAssertEqualObjects(@"auto", FLTGetStringForFLTExposureMode(FLTExposureModeAuto)); XCTAssertEqualObjects(@"locked", FLTGetStringForFLTExposureMode(FLTExposureModeLocked)); - XCTAssertThrows(FLTGetStringForFLTExposureMode(-1)); + XCTAssertNil(FLTGetStringForFLTExposureMode(-1)); } - (void)testFLTGetFLTExposureModeForString { XCTAssertEqual(FLTExposureModeAuto, FLTGetFLTExposureModeForString(@"auto")); XCTAssertEqual(FLTExposureModeLocked, FLTGetFLTExposureModeForString(@"locked")); - XCTAssertThrows(FLTGetFLTExposureModeForString(@"unknown")); + XCTAssertEqual(FLTExposureModeInvalid, FLTGetFLTExposureModeForString(@"unknown")); } #pragma mark - focus mode tests @@ -48,13 +48,13 @@ - (void)testFLTGetFLTExposureModeForString { - (void)testFLTGetStringForFLTFocusMode { XCTAssertEqualObjects(@"auto", FLTGetStringForFLTFocusMode(FLTFocusModeAuto)); XCTAssertEqualObjects(@"locked", FLTGetStringForFLTFocusMode(FLTFocusModeLocked)); - XCTAssertThrows(FLTGetStringForFLTFocusMode(-1)); + XCTAssertNil(FLTGetStringForFLTFocusMode(-1)); } - (void)testFLTGetFLTFocusModeForString { XCTAssertEqual(FLTFocusModeAuto, FLTGetFLTFocusModeForString(@"auto")); XCTAssertEqual(FLTFocusModeLocked, FLTGetFLTFocusModeForString(@"locked")); - XCTAssertThrows(FLTGetFLTFocusModeForString(@"unknown")); + XCTAssertEqual(FLTFocusModeInvalid, FLTGetFLTFocusModeForString(@"unknown")); } #pragma mark - resolution preset tests @@ -67,7 +67,7 @@ - (void)testFLTGetFLTResolutionPresetForString { XCTAssertEqual(FLTResolutionPresetVeryHigh, FLTGetFLTResolutionPresetForString(@"veryHigh")); XCTAssertEqual(FLTResolutionPresetUltraHigh, FLTGetFLTResolutionPresetForString(@"ultraHigh")); XCTAssertEqual(FLTResolutionPresetMax, FLTGetFLTResolutionPresetForString(@"max")); - XCTAssertThrows(FLTGetFLTFlashModeForString(@"unknown")); + XCTAssertEqual(FLTResolutionPresetInvalid, FLTGetFLTResolutionPresetForString(@"unknown")); } #pragma mark - video format tests @@ -89,7 +89,7 @@ - (void)testFLTGetUIDeviceOrientationForString { XCTAssertEqual(UIDeviceOrientationLandscapeLeft, FLTGetUIDeviceOrientationForString(@"landscapeRight")); XCTAssertEqual(UIDeviceOrientationPortrait, FLTGetUIDeviceOrientationForString(@"portraitUp")); - XCTAssertThrows(FLTGetUIDeviceOrientationForString(@"unknown")); + XCTAssertEqual(UIDeviceOrientationUnknown, FLTGetUIDeviceOrientationForString(@"unknown")); } - (void)testFLTGetStringForUIDeviceOrientation { diff --git a/packages/camera/camera_avfoundation/ios/Classes/CameraProperties.h b/packages/camera/camera_avfoundation/ios/Classes/CameraProperties.h index aee4d643f64..4d0818dc816 100644 --- a/packages/camera/camera_avfoundation/ios/Classes/CameraProperties.h +++ b/packages/camera/camera_avfoundation/ios/Classes/CameraProperties.h @@ -17,6 +17,9 @@ typedef NS_ENUM(NSInteger, FLTFlashMode) { FLTFlashModeAuto, FLTFlashModeAlways, FLTFlashModeTorch, + // This should never occur; it indicates an unknown value was received over + // the platform channel. + FLTFlashModeInvalid, }; /** @@ -39,6 +42,9 @@ extern AVCaptureFlashMode FLTGetAVCaptureFlashModeForFLTFlashMode(FLTFlashMode m typedef NS_ENUM(NSInteger, FLTExposureMode) { FLTExposureModeAuto, FLTExposureModeLocked, + // This should never occur; it indicates an unknown value was received over + // the platform channel. + FLTExposureModeInvalid, }; /** @@ -61,6 +67,9 @@ extern FLTExposureMode FLTGetFLTExposureModeForString(NSString *mode); typedef NS_ENUM(NSInteger, FLTFocusMode) { FLTFocusModeAuto, FLTFocusModeLocked, + // This should never occur; it indicates an unknown value was received over + // the platform channel. + FLTFocusModeInvalid, }; /** @@ -100,6 +109,9 @@ typedef NS_ENUM(NSInteger, FLTResolutionPreset) { FLTResolutionPresetVeryHigh, FLTResolutionPresetUltraHigh, FLTResolutionPresetMax, + // This should never occur; it indicates an unknown value was received over + // the platform channel. + FLTResolutionPresetInvalid, }; /** diff --git a/packages/camera/camera_avfoundation/ios/Classes/CameraProperties.m b/packages/camera/camera_avfoundation/ios/Classes/CameraProperties.m index e36f98af27f..147d3e3670b 100644 --- a/packages/camera/camera_avfoundation/ios/Classes/CameraProperties.m +++ b/packages/camera/camera_avfoundation/ios/Classes/CameraProperties.m @@ -16,13 +16,7 @@ FLTFlashMode FLTGetFLTFlashModeForString(NSString *mode) { } else if ([mode isEqualToString:@"torch"]) { return FLTFlashModeTorch; } else { - NSError *error = [NSError errorWithDomain:NSCocoaErrorDomain - code:NSURLErrorUnknown - userInfo:@{ - NSLocalizedDescriptionKey : [NSString - stringWithFormat:@"Unknown flash mode %@", mode] - }]; - @throw error; + return FLTFlashModeInvalid; } } @@ -48,14 +42,11 @@ AVCaptureFlashMode FLTGetAVCaptureFlashModeForFLTFlashMode(FLTFlashMode mode) { return @"auto"; case FLTExposureModeLocked: return @"locked"; + case FLTExposureModeInvalid: + // This value should never actually be used. + return nil; } - NSError *error = [NSError errorWithDomain:NSCocoaErrorDomain - code:NSURLErrorUnknown - userInfo:@{ - NSLocalizedDescriptionKey : [NSString - stringWithFormat:@"Unknown string for exposure mode"] - }]; - @throw error; + return nil; } FLTExposureMode FLTGetFLTExposureModeForString(NSString *mode) { @@ -64,13 +55,7 @@ FLTExposureMode FLTGetFLTExposureModeForString(NSString *mode) { } else if ([mode isEqualToString:@"locked"]) { return FLTExposureModeLocked; } else { - NSError *error = [NSError errorWithDomain:NSCocoaErrorDomain - code:NSURLErrorUnknown - userInfo:@{ - NSLocalizedDescriptionKey : [NSString - stringWithFormat:@"Unknown exposure mode %@", mode] - }]; - @throw error; + return FLTExposureModeInvalid; } } @@ -82,14 +67,11 @@ FLTExposureMode FLTGetFLTExposureModeForString(NSString *mode) { return @"auto"; case FLTFocusModeLocked: return @"locked"; + case FLTFocusModeInvalid: + // This value should never actually be used. + return nil; } - NSError *error = [NSError errorWithDomain:NSCocoaErrorDomain - code:NSURLErrorUnknown - userInfo:@{ - NSLocalizedDescriptionKey : [NSString - stringWithFormat:@"Unknown string for focus mode"] - }]; - @throw error; + return nil; } FLTFocusMode FLTGetFLTFocusModeForString(NSString *mode) { @@ -98,13 +80,7 @@ FLTFocusMode FLTGetFLTFocusModeForString(NSString *mode) { } else if ([mode isEqualToString:@"locked"]) { return FLTFocusModeLocked; } else { - NSError *error = [NSError errorWithDomain:NSCocoaErrorDomain - code:NSURLErrorUnknown - userInfo:@{ - NSLocalizedDescriptionKey : [NSString - stringWithFormat:@"Unknown focus mode %@", mode] - }]; - @throw error; + return FLTFocusModeInvalid; } } @@ -120,14 +96,7 @@ UIDeviceOrientation FLTGetUIDeviceOrientationForString(NSString *orientation) { } else if ([orientation isEqualToString:@"portraitUp"]) { return UIDeviceOrientationPortrait; } else { - NSError *error = [NSError - errorWithDomain:NSCocoaErrorDomain - code:NSURLErrorUnknown - userInfo:@{ - NSLocalizedDescriptionKey : - [NSString stringWithFormat:@"Unknown device orientation %@", orientation] - }]; - @throw error; + return UIDeviceOrientationUnknown; } } @@ -163,13 +132,7 @@ FLTResolutionPreset FLTGetFLTResolutionPresetForString(NSString *preset) { } else if ([preset isEqualToString:@"max"]) { return FLTResolutionPresetMax; } else { - NSError *error = [NSError errorWithDomain:NSCocoaErrorDomain - code:NSURLErrorUnknown - userInfo:@{ - NSLocalizedDescriptionKey : [NSString - stringWithFormat:@"Unknown resolution preset %@", preset] - }]; - @throw error; + return FLTResolutionPresetInvalid; } } diff --git a/packages/camera/camera_avfoundation/ios/Classes/FLTCam.m b/packages/camera/camera_avfoundation/ios/Classes/FLTCam.m index e0f03000d45..e932bc4824c 100644 --- a/packages/camera/camera_avfoundation/ios/Classes/FLTCam.m +++ b/packages/camera/camera_avfoundation/ios/Classes/FLTCam.m @@ -118,10 +118,16 @@ - (instancetype)initWithCameraName:(NSString *)cameraName error:(NSError **)error { self = [super init]; NSAssert(self, @"super init cannot be nil"); - @try { - _resolutionPreset = FLTGetFLTResolutionPresetForString(resolutionPreset); - } @catch (NSError *e) { - *error = e; + _resolutionPreset = FLTGetFLTResolutionPresetForString(resolutionPreset); + if (_resolutionPreset == FLTResolutionPresetInvalid) { + *error = [NSError + errorWithDomain:NSCocoaErrorDomain + code:NSURLErrorUnknown + userInfo:@{ + NSLocalizedDescriptionKey : + [NSString stringWithFormat:@"Unknown resolution preset %@", resolutionPreset] + }]; + return nil; } _enableAudio = enableAudio; _captureSessionQueue = captureSessionQueue; @@ -162,7 +168,9 @@ - (instancetype)initWithCameraName:(NSString *)cameraName _motionManager = [[CMMotionManager alloc] init]; [_motionManager startAccelerometerUpdates]; - [self setCaptureSessionPreset:_resolutionPreset]; + if (![self setCaptureSessionPreset:_resolutionPreset withError:error]) { + return nil; + } [self updateOrientation]; return self; @@ -337,7 +345,7 @@ - (NSString *)getTemporaryFilePathWithExtension:(NSString *)extension return file; } -- (void)setCaptureSessionPreset:(FLTResolutionPreset)resolutionPreset { +- (BOOL)setCaptureSessionPreset:(FLTResolutionPreset)resolutionPreset withError:(NSError **)error { switch (resolutionPreset) { case FLTResolutionPresetMax: case FLTResolutionPresetUltraHigh: @@ -382,17 +390,17 @@ - (void)setCaptureSessionPreset:(FLTResolutionPreset)resolutionPreset { _videoCaptureSession.sessionPreset = AVCaptureSessionPresetLow; _previewSize = CGSizeMake(352, 288); } else { - NSError *error = - [NSError errorWithDomain:NSCocoaErrorDomain - code:NSURLErrorUnknown - userInfo:@{ - NSLocalizedDescriptionKey : - @"No capture session available for current capture session." - }]; - @throw error; + *error = [NSError errorWithDomain:NSCocoaErrorDomain + code:NSURLErrorUnknown + userInfo:@{ + NSLocalizedDescriptionKey : + @"No capture session available for current capture session." + }]; + return NO; } } _audioCaptureSession.sessionPreset = _videoCaptureSession.sessionPreset; + return YES; } - (void)captureOutput:(AVCaptureOutput *)output @@ -726,11 +734,17 @@ - (void)resumeVideoRecordingWithResult:(FLTThreadSafeFlutterResult *)result { - (void)lockCaptureOrientationWithResult:(FLTThreadSafeFlutterResult *)result orientation:(NSString *)orientationStr { - UIDeviceOrientation orientation; - @try { - orientation = FLTGetUIDeviceOrientationForString(orientationStr); - } @catch (NSError *e) { - [result sendError:e]; + UIDeviceOrientation orientation = FLTGetUIDeviceOrientationForString(orientationStr); + // "Unknown" should never be sent, so is used to represent an unexpected + // value. + if (orientation == UIDeviceOrientationUnknown) { + [result sendError:[NSError errorWithDomain:NSCocoaErrorDomain + code:NSURLErrorUnknown + userInfo:@{ + NSLocalizedDescriptionKey : [NSString + stringWithFormat:@"Unknown device orientation %@", + orientationStr] + }]]; return; } @@ -749,11 +763,14 @@ - (void)unlockCaptureOrientationWithResult:(FLTThreadSafeFlutterResult *)result } - (void)setFlashModeWithResult:(FLTThreadSafeFlutterResult *)result mode:(NSString *)modeStr { - FLTFlashMode mode; - @try { - mode = FLTGetFLTFlashModeForString(modeStr); - } @catch (NSError *e) { - [result sendError:e]; + FLTFlashMode mode = FLTGetFLTFlashModeForString(modeStr); + if (mode == FLTFlashModeInvalid) { + [result sendError:[NSError errorWithDomain:NSCocoaErrorDomain + code:NSURLErrorUnknown + userInfo:@{ + NSLocalizedDescriptionKey : [NSString + stringWithFormat:@"Unknown flash mode %@", modeStr] + }]]; return; } if (mode == FLTFlashModeTorch) { @@ -800,11 +817,14 @@ - (void)setFlashModeWithResult:(FLTThreadSafeFlutterResult *)result mode:(NSStri } - (void)setExposureModeWithResult:(FLTThreadSafeFlutterResult *)result mode:(NSString *)modeStr { - FLTExposureMode mode; - @try { - mode = FLTGetFLTExposureModeForString(modeStr); - } @catch (NSError *e) { - [result sendError:e]; + FLTExposureMode mode = FLTGetFLTExposureModeForString(modeStr); + if (mode == FLTExposureModeInvalid) { + [result sendError:[NSError errorWithDomain:NSCocoaErrorDomain + code:NSURLErrorUnknown + userInfo:@{ + NSLocalizedDescriptionKey : [NSString + stringWithFormat:@"Unknown exposure mode %@", modeStr] + }]]; return; } _exposureMode = mode; @@ -830,11 +850,14 @@ - (void)applyExposureMode { } - (void)setFocusModeWithResult:(FLTThreadSafeFlutterResult *)result mode:(NSString *)modeStr { - FLTFocusMode mode; - @try { - mode = FLTGetFLTFocusModeForString(modeStr); - } @catch (NSError *e) { - [result sendError:e]; + FLTFocusMode mode = FLTGetFLTFocusModeForString(modeStr); + if (mode == FLTFocusModeInvalid) { + [result sendError:[NSError errorWithDomain:NSCocoaErrorDomain + code:NSURLErrorUnknown + userInfo:@{ + NSLocalizedDescriptionKey : [NSString + stringWithFormat:@"Unknown focus mode %@", modeStr] + }]]; return; } _focusMode = mode; diff --git a/packages/camera/camera_avfoundation/pubspec.yaml b/packages/camera/camera_avfoundation/pubspec.yaml index e13e957cad7..b4a4396926d 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/packages/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.13+5 +version: 0.9.13+6 environment: sdk: ">=2.19.0 <4.0.0" From c1bffdc9d37dbc60283140e63b38849091da8c2d Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 29 Sep 2023 12:48:43 -0400 Subject: [PATCH 2/2] Add switch handling --- .../example/ios/Runner.xcodeproj/project.pbxproj | 2 +- .../xcshareddata/xcschemes/Runner.xcscheme | 2 +- .../camera/camera_avfoundation/ios/Classes/FLTCam.m | 10 ++++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/camera/camera_avfoundation/example/ios/Runner.xcodeproj/project.pbxproj b/packages/camera/camera_avfoundation/example/ios/Runner.xcodeproj/project.pbxproj index a20638d18e1..6006b9f2b4b 100644 --- a/packages/camera/camera_avfoundation/example/ios/Runner.xcodeproj/project.pbxproj +++ b/packages/camera/camera_avfoundation/example/ios/Runner.xcodeproj/project.pbxproj @@ -280,7 +280,7 @@ 97C146E61CF9000F007C117D /* Project object */ = { isa = PBXProject; attributes = { - LastUpgradeCheck = 1300; + LastUpgradeCheck = 1430; ORGANIZATIONNAME = "The Flutter Authors"; TargetAttributes = { 03BB76672665316900CE5A93 = { diff --git a/packages/camera/camera_avfoundation/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme b/packages/camera/camera_avfoundation/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme index f4b3c109900..1ff4b573d76 100644 --- a/packages/camera/camera_avfoundation/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme +++ b/packages/camera/camera_avfoundation/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme @@ -1,6 +1,6 @@