-
Notifications
You must be signed in to change notification settings - Fork 29.7k
fix: check both pointer count and action before reusing MotionEvent #178528
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.
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
fix: check both pointer count and action before reusing MotionEvent #178528
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request provides a solid fix for a gesture-blocking issue on Android platform views. The problem, which occurred when the framework sent a MotionEvent with a different action than the original cached event, is correctly resolved by adding a check for the event action in addition to the pointer count before reusing a cached event. The logic is sound, and the changes are clean and well-commented. Furthermore, a new unit test, toMotionEvent_handlesActionMismatch, has been added. This test is comprehensive and accurately simulates the bug scenario, ensuring the fix is effective and preventing future regressions. The overall quality of the change is excellent.
bd780d2 to
2ae4107
Compare
2ae4107 to
ca107d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I ask what behavior you expect to see? I no longer see the behavior from #178528 that your original PR intended to fix (the platform view becoming unresponsive) on master. I'm not sure I can see a behavior difference when testing your app on this branch vs master:
on your branch:
https://github.com/user-attachments/assets/47fe92b4-e71f-4830-8cb1-1e808512dd7e
Naively, they both look correct to me (they discard multi touch gestures, as one would expect from a SingleTouchContainer and otherwise don't become unresponsive).
I also am doubtful that #178015 introduced the action mismatch you are highlighting here. Anecdotally I had noticed that this bug existed before either your original PR or @jason-simmons's pr had landed.
Given that your original PR still used the action from the tracked event when making the reconstructed event, I would expect no effect from @jason-simmons's PR on the behavior you see with regard to action delivered.
|
@gmackall cc @reidbaker If you need any other information I will provide it right away. |
|
We will need to resolve #179275 before landing this |
gmackall
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok it turns out this actually resolves #179275!
I also wanted to double check if we could, for the case where only the actions mismatch, call MotionEvent.setAction() and maintain verifiability. But it turns out that method renders the event not verified, so it seems pointer mismatch and action mismatch are the two cases where we can't fix the event and thus need to construct a new event. So this LGTM.
|
autosubmit label was removed for flutter/flutter/178528, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR. |
|
autosubmit label was removed for flutter/flutter/178528, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.
|
Roll Flutter from 69d8710fadcd to 5b8720312a94 (42 revisions) flutter/flutter@69d8710...5b87203 2025-12-05 [email protected] Roll Packages from d39e481 to b17d3ff (3 revisions) (flutter/flutter#179505) 2025-12-05 [email protected] Move target_cpu into use_rbe block in Fuchsia build (flutter/flutter#179458) 2025-12-05 [email protected] Roll Skia from fe6bf18a3f6b to a31411879251 (3 revisions) (flutter/flutter#179499) 2025-12-05 [email protected] Force WASM single threading in Chrome extensions. (flutter/flutter#179400) 2025-12-05 [email protected] Roll Skia from aea282ea0bcd to fe6bf18a3f6b (7 revisions) (flutter/flutter#179489) 2025-12-05 [email protected] fix: check both pointer count and action before reusing MotionEvent (flutter/flutter#178528) 2025-12-05 [email protected] Roll Fuchsia Linux SDK from VtDPX2k1kosHxjKUE... to vDeTl_KBeLJY8nCAx... (flutter/flutter#179478) 2025-12-05 [email protected] [ Widget Preview ] Add embedded Widget Inspector support (flutter/flutter#178116) 2025-12-05 [email protected] bubble up fragment shader errors (flutter/flutter#179363) 2025-12-05 [email protected] Roll Dart SDK from eda82318e193 to 42fe3327abca (1 revision) (flutter/flutter#179472) 2025-12-05 [email protected] [Android 16] Use AVD With New Android Renderer (flutter/flutter#179306) 2025-12-05 [email protected] Unpin `package:dwds` dependency (flutter/flutter#179462) 2025-12-05 [email protected] [Impeller] include uniform info in impellerc json reflections (flutter/flutter#179317) 2025-12-05 [email protected] Roll Skia from 1591b066f49b to aea282ea0bcd (1 revision) (flutter/flutter#179468) 2025-12-04 [email protected] Roll Skia from e1923478562b to 1591b066f49b (3 revisions) (flutter/flutter#179460) 2025-12-04 [email protected] Cherry-pick flutter 3.38.4 changelog back to master. (flutter/flutter#179463) 2025-12-04 [email protected] [web] Add clone method to LayerPicture and dispose pictures in PictureLayer (flutter/flutter#179162) 2025-12-04 [email protected] Fixes merge conflict from high bitrate texture tests (flutter/flutter#179416) 2025-12-04 [email protected] Fix typos in `VirtualDisplayController.java` (flutter/flutter#179411) 2025-12-04 [email protected] Roll Dart SDK from 2de44cc08970 to eda82318e193 (1 revision) (flutter/flutter#179453) 2025-12-04 [email protected] Roll Skia from 55d94a54f453 to e1923478562b (1 revision) (flutter/flutter#179449) 2025-12-04 [email protected] Roll Packages from 8cb4903 to d39e481 (8 revisions) (flutter/flutter#179451) 2025-12-04 [email protected] Add DropdownMenu.selectOnly (flutter/flutter#179189) 2025-12-04 [email protected] Roll Dart SDK from 7e6bfc6af55c to 2de44cc08970 (6 revisions) (flutter/flutter#179443) 2025-12-04 [email protected] Roll Skia from b8f79d7316c0 to 55d94a54f453 (1 revision) (flutter/flutter#179439) 2025-12-04 [email protected] Revise README for link updates and terminology changes (flutter/flutter#179357) 2025-12-04 [email protected] Roll Skia from ce19122e3982 to b8f79d7316c0 (3 revisions) (flutter/flutter#179436) 2025-12-04 [email protected] Update some BottomNavigationBar comments to reflect theme normalization (flutter/flutter#179404) 2025-12-04 [email protected] Replace use of eglCreateImage with eglCreateImageKHR to reduce EGL requirement (flutter/flutter#179310) 2025-12-04 [email protected] Roll Skia from 81a9a0751f00 to ce19122e3982 (5 revisions) (flutter/flutter#179430) 2025-12-04 [email protected] [impellerc] adds entry prefix flag to avoid shader collisions (flutter/flutter#179160) 2025-12-04 [email protected] [Windows] Allow apps to prefer high power GPUs (flutter/flutter#177653) 2025-12-04 [email protected] [ Infra ] Shard `Windows tool_tests_commands` (flutter/flutter#179409) 2025-12-04 [email protected] [flutter_tools] Fix filename typo (flutter/flutter#179427) 2025-12-04 [email protected] Marks Linux_pixel_7pro draw_arcs_all_stroke_styles_perf__timeline_summary to be unflaky (flutter/flutter#179392) 2025-12-03 [email protected] Marks Mac_ios draw_arcs_all_fill_styles_perf_ios__timeline_summary to be unflaky (flutter/flutter#179391) 2025-12-03 [email protected] Marks Linux_pixel_7pro draw_arcs_all_fill_styles_perf__timeline_summary to be unflaky (flutter/flutter#179390) 2025-12-03 [email protected] Roll Abseil to 564023aa5376 (flutter/flutter#179421) 2025-12-03 [email protected] Made wide gamut tests give more information in their failure (flutter/flutter#179415) 2025-12-03 [email protected] Roll Skia from c20f797ab6f9 to 81a9a0751f00 (5 revisions) (flutter/flutter#179417) 2025-12-03 [email protected] Migrate samples and docs to RadioGroup (flutter/flutter#179158) 2025-12-03 [email protected] Roll Dart SDK to 3.11.0-200.1.beta (flutter/flutter#179399) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages ...
…lutter#178528) Fix issue flutter#169486 ## Problem After [PR flutter#178015](flutter#178015) was merged, Android platform views experience gesture blocking after multi-touch operations (e.g., pinch-to-zoom). The issue occurs because: 1. PR flutter#178015 changed framework behavior to encode original pointer count in events 2. Framework now sends events with **different actions** than the original MotionEvent - Example: Framework sends `ACTION_MOVE (2)` while original event is `ACTION_POINTER_UP (6)` 3. The existing engine code only checked **pointer count**, not action 4. This caused the engine to use original event with wrong action → gesture blocked ### Logs Showing the Issue trackedEvent.pointerCount=2, touch.pointerCount=2, trackedEvent.action=6 (ACTION_POINTER_UP), touch.action=2 (ACTION_MOVE) → Pointer counts MATCH, using original event → WebView receives ACTION_POINTER_UP but expects ACTION_MOVE → Gesture blocked! ❌
Fix issue #169486
Problem
After PR #178015 was merged, Android platform views experience gesture blocking after multi-touch operations (e.g., pinch-to-zoom). The issue occurs because:
ACTION_MOVE (2)while original event isACTION_POINTER_UP (6)Logs Showing the Issue
trackedEvent.pointerCount=2, touch.pointerCount=2, trackedEvent.action=6 (ACTION_POINTER_UP), touch.action=2 (ACTION_MOVE) → Pointer counts MATCH, using original event → WebView receives ACTION_POINTER_UP but expects ACTION_MOVE → Gesture blocked! ❌