Switch to WKShareSheet for WKPDFView

• Component: New Bugs
• Status: RESOLVED FIXED

[Megan Gardner]

Switch to WKShareSheet for WKPDFView

[Megan Gardner]

Created attachment 379775 [details]
Patch

[Megan Gardner]

Created attachment 379778 [details]
Patch

[Daniel Bates]

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]

Created attachment 380342 [details]
Patch

[Tim Horton]

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 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]

Created attachment 380366 [details]
Patch for landing

[Radar WebKit Bug Importer]

<rdar://problem/56052711>

[WebKit Commit Bot]

Comment on attachment 380366 [details]
Patch for landing

Clearing flags on attachment: 380366

Committed r250801: <https://trac.webkit.org/changeset/250801>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.