Check GTK calls are done on the same thread. #174488
Merged
Conversation
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
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces thread-safety checks for mocked GTK calls in the test suite. It stores the thread ID when the MockGtk object is created and adds a check_thread function that verifies subsequent GTK function calls are made on the same thread. This check is integrated into numerous existing and newly added mock GTK/GDK functions to prevent race conditions and ensure thread-correctness in tests. My feedback focuses on adhering to the project's C++ style guide for a new data member.
| gtk_im_context_set_surrounding, | ||
| (GtkIMContext * context, const gchar* text, gint len, gint cursor_index)); | ||
|
|
||
| GThread* thread; |
Contributor
There was a problem hiding this comment.
According to the Google C++ Style Guide, which this repository follows, data member names must end with a trailing underscore.1 Please rename thread to thread_. This will require corresponding changes in mock_gtk.cc where this member is used.
Suggested change
| GThread* thread; | |
| GThread* thread_; |
Style Guide References
Footnotes
| static MockGtk* mock = nullptr; | ||
|
|
||
| MockGtk::MockGtk() { | ||
| thread = g_thread_self(); |
Contributor
There was a problem hiding this comment.
Following the style guide update for the thread member in mock_gtk.h, this usage should be updated to thread_.
thread_ = g_thread_self();| // Check we are running on the GTK thread. | ||
| static void check_thread() { | ||
| if (mock != nullptr) { | ||
| EXPECT_EQ(mock->thread, g_thread_self()); |
Contributor
There was a problem hiding this comment.
Following the style guide update for the thread member in mock_gtk.h, this usage should be updated to thread_.
EXPECT_EQ(mock->thread_, g_thread_self());
mattkae
approved these changes
Aug 27, 2025
Contributor
mattkae
left a comment
There was a problem hiding this comment.
One question, but approving it if you have good reason!
Contributor
|
This appears to be causing build errors due to conflicting definitions of |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 28, 2025
Reverts #174488 It is causing failures in testing: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8705343407724739553/+/u/build_ci_host_release_benchmarks_flutter_build_dart:copy_dart_sdk_flutter_display_list:display_list_benchmarks_flutter_display_list:display_list_builder_benchmarks_flutter_display_list:display_list_region_benchmarks_flutter_display_list:display_list_transform_benchmarks_flutter_fml:fml_benchmarks_flutter_impeller_geometry:geometry_benchmarks_flutter_lib_ui:ui_benchmarks_flutter_shell_common:shell_benchmarks_flutter_shell_testing_flutter_txt:txt_benchmarks_flutter_tools_path_ops_flutter_build_archives:flutter_patched_sdk_flutter:unittests/stdout
robert-ancell
added a commit
to robert-ancell/flutter
that referenced
this pull request
Aug 28, 2025
Resubmitted, as the previous PR had a merge error. Fixes flutter#173660
Contributor
|
Revert has landed. Feel free to rebase and reland now. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 28, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 28, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 28, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 29, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 29, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 29, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 30, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 30, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 31, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 31, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 1, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 1, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 1, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 2, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 2, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 2, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 2, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 2, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 2, 2025
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Sep 2, 2025
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Sep 2, 2025
Roll Flutter from da5523a to 6b18740 (49 revisions) flutter/flutter@da5523a...6b18740 2025-08-29 engine-flutter-autoroll@skia.org Roll Dart SDK from 11f6cd99f6b3 to 72cda0f3dc42 (2 revisions) (flutter/flutter#174697) 2025-08-29 sokolovskyi.konstantin@gmail.com Fix empty adaptive text selection toolbars building. (flutter/flutter#174656) 2025-08-29 jhy03261997@gmail.com [flutter_test] update the _isImportantForAccessibility method in SemanticsController to include tooltip (flutter/flutter#174476) 2025-08-29 engine-flutter-autoroll@skia.org Roll Skia from 89794f0b5384 to 43e79dc80ca8 (1 revision) (flutter/flutter#174678) 2025-08-29 engine-flutter-autoroll@skia.org Roll Skia from f3c8b4c677f5 to 89794f0b5384 (6 revisions) (flutter/flutter#174675) 2025-08-29 47866232+chunhtai@users.noreply.github.com Implement Overlay.of with inherited widget (flutter/flutter#174315) 2025-08-29 jacksongardner@google.com [impeller] Support partitioned host buffer (flutter/flutter#174463) 2025-08-29 47866232+chunhtai@users.noreply.github.com Adds semantics for disabled buttons in date picker (flutter/flutter#174064) 2025-08-29 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from bHYRvLv2Dg56RWZF2... to 00VSr-5B7hq0G2eZx... (flutter/flutter#174667) 2025-08-29 robert.ancell@canonical.com Check GTK calls are done on the same thread. (#174488) (flutter/flutter#174624) 2025-08-29 bkonyi@google.com [ Tool ] Only listen for DebugConnectionInfo if the service protocol is supported (flutter/flutter#174664) 2025-08-29 engine-flutter-autoroll@skia.org Roll Dart SDK from 89023922f96d to 11f6cd99f6b3 (9 revisions) (flutter/flutter#174669) 2025-08-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[web] Refactor renderers to use the same frontend code (#174588)" (flutter/flutter#174672) 2025-08-28 jhy03261997@gmail.com [a11y] [test] containsSemantics can ignore SemanticsValidationResult (flutter/flutter#174608) 2025-08-28 sokolovskyi.konstantin@gmail.com Fix some issues in engine-tool README. (flutter/flutter#174512) 2025-08-28 fluttergithubbot@gmail.com Marks Linux_pixel_7pro new_gallery__transition_perf to be flaky (flutter/flutter#174106) 2025-08-28 ahmedsameha1@gmail.com Make sure that an AlertDialog doesn't crash in 0x0 environment (flutter/flutter#174091) 2025-08-28 fluttergithubbot@gmail.com Marks Linux_pixel_7pro hello_world_impeller to be flaky (flutter/flutter#173699) 2025-08-28 fluttergithubbot@gmail.com Marks Linux_pixel_7pro drive_perf_debug_warning to be flaky (flutter/flutter#174112) 2025-08-28 fluttergithubbot@gmail.com Marks Linux_android_emu android_display_cutout to be flaky (flutter/flutter#174501) 2025-08-28 fluttergithubbot@gmail.com Marks Linux_pixel_7pro service_extensions_test to be flaky (flutter/flutter#174114) 2025-08-28 fluttergithubbot@gmail.com Marks Windows plugin_test to be flaky (flutter/flutter#174117) 2025-08-28 fluttergithubbot@gmail.com Marks Windows_mokey basic_material_app_win__compile to be flaky (flutter/flutter#173702) 2025-08-28 fluttergithubbot@gmail.com Marks Mac_mokey microbenchmarks to be flaky (flutter/flutter#174102) 2025-08-28 fluttergithubbot@gmail.com Marks Linux_mokey complex_layout__start_up to be flaky (flutter/flutter#173692) 2025-08-28 fluttergithubbot@gmail.com Marks Linux build_android_host_app_with_module_aar to be flaky (flutter/flutter#172631) 2025-08-28 fluttergithubbot@gmail.com Marks Linux_pixel_7pro new_gallery_opengles_impeller__transition_perf to be flaky (flutter/flutter#173338) 2025-08-28 fluttergithubbot@gmail.com Marks Linux_pixel_7pro platform_views_scroll_perf_impeller__timeline_summary to be flaky (flutter/flutter#172211) 2025-08-28 jason-simmons@users.noreply.github.com Remove the option to disable the merged platform/UI thread on Android and iOS (flutter/flutter#174408) 2025-08-28 mdebbar@google.com Don't fail when hot restarting `web-server` and there are no connected clients (flutter/flutter#174600) 2025-08-28 engine-flutter-autoroll@skia.org Roll Skia from 7c2fe2629d4a to f3c8b4c677f5 (7 revisions) (flutter/flutter#174654) 2025-08-28 mdebbar@google.com [WebParagraph] More plumbing towards making it usable in Flutter apps (flutter/flutter#174587) 2025-08-28 engine-flutter-autoroll@skia.org Roll Packages from 86fbeec to 141d8e3 (6 revisions) (flutter/flutter#174645) 2025-08-28 1961493+harryterkelsen@users.noreply.github.com [web] Refactor renderers to use the same frontend code (flutter/flutter#174588) 2025-08-28 engine-flutter-autoroll@skia.org Roll Skia from eb000b138a9d to 7c2fe2629d4a (3 revisions) (flutter/flutter#174637) 2025-08-28 bkonyi@google.com [ Tool ] Roll package:dwds 25.0.4 (flutter/flutter#174601) 2025-08-28 engine-flutter-autoroll@skia.org Roll Skia from 9b1642f2cfea to eb000b138a9d (2 revisions) (flutter/flutter#174627) 2025-08-28 engine-flutter-autoroll@skia.org Roll Skia from 430d60054d66 to 9b1642f2cfea (7 revisions) (flutter/flutter#174625) 2025-08-28 30870216+gaaclarke@users.noreply.github.com Refactored Canvas to disallow null inline contexts. (flutter/flutter#174530) 2025-08-28 flar@google.com Revert "Check GTK calls are done on the same thread." (flutter/flutter#174604) 2025-08-27 engine-flutter-autoroll@skia.org Roll Skia from 2a12b57fbbf0 to 430d60054d66 (3 revisions) (flutter/flutter#174590) 2025-08-27 ttankkeo112@gmail.com Retry "Implements the Android native stretch effect as a fragment shader (Impeller-only)." (flutter/flutter#173885) 2025-08-27 matanlurey@users.noreply.github.com Use raw `--removal-label "cp: ..."` when removing labels for unmerged PRs (flutter/flutter#174596) 2025-08-27 jakemac@google.com Flutter driver deserialization (flutter/flutter#172927) 2025-08-27 robert.ancell@canonical.com Check GTK calls are done on the same thread. (flutter/flutter#174488) 2025-08-27 matanlurey@users.noreply.github.com Fix broken reference to `PULL_REQUEST_CP_TEMPLATE.md` after refactor (flutter/flutter#174595) ...
mboetger
pushed a commit
to mboetger/flutter
that referenced
this pull request
Sep 18, 2025
mboetger
pushed a commit
to mboetger/flutter
that referenced
this pull request
Sep 18, 2025
Reverts flutter#174488 It is causing failures in testing: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8705343407724739553/+/u/build_ci_host_release_benchmarks_flutter_build_dart:copy_dart_sdk_flutter_display_list:display_list_benchmarks_flutter_display_list:display_list_builder_benchmarks_flutter_display_list:display_list_region_benchmarks_flutter_display_list:display_list_transform_benchmarks_flutter_fml:fml_benchmarks_flutter_impeller_geometry:geometry_benchmarks_flutter_lib_ui:ui_benchmarks_flutter_shell_common:shell_benchmarks_flutter_shell_testing_flutter_txt:txt_benchmarks_flutter_tools_path_ops_flutter_build_archives:flutter_patched_sdk_flutter:unittests/stdout
mboetger
pushed a commit
to mboetger/flutter
that referenced
this pull request
Sep 18, 2025
…r#174624) Resubmitted, as the previous PR had a merge error. Fixes flutter#173660
korca0220
pushed a commit
to korca0220/flutter
that referenced
this pull request
Sep 22, 2025
korca0220
pushed a commit
to korca0220/flutter
that referenced
this pull request
Sep 22, 2025
Reverts flutter#174488 It is causing failures in testing: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8705343407724739553/+/u/build_ci_host_release_benchmarks_flutter_build_dart:copy_dart_sdk_flutter_display_list:display_list_benchmarks_flutter_display_list:display_list_builder_benchmarks_flutter_display_list:display_list_region_benchmarks_flutter_display_list:display_list_transform_benchmarks_flutter_fml:fml_benchmarks_flutter_impeller_geometry:geometry_benchmarks_flutter_lib_ui:ui_benchmarks_flutter_shell_common:shell_benchmarks_flutter_shell_testing_flutter_txt:txt_benchmarks_flutter_tools_path_ops_flutter_build_archives:flutter_patched_sdk_flutter:unittests/stdout
korca0220
pushed a commit
to korca0220/flutter
that referenced
this pull request
Sep 22, 2025
…r#174624) Resubmitted, as the previous PR had a merge error. Fixes flutter#173660
Jaineel-Mamtora
pushed a commit
to Jaineel-Mamtora/flutter_forked
that referenced
this pull request
Sep 24, 2025
Jaineel-Mamtora
pushed a commit
to Jaineel-Mamtora/flutter_forked
that referenced
this pull request
Sep 24, 2025
Reverts flutter#174488 It is causing failures in testing: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8705343407724739553/+/u/build_ci_host_release_benchmarks_flutter_build_dart:copy_dart_sdk_flutter_display_list:display_list_benchmarks_flutter_display_list:display_list_builder_benchmarks_flutter_display_list:display_list_region_benchmarks_flutter_display_list:display_list_transform_benchmarks_flutter_fml:fml_benchmarks_flutter_impeller_geometry:geometry_benchmarks_flutter_lib_ui:ui_benchmarks_flutter_shell_common:shell_benchmarks_flutter_shell_testing_flutter_txt:txt_benchmarks_flutter_tools_path_ops_flutter_build_archives:flutter_patched_sdk_flutter:unittests/stdout
Jaineel-Mamtora
pushed a commit
to Jaineel-Mamtora/flutter_forked
that referenced
this pull request
Sep 24, 2025
…r#174624) Resubmitted, as the previous PR had a merge error. Fixes flutter#173660
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 12, 2025
lucaantonelli
pushed a commit
to lucaantonelli/flutter
that referenced
this pull request
Nov 21, 2025
lucaantonelli
pushed a commit
to lucaantonelli/flutter
that referenced
this pull request
Nov 21, 2025
Reverts flutter#174488 It is causing failures in testing: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8705343407724739553/+/u/build_ci_host_release_benchmarks_flutter_build_dart:copy_dart_sdk_flutter_display_list:display_list_benchmarks_flutter_display_list:display_list_builder_benchmarks_flutter_display_list:display_list_region_benchmarks_flutter_display_list:display_list_transform_benchmarks_flutter_fml:fml_benchmarks_flutter_impeller_geometry:geometry_benchmarks_flutter_lib_ui:ui_benchmarks_flutter_shell_common:shell_benchmarks_flutter_shell_testing_flutter_txt:txt_benchmarks_flutter_tools_path_ops_flutter_build_archives:flutter_patched_sdk_flutter:unittests/stdout
lucaantonelli
pushed a commit
to lucaantonelli/flutter
that referenced
this pull request
Nov 21, 2025
…r#174624) Resubmitted, as the previous PR had a merge error. Fixes flutter#173660
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes #173660