breautek commented on code in PR #942:
URL:
https://github.com/apache/cordova-plugin-camera/pull/942#discussion_r2687881749
##########
src/ios/CDVCamera.m:
##########
@@ -396,17 +396,29 @@ - (void)picker:(PHPickerViewController*)picker
didFinishPicking:(NSArray<PHPicke
// Check if it's a video
if ([pickerResult.itemProvider
hasItemConformingToTypeIdentifier:UTTypeMovie.identifier]) {
- [pickerResult.itemProvider
loadFileRepresentationForTypeIdentifier:UTTypeMovie.identifier
completionHandler:^(NSURL * _Nullable url, NSError * _Nullable error) {
+ // Writes a copy of the data to a temporary file. This file will
be deleted
+ // when the completion handler returns. The program should copy or
move the file within the completion handler.
+ [pickerResult.itemProvider
loadFileRepresentationForTypeIdentifier:UTTypeMovie.identifier
+
completionHandler:^(NSURL * _Nullable url, NSError * _Nullable error) {
if (error) {
- CDVPluginResult* result = [CDVPluginResult
resultWithStatus:CDVCommandStatus_ERROR messageAsString:[error
localizedDescription]];
+ CDVPluginResult* result = [CDVPluginResult
resultWithStatus:CDVCommandStatus_ERROR
+
messageAsString:[error localizedDescription]];
[weakSelf.commandDelegate sendPluginResult:result
callbackId:callbackId];
weakSelf.hasPendingOperation = NO;
return;
}
dispatch_async(dispatch_get_main_queue(), ^{
NSString* videoPath = [weakSelf createTmpVideo:[url path]];
- CDVPluginResult* result = [CDVPluginResult
resultWithStatus:CDVCommandStatus_OK messageAsString:videoPath];
+ CDVPluginResult* result = nil;
+
+ if (videoPath == nil) {
+ result = [CDVPluginResult
resultWithStatus:CDVCommandStatus_IO_EXCEPTION
+ messageAsString:@"Failed to
copy video file to temporary location"];
+ } else {
+ result = [CDVPluginResult
resultWithStatus:CDVCommandStatus_OK messageAsString:videoPath];
+ }
+
[weakSelf.commandDelegate sendPluginResult:result
callbackId:callbackId];
weakSelf.hasPendingOperation = NO;
Review Comment:
If we follow the above suggestion regarding weakSelf then these should be
updated as well
```suggestion
[strongSelf.commandDelegate sendPluginResult:result callbackId:callbackId];
strongSelf.hasPendingOperation = NO;
```
##########
src/ios/CDVCamera.m:
##########
@@ -396,17 +396,29 @@ - (void)picker:(PHPickerViewController*)picker
didFinishPicking:(NSArray<PHPicke
// Check if it's a video
if ([pickerResult.itemProvider
hasItemConformingToTypeIdentifier:UTTypeMovie.identifier]) {
- [pickerResult.itemProvider
loadFileRepresentationForTypeIdentifier:UTTypeMovie.identifier
completionHandler:^(NSURL * _Nullable url, NSError * _Nullable error) {
+ // Writes a copy of the data to a temporary file. This file will
be deleted
+ // when the completion handler returns. The program should copy or
move the file within the completion handler.
+ [pickerResult.itemProvider
loadFileRepresentationForTypeIdentifier:UTTypeMovie.identifier
+
completionHandler:^(NSURL * _Nullable url, NSError * _Nullable error) {
if (error) {
- CDVPluginResult* result = [CDVPluginResult
resultWithStatus:CDVCommandStatus_ERROR messageAsString:[error
localizedDescription]];
+ CDVPluginResult* result = [CDVPluginResult
resultWithStatus:CDVCommandStatus_ERROR
+
messageAsString:[error localizedDescription]];
[weakSelf.commandDelegate sendPluginResult:result
callbackId:callbackId];
weakSelf.hasPendingOperation = NO;
return;
}
dispatch_async(dispatch_get_main_queue(), ^{
NSString* videoPath = [weakSelf createTmpVideo:[url path]];
Review Comment:
I realized that this PR doesn't touch this line but this usage of `weakSelf`
is technically unsafe because it could go away at any time. Attempting to use
it when it was GC'ed would result in a fatal crash.
This would only happen if the webview was in the process of being destroyed
in practice I believe, so in this event we can basically treat any action as
invalid, and it is pretty simple to handle this.
```suggestion
CDVCamera* strongSelf = weakSelf;
if (strongSelf == nil) return;
NSString* videoPath = [strongSelf createTmpVideo:[url
path]];
```
This way if strongSelf has the CDVCamera reference, that reference is
guarenteed to be retained for the duration of the block. Once the block exits,
the strongSelf goes out of scope and will be cleaned up.
We check if it's nil in case `weakSelf` was already GC'ed when we entered
the main queue execution routine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
