Bug 173419 - [WPE] Implement EventSenderProxy in WTR
Summary: [WPE] Implement EventSenderProxy in WTR
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Claudio Saavedra
URL:
Keywords:
Depends on:
Blocks: 174674
  Show dependency treegraph
 
Reported: 2017-06-15 07:07 PDT by Claudio Saavedra
Modified: 2017-09-05 03:50 PDT (History)
7 users (show)

See Also:


Attachments
Patch (43.84 KB, patch)
2017-07-27 03:05 PDT, Claudio Saavedra
no flags Details | Formatted Diff | Diff
Patch (46.51 KB, patch)
2017-07-27 03:25 PDT, Claudio Saavedra
no flags Details | Formatted Diff | Diff
Patch for landing (46.57 KB, patch)
2017-07-28 02:19 PDT, Claudio Saavedra
no flags Details | Formatted Diff | Diff
Patch for landing (46.57 KB, patch)
2017-07-28 02:38 PDT, Claudio Saavedra
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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