Bug 130081 – [GTK] Use GMainLoopSource for idle and timeout sources in WebKit2

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Carlos Garcia Campos]

ssia

[Carlos Garcia Campos]

Created attachment 226408 [details]
Patch

[WebKit Commit Bot]

Attachment 226408 [details] did not pass style-queue:


ERROR: Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:112:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Created attachment 227404 [details]
Rebased patch

[WebKit Commit Bot]

Attachment 227404 [details] did not pass style-queue:


ERROR: Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:112:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Created attachment 232121 [details]
Rebased patch

[WebKit Commit Bot]

Attachment 232121 [details] did not pass style-queue:


ERROR: Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:112:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

Comment on attachment 232121 [details]
Rebased patch

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

Nice. Just consider fixing the small style nit and the source naming issue that we discussed earlier when landing.

> Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:89
> +        if (condition & G_IO_HUP || condition & G_IO_ERR || condition & G_IO_NVAL) {
> +            closeFunction();
> +            return GMainLoopSource::Stop;
> +        }

Nit: Maybe one more level of indentation here?

> Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:98
> +        }, socket.get(), G_IO_IN,

It might make this a little less strange.

[Martin Robinson]

Comment on attachment 232121 [details]
Rebased patch

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

Nice. Just consider fixing the small style nit and the source naming issue that we discussed earlier when landing.

> Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:89
> +        if (condition & G_IO_HUP || condition & G_IO_ERR || condition & G_IO_NVAL) {
> +            closeFunction();
> +            return GMainLoopSource::Stop;
> +        }

Nit: Maybe one more level of indentation here?

> Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:98
> +        }, socket.get(), G_IO_IN,

It might make this a little less strange.

[Carlos Garcia Campos]

Committed r169423: <http://trac.webkit.org/changeset/169423>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 133348

[Carlos Garcia Campos]

I'm pretty sure the assert is harmless, I've been using this patch for a long time with release builds and it has never crashed. I think the problem is in GMainLoopSource itself, not in this patch, or simply the assert is wrong. I'll debug it.

[Carlos Garcia Campos]

See also https://bugs.webkit.org/show_bug.cgi?id=131220

[Carlos Garcia Campos]

Committed r169444: <http://trac.webkit.org/changeset/169444>