[webkit-reviews] review denied: [Bug 75553] [GTK][WK2] Initial FullScreen support : [Attachment 124729] Initial FullScreen support

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 75553: [GTK][WK2] Initial FullScreen support
https://bugs.webkit.org/show_bug.cgi?id=75553

Attachment 124729: Initial FullScreen support
https://bugs.webkit.org/attachment.cgi?id=124729&action=review

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


> Source/WebKit2/UIProcess/gtk/FullScreenWindowController.cpp:32
> +#include <WebCore/PlatformScreen.h>
> +
> +#include <gdk/gdk.h>

Please remove the extra newline here.

> Source/WebKit2/UIProcess/gtk/FullScreenWindowController.h:48
> +    GRefPtr<WebKitWebViewBase> m_webViewBase;

This actually means that all WebKitWebViewBase will leak because the
WebViewBase owns the fullscreen controller and the fullscreen controller holds
a reference. :)

> Source/WebKit2/UIProcess/gtk/WebFullScreenManagerProxyGtk.cpp:32
>  #include "WebProcess.h"
>  
>  #include <WebCore/NotImplemented.h>

Please remove the extra newline here.

> Source/WebKit2/UIProcess/gtk/WebFullScreenManagerProxyGtk.cpp:60
> +    WebKitWebViewBase* webViewBase = WEBKIT_WEB_VIEW_BASE(m_webView);
> +    FullScreenWindowController& controller =
webkitWebViewBaseFullscreenWindowController(webViewBase);
> +    controller.exitFullScreen();

This can be one line.

> Source/WebKit2/UIProcess/gtk/WebFullScreenManagerProxyGtk.cpp:100
> +    FullScreenWindowController& controller =
webkitWebViewBaseFullscreenWindowController(webViewBase);
> +    rect = controller.getFullScreenRect();

Ditto.