| Summary: | [GTK] Use GMainLoopSource for idle and timeout sources in WebKit2 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||
| Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | commit-queue, gustavo, pnormand, svillar | ||||||||
| Priority: | P2 | Keywords: | Gtk | ||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Bug Depends on: | 130027, 131220, 133348 | ||||||||||
| Bug Blocks: | 132326 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Carlos Garcia Campos
2014-03-11 02:52:49 PDT
Created attachment 226408 [details]
Patch
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.
Created attachment 227404 [details]
Rebased patch
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.
Created attachment 232121 [details]
Rebased patch
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 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 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. Committed r169423: <http://trac.webkit.org/changeset/169423> Re-opened since this is blocked by bug 133348 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. Committed r169444: <http://trac.webkit.org/changeset/169444> |