RESOLVED FIXED 202338
Switch to WKShareSheet for WKPDFView
https://bugs.webkit.org/show_bug.cgi?id=202338
Summary Switch to WKShareSheet for WKPDFView
Megan Gardner
Reported 2019-09-27 17:15:54 PDT
Switch to WKShareSheet for WKPDFView
Attachments
Patch (3.99 KB, patch)
2019-09-27 17:18 PDT, Megan Gardner
no flags
Patch (5.02 KB, patch)
2019-09-27 17:31 PDT, Megan Gardner
no flags
Patch (4.99 KB, patch)
2019-10-07 12:02 PDT, Megan Gardner
no flags
Patch for landing (5.02 KB, patch)
2019-10-07 15:12 PDT, Megan Gardner
no flags
Megan Gardner
Comment 1 2019-09-27 17:18:38 PDT
Megan Gardner
Comment 2 2019-09-27 17:31:07 PDT
Daniel Bates
Comment 3 2019-09-29 10:04:52 PDT
Comment on attachment 379778 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379778&action=review This is a good first stab. It looks like this patch has a use-after-free because: 1. The allocated WKShareSheet is assigned to a local RetainPtr. 2. _shareSheet is never assigned a non-nil value > Source/WebKit/UIProcess/ios/WKPDFView.mm:560 > + if (_shareSheet) Ok as-is. The optimal solution would be to unconditionally call -dismiss because: 1. Sending a message to a possible nil object is okay in Obj-C. 2. Because of (1) a branch can be removed.
Megan Gardner
Comment 4 2019-10-07 12:02:20 PDT
Tim Horton
Comment 5 2019-10-07 14:18:56 PDT
Comment on attachment 380342 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380342&action=review > Source/WebKit/ChangeLog:10 > + us to eliminate this mostly unused class. Now entirely unused? > Source/WebKit/UIProcess/ios/WKPDFView.h:32 > +@interface WKPDFView : WKApplicationStateTrackingView <WKWebViewContentProvider, WKShareSheetDelegate> Keep these sorted :P > Source/WebKit/UIProcess/ios/WKPDFView.mm:564 > + [_shareSheet presentWithParameters:shareData inRect: { [[_hostViewController view] convertRect:boundingRect toView:webView] } completionHandler:[] (bool success) { }]; Is there not a way to pass a null completion handler? Maybe not.
Megan Gardner
Comment 6 2019-10-07 15:11:21 PDT
Comment on attachment 380342 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380342&action=review >> Source/WebKit/UIProcess/ios/WKPDFView.mm:564 >> + [_shareSheet presentWithParameters:shareData inRect: { [[_hostViewController view] convertRect:boundingRect toView:webView] } completionHandler:[] (bool success) { }]; > > Is there not a way to pass a null completion handler? Maybe not. I took this from the calls in WKContentViewInteraction.
Megan Gardner
Comment 7 2019-10-07 15:12:10 PDT
Created attachment 380366 [details] Patch for landing
Radar WebKit Bug Importer
Comment 8 2019-10-07 15:13:00 PDT
WebKit Commit Bot
Comment 9 2019-10-07 15:53:33 PDT
Comment on attachment 380366 [details] Patch for landing Clearing flags on attachment: 380366 Committed r250801: <https://trac.webkit.org/changeset/250801>
WebKit Commit Bot
Comment 10 2019-10-07 15:53:34 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.