Bug 107398 - [GTK][GStreamer] FullscreenVideoControllerGtk implementation
Summary: [GTK][GStreamer] FullscreenVideoControllerGtk implementation
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on: 106760 107458
  Show dependency treegraph
Reported: 2013-01-20 07:12 PST by Philippe Normand
Modified: 2013-02-01 02:28 PST (History)
8 users (show)

See Also:

Patch (41.86 KB, patch)
2013-01-20 07:23 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (41.85 KB, patch)
2013-01-26 07:15 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (41.89 KB, patch)
2013-01-30 07:00 PST, Philippe Normand
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2013-01-20 07:12:07 PST
See also bug 106760
Comment 1 Philippe Normand 2013-01-20 07:23:51 PST
Created attachment 183672 [details]
Comment 2 Philippe Normand 2013-01-26 07:15:10 PST
Created attachment 184864 [details]
Comment 3 Philippe Normand 2013-01-30 07:00:42 PST
Created attachment 185497 [details]

Rebased against ToT
Comment 4 Gustavo Noronha (kov) 2013-01-31 05:11:31 PST
Comment on attachment 185497 [details]

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?
Comment 5 Philippe Normand 2013-01-31 06:16:17 PST
Hum but how would that work?

I expect each port to have its own FullscreenController implementation deriving from the base class. Having a Gtk subsclass kinda makes sense to me :)

I guess I'm misunderstanding your point
Comment 6 Philippe Normand 2013-02-01 02:21:18 PST
Carlos clarified this for me, the create method should be moved to the GStreamer base class, avoiding the ugly #define FullscreenVideoControllerClass in the player so I can directly use the base class there.
Comment 7 Philippe Normand 2013-02-01 02:27:25 PST
Committed r141566: <http://trac.webkit.org/changeset/141566>
Comment 8 Philippe Normand 2013-02-01 02:28:26 PST
Comment on attachment 185497 [details]

Clearing r+ flag of landed patch