Bug 130081 - [GTK] Use GMainLoopSource for idle and timeout sources in WebKit2
Summary: [GTK] Use GMainLoopSource for idle and timeout sources in WebKit2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on: 130027 131220 133348
Blocks: 132326
  Show dependency treegraph
 
Reported: 2014-03-11 02:52 PDT by Carlos Garcia Campos
Modified: 2014-05-29 03:33 PDT (History)
4 users (show)

See Also:


Attachments
Patch (18.68 KB, patch)
2014-03-11 02:54 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Rebased patch (18.87 KB, patch)
2014-03-21 02:43 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Rebased patch (18.96 KB, patch)
2014-05-27 03:25 PDT, Carlos Garcia Campos
mrobinson: review+
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:52:49 PDT
ssia
Comment 1 Carlos Garcia Campos 2014-03-11 02:54:14 PDT
Created attachment 226408 [details]
Patch
Comment 2 WebKit Commit Bot 2014-03-11 02:55:29 PDT
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.
Comment 3 Carlos Garcia Campos 2014-03-21 02:43:26 PDT
Created attachment 227404 [details]
Rebased patch
Comment 4 WebKit Commit Bot 2014-03-21 02:45:19 PDT
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.
Comment 5 Carlos Garcia Campos 2014-05-27 03:25:01 PDT
Created attachment 232121 [details]
Rebased patch
Comment 6 WebKit Commit Bot 2014-05-27 03:28:18 PDT
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.
Comment 7 Martin Robinson 2014-05-28 06:51:32 PDT
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.
Comment 8 Martin Robinson 2014-05-28 06:51:32 PDT
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.
Comment 9 Carlos Garcia Campos 2014-05-28 08:35:31 PDT
Committed r169423: <http://trac.webkit.org/changeset/169423>
Comment 10 WebKit Commit Bot 2014-05-28 09:19:55 PDT
Re-opened since this is blocked by bug 133348
Comment 11 Carlos Garcia Campos 2014-05-28 09:22:43 PDT
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.
Comment 12 Carlos Garcia Campos 2014-05-28 09:23:06 PDT
See also https://bugs.webkit.org/show_bug.cgi?id=131220
Comment 13 Carlos Garcia Campos 2014-05-29 03:33:40 PDT
Committed r169444: <http://trac.webkit.org/changeset/169444>