[webkit-reviews] review granted: [Bug 173419] [WPE] Implement EventSenderProxy in WTR : [Attachment 316543] Patch

[bugzilla-daemon at webkit.org]

Zan Dobersek <zan at falconsigh.net> has granted Claudio Saavedra
<csaavedra at igalia.com>'s request for review:
Bug 173419: [WPE] Implement EventSenderProxy in WTR
https://bugs.webkit.org/show_bug.cgi?id=173419

Attachment 316543: Patch

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




--- Comment #5 from Zan Dobersek <zan at falconsigh.net> ---
Comment on attachment 316543
  --> https://bugs.webkit.org/attachment.cgi?id=316543
Patch

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

> Tools/WebKitTestRunner/EventSenderProxy.h:123
> +    Vector<wpe_input_touch_event_raw> getUpdatedTouchEvents();

Should be `struct wpe_input_touch_event_raw`.

> Tools/WebKitTestRunner/EventSenderProxy.h:125
> +    void prepareAndDispatchTouchEvent(wpe_input_touch_event_type);

Should be `enum wpe_input_touch_event_type`.

> Tools/WebKitTestRunner/EventSenderProxy.h:146
> +    HashSet<int, DefaultHash<int>::Hash,
WTF::UnsignedWithZeroKeyHashTraits<int>> m_updatedTouchEvents;

Can this be unsigned? I don't think you have to store negative indices here.

> Tools/WebKitTestRunner/wpe/EventSenderProxyWPE.cpp:298
> +    const auto rawEvent = &m_touchEvents[index];
> +    rawEvent->x = x;
> +    rawEvent->y = y;
> +    rawEvent->time = m_time;
> +    rawEvent->type = wpe_input_touch_event_type_motion;

Usually pointers are avoided in such cases, and references are preferred:
  `auto& rawEvent = m_touchEvents[index];`

> Tools/WebKitTestRunner/wpe/EventSenderProxyWPE.cpp:314
> +Vector<wpe_input_touch_event_raw> EventSenderProxy::getUpdatedTouchEvents()
> +{
> +    Vector<wpe_input_touch_event_raw> events;

Should be `struct wpe_input_touch_event_raw`.

> Tools/WebKitTestRunner/wpe/EventSenderProxyWPE.cpp:329
> +void
EventSenderProxy::prepareAndDispatchTouchEvent(wpe_input_touch_event_type
eventType)

Should be `enum wpe_input_touch_event_type`.

> Tools/WebKitTestRunner/wpe/EventSenderProxyWPE.cpp:369
> +    const auto rawEvent = &m_touchEvents[index];

`auto& rawEvent = m_touchEvents[index]`.