WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(5.02 KB, patch)
2019-09-27 17:31 PDT
,
Megan Gardner
no flags
Details
Formatted Diff
Diff
Patch
(4.99 KB, patch)
2019-10-07 12:02 PDT
,
Megan Gardner
no flags
Details
Formatted Diff
Diff
Patch for landing
(5.02 KB, patch)
2019-10-07 15:12 PDT
,
Megan Gardner
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Megan Gardner
Comment 1
2019-09-27 17:18:38 PDT
Created
attachment 379775
[details]
Patch
Megan Gardner
Comment 2
2019-09-27 17:31:07 PDT
Created
attachment 379778
[details]
Patch
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
Created
attachment 380342
[details]
Patch
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
<
rdar://problem/56052711
>
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.
Top of Page
Format For Printing
XML
Clone This Bug