Repository navigation
Conversation
… set Platforms that pass `require_merged_platform_ui_thread` to `SettingsFromCommandLine` (iOS) already reject `--no-enable-merged-platform-ui-thread` and `--merged-platform-ui-thread=disabled` with an FML_CHECK. `--merged-platform-ui-thread=mergeAfterLaunch` seems to have accidentally slipped through allowing a command-line flag to select a threading model our embedders no longer implement. This adds the missing assertion and logging to prevent an invalid flag combination. This is cleanup prior to an eventual migration of the iOS embedder to the embedder API. Issue: flutter#112232
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds a check to ensure that the mergeAfterLaunch option for the merged-platform-ui-thread switch is rejected on platforms where a merged platform UI thread is required. It also adds a corresponding unit test to verify this behavior. There are no review comments, and I have no feedback to provide.
Member
|
|
Member
Author
|
Confirmed by some poking around; closing. Thanks for the additional context @jason-simmons . |
cbracken
added a commit
to cbracken/flutter
that referenced
this pull request
Jul 27, 2026
iOS runs with the platform and UI threads merged from launch and only implements fully-merged (`kEnabled`) threading. The `mergeAfterLaunch` approach starts the engine on a separate UI thread and merges it onto the platform thread once the root isolate is running. The `mergeAfterLaunch` merge/unmerge code lives entirely in `shell/common`. The iOS embedder has never implemented it, and no iOS code has ever referenced it. On iOS, the flag was never wired up, and it isn't reachable through any supported configuration. The `FLTEnableMergedPlatformUIThread` Info.plist key only ever selects `kEnabled` or `kDisabled`. The only way to reach it is the raw command-line flag, which nothing rejects today, so passing `--merged-platform-ui-thread=mergeAfterLaunch` slips through and produces a broken engine since: * `Shell::Spawn` refuses `kMergeAfterLaunch` and returns null, so `FlutterEngineGroup` fails to spawn secondary engines outright. * The CADisplayLink-backed vsync waiter registers on the run loop of whichever thread services the UI task runner when it's constructed. Under the merged model iOS actually ships (`kEnabled`) that's the platform thread, which is correct. Under `mergeAfterLaunch` it would be the temporary UI thread that the model abandons after launch, and which won't work. It turns out this has been around since `mergeAfterLaunch` was introduced. Initially it might have been introduced by the recent migration of the vsync waiter to Swift, but it's been there since day one. Since every iOS `Settings` setting passes through `FlutterDartProject`, we can fail startup there with an `FML_CHECK` if this configuration is set. Normally this would live in `shell/common/switches.cc` but Android also passes `require_merged_platform_ui_thread` to `SettingsFromCommandLine` and still supports `mergeAfterLaunch` as a launch-latency optimization used by internal customers. Also added a test that hints that `mergeAfterLaunch` is an intentional, supported configuration so that people like me don't try to lock it down again. See: flutter#189893 This is part of cleanup work intended to simplify the iOS embedder prior to an eventual migration to the embedder API. Issue: flutter#112232
cbracken
added a commit
to cbracken/flutter
that referenced
this pull request
Jul 27, 2026
iOS runs with the platform and UI threads merged from launch and only implements fully-merged (`kEnabled`) threading. The `mergeAfterLaunch` approach starts the engine on a separate UI thread and merges it onto the platform thread once the root isolate is running. The `mergeAfterLaunch` merge/unmerge code lives entirely in `shell/common`. The iOS embedder has never implemented it, and no iOS code has ever referenced it. On iOS, the flag was never wired up, and it isn't reachable through any supported configuration. The `FLTEnableMergedPlatformUIThread` Info.plist key only ever selects `kEnabled` or `kDisabled`. The only way to reach it is the raw command-line flag, which nothing rejects today, so passing `--merged-platform-ui-thread=mergeAfterLaunch` slips through and produces a broken engine since: * `Shell::Spawn` refuses `kMergeAfterLaunch` and returns null, so `FlutterEngineGroup` fails to spawn secondary engines outright. * The CADisplayLink-backed vsync waiter registers on the run loop of whichever thread services the UI task runner when it's constructed. Under the merged model iOS actually ships (`kEnabled`) that's the platform thread, which is correct. Under `mergeAfterLaunch` it would be the temporary UI thread that the model abandons after launch, and which won't work. It turns out this has been around since `mergeAfterLaunch` was introduced. Initially it might have been introduced by the recent migration of the vsync waiter to Swift, but it's been there since day one. Since every iOS `Settings` setting passes through `FlutterDartProject`, we can fail startup there with an `FML_CHECK` if this configuration is set. Normally this would live in `shell/common/switches.cc` but Android also passes `require_merged_platform_ui_thread` to `SettingsFromCommandLine` and still supports `mergeAfterLaunch` as a launch-latency optimization used by internal customers. Also added a test that hints that `mergeAfterLaunch` is an intentional, supported configuration so that people like me don't try to lock it down again. See: flutter#189893 This is part of cleanup work intended to simplify the iOS embedder prior to an eventual migration to the embedder API. Issue: flutter#112232
cbracken
added a commit
to cbracken/flutter
that referenced
this pull request
Jul 27, 2026
iOS runs with the platform and UI threads merged from launch and only implements fully-merged (`kEnabled`) threading. The `mergeAfterLaunch` approach starts the engine on a separate UI thread and merges it onto the platform thread once the root isolate is running. The `mergeAfterLaunch` merge/unmerge code lives entirely in `shell/common`. The iOS embedder has never implemented it, and no iOS code has ever referenced it. On iOS, the flag was never wired up, and it isn't reachable through any supported configuration. The `FLTEnableMergedPlatformUIThread` Info.plist key only ever selects `kEnabled` or `kDisabled`. The only way to reach it is the raw command-line flag, which nothing rejects today, so passing `--merged-platform-ui-thread=mergeAfterLaunch` slips through and produces a broken engine since: * `Shell::Spawn` refuses `kMergeAfterLaunch` and returns null, so `FlutterEngineGroup` fails to spawn secondary engines outright. * The CADisplayLink-backed vsync waiter registers on the run loop of whichever thread services the UI task runner when it's constructed. Under the merged model iOS actually ships (`kEnabled`) that's the platform thread, which is correct. Under `mergeAfterLaunch` it would be the temporary UI thread that the model abandons after launch, and which won't work. This has been unsupported since `mergeAfterLaunch` was introduced. Since every iOS `Settings` setting passes through `FlutterDartProject`, we can fail startup there with an `FML_CHECK` if this configuration is set. Normally this would live in `shell/common/switches.cc` but Android also passes `require_merged_platform_ui_thread` to `SettingsFromCommandLine` and still supports `mergeAfterLaunch` as a launch-latency optimization used by internal customers. Also added a test that hints that `mergeAfterLaunch` is an intentional, supported configuration so that people like me don't try to lock it down again. See: flutter#189893 This is part of cleanup work intended to simplify the iOS embedder prior to an eventual migration to the embedder API. Issue: flutter#112232
10 tasks done
pull Bot
pushed a commit
to chunfengyao/flutter
that referenced
this pull request
Jul 28, 2026
iOS runs with the platform and UI threads merged from launch and only implements fully-merged (`kEnabled`) threading. The `mergeAfterLaunch` approach starts the engine on a separate UI thread and merges it onto the platform thread once the root isolate is running. The `mergeAfterLaunch` merge/unmerge code lives entirely in `shell/common`. The iOS embedder has never implemented it, and no iOS code has ever referenced it. On iOS, the flag was never wired up, and it isn't reachable through any supported configuration. The `FLTEnableMergedPlatformUIThread` Info.plist key only ever selects `kEnabled` or `kDisabled`. The only way to reach it is the raw command-line flag, which nothing rejects today, so passing `--merged-platform-ui-thread=mergeAfterLaunch` slips through and produces a broken engine since: * `Shell::Spawn` refuses `kMergeAfterLaunch` and returns null, so `FlutterEngineGroup` fails to spawn secondary engines outright. * The CADisplayLink-backed vsync waiter registers on the run loop of whichever thread services the UI task runner when it's constructed. Under the merged model iOS actually ships (`kEnabled`) that's the platform thread, which is correct. Under `mergeAfterLaunch` it would be the temporary UI thread that the model abandons after launch, and which won't work. This has been unsupported since `mergeAfterLaunch` was introduced. Since every iOS `Settings` setting passes through `FlutterDartProject`, we can fail startup there with an `FML_CHECK` if this configuration is set. Normally this would live in `shell/common/switches.cc` but Android also passes `require_merged_platform_ui_thread` to `SettingsFromCommandLine` and still supports `mergeAfterLaunch` as a launch-latency optimization used by internal customers. Also added a test that hints that `mergeAfterLaunch` is an intentional, supported configuration so that people like me don't try to lock it down again. See: flutter#189893 This is part of cleanup work intended to simplify the iOS embedder prior to an eventual migration to the embedder API. Issue: flutter#112232 ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [X] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [X] I updated/added relevant documentation (doc comments with `///`). - [X] I added new tests to check the change I am making, or this PR is [test-exempt]. - [X] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [X] All existing and new tests are passing. 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](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This file contains hidden or 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
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.
Platforms that pass
require_merged_platform_ui_threadtoSettingsFromCommandLine(i.e. iOS) already reject--no-enable-merged-platform-ui-threadand--merged-platform-ui-thread=disabledwith an FML_CHECK.--merged-platform-ui-thread=mergeAfterLaunchseems to have accidentally slipped through allowing a command-line flag to select a threading model our embedders no longer support. This adds the missing assertion and logging to prevent an invalid flag combination.This is cleanup prior to an eventual migration of the iOS embedder to the embedder API.
Issue: #112232
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.