wl: do not set to null the surface and target at touch_on_up() #720
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aperezdc
requested changes
Aug 26, 2024
There was a problem hiding this comment.
I think it would be better to make seat->touch_target a weak reference, that way if the CogViewport gets destroyed before the touch-move and/or touch-up events arrive it will be set to NULL automatically and we avoid use-after-free of a stale pointer.
When assigning inside touch_on_down we want to use:
- seat->touch_target = viewport;
+ g_set_weak_pointer(&seat->touch_target, viewport);On a multitouch screen, when several fingers are on the screen at the same time, if we set to null surface and target on touch_on_up() then we stop all the ongoing touch gestures and not only the one of the finger that caused the up event. That is becasue touch_on_motion() will refuse to process further events if surface or target are null. Instead of doing that do not set to null this pointers on the touch_on_up() event. The value of those will be set again on the next touch_on_down() event in any case. Also make seat->touch_target a weak reference, that way if the CogViewport gets destroyed before the touch-move and/or touch-up events arrive it will be set to NULL automatically and we avoid use-after-free of a stale pointer. Related: Igalia#698 Related: https://bugs.webkit.org/show_bug.cgi?id=270516
|
changed: make seat->touch_target a weak reference |
aperezdc
approved these changes
Aug 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On a multitouch screen, when several fingers are on the screen at the same time, if we set to null surface and target on touch_on_up() then we stop all the ongoing touch gestures and not only the one of the finger that caused the up event. That is becasue touch_on_motion() will refuse to process further events if surface or target are null.
Instead of doing that do not set to null this pointers on the touch_on_up() event. The value of those will be set again on the next touch_on_down() event in any case.
Related: #698
Related: https://bugs.webkit.org/show_bug.cgi?id=270516