[webkit-reviews] review denied: [Bug 172048] Add objc version of WK2 UIPageClient setHasVideoInPictureInPicture : [Attachment 309944] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Jeremy Jones
<jeremyj-wk at apple.com>'s request for review:
Bug 172048: Add objc version of WK2 UIPageClient setHasVideoInPictureInPicture
https://bugs.webkit.org/show_bug.cgi?id=172048

Attachment 309944: Patch

https://bugs.webkit.org/attachment.cgi?id=309944&action=review




--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 309944
  --> https://bugs.webkit.org/attachment.cgi?id=309944
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=309944&action=review

> Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegatePrivate.h:63
> +- (void)_webView:(WKWebView *)webView
setHasVideoInPictureInPicture:(BOOL)hasVideoInPictureInPicture
WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));

This is a weird signature for a delegate callback. It should be more like:

- (void)_webViewDidGainPictureInPictureVideo:(WKWebView *)webView
- (void)_webViewDidLosePictureInPictureVideo:(WKWebView *)webView

or 
- (void)_webView:(WKWebView *)webView
hasVideoInPictureInPictureChangedTo:(BOOL)hasPIP

or something.

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.h:114
> +	   void setHasVideoInPictureInPicture(WebKit::WebPageProxy*, bool)
override;

Should be be a "set".