It looks GTK port DRT doesn't support scheduleAsynchronousKeyDown yet, so it causes some layout test failures
Created attachment 127978 [details] Patch
Comment on attachment 127978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127978&action=review r-, although it's just for a few nitpicks and a possible useless include, I think you'll prefer to upload a new patch anyway? =) > Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:53 > +#include "PlatformKeyboardEvent.h" Why do we need this extra include? > Tools/DumpRenderTree/gtk/EventSender.cpp:761 > + sendKeyDown(static_cast<gpointer>(pressEvent)); I think I would prefer to have a proper function with the appropriate type for the argument that is called from here and from the idle callback.
Comment on attachment 127978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127978&action=review > Tools/DumpRenderTree/gtk/EventSender.cpp:834 > + g_idle_add(sendKeyDown, static_cast<gpointer>(pressEvent)); Perhaps g_timeout_add after a 0 delay would be better here?
Comment on attachment 127978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127978&action=review Thanks for the review :) >> Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:53 >> +#include "PlatformKeyboardEvent.h" > > Why do we need this extra include? Oops. Sorry :p >> Tools/DumpRenderTree/gtk/EventSender.cpp:761 >> + sendKeyDown(static_cast<gpointer>(pressEvent)); > > I think I would prefer to have a proper function with the appropriate type for the argument that is called from here and from the idle callback. Do you mean it looks better to wrap sendKeyDown and g_idle_add(sendKeyDown... in a function with arguments? For example, bool sendKeyDown(bool asynchronous)?
Comment on attachment 127978 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=127978&action=review >> Tools/DumpRenderTree/gtk/EventSender.cpp:834 >> + g_idle_add(sendKeyDown, static_cast<gpointer>(pressEvent)); > > Perhaps g_timeout_add after a 0 delay would be better here? I followed the way of scheduleAsynchronousClickCallback. It has used g_idle_add. Do we need to modify this, too? :)
Created attachment 128132 [details] Patch
(In reply to comment #4) > >> Tools/DumpRenderTree/gtk/EventSender.cpp:761 > >> + sendKeyDown(static_cast<gpointer>(pressEvent)); > > > > I think I would prefer to have a proper function with the appropriate type for the argument that is called from here and from the idle callback. > > Do you mean it looks better to wrap sendKeyDown and g_idle_add(sendKeyDown... in a function with arguments? > For example, bool sendKeyDown(bool asynchronous)? No, I mean sendKeyDown should take GdkEvent* pressEvent as the only argument. You would call it directly here, and you would have a handler for the idle that would also call it.
Created attachment 128229 [details] Patch
Comment on attachment 128229 [details] Patch Yep! Thanks
Comment on attachment 128229 [details] Patch Clearing flags on attachment: 128229 Committed r108628: <http://trac.webkit.org/changeset/108628>
All reviewed patches have been landed. Closing bug.