Repository navigation
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the iOS and macOS shell implementation to dispatch pointer events and request vsync frames synchronously, utilizing a shared FlutterRunLoop and a custom EmbedderTaskRunner. Feedback on these changes highlights critical issues: the synchronous execution of vsync callbacks during touch dispatch can cause infinite recursion and stack overflows, and capturing a raw task runner pointer in deferred blocks introduces a Use-After-Free vulnerability. Additionally, the use of global state variables breaks multi-engine support, and nestedly pumping the run loop during touch dispatch poses re-entrancy risks. Finally, the reviewer recommends removing commented-out code and #if 0 blocks to adhere to style guidelines.
| CFTimeInterval startTime = CACurrentMediaTime(); | ||
| while (!FlutterDidPresentSurface && CACurrentMediaTime() - startTime < 0.5) { | ||
| [FlutterRunLoop.mainRunLoop pollFlutterMessagesOnce]; | ||
| } |
There was a problem hiding this comment.
Pumping the run loop nestedly using pollFlutterMessagesOnce in a while loop on the main thread while holding the touch dispatch is highly risky. This can lead to unexpected re-entrancy issues where other events (timers, network callbacks, or other touch events) are processed in an unexpected state, potentially causing crashes or inconsistent UI state. It also risks blocking the main thread and triggering watchdog terminations if the frame presentation is delayed.
There was a problem hiding this comment.
There is very reasonable risk of reentrancy issues because we have a custom run loop mode just for this.
github-actions
Bot
added
platform-ios
This we block after other UIKit widgets had chance to update.
knopp
changed the title
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
This PR builds on top of #191368 to fix the last remaining issue - delaying CA commit until we have frame available. This gets rid of the second frame of latency.
This is a very hacky proof of concept PR that is cutting a lot of corners. But it works.
(Note that we can match UIKit while gesture is in progress, but the gesture starts slower because of recognizer logic differences. Also there is some weirdness in
ScrollDragController._adjustForScrollStartThresholdthat maybe need a look.)zero-additional-latency.mov
List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.