-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[Embedder] Implement merged platform and UI thread #162944
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
Conversation
From the conversation here, the direction we want to take is to not make this be an embedder concern. As I understand, this is needed for the WIP prototype to work. Are you looking for a review here over just the implementation of a separate platform task runner configuration exposed via the embedder API? |
@chinmaygarde, the concern here was about embedder API changes needed to flush microtask queue. That is gone in this PR. It is handled internally and does not leak into the API. The only API addition is the |
Now thinking about it a bit, maybe rather than introducing |
2f7e60a
to
6ef97ff
Compare
I have modified the PR to allow specifying |
6ef97ff
to
8cd39c9
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.
LGTM
Co-authored-by: Jonah Williams <jonahwilliams@google.com>
Thank you. That is much more preferable as it lines up identically with how the other runners are specified. Can you also add a test that tests the embedder specifying a UI task runner that is not in a merged configuration (like its own thread)? |
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.
Thanks.
0e462a2
to
17d2402
Compare
autosubmit label was removed for flutter/flutter/162944, because - The status or check suite Linux linux_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label. |
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. 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. |
@@ -17,6 +17,7 @@ | |||
namespace fml { | |||
|
|||
const size_t TaskQueueId::kUnmerged = ULONG_MAX; | |||
const size_t TaskQueueId::kInvalid = ULONG_MAX - 1; |
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.
What do you think of calling this something like kNone
and renaming TaskQueueId::is_valid
to TaskQueueId::exists
?
I worry we might be tempted to reuse the "invalid" state for more than "This task runner does not have its own task queue". I don't feel strongly about this, feel free to skip
@@ -55,6 +55,9 @@ class TaskRunner : public fml::RefCountedThreadSafe<TaskRunner>, | |||
|
|||
/// Returns the unique identifier associated with the TaskRunner. | |||
/// \see fml::MessageLoopTaskQueues | |||
/// | |||
/// Will be TaskQueueId::kInvalid for embedder supplied task runners | |||
/// that are not associated with a task queue. | |||
virtual TaskQueueId GetTaskQueueId(); |
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.
I worry people will forget to check the TaskQueueId::kInvalid
case everywhere. Would it make sense to make this return a std::optional<TaskQueueId>
and return std::nullopt
if the runner has no associated task queue?
I don't feel strongly about this, feel free to skip
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.
I'll make a follow-up PR with this change. I feel like that's going to add a bit of noise so maybe a separate PR will be better.
if (has_ui_thread_message_loop) { | ||
fml::MessageLoop::GetCurrent().RemoveTaskObserver(key); | ||
} | ||
}; |
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.
Just for my own curiosity, what are the task observers used for? I don't understand the implications of not registering these observers
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.
Ah from the comment in EmbedderEngine::RunTask
, it sounds like the observers are used to flush microtasks?
If so, what do you think adding a quick comment here to explain that this is used to flush microtasks if there's a separate UI thread, otherwise, microtasks are flushed by EmbedderEngine::RunTask
?
Roll Flutter from 892f9c1 to e8f34a9 (71 revisions) flutter/flutter@892f9c1...e8f34a9 2025-02-12 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Skia from f31c733c86c4 to 25937c31f153 (2 revisions) (#163127)" (flutter/flutter#163133) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from f31c733c86c4 to 25937c31f153 (2 revisions) (flutter/flutter#163127) 2025-02-12 jonnywang@google.com Update .ci.yaml to support Fuchsia cherrypick branches (flutter/flutter#163000) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from 6f17f2ebb2e5 to f31c733c86c4 (1 revision) (flutter/flutter#163112) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from a9dbb2479c26 to 6f17f2ebb2e5 (2 revisions) (flutter/flutter#163109) 2025-02-12 jonahwilliams@google.com [devicelab] dont strip symbols in platform views layout test. (flutter/flutter#163101) 2025-02-12 jonahwilliams@google.com [Impeller] mirror tile mode requires highp for Adreno. (flutter/flutter#163066) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from 5b56d9a91633 to a9dbb2479c26 (6 revisions) (flutter/flutter#163100) 2025-02-12 engine-flutter-autoroll@skia.org Roll Dart SDK from d9d7f103b6b7 to fcef25f18e4d (3 revisions) (flutter/flutter#163098) 2025-02-12 matanlurey@users.noreply.github.com Generate a correct `.flutter-plugin-dependencies` file for iOS/macOS projects (flutter/flutter#162834) 2025-02-12 matanlurey@users.noreply.github.com Remove unsound artifacts, remove `*Sound` qualifier. (flutter/flutter#163015) 2025-02-12 chinmaygarde@google.com [Impeller] libImpeller: Add support for Metal and Vulkan rendering. (flutter/flutter#161547) 2025-02-11 fluttergithubbot@gmail.com Marks Mac_benchmark basic_material_app_macos__compile to be flaky (flutter/flutter#162365) 2025-02-11 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#163083) 2025-02-11 47866232+chunhtai@users.noreply.github.com Adds hasSelectedState parameter to matchesSemantics for migration (flutter/flutter#163014) 2025-02-11 koji.wakamiya@gmail.com fix: Dispose codec after completing frame creation (flutter/flutter#159945) 2025-02-11 41930132+hellohuanlin@users.noreply.github.com [ios][secure_paste]show menu item based on info sent from framework (flutter/flutter#161103) 2025-02-11 christopherfujino@gmail.com Update conductor to support monorepos (flutter/flutter#161704) 2025-02-11 jonahwilliams@google.com [Android] fix hcpp tapping, again, and add test. (flutter/flutter#163035) 2025-02-11 2049220+sigmundch@users.noreply.github.com Add new builder for experiment with dynamic modules. (flutter/flutter#162855) 2025-02-11 jason-simmons@users.noreply.github.com Roll vulkan-deps to 9edf248c597b (flutter/flutter#162549) 2025-02-11 47866232+chunhtai@users.noreply.github.com Adds dialog and alertdialog role (flutter/flutter#162692) 2025-02-11 engine-flutter-autoroll@skia.org Roll Dart SDK from 99789828cc95 to d9d7f103b6b7 (12 revisions) (flutter/flutter#163060) 2025-02-11 bkonyi@google.com [ Widget Preview ] Cleanup PreviewDetector code (flutter/flutter#163050) 2025-02-11 matanlurey@users.noreply.github.com Fix `SkiaException` -> `TestFailure`, add tests. (flutter/flutter#163054) 2025-02-11 jonahwilliams@google.com [Android] fix hcpp overlay layer intersection. (flutter/flutter#163024) 2025-02-11 bkonyi@google.com [ Widget Preview ] Update generated scaffold project to include early preview rendering (flutter/flutter#162847) 2025-02-11 matej.knopp@gmail.com [Embedder] Implement merged platform and UI thread (flutter/flutter#162944) 2025-02-11 jonahwilliams@google.com [Android] Remove overlay when platform views are removed from screen. (flutter/flutter#162908) 2025-02-11 jason-simmons@users.noreply.github.com Roll Dart to 3.8.0-76.0.dev (flutter/flutter#162913) 2025-02-11 jonahwilliams@google.com [Android] add HCPP platform views benchmark and integration test. (flutter/flutter#163018) 2025-02-11 engine-flutter-autoroll@skia.org Roll Skia from 8c377e8bedd2 to 5b56d9a91633 (9 revisions) (flutter/flutter#163021) 2025-02-11 matanlurey@users.noreply.github.com Try golden-testing on a Mokey (`bringup: true`), retry on an emulator (flutter/flutter#163029) 2025-02-11 robert.ancell@canonical.com Fix Linux keyboard support for AltGr (flutter/flutter#162495) 2025-02-11 robert.ancell@canonical.com Update gen_keycodes output to new engine location. (flutter/flutter#162479) 2025-02-10 jonahwilliams@google.com [Android] add runtime flag to determine if HCPP is supported. (flutter/flutter#163004) 2025-02-10 koji.wakamiya@gmail.com [iOS][Engine] Fix view removal process for AutofillContextAction.cancel (flutter/flutter#160653) 2025-02-10 matanlurey@users.noreply.github.com Re-land #162644: Remove `--verbose` from devicelab task executions. (flutter/flutter#163017) 2025-02-10 reidbaker@google.com Include device lab version for how to run test (flutter/flutter#163010) 2025-02-10 jacksongardner@google.com Change the default optimization level to `-O2` for wasm in release mode. (flutter/flutter#162917) 2025-02-10 yjbanov@google.com [web] robustify safaridriver launch sequence (flutter/flutter#162919) 2025-02-10 jakemac@google.com Return more eagerly when toggling service extensions (flutter/flutter#162774) 2025-02-10 goderbauer@google.com Fix doc reference typos (flutter/flutter#162893) 2025-02-10 engine-flutter-autoroll@skia.org Roll Skia from 180ed4fc263d to 8c377e8bedd2 (4 revisions) (flutter/flutter#162998) 2025-02-10 matanlurey@users.noreply.github.com FYI matanlurey (does not require review, but probably should) on dev/test infra. (flutter/flutter#162642) 2025-02-10 30870216+gaaclarke@users.noreply.github.com [Impeller] rrect_blur: scale max radius clamp by transform (flutter/flutter#161238) ...
Roll Flutter from 892f9c1 to e8f34a9 (71 revisions) flutter/flutter@892f9c1...e8f34a9 2025-02-12 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Skia from f31c733c86c4 to 25937c31f153 (2 revisions) (#163127)" (flutter/flutter#163133) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from f31c733c86c4 to 25937c31f153 (2 revisions) (flutter/flutter#163127) 2025-02-12 jonnywang@google.com Update .ci.yaml to support Fuchsia cherrypick branches (flutter/flutter#163000) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from 6f17f2ebb2e5 to f31c733c86c4 (1 revision) (flutter/flutter#163112) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from a9dbb2479c26 to 6f17f2ebb2e5 (2 revisions) (flutter/flutter#163109) 2025-02-12 jonahwilliams@google.com [devicelab] dont strip symbols in platform views layout test. (flutter/flutter#163101) 2025-02-12 jonahwilliams@google.com [Impeller] mirror tile mode requires highp for Adreno. (flutter/flutter#163066) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from 5b56d9a91633 to a9dbb2479c26 (6 revisions) (flutter/flutter#163100) 2025-02-12 engine-flutter-autoroll@skia.org Roll Dart SDK from d9d7f103b6b7 to fcef25f18e4d (3 revisions) (flutter/flutter#163098) 2025-02-12 matanlurey@users.noreply.github.com Generate a correct `.flutter-plugin-dependencies` file for iOS/macOS projects (flutter/flutter#162834) 2025-02-12 matanlurey@users.noreply.github.com Remove unsound artifacts, remove `*Sound` qualifier. (flutter/flutter#163015) 2025-02-12 chinmaygarde@google.com [Impeller] libImpeller: Add support for Metal and Vulkan rendering. (flutter/flutter#161547) 2025-02-11 fluttergithubbot@gmail.com Marks Mac_benchmark basic_material_app_macos__compile to be flaky (flutter/flutter#162365) 2025-02-11 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#163083) 2025-02-11 47866232+chunhtai@users.noreply.github.com Adds hasSelectedState parameter to matchesSemantics for migration (flutter/flutter#163014) 2025-02-11 koji.wakamiya@gmail.com fix: Dispose codec after completing frame creation (flutter/flutter#159945) 2025-02-11 41930132+hellohuanlin@users.noreply.github.com [ios][secure_paste]show menu item based on info sent from framework (flutter/flutter#161103) 2025-02-11 christopherfujino@gmail.com Update conductor to support monorepos (flutter/flutter#161704) 2025-02-11 jonahwilliams@google.com [Android] fix hcpp tapping, again, and add test. (flutter/flutter#163035) 2025-02-11 2049220+sigmundch@users.noreply.github.com Add new builder for experiment with dynamic modules. (flutter/flutter#162855) 2025-02-11 jason-simmons@users.noreply.github.com Roll vulkan-deps to 9edf248c597b (flutter/flutter#162549) 2025-02-11 47866232+chunhtai@users.noreply.github.com Adds dialog and alertdialog role (flutter/flutter#162692) 2025-02-11 engine-flutter-autoroll@skia.org Roll Dart SDK from 99789828cc95 to d9d7f103b6b7 (12 revisions) (flutter/flutter#163060) 2025-02-11 bkonyi@google.com [ Widget Preview ] Cleanup PreviewDetector code (flutter/flutter#163050) 2025-02-11 matanlurey@users.noreply.github.com Fix `SkiaException` -> `TestFailure`, add tests. (flutter/flutter#163054) 2025-02-11 jonahwilliams@google.com [Android] fix hcpp overlay layer intersection. (flutter/flutter#163024) 2025-02-11 bkonyi@google.com [ Widget Preview ] Update generated scaffold project to include early preview rendering (flutter/flutter#162847) 2025-02-11 matej.knopp@gmail.com [Embedder] Implement merged platform and UI thread (flutter/flutter#162944) 2025-02-11 jonahwilliams@google.com [Android] Remove overlay when platform views are removed from screen. (flutter/flutter#162908) 2025-02-11 jason-simmons@users.noreply.github.com Roll Dart to 3.8.0-76.0.dev (flutter/flutter#162913) 2025-02-11 jonahwilliams@google.com [Android] add HCPP platform views benchmark and integration test. (flutter/flutter#163018) 2025-02-11 engine-flutter-autoroll@skia.org Roll Skia from 8c377e8bedd2 to 5b56d9a91633 (9 revisions) (flutter/flutter#163021) 2025-02-11 matanlurey@users.noreply.github.com Try golden-testing on a Mokey (`bringup: true`), retry on an emulator (flutter/flutter#163029) 2025-02-11 robert.ancell@canonical.com Fix Linux keyboard support for AltGr (flutter/flutter#162495) 2025-02-11 robert.ancell@canonical.com Update gen_keycodes output to new engine location. (flutter/flutter#162479) 2025-02-10 jonahwilliams@google.com [Android] add runtime flag to determine if HCPP is supported. (flutter/flutter#163004) 2025-02-10 koji.wakamiya@gmail.com [iOS][Engine] Fix view removal process for AutofillContextAction.cancel (flutter/flutter#160653) 2025-02-10 matanlurey@users.noreply.github.com Re-land #162644: Remove `--verbose` from devicelab task executions. (flutter/flutter#163017) 2025-02-10 reidbaker@google.com Include device lab version for how to run test (flutter/flutter#163010) 2025-02-10 jacksongardner@google.com Change the default optimization level to `-O2` for wasm in release mode. (flutter/flutter#162917) 2025-02-10 yjbanov@google.com [web] robustify safaridriver launch sequence (flutter/flutter#162919) 2025-02-10 jakemac@google.com Return more eagerly when toggling service extensions (flutter/flutter#162774) 2025-02-10 goderbauer@google.com Fix doc reference typos (flutter/flutter#162893) 2025-02-10 engine-flutter-autoroll@skia.org Roll Skia from 180ed4fc263d to 8c377e8bedd2 (4 revisions) (flutter/flutter#162998) 2025-02-10 matanlurey@users.noreply.github.com FYI matanlurey (does not require review, but probably should) on dev/test infra. (flutter/flutter#162642) 2025-02-10 30870216+gaaclarke@users.noreply.github.com [Impeller] rrect_blur: scale max radius clamp by transform (flutter/flutter#161238) ...
Roll Flutter from 892f9c1 to e8f34a9 (71 revisions) flutter/flutter@892f9c1...e8f34a9 2025-02-12 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Skia from f31c733c86c4 to 25937c31f153 (2 revisions) (#163127)" (flutter/flutter#163133) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from f31c733c86c4 to 25937c31f153 (2 revisions) (flutter/flutter#163127) 2025-02-12 jonnywang@google.com Update .ci.yaml to support Fuchsia cherrypick branches (flutter/flutter#163000) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from 6f17f2ebb2e5 to f31c733c86c4 (1 revision) (flutter/flutter#163112) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from a9dbb2479c26 to 6f17f2ebb2e5 (2 revisions) (flutter/flutter#163109) 2025-02-12 jonahwilliams@google.com [devicelab] dont strip symbols in platform views layout test. (flutter/flutter#163101) 2025-02-12 jonahwilliams@google.com [Impeller] mirror tile mode requires highp for Adreno. (flutter/flutter#163066) 2025-02-12 engine-flutter-autoroll@skia.org Roll Skia from 5b56d9a91633 to a9dbb2479c26 (6 revisions) (flutter/flutter#163100) 2025-02-12 engine-flutter-autoroll@skia.org Roll Dart SDK from d9d7f103b6b7 to fcef25f18e4d (3 revisions) (flutter/flutter#163098) 2025-02-12 matanlurey@users.noreply.github.com Generate a correct `.flutter-plugin-dependencies` file for iOS/macOS projects (flutter/flutter#162834) 2025-02-12 matanlurey@users.noreply.github.com Remove unsound artifacts, remove `*Sound` qualifier. (flutter/flutter#163015) 2025-02-12 chinmaygarde@google.com [Impeller] libImpeller: Add support for Metal and Vulkan rendering. (flutter/flutter#161547) 2025-02-11 fluttergithubbot@gmail.com Marks Mac_benchmark basic_material_app_macos__compile to be flaky (flutter/flutter#162365) 2025-02-11 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#163083) 2025-02-11 47866232+chunhtai@users.noreply.github.com Adds hasSelectedState parameter to matchesSemantics for migration (flutter/flutter#163014) 2025-02-11 koji.wakamiya@gmail.com fix: Dispose codec after completing frame creation (flutter/flutter#159945) 2025-02-11 41930132+hellohuanlin@users.noreply.github.com [ios][secure_paste]show menu item based on info sent from framework (flutter/flutter#161103) 2025-02-11 christopherfujino@gmail.com Update conductor to support monorepos (flutter/flutter#161704) 2025-02-11 jonahwilliams@google.com [Android] fix hcpp tapping, again, and add test. (flutter/flutter#163035) 2025-02-11 2049220+sigmundch@users.noreply.github.com Add new builder for experiment with dynamic modules. (flutter/flutter#162855) 2025-02-11 jason-simmons@users.noreply.github.com Roll vulkan-deps to 9edf248c597b (flutter/flutter#162549) 2025-02-11 47866232+chunhtai@users.noreply.github.com Adds dialog and alertdialog role (flutter/flutter#162692) 2025-02-11 engine-flutter-autoroll@skia.org Roll Dart SDK from 99789828cc95 to d9d7f103b6b7 (12 revisions) (flutter/flutter#163060) 2025-02-11 bkonyi@google.com [ Widget Preview ] Cleanup PreviewDetector code (flutter/flutter#163050) 2025-02-11 matanlurey@users.noreply.github.com Fix `SkiaException` -> `TestFailure`, add tests. (flutter/flutter#163054) 2025-02-11 jonahwilliams@google.com [Android] fix hcpp overlay layer intersection. (flutter/flutter#163024) 2025-02-11 bkonyi@google.com [ Widget Preview ] Update generated scaffold project to include early preview rendering (flutter/flutter#162847) 2025-02-11 matej.knopp@gmail.com [Embedder] Implement merged platform and UI thread (flutter/flutter#162944) 2025-02-11 jonahwilliams@google.com [Android] Remove overlay when platform views are removed from screen. (flutter/flutter#162908) 2025-02-11 jason-simmons@users.noreply.github.com Roll Dart to 3.8.0-76.0.dev (flutter/flutter#162913) 2025-02-11 jonahwilliams@google.com [Android] add HCPP platform views benchmark and integration test. (flutter/flutter#163018) 2025-02-11 engine-flutter-autoroll@skia.org Roll Skia from 8c377e8bedd2 to 5b56d9a91633 (9 revisions) (flutter/flutter#163021) 2025-02-11 matanlurey@users.noreply.github.com Try golden-testing on a Mokey (`bringup: true`), retry on an emulator (flutter/flutter#163029) 2025-02-11 robert.ancell@canonical.com Fix Linux keyboard support for AltGr (flutter/flutter#162495) 2025-02-11 robert.ancell@canonical.com Update gen_keycodes output to new engine location. (flutter/flutter#162479) 2025-02-10 jonahwilliams@google.com [Android] add runtime flag to determine if HCPP is supported. (flutter/flutter#163004) 2025-02-10 koji.wakamiya@gmail.com [iOS][Engine] Fix view removal process for AutofillContextAction.cancel (flutter/flutter#160653) 2025-02-10 matanlurey@users.noreply.github.com Re-land #162644: Remove `--verbose` from devicelab task executions. (flutter/flutter#163017) 2025-02-10 reidbaker@google.com Include device lab version for how to run test (flutter/flutter#163010) 2025-02-10 jacksongardner@google.com Change the default optimization level to `-O2` for wasm in release mode. (flutter/flutter#162917) 2025-02-10 yjbanov@google.com [web] robustify safaridriver launch sequence (flutter/flutter#162919) 2025-02-10 jakemac@google.com Return more eagerly when toggling service extensions (flutter/flutter#162774) 2025-02-10 goderbauer@google.com Fix doc reference typos (flutter/flutter#162893) 2025-02-10 engine-flutter-autoroll@skia.org Roll Skia from 180ed4fc263d to 8c377e8bedd2 (4 revisions) (flutter/flutter#162998) 2025-02-10 matanlurey@users.noreply.github.com FYI matanlurey (does not require review, but probably should) on dev/test infra. (flutter/flutter#162642) 2025-02-10 30870216+gaaclarke@users.noreply.github.com [Impeller] rrect_blur: scale max radius clamp by transform (flutter/flutter#161238) ...
Fixes #152337
Introduces
ui_task_runner
field onFlutterCustomTaskRunners
. This lets the embedder to specify task runner for UI isolate tasks, allowing for merging platform and UI threads.With custom UI task runner there is no
MessageLoop
anymore for the UI thread, so the message loop task observer can no longer be used to flush microtask queue. Instead the microtask queue is flushed at the end of eachFlutterEngineRunTask
invocation if needed. This is handled internally by the embedder.Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.