[webkit-reviews] review denied: [Bug 69410] [WK2] [GTK] Implement KeyDown function for WebKit2 EventSender. : [Attachment 111951] Updated Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Kaustubh Atrawalkar
<kaustubh at motorola.com>'s request for review:
Bug 69410: [WK2] [GTK] Implement KeyDown function for WebKit2 EventSender.
https://bugs.webkit.org/show_bug.cgi?id=69410

Attachment 111951: Updated Patch
https://bugs.webkit.org/attachment.cgi?id=111951&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=111951&action=review


Looks good, but I have a couple questions below.

> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:63
> +bool dispatchEvent(GdkEvent* event)
> +{
> +    gtk_main_do_event(event);
> +    gdk_event_free(event);
> +    return 0;
> +}

This function should be static. Why does it return a bool, while you
unconditionally return 0?

> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:193
> +    GdkKeymapKey* keys;
> +    gint nKeys;
> +    if (gdk_keymap_get_entries_for_keyval(gdk_keymap_get_default(),
gdkKeySym, &keys, &nKeys)) {
> +	   pressEvent->key.hardware_keycode = keys[0].keycode;
> +	   g_free(keys);
> +    }

Can you use GOwnPtr and .outPtr() for keys?