.
Created attachment 215006 [details] Replace 0 timeouts g_timeout_add() by g_idle_add() A zero timeout should be equivalent to using g_idle_add() without the nagging feeling that the wrong API was used.
Created attachment 215007 [details] Name all the GLib timeout sources See http://www.hadess.net/2013/10/reducing-wake-ups-2013-edition.html
Comment on attachment 215006 [details] Replace 0 timeouts g_timeout_add() by g_idle_add() Not exactly true, because g_idle and g_timeout use different priorities by default. g_timeout_add(0 is equivalent to g_idle_add_full(G_PRIORITY_DEFAULT. But I agree with you and I have changed some of those in the past.
Comment on attachment 215007 [details] Name all the GLib timeout sources View in context: https://bugs.webkit.org/attachment.cgi?id=215007&action=review We don't use usually attach more than one patch to the same bug report, could you please file a new bug report and upload an updated patch with the style issues fixed please? You can run Tools/Scripts/check-webkit-style script to make sure your patch doesn't have style issues before uploading them. Also, if you ask for review, setting the review flag to ?, the style bot will also run the style checker for you. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:415 > + g_source_set_name_by_id (m_readyTimerHandler, "[WebKit] mediaPlayerPrivateReadyStateTimeoutCallback"); WebKit coding style doesn't allow space between function name and parentheses. > Tools/DumpRenderTree/gtk/TestRunnerGtk.cpp:319 > + guint id; > + id = g_timeout_add_seconds(timeoutSeconds, waitToDumpWatchdogFired, 0); Use a single line here. > Tools/MiniBrowser/gtk/BrowserWindow.c:164 > + guint id; > + id = g_timeout_add(500, (GSourceFunc)resetEntryProgress, window->uriEntry); Ditto.
WebKit patches should include a ChangeLog entry, you can use Tools/Scripts/prepare-ChangeLog script to generate the changelog entry for your patch before submitting it. See also: http://www.webkit.org/coding/contributing.html Thanks the patches.
Created attachment 215042 [details] Name all the GLib timeout sources See http://www.hadess.net/2013/10/reducing-wake-ups-2013-edition.html
(In reply to comment #5) > WebKit patches should include a ChangeLog entry, you can use Tools/Scripts/prepare-ChangeLog script to generate the changelog entry for your patch before submitting it. I can't do that, I'm working off a tarball.
(In reply to comment #7) > (In reply to comment #5) > > WebKit patches should include a ChangeLog entry, you can use Tools/Scripts/prepare-ChangeLog script to generate the changelog entry for your patch before submitting it. > > I can't do that, I'm working off a tarball. Ok, I'll add the changelog entry for you then
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #5) > > > WebKit patches should include a ChangeLog entry, you can use Tools/Scripts/prepare-ChangeLog script to generate the changelog entry for your patch before submitting it. > > > > I can't do that, I'm working off a tarball. > > Ok, I'll add the changelog entry for you then Thanks. Bug 123260 will have the same problem.
Carlos, I can take care of this if you haven't already?
(In reply to comment #10) > Carlos, I can take care of this if you haven't already? Of course, I didn't have time yesterday in the end.
Created attachment 215288 [details] patch
Comment on attachment 215288 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=215288&action=review > Source/WebCore/platform/gtk/SharedTimerGtk.cpp:61 > + g_source_set_name_by_id(sharedTimer, "[WebKit] timeout_cb"); I wonder how useful the name is in cases like this one where a generic name is used. Maybe we could use the class name SharedTimer::timeout_cb, or simply rename these callbacks to something more descriptive (in this case, it doesn't even follow the webkit style), like sharedTimerTimeoutCallback.
Created attachment 215292 [details] patch
Comment on attachment 215292 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=215292&action=review > Source/WebCore/ChangeLog:8 > + Give a name to GLiib timeout sources, this is helpful when Typo, GLiib.
Created attachment 215307 [details] Patch for landing
Comment on attachment 215307 [details] Patch for landing Clearing flags on attachment: 215307 Committed r158110: <http://trac.webkit.org/changeset/158110>
All reviewed patches have been landed. Closing bug.