Bug 130078

Summary: [GTK] Use GMainLoopSource for idle and timeout sources in WebCore
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: agomez, bfulgham, calvaris, cdumez, commit-queue, eric.carlson, glenn, gustavo, jer.noble, menard, mrobinson, ossy, peavo, philipj, pnormand, sergio, vjaquez
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 130027, 130571, 130585    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Rebased patch
none
Try to fix the build
pnormand: review+, pnormand: commit-queue-
Patch for landing none

Description Carlos Garcia Campos 2014-03-11 02:40:04 PDT 
ssia
Comment 1 Carlos Garcia Campos 2014-03-11 02:41:33 PDT 
Created attachment 226404 [details]
Patch
Comment 2 WebKit Commit Bot 2014-03-11 02:43:01 PDT 
Attachment 226404 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:374:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:378:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:388:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:392:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:532:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:551:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:588:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:608:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:649:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:682:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:539:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:547:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:689:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:719:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:748:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:218:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 16 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Carlos Garcia Campos 2014-03-20 01:10:05 PDT 
Created attachment 227270 [details]
Rebased patch

Updated the the changes in GMainLoopSource and fixed some edge cases
Comment 4 WebKit Commit Bot 2014-03-20 01:11:28 PDT 
Attachment 227270 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:374:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:378:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:388:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:392:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:532:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:551:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:588:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:608:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:650:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:683:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:536:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:544:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:686:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:716:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:745:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:219:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 16 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Carlos Garcia Campos 2014-03-20 01:28:05 PDT 
Created attachment 227271 [details]
Try to fix the build
Comment 6 Carlos Garcia Campos 2014-03-20 01:33:50 PDT 
Comment on attachment 227271 [details]
Try to fix the build

Oops, I used the wrong flag
Comment 7 WebKit Commit Bot 2014-03-20 01:34:29 PDT 
Attachment 227271 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:370:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:374:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:384:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:388:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:528:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:547:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:584:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:604:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:646:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:679:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:536:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:544:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:686:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:716:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:745:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:219:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 16 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Philippe Normand 2014-03-20 02:01:21 PDT 
Comment on attachment 227271 [details]
Try to fix the build

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

> Source/WebCore/ChangeLog:8
> +        * platform/audio/gstreamer/AudioFileReaderGStreamer.cpp:

All this can be removed if you don't plan to comment each change :)

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:-145
> -    // Reset pipeline if we are sitting on READY state when timeout is reached

Can this comment be kept in the new code please?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:33
> +#include <wtf/gobject/GMainLoopSource.h>

Can the class be forward-declared instead?
Comment 9 Carlos Garcia Campos 2014-03-20 03:57:28 PDT 
Comment on attachment 227271 [details]
Try to fix the build

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

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:-145
>> -    // Reset pipeline if we are sitting on READY state when timeout is reached
> 
> Can this comment be kept in the new code please?

There's already a comment about this where the new code is, see:

// Create a timer when entering the READY state so that we can free resources                                                                                                             
// if we stay for too long on READY.                                                                                                                                                      
// Also lets remove the timer if we request a state change for any state other than READY.                                                                                                
// See also https://bugs.webkit.org/show_bug.cgi?id=117354

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.h:33
>> +#include <wtf/gobject/GMainLoopSource.h>
> 
> Can the class be forward-declared instead?

I don't think so, members are stack allocated.
Comment 10 Carlos Garcia Campos 2014-03-21 01:05:35 PDT 
Committed r166052: <http://trac.webkit.org/changeset/166052>
Comment 11 Csaba Osztrogonác 2014-03-21 01:27:40 PDT 
(In reply to comment #10)
> Committed r166052: <http://trac.webkit.org/changeset/166052>

It broke the EFL build as the EWS noticed it. :-/
Comment 12 WebKit Commit Bot 2014-03-21 01:49:33 PDT 
Re-opened since this is blocked by bug 130571
Comment 13 Carlos Garcia Campos 2014-03-21 01:50:39 PDT 
(In reply to comment #11)
> (In reply to comment #10)
> > Committed r166052: <http://trac.webkit.org/changeset/166052>
> 
> It broke the EFL build as the EWS noticed it. :-/

Yes, I know, as we discussed in bug #130027, tis is due to the gcc version used. I assumed EFL bots had a higer gcc version than the EWS bots. I'm rolling it out.
Comment 14 Andres Gomez Garcia 2014-03-21 07:21:39 PDT 
We need to decide first whether to bump the GCC version or use explicit casts.

Blocking on bug 130585.
Comment 15 Carlos Garcia Campos 2014-03-31 05:24:38 PDT 
Created attachment 228175 [details]
Patch for landing

Let's try again now that EFL bots have been upgraded.
Comment 16 WebKit Commit Bot 2014-03-31 05:26:07 PDT 
Attachment 228175 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:370:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:374:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:384:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:388:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:528:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:547:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:584:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:604:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:646:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitMediaSourceGStreamer.cpp:679:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:536:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:544:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:686:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:716:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:745:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:219:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 16 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Carlos Garcia Campos 2014-03-31 05:27:19 PDT 
Comment on attachment 228175 [details]
Patch for landing

Oops, no need for r?
Comment 18 Carlos Garcia Campos 2014-03-31 05:33:30 PDT 
Committed r166496: <http://trac.webkit.org/changeset/166496>
Comment 19 Csaba Osztrogonác 2014-03-31 08:21:58 PDT 
(In reply to comment #18)
> Committed r166496: <http://trac.webkit.org/changeset/166496>

It broke the WinCairo build, cc the WinCairo maintainers.

    1>..\platform\graphics\gstreamer\InbandTextTrackPrivateGStreamer.cpp(83): error C2668: 'WTF::GMainLoopSource::schedule' : ambiguous call to overloaded function
                 C:\Projects\BuildSlave\win-cairo-release\build\WebKitBuild\Release_WinCairo\include\private\wtf/gobject/GMainLoopSource.h(54): could be 'void WTF::GMainLoopSource::schedule(const char *,std::function<bool (void)>,int,std::function<void (void)>,GMainContext *)'
                 C:\Projects\BuildSlave\win-cairo-release\build\WebKitBuild\Release_WinCairo\include\private\wtf/gobject/GMainLoopSource.h(53): or       'void WTF::GMainLoopSource::schedule(const char *,std::function<void (void)>,int,std::function<void (void)>,GMainContext *)'
                 while trying to match the argument list '(const char [62], std::_Bind<true,void,std::_Pmf_wrap<void (__thiscall WebCore::InbandTextTrackPrivateGStreamer::* )(void),void,WebCore::InbandTextTrackPrivateGStreamer,>,WebCore::InbandTextTrackPrivateGStreamer *const >)'