[Webkit-unassigned] [Bug 130027] [GLIB] Add GMainLoopSource class to wrap idle and timeout sources

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=130027





--- Comment #30 from Carlos Garcia Campos <cgarcia at igalia.com>  2014-03-17 02:20:49 PST ---
(In reply to comment #29)
> (From update of attachment 226675 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226675&action=review
> 
> >> Source/WTF/wtf/gobject/GMainLoopSource.cpp:46
> >> +}
> > 
> > This is quite clever.
> 
> Just thinking about this, the ::create thing is a bit confusing, because it doesn't actually return something that you are supposed to delete (like other create methods).

hmm, this is a good point.

> I wonder if you can just make static versions of the schedule methods that take care of creating the source? So when you call it, it would look something like:
> 
> GMainLoopSource::schedule("[WebKit] RunLoop work", std::bind(&RunLoop::performWork, this), G_PRIORITY_DEFAULT, [this]() { deref(); });
> 
> One issue with this is that "schedule" cannot be both static and an instance method, so we'd need to pick one name for the static version and one for the instance version.

I think this is a good idea, the problem is indeed choosing good names :-) scheduleAndDeleteOnDestroy? scheduleDeletingOnDestroy? ...

> I think another option would be to rename "create" to something else that makes it clear that this isn't a traditional factory method.

Well, I agree this is not consistent with other cases where we use create(), but we are actually creating a new object, so I can't think of a better name. createdAutoDeleted? ...

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.