[webkit-reviews] review denied: [Bug 57434] [GTK] Implement scheduleWorkAfterDelay() in WorkQueueGtk : [Attachment 87487] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Mar 30 09:51:09 PDT 2011
Martin Robinson <mrobinson at webkit.org> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 57434: [GTK] Implement scheduleWorkAfterDelay() in WorkQueueGtk
https://bugs.webkit.org/show_bug.cgi?id=57434
Attachment 87487: Patch
https://bugs.webkit.org/attachment.cgi?id=87487&action=review
------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=87487&action=review
Great patch. I'd r+ it except for the missing ChangeLog information.
> Source/WebKit2/ChangeLog:19
> + * Platform/gtk/WorkQueueGtk.cpp:
> + (WorkQueue::EventSource::executeEventSource):
> + (WorkQueue::EventSource::performWorkOnce):
> + (WorkQueue::EventSource::performWork):
> + (WorkQueue::workQueueThreadBody):
> + (WorkQueue::registerEventSourceHandler):
> + (WorkQueue::unregisterEventSourceHandler):
> + (WorkQueue::scheduleWorkOnSource):
> + (WorkQueue::scheduleWork):
You should fill these out.
> Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:139
> {
> - GIOChannel* channel = g_io_channel_unix_new(fileDescriptor);
> - ASSERT(channel);
> - GSource* dispatchSource = g_io_create_watch(channel,
static_cast<GIOCondition>(condition));
> + GRefPtr<GSocket> socket = adoptGRef(g_socket_new_from_fd(fileDescriptor,
0));
> + ASSERT(socket);
> + GSource* dispatchSource = g_socket_create_source(socket.get(),
static_cast<GIOCondition>(condition), 0);
For instance here in the ChangeLog it would be useful to explain why a GSocket
is preferred to a GIOChannel.
> Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:195
> + GRefPtr<GSource> dispatchSource = adoptGRef(g_idle_source_new());
In the past we've had issues with starvation of sources created with
g_idle_add. We might want to stick with a timeout source here. Gustavo has more
information about this, I believe.
More information about the webkit-reviews
mailing list