[Webkit-unassigned] [Bug 57434] [GTK] Implement scheduleWorkAfterDelay() in WorkQueueGtk
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Mar 30 09:58:58 PDT 2011
https://bugs.webkit.org/show_bug.cgi?id=57434
--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com> 2011-03-30 09:58:58 PST ---
(In reply to comment #2)
> (From update of attachment 87487 [details])
> 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.
Ok, I did it too fast.
> > 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.
GSocket api is newer and allow passing a cancellable to the source, so that we can cacel the source from any thread. I have a patch on top of this one that implements also void WorkQueue::scheduleWorkOnTermination() and uses a cancellable to cancel the source when the webprocess dies.
> > 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.
I change the priority to avoid issues, I think that a timeout source with 0 it's mostly the same than an idle source with a default priority, but I'm not sure. I haven't seen any problems in my tests.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list