[webkit-reviews] review granted: [Bug 125427] [Cocoa] Remove knowledge of the WKView from the WebFullScreenManagerProxy by adding a proper client : [Attachment 218716] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org <mitz at webkit.org> has granted Sam Weinig <sam at webkit.org>'s
request for review:
Bug 125427: [Cocoa] Remove knowledge of the WKView from the
WebFullScreenManagerProxy by adding a proper client
https://bugs.webkit.org/show_bug.cgi?id=125427

Attachment 218716: Patch
https://bugs.webkit.org/attachment.cgi?id=218716&action=review

------- Additional Comments from mitz at webkit.org <mitz at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=218716&action=review


> Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm:567
> +#endif
> +
> +#if ENABLE(FULLSCREEN_API)

This is unnecessary

> Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm:580
> +    if (!m_wkView.hasFullScreenWindowController)
> +	   return false;
> +

Is this part necessary?

> Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm:603
> +#endif

This #endif could use a newline before it and a comment after it.

> Source/WebKit2/UIProcess/API/mac/WKViewInternal.h:98
> + at property (readonly) BOOL hasFullScreenWindowController;
> + at property (readonly) WKFullScreenWindowController
*fullScreenWindowController;
> +- (void)closeFullScreenWindowController;

As internal methods and properties, these should probably be prefixed with an
underscore.