[webkit-reviews] review granted: [Bug 133850] [GTK] Implement button-press-event, button-release-event, and absolute-axis-event of GAMEPAD API. : [Attachment 401508] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted ChangSeok Oh
<changseok at webkit.org>'s request for review:
Bug 133850: [GTK] Implement button-press-event, button-release-event, and
absolute-axis-event of GAMEPAD API.
https://bugs.webkit.org/show_bug.cgi?id=133850

Attachment 401508: Patch

https://bugs.webkit.org/attachment.cgi?id=401508&action=review




--- Comment #12 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 401508
  --> https://bugs.webkit.org/attachment.cgi?id=401508
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401508&action=review

> Source/WebCore/platform/gamepad/manette/ManetteGamepad.cpp:32
> +#include <linux/input-event-codes.h>

I didn't notice this in the previous revision. Is gamepad only enabled for
linux?

> Source/WebCore/platform/gamepad/manette/ManetteGamepad.cpp:150
> +    if (m_buttonPressEventHandlerID)
> +	   g_signal_handler_disconnect(m_device.get(),
m_buttonPressEventHandlerID);
> +    if (m_buttonReleaseEventHandlerID)
> +	   g_signal_handler_disconnect(m_device.get(),
m_buttonReleaseEventHandlerID);
> +    if (m_absoluteAxisEventHandlerID)
> +	   g_signal_handler_disconnect(m_device.get(),
m_absoluteAxisEventHandlerID);

You can do g_signal_handlers_disconnect_by_data(m_device.get(), this); and then
you don't need to keep the ids.

> Source/WebCore/platform/gamepad/manette/ManetteGamepadProvider.cpp:76
> +    g_signal_handlers_disconnect_by_func(m_monitor.get(),
reinterpret_cast<gpointer>(onDeviceConnected), this);
> +    g_signal_handlers_disconnect_by_func(m_monitor.get(),
reinterpret_cast<gpointer>(onDeviceDisconnected), this);

Same here, better use by_data().