Bug 130078

Summary:

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

Product:

WebKit

Reporter:

Carlos Garcia Campos <cgarcia>

Component:

Platform

Assignee:

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:

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

Carlos Garcia Campos

Reported 2014-03-11 02:40:04 PDT

ssia

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
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Carlos Garcia Campos

Comment 1 2014-03-11 02:41:33 PDT

Created attachment 226404 [details] Patch

WebKit Commit Bot

Comment 2 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.

Carlos Garcia Campos

Comment 3 2014-03-20 01:10:05 PDT

Created attachment 227270 [details] Rebased patch Updated the the changes in GMainLoopSource and fixed some edge cases

WebKit Commit Bot

Comment 4 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.

Carlos Garcia Campos

Comment 5 2014-03-20 01:28:05 PDT

Created attachment 227271 [details] Try to fix the build

Carlos Garcia Campos

Comment 6 2014-03-20 01:33:50 PDT

Comment on attachment 227271 [details] Try to fix the build Oops, I used the wrong flag

WebKit Commit Bot

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

Philippe Normand

Comment 8 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?

Carlos Garcia Campos

Comment 9 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.

Carlos Garcia Campos

Comment 10 2014-03-21 01:05:35 PDT

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

Csaba Osztrogonác

Comment 11 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. :-/

WebKit Commit Bot

Comment 12 2014-03-21 01:49:33 PDT

Re-opened since this is blocked by bug 130571

Carlos Garcia Campos

Comment 13 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.

Andres Gomez Garcia

Comment 14 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.

Carlos Garcia Campos

Comment 15 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.

WebKit Commit Bot

Comment 16 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.

Carlos Garcia Campos

Comment 17 2014-03-31 05:27:19 PDT

Comment on attachment 228175 [details] Patch for landing Oops, no need for r?

Carlos Garcia Campos

Comment 18 2014-03-31 05:33:30 PDT

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

Csaba Osztrogonác

Comment 19 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 >)'

Note You need to log in before you can comment on or make changes to this bug.