Bug 130078 – [GTK] Use GMainLoopSource for idle and timeout sources in WebCore

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Carlos Garcia Campos]

ssia

[Carlos Garcia Campos]

Created attachment 226404 [details]
Patch

[WebKit Commit Bot]

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.

[Carlos Garcia Campos]

Created attachment 227270 [details]
Rebased patch

Updated the the changes in GMainLoopSource and fixed some edge cases

[WebKit Commit Bot]

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.

[Carlos Garcia Campos]

Created attachment 227271 [details]
Try to fix the build

[Carlos Garcia Campos]

Comment on attachment 227271 [details]
Try to fix the build

Oops, I used the wrong flag

[WebKit Commit Bot]

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.

[Philippe Normand]

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?

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

Committed r166052: <http://trac.webkit.org/changeset/166052>

[Csaba Osztrogonác]

(In reply to comment #10)
> Committed r166052: <http://trac.webkit.org/changeset/166052>

It broke the EFL build as the EWS noticed it. :-/

[WebKit Commit Bot]

Re-opened since this is blocked by bug 130571

[Carlos Garcia Campos]

(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.

[Andres Gomez Garcia]

We need to decide first whether to bump the GCC version or use explicit casts.

Blocking on bug 130585.

[Carlos Garcia Campos]

Created attachment 228175 [details]
Patch for landing

Let's try again now that EFL bots have been upgraded.

[WebKit Commit Bot]

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.

[Carlos Garcia Campos]

Comment on attachment 228175 [details]
Patch for landing

Oops, no need for r?

[Carlos Garcia Campos]

Committed r166496: <http://trac.webkit.org/changeset/166496>

[Csaba Osztrogonác]

(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 >)'