On iOS, the file chooser needs access to frontboard and icon services in the WebContent process. Create and consume extensions for these services when choosing files. When done, the extensions should be revoked.
Created attachment 397484 [details] Patch
Created attachment 397486 [details] Patch
Created attachment 397497 [details] Patch
Created attachment 397511 [details] Patch
Comment on attachment 397511 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397511&action=review > Source/WebKit/Shared/ios/WebIconUtilities.mm:95 > + if (![interactionController.icons count]) > + return nullptr; How does this change relate to the rest? No comment in the change log to explain it. Definitely not obvious. Do we have to carefully call this before the extension is revoked? Also, UIImage * return value would normally be a nil, not *. Also, looks like there is a mistake in thumbnailSizedImageForImage, which should return a RetainPtr<UIImage> or should retain/autorelease the result of UIGraphicsGetImageFromCurrentImageContext. Given that name it doesn’t sound safe to keep the pointer around before deallocating the image context.
Created attachment 397732 [details] Patch
(In reply to Darin Adler from comment #5) > Comment on attachment 397511 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=397511&action=review > > > Source/WebKit/Shared/ios/WebIconUtilities.mm:95 > > + if (![interactionController.icons count]) > > + return nullptr; > > How does this change relate to the rest? No comment in the change log to > explain it. Definitely not obvious. > When running the test associated with this patch in the iOS simulator, I encountered an out-of-bounds array exception, which was fixed by the change above. I added a comment about this in the change log. > Do we have to carefully call this before the extension is revoked? > > Also, UIImage * return value would normally be a nil, not *. > Fixed. > Also, looks like there is a mistake in thumbnailSizedImageForImage, which > should return a RetainPtr<UIImage> or should retain/autorelease the result > of UIGraphicsGetImageFromCurrentImageContext. Given that name it doesn’t > sound safe to keep the pointer around before deallocating the image context. I changed the return type to RetainPtr<UIImage> in the most recent patch. This lead to a few other changes. I did not put the bug back into the Review state, but it would perhaps be good if you could have another quick look to make sure the changes look good to you. Thanks for reviewing!
Created attachment 397738 [details] Patch
Comment on attachment 397738 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397738&action=review > Source/WebKit/Shared/ios/WebIconUtilities.mm:86 > return resultImage; Need to change the resultImage local variable to a RetainPtr. If UIGraphicsEndImageContext released then image, then this return statement is too late.
Created attachment 397749 [details] Patch
(In reply to Darin Adler from comment #9) > Comment on attachment 397738 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=397738&action=review > > > Source/WebKit/Shared/ios/WebIconUtilities.mm:86 > > return resultImage; > > Need to change the resultImage local variable to a RetainPtr. If > UIGraphicsEndImageContext released then image, then this return statement is > too late. Good catch, thanks for reviewing!
Committed r260820: <https://trac.webkit.org/changeset/260820> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397749 [details].
<rdar://problem/62513703>