Bug 173419

Summary: [WPE] Implement EventSenderProxy in WTR
Product: WebKit Reporter: Claudio Saavedra <csaavedra>
Component: WPE WebKitAssignee: Claudio Saavedra <csaavedra>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, buildbot, commit-queue, cturner, lforschler, webkit-bug-importer, zan
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=175864
Bug Depends on:    
Bug Blocks: 174674    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

Description Claudio Saavedra 2017-06-15 07:07:36 PDT
The implementation is missing so tests injecting events are non-testable at the moment.
Comment 1 Claudio Saavedra 2017-07-27 03:05:23 PDT
Created attachment 316541 [details]
Patch
Comment 2 Build Bot 2017-07-27 03:07:49 PDT
Attachment 316541 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/EventSenderProxy.h:43:  "wtf/HashSet.h" already included at Tools/WebKitTestRunner/EventSenderProxy.h:38  [build/include] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Claudio Saavedra 2017-07-27 03:25:10 PDT
Created attachment 316543 [details]
Patch
Comment 4 Build Bot 2017-07-27 03:27:59 PDT
Attachment 316543 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/EventSenderProxy.h:43:  "wtf/HashSet.h" already included at Tools/WebKitTestRunner/EventSenderProxy.h:38  [build/include] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Zan Dobersek 2017-07-27 23:31:52 PDT
Comment on attachment 316543 [details]
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]`.
Comment 6 Claudio Saavedra 2017-07-28 02:19:05 PDT
Created attachment 316628 [details]
Patch for landing
Comment 7 Claudio Saavedra 2017-07-28 02:19:50 PDT
Comment on attachment 316628 [details]
Patch for landing

I might have forgotten something..
Comment 8 Claudio Saavedra 2017-07-28 02:38:18 PDT
Created attachment 316629 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2017-07-28 03:26:36 PDT
The commit-queue encountered the following flaky tests while processing attachment 316629 [details]:

svg/animations/smil-leak-dynamically-added-element-instances.svg bug 114280 (authors: ap@webkit.org and fmalita@chromium.org)
The commit-queue is continuing to process your patch.
Comment 10 WebKit Commit Bot 2017-07-28 03:27:07 PDT
Comment on attachment 316629 [details]
Patch for landing

Clearing flags on attachment: 316629

Committed r220000: <http://trac.webkit.org/changeset/220000>
Comment 11 WebKit Commit Bot 2017-07-28 03:27:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Charlie Turner 2017-09-05 03:50:05 PDT
This no longer times out, but instead fails after r221234

webkit.org/b/173419 fast/events/before-input-prevent-insert-replacement.html [ Timeout ]

Diff
--- /home/buildbot/wpe/wpe-linux-64-release-tests/build/layout-test-results/fast/events/before-input-prevent-insert-replacement-expected.txt
+++ /home/buildbot/wpe/wpe-linux-64-release-tests/build/layout-test-results/fast/events/before-input-prevent-insert-replacement-actual.txt
@@ -2,19 +2,15 @@
 You should observe that the replacement accented character does not replace 'a'.
 Then insert an emoji using the emoji picker. This should insert something into the editor.
 Finally, select all the content and attempt to replace it with an emoji. This should do nothing.
-ac
+a
 (1) Typing 'a'...
 (editable): type=beforeinput, inputType=insertText, data=a, dataTransfer=(null)
 (editable): type=input, inputType=insertText, data=a, dataTransfer=(null)
 The editor now has text content: a
 (2) Preventing default when replacing 'a' with 'b'...
-(editable): type=beforeinput, inputType=insertReplacementText, data=null, dataTransfer=plain:"b", html:"b"
 The editor now has text content: a
 (3) Inserting 'c' after 'a'...
-(editable): type=beforeinput, inputType=insertText, data=c, dataTransfer=(null)
-(editable): type=input, inputType=insertText, data=c, dataTransfer=(null)
-The editor now has text content: ac
+The editor now has text content: a
 (4) Selecting all and preventing replacement with 'd'...
-(editable): type=beforeinput, inputType=insertReplacementText, data=null, dataTransfer=plain:"d", html:"d"
-The editor now has text content: ac
+The editor now has text content: a