Skip to content

[camera] Remove @throw from iOS implementation #5034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 29, 2023
Merged

[camera] Remove @throw from iOS implementation #5034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
741d98b
[camera] Remove `@throw` from iOS implementation
stuartmorgan-g Sep 29, 2023
c1bffdc
Add switch handling
stuartmorgan-g Sep 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/camera/camera_avfoundation/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.9.13+6

* Fixes incorrect use of `NSError` that could cause crashes on launch.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: ... cause crashes on launch on iOS 17

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I unfortunately don't know if it's "on iOS 17" or "when compiled against the iOS 17 SDK" or some other slight variant of that, which is why I left it vague.


## 0.9.13+5

* Ignores audio samples until the first video sample arrives.
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera_avfoundation/example/ios/Runner.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@
97C146E61CF9000F007C117D /* Project object */ = {
isa = PBXProject;
attributes = {
LastUpgradeCheck = 1300;
LastUpgradeCheck = 1430;
ORGANIZATIONNAME = "The Flutter Authors";
TargetAttributes = {
03BB76672665316900CE5A93 = {
Expand Down
2 changes: 1 addition & 1 deletion ...a/camera_avfoundation/example/ios/Runner.xcodeproj/xcshareddata/xcschemes/Runner.xcscheme
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "1300"
LastUpgradeVersion = "1430"
version = "1.3">
<BuildAction
parallelizeBuildables = "YES"
Expand Down
14 changes: 7 additions & 7 deletions packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraPropertiesTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: should it be FLTFlashModeUnknown? in case we have new flash mode in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have a new flash mode in the future then we need to update both the Dart and Obj-C code at the same time. This code is purely for manual platform channel conversions, which means the only way to get a value we don't recognize is if someone adds serialization for that mode on the Dart side but not the corresponding deserialization code on the native side, which would make no sense.

If we have flash modes that the Dart side of camera_avfoundation has been updated to send over the method channel but the Obj-C side doesn't know about, then someone has made a serious mistake in adding a new flash mode. We shouldn't explicitly allow for that case with an "unknown", implying that's an okay state to be in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This is why Pigeon will moot all of this; in Pigeon it's impossible for an enum value defined in the interface to exist on one side but not the other.)

}

- (void)testFLTGetAVCaptureFlashModeForFLTFlashMode {
Expand All @@ -34,27 +34,27 @@ - (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

- (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
Expand All @@ -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
Expand All @@ -89,7 +89,7 @@ - (void)testFLTGetUIDeviceOrientationForString {
XCTAssertEqual(UIDeviceOrientationLandscapeLeft,
FLTGetUIDeviceOrientationForString(@"landscapeRight"));
XCTAssertEqual(UIDeviceOrientationPortrait, FLTGetUIDeviceOrientationForString(@"portraitUp"));
XCTAssertThrows(FLTGetUIDeviceOrientationForString(@"unknown"));
XCTAssertEqual(UIDeviceOrientationUnknown, FLTGetUIDeviceOrientationForString(@"unknown"));
}

- (void)testFLTGetStringForUIDeviceOrientation {
Expand Down
12 changes: 12 additions & 0 deletions packages/camera/camera_avfoundation/ios/Classes/CameraProperties.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

/**
Expand All @@ -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,
};

/**
Expand All @@ -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,
};

/**
Expand Down Expand Up @@ -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,
};

/**
Expand Down
63 changes: 13 additions & 50 deletions packages/camera/camera_avfoundation/ios/Classes/CameraProperties.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand All @@ -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) {
Expand All @@ -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;
}
}

Expand All @@ -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) {
Expand All @@ -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;
}
}

Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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;
}
}

Expand Down
Loading