Skip to content

[WPE] Extract common logic from EventSenderProxyClientWPE touch event handling #45692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[WPE] Extract common logic from EventSenderProxyClientWPE touch event handling #45692

wants to merge 1 commit into from

Conversation

spenap
Copy link
Contributor

@spenap spenap commented May 21, 2025 •
edited by webkit-early-warning-system
Loading

c53cb8a

[WPE] Extract common logic from EventSenderProxyClientWPE touch event handling
https://bugs.webkit.org/show_bug.cgi?id=293352

Reviewed by NOBODY (OOPS!).

Extract the common logic from touchStart, touchMove, touchEnd and
touchCancel into a lambda-generating function. As these 4 methods
transverse the m_touchPoints, they can all invoke the lambda on each
point: two in a range-based for loop, two in removeAllMatching.

The return value in the lambda is ignored in the range-based case, and
used by removeAllMatching in the other, and a context structure is used
to keep all arguments together.

* Tools/WebKitTestRunner/wpe/EventSenderProxyClientWPE.cpp:
(WTR::std::function<bool):
(WTR::EventSenderProxyClientWPE::touchStart):
(WTR::EventSenderProxyClientWPE::touchMove):
(WTR::EventSenderProxyClientWPE::touchEnd):
(WTR::EventSenderProxyClientWPE::touchCancel):
* Tools/WebKitTestRunner/wpe/EventSenderProxyClientWPE.h:

c53cb8a

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@spenap spenap requested a review from a team as a code owner May 21, 2025 12:18
@spenap spenap self-assigned this May 21, 2025
@spenap spenap added the WPE WebKit WebKit WPE component label May 21, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 21, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 8f09b45)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
⏳ 🧪 ios-wk2-wpt   🧪 mac-wk1 ✅ 🛠 wpe-cairo
⏳ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ⏳ 🧪 mac-AS-debug-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim ⏳ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2   🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@spenap spenap marked this pull request as draft May 21, 2025 12:18
@spenap
Copy link
Contributor Author

spenap commented May 21, 2025

@carlosgcampos , this is a follow-up to #45156. I haven't tested it yet, just wanted to try it out quickly to understand if you'd be happy with this direction

@spenap spenap force-pushed the eng/WPE-Extract-common-logic-from-EventSenderProxyClientWPE-touch-event-handling branch from 8f09b45 to bea49a4 Compare May 21, 2025 12:49
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 21, 2025 •
edited
Loading

EWS run on previous version of this PR (hash bea49a4)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
⏳ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
  🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ⏳ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ⏳ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

aperezdc
aperezdc reviewed May 21, 2025
View reviewed changes
Copy link
Contributor

@aperezdc aperezdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I think it better if @carlosgcampos could also take a look. I left a small suggestion.

@spenap spenap force-pushed the eng/WPE-Extract-common-logic-from-EventSenderProxyClientWPE-touch-event-handling branch from bea49a4 to 9af98fe Compare May 21, 2025 14:14
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 21, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 9af98fe)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@spenap spenap marked this pull request as ready for review May 22, 2025 04:19
@spenap
[WPE] Extract common logic from EventSenderProxyClientWPE touch event…
c53cb8a 
… handling

https://bugs.webkit.org/show_bug.cgi?id=293352

Reviewed by NOBODY (OOPS!).

Extract the common logic from touchStart, touchMove, touchEnd and
touchCancel into a lambda-generating function. As these 4 methods
transverse the m_touchPoints, they can all invoke the lambda on each
point: two in a range-based for loop, two in removeAllMatching.

The return value in the lambda is ignored in the range-based case, and
used by removeAllMatching in the other, and a context structure is used
to keep all arguments together.

* Tools/WebKitTestRunner/wpe/EventSenderProxyClientWPE.cpp:
(WTR::std::function<bool):
(WTR::EventSenderProxyClientWPE::touchStart):
(WTR::EventSenderProxyClientWPE::touchMove):
(WTR::EventSenderProxyClientWPE::touchEnd):
(WTR::EventSenderProxyClientWPE::touchCancel):
* Tools/WebKitTestRunner/wpe/EventSenderProxyClientWPE.h:
@spenap spenap force-pushed the eng/WPE-Extract-common-logic-from-EventSenderProxyClientWPE-touch-event-handling branch from 9af98fe to c53cb8a Compare June 2, 2025 14:40
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 2, 2025 •
edited
Loading

EWS run on current version of this PR (hash c53cb8a)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

aperezdc
aperezdc approved these changes Jun 27, 2025
View reviewed changes
Copy link
Contributor

@aperezdc aperezdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge this, patch LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aperezdc aperezdc aperezdc approved these changes

Assignees

@spenap spenap

Labels
WPE WebKit WebKit WPE component
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@spenap @webkit-early-warning-system @aperezdc