[webkit-reviews] review granted: [Bug 107398] [GTK][GStreamer] FullscreenVideoControllerGtk implementation : [Attachment 185497] Patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 107398: [GTK][GStreamer] FullscreenVideoControllerGtk implementation
https://bugs.webkit.org/show_bug.cgi?id=107398

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

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=185497&action=review


> Source/WebCore/platform/graphics/gtk/FullscreenVideoControllerGtk.h:82
> +    unsigned long m_exitFullcreenActionActivateSignalHandler;

Two naming nits about this class: these really aren't handlers but rather ids,
so how about s/Handler/ID/? And secondly, keeping Gtk out of this class' name
would avoid having an #if PLATFORM() block for each of GStreamer-based ports in
the MediaPlayerPrivate part of this patch, wouldn't it?