From 9b0c5c6c1c8c3f39921613c953296bd352ee5763 Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Wed, 26 Mar 2025 13:50:17 +1300 Subject: [PATCH] Fix regression in semantics update handling. (#165842) This was shown with the following errors in stdout: GLib-GObject-CRITICAL **: 11:41:35.775: invalid cast from 'FlEngine' to 'FlView' This was broken in 554c814ada6419c4ece39a8de5ff38e16cda614b --- .../ci/licenses_golden/licenses_flutter | 2 + .../src/flutter/shell/platform/linux/BUILD.gn | 1 + .../flutter/shell/platform/linux/fl_view.cc | 50 ++++++++++-------- .../platform/linux/fl_view_accessible.cc | 2 + .../shell/platform/linux/fl_view_private.h | 25 +++++++++ .../shell/platform/linux/fl_view_test.cc | 52 +++++++++++++++++++ 6 files changed, 109 insertions(+), 23 deletions(-) create mode 100644 engine/src/flutter/shell/platform/linux/fl_view_private.h diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index 82256f6c5b..ffbe0003f4 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -53327,6 +53327,7 @@ ORIGIN: ../../../flutter/shell/platform/linux/fl_view.cc + ../../../flutter/LICE ORIGIN: ../../../flutter/shell/platform/linux/fl_view_accessible.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/linux/fl_view_accessible.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/linux/fl_view_accessible_test.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/shell/platform/linux/fl_view_private.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/linux/fl_view_test.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/linux/fl_window_state_monitor.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/shell/platform/linux/fl_window_state_monitor.h + ../../../flutter/LICENSE @@ -56336,6 +56337,7 @@ FILE: ../../../flutter/shell/platform/linux/fl_view.cc FILE: ../../../flutter/shell/platform/linux/fl_view_accessible.cc FILE: ../../../flutter/shell/platform/linux/fl_view_accessible.h FILE: ../../../flutter/shell/platform/linux/fl_view_accessible_test.cc +FILE: ../../../flutter/shell/platform/linux/fl_view_private.h FILE: ../../../flutter/shell/platform/linux/fl_view_test.cc FILE: ../../../flutter/shell/platform/linux/fl_window_state_monitor.cc FILE: ../../../flutter/shell/platform/linux/fl_window_state_monitor.h diff --git a/engine/src/flutter/shell/platform/linux/BUILD.gn b/engine/src/flutter/shell/platform/linux/BUILD.gn index 4607d30b95..31b4d8e0f8 100644 --- a/engine/src/flutter/shell/platform/linux/BUILD.gn +++ b/engine/src/flutter/shell/platform/linux/BUILD.gn @@ -91,6 +91,7 @@ source_set("flutter_linux_sources") { "fl_method_codec_private.h", "fl_plugin_registrar_private.h", "fl_pointer_manager.h", + "fl_view_private.h", "fl_window_state_monitor.h", "key_mapping.h", ] diff --git a/engine/src/flutter/shell/platform/linux/fl_view.cc b/engine/src/flutter/shell/platform/linux/fl_view.cc index e972578aa7..539c58c94d 100644 --- a/engine/src/flutter/shell/platform/linux/fl_view.cc +++ b/engine/src/flutter/shell/platform/linux/fl_view.cc @@ -20,6 +20,7 @@ #include "flutter/shell/platform/linux/fl_socket_accessible.h" #include "flutter/shell/platform/linux/fl_touch_manager.h" #include "flutter/shell/platform/linux/fl_view_accessible.h" +#include "flutter/shell/platform/linux/fl_view_private.h" #include "flutter/shell/platform/linux/fl_window_state_monitor.h" #include "flutter/shell/platform/linux/public/flutter_linux/fl_engine.h" #include "flutter/shell/platform/linux/public/flutter_linux/fl_plugin_registry.h" @@ -222,11 +223,8 @@ static void view_added_cb(GObject* object, } // Called when the engine updates accessibility. -static void update_semantics_cb(FlEngine* engine, - const FlutterSemanticsUpdate2* update, - gpointer user_data) { - FlView* self = FL_VIEW(user_data); - +static void update_semantics_cb(FlView* self, + const FlutterSemanticsUpdate2* update) { // A semantics update is routed to a particular view. if (update->view_id != self->view_id) { return; @@ -496,11 +494,6 @@ static void realize_cb(FlView* self) { setup_cursor(self); handle_geometry_changed(self); - - self->view_accessible = fl_view_accessible_new(self->engine, self->view_id); - fl_socket_accessible_embed( - FL_SOCKET_ACCESSIBLE(gtk_widget_get_accessible(GTK_WIDGET(self))), - atk_plug_get_id(ATK_PLUG(self->view_accessible))); } static void secondary_realize_cb(FlView* self) { @@ -685,6 +678,22 @@ static void fl_view_class_init(FlViewClass* klass) { fl_socket_accessible_get_type()); } +// Engine related construction. +static void setup_engine(FlView* self) { + self->view_accessible = fl_view_accessible_new(self->engine, self->view_id); + fl_socket_accessible_embed( + FL_SOCKET_ACCESSIBLE(gtk_widget_get_accessible(GTK_WIDGET(self))), + atk_plug_get_id(ATK_PLUG(self->view_accessible))); + + self->pointer_manager = fl_pointer_manager_new(self->view_id, self->engine); + + self->on_pre_engine_restart_cb_id = + g_signal_connect_swapped(self->engine, "on-pre-engine-restart", + G_CALLBACK(on_pre_engine_restart_cb), self); + self->update_semantics_cb_id = g_signal_connect_swapped( + self->engine, "update-semantics", G_CALLBACK(update_semantics_cb), self); +} + static void fl_view_init(FlView* self) { self->cancellable = g_cancellable_new(); @@ -755,13 +764,7 @@ G_MODULE_EXPORT FlView* fl_view_new(FlDartProject* project) { g_assert(FL_IS_RENDERER_GDK(renderer)); self->renderer = FL_RENDERER_GDK(g_object_ref(renderer)); - self->pointer_manager = fl_pointer_manager_new(self->view_id, engine); - - self->on_pre_engine_restart_cb_id = - g_signal_connect_swapped(self->engine, "on-pre-engine-restart", - G_CALLBACK(on_pre_engine_restart_cb), self); - self->update_semantics_cb_id = g_signal_connect_swapped( - engine, "update-semantics", G_CALLBACK(update_semantics_cb), self); + setup_engine(self); g_signal_connect_swapped(self->gl_area, "create-context", G_CALLBACK(create_context_cb), self); @@ -781,18 +784,14 @@ G_MODULE_EXPORT FlView* fl_view_new_for_engine(FlEngine* engine) { g_assert(FL_IS_RENDERER_GDK(renderer)); self->renderer = FL_RENDERER_GDK(g_object_ref(renderer)); - self->on_pre_engine_restart_cb_id = - g_signal_connect_swapped(engine, "on-pre-engine-restart", - G_CALLBACK(on_pre_engine_restart_cb), self); - self->view_id = fl_engine_add_view(engine, 1, 1, 1.0, self->cancellable, view_added_cb, self); + setup_engine(self); + fl_renderer_add_renderable(FL_RENDERER(self->renderer), self->view_id, FL_RENDERABLE(self)); - self->pointer_manager = fl_pointer_manager_new(self->view_id, engine); - g_signal_connect_swapped(self->gl_area, "realize", G_CALLBACK(secondary_realize_cb), self); return self; @@ -815,3 +814,8 @@ G_MODULE_EXPORT void fl_view_set_background_color(FlView* self, gdk_rgba_free(self->background_color); self->background_color = gdk_rgba_copy(color); } + +FlViewAccessible* fl_view_get_accessible(FlView* self) { + g_return_val_if_fail(FL_IS_VIEW(self), nullptr); + return self->view_accessible; +} diff --git a/engine/src/flutter/shell/platform/linux/fl_view_accessible.cc b/engine/src/flutter/shell/platform/linux/fl_view_accessible.cc index f78a48fdb2..a7cb0c48c8 100644 --- a/engine/src/flutter/shell/platform/linux/fl_view_accessible.cc +++ b/engine/src/flutter/shell/platform/linux/fl_view_accessible.cc @@ -143,6 +143,8 @@ FlViewAccessible* fl_view_accessible_new(FlEngine* engine, void fl_view_accessible_handle_update_semantics( FlViewAccessible* self, const FlutterSemanticsUpdate2* update) { + g_return_if_fail(FL_IS_VIEW_ACCESSIBLE(self)); + g_autoptr(GHashTable) pending_children = g_hash_table_new_full(g_direct_hash, g_direct_equal, nullptr, reinterpret_cast(fl_value_unref)); diff --git a/engine/src/flutter/shell/platform/linux/fl_view_private.h b/engine/src/flutter/shell/platform/linux/fl_view_private.h new file mode 100644 index 0000000000..cbdcef2ba3 --- /dev/null +++ b/engine/src/flutter/shell/platform/linux/fl_view_private.h @@ -0,0 +1,25 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_SHELL_PLATFORM_LINUX_FL_VIEW_PRIVATE_H_ +#define FLUTTER_SHELL_PLATFORM_LINUX_FL_VIEW_PRIVATE_H_ + +#include "flutter/shell/platform/linux/fl_view_accessible.h" +#include "flutter/shell/platform/linux/public/flutter_linux/fl_view.h" + +G_BEGIN_DECLS + +/** + * fl_view_get_accessible: + * @view: an #FlView. + * + * Get the accessible object for this view. + * + * Returns: an #FlViewAccessible. + */ +FlViewAccessible* fl_view_get_accessible(FlView* view); + +G_END_DECLS + +#endif // FLUTTER_SHELL_PLATFORM_LINUX_FL_VIEW_PRIVATE_H_ diff --git a/engine/src/flutter/shell/platform/linux/fl_view_test.cc b/engine/src/flutter/shell/platform/linux/fl_view_test.cc index 24e2d5c617..a3b4f189f3 100644 --- a/engine/src/flutter/shell/platform/linux/fl_view_test.cc +++ b/engine/src/flutter/shell/platform/linux/fl_view_test.cc @@ -5,6 +5,7 @@ #include "flutter/shell/platform/linux/public/flutter_linux/fl_view.h" #include "flutter/shell/platform/embedder/test_utils/proc_table_replacement.h" #include "flutter/shell/platform/linux/fl_engine_private.h" +#include "flutter/shell/platform/linux/fl_view_private.h" #include "flutter/shell/platform/linux/testing/fl_test.h" #include "flutter/shell/platform/linux/testing/fl_test_gtk_logs.h" #include "flutter/shell/platform/linux/testing/mock_gtk.h" @@ -61,6 +62,57 @@ TEST(FlViewTest, FirstFrameSignal) { EXPECT_TRUE(first_frame_emitted); } +// Check semantics update applied +TEST(FlViewTest, SemanticsUpdate) { + flutter::testing::fl_ensure_gtk_init(); + + g_autoptr(FlDartProject) project = fl_dart_project_new(); + FlView* view = fl_view_new(project); + + FlEngine* engine = fl_view_get_engine(view); + g_autoptr(GError) error = nullptr; + EXPECT_TRUE(fl_engine_start(engine, &error)); + + FlutterSemanticsNode2 root_node = { + .id = 0, + .label = "root", + }; + FlutterSemanticsNode2* nodes[1] = {&root_node}; + FlutterSemanticsUpdate2 update = { + .node_count = 1, .nodes = nodes, .view_id = 0}; + g_signal_emit_by_name(engine, "update-semantics", &update); + + FlViewAccessible* accessible = fl_view_get_accessible(view); + EXPECT_EQ(atk_object_get_n_accessible_children(ATK_OBJECT(accessible)), 1); + AtkObject* root_object = + atk_object_ref_accessible_child(ATK_OBJECT(accessible), 0); + EXPECT_STREQ(atk_object_get_name(root_object), "root"); +} + +// Check semantics update ignored for other view +TEST(FlViewTest, SemanticsUpdateOtherView) { + flutter::testing::fl_ensure_gtk_init(); + + g_autoptr(FlDartProject) project = fl_dart_project_new(); + FlView* view = fl_view_new(project); + + FlEngine* engine = fl_view_get_engine(view); + g_autoptr(GError) error = nullptr; + EXPECT_TRUE(fl_engine_start(engine, &error)); + + FlutterSemanticsNode2 root_node = { + .id = 0, + .label = "root", + }; + FlutterSemanticsNode2* nodes[1] = {&root_node}; + FlutterSemanticsUpdate2 update = { + .node_count = 1, .nodes = nodes, .view_id = 99}; + g_signal_emit_by_name(engine, "update-semantics", &update); + + FlViewAccessible* accessible = fl_view_get_accessible(view); + EXPECT_EQ(atk_object_get_n_accessible_children(ATK_OBJECT(accessible)), 0); +} + // Check secondary view is registered with engine. TEST(FlViewTest, SecondaryView) { flutter::testing::fl_ensure_gtk_init();