Bug 130078 - [GTK] Use GMainLoopSource for idle and timeout sources in WebCore
Summary: [GTK] Use GMainLoopSource for idle and timeout sources in WebCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on: 130027 130571 130585
Blocks:
  Show dependency treegraph
 
Reported: 2014-03-11 02:40 PDT by Carlos Garcia Campos
Modified: 2014-03-31 08:21 PDT (History)
17 users (show)

See Also:


Attachments
Patch (63.19 KB, patch)
2014-03-11 02:41 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Rebased patch (64.50 KB, patch)
2014-03-20 01:10 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Try to fix the build (64.50 KB, patch)
2014-03-20 01:28 PDT, Carlos Garcia Campos
pnormand: review+
pnormand: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (60.77 KB, patch)
2014-03-31 05:24 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 >)'