There are only a few places where we still use GMainLoopSource, once we migrate those, we could remove GMainLoopSource entirely.
Created attachment 266196 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment on attachment 266196 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266196&action=review > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestResources.cpp:714 > + WTF::GMutexLocker<GMutex>* lock; This can be a simple reference instead of a pointer. > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestResources.cpp:719 > + auto* context = static_cast<UnlockServerSourceContext*>(userData); You can dereference the casted user data immediately and bind it to a reference (auto&), instead of a pointer. > Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp:80 > + g_source_set_ready_time(timeoutSource(), g_get_monotonic_time() + std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(timeout)).count()); That summation can overflow. It likely won't, but it's possible. Also, using std::chrono also maybe isn't a requirement here, G_USEC_PER_SEC would work just fine.
Comment on attachment 266196 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266196&action=review >> Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp:80 >> + g_source_set_ready_time(timeoutSource(), g_get_monotonic_time() + std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(timeout)).count()); > > That summation can overflow. It likely won't, but it's possible. Also, using std::chrono also maybe isn't a requirement here, G_USEC_PER_SEC would work just fine. The complete overflow protection (the one that roundtrips to std::chrono::microseconds) could be abstracted and put in WTF, in a small header. It's now used in maybe 5 places in the code. Otherwise the patch looks fine.
Comment on attachment 266196 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266196&action=review >>> Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp:80 >>> + g_source_set_ready_time(timeoutSource(), g_get_monotonic_time() + std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::duration<double>(timeout)).count()); >> >> That summation can overflow. It likely won't, but it's possible. Also, using std::chrono also maybe isn't a requirement here, G_USEC_PER_SEC would work just fine. > > The complete overflow protection (the one that roundtrips to std::chrono::microseconds) could be abstracted and put in WTF, in a small header. It's now used in maybe 5 places in the code. > > Otherwise the patch looks fine. I didn't add the overflow check because here the timeout is a fixed value, not something calculated, but I agree it would be better to move it a common place and use it everywhere else. Thanks for the review.
Committed r192782: <http://trac.webkit.org/changeset/192782>