From f3acc7869e791d1dab908829b39905c48f4261f8 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 8 Apr 2022 18:18:52 +0200 Subject: [PATCH 1/4] [Linux] make FlViewAccessible testable Add construct-only 'engine' property to make it possible to inject a mock engine. --- shell/platform/linux/fl_view_accessible.cc | 61 +++++++++++++++++++++- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/shell/platform/linux/fl_view_accessible.cc b/shell/platform/linux/fl_view_accessible.cc index 814bbb4faf052..da99aaaa0cc09 100644 --- a/shell/platform/linux/fl_view_accessible.cc +++ b/shell/platform/linux/fl_view_accessible.cc @@ -9,14 +9,33 @@ struct _FlViewAccessible { GtkContainerAccessible parent_instance; + FlEngine* engine; + // Semantics nodes keyed by ID GHashTable* semantics_nodes_by_id; }; +enum { PROP_0, PROP_ENGINE, PROP_LAST }; + G_DEFINE_TYPE(FlViewAccessible, fl_view_accessible, GTK_TYPE_CONTAINER_ACCESSIBLE) +static void fl_view_accessible_set_engine(FlViewAccessible* self, + FlEngine* engine) { + self->engine = engine; + g_object_add_weak_pointer(G_OBJECT(self), + reinterpret_cast(&self->engine)); +} + +static FlEngine* fl_view_accessible_get_engine(FlViewAccessible* self) { + if (self->engine == nullptr) { + FlView* view = FL_VIEW(gtk_accessible_get_widget(GTK_ACCESSIBLE(self))); + fl_view_accessible_set_engine(self, fl_view_get_engine(view)); + } + return self->engine; +} + // Gets the ATK node for the given id. // If the node doesn't exist it will be created. static FlAccessibleNode* get_node(FlViewAccessible* self, int32_t id) { @@ -26,8 +45,8 @@ static FlAccessibleNode* get_node(FlViewAccessible* self, int32_t id) { return node; } - FlView* view = FL_VIEW(gtk_accessible_get_widget(GTK_ACCESSIBLE(self))); - node = fl_accessible_node_new(fl_view_get_engine(view), id); + FlEngine* engine = fl_view_accessible_get_engine(self); + node = fl_accessible_node_new(engine, id); if (id == 0) { fl_accessible_node_set_parent(node, ATK_OBJECT(self), 0); } @@ -59,10 +78,48 @@ static AtkRole fl_view_accessible_get_role(AtkObject* accessible) { return ATK_ROLE_PANEL; } +// Implements GObject::set_property +static void fl_view_accessible_set_property(GObject* object, + guint prop_id, + const GValue* value, + GParamSpec* pspec) { + FlViewAccessible* self = FL_VIEW_ACCESSIBLE(object); + switch (prop_id) { + case PROP_ENGINE: + fl_view_accessible_set_engine(self, FL_ENGINE(g_value_get_object(value))); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); + break; + } +} + +static void fl_view_accessible_dispose(GObject* object) { + FlViewAccessible* self = FL_VIEW_ACCESSIBLE(object); + + if (self->engine != nullptr) { + g_object_remove_weak_pointer(object, + reinterpret_cast(&self->engine)); + self->engine = nullptr; + } + + G_OBJECT_CLASS(fl_view_accessible_parent_class)->dispose(object); +} + static void fl_view_accessible_class_init(FlViewAccessibleClass* klass) { ATK_OBJECT_CLASS(klass)->get_n_children = fl_view_accessible_get_n_children; ATK_OBJECT_CLASS(klass)->ref_child = fl_view_accessible_ref_child; ATK_OBJECT_CLASS(klass)->get_role = fl_view_accessible_get_role; + + G_OBJECT_CLASS(klass)->dispose = fl_view_accessible_dispose; + G_OBJECT_CLASS(klass)->set_property = fl_view_accessible_set_property; + + g_object_class_install_property( + G_OBJECT_CLASS(klass), PROP_ENGINE, + g_param_spec_object( + "engine", "engine", "Flutter engine", fl_engine_get_type(), + static_cast(G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS))); } static void fl_view_accessible_init(FlViewAccessible* self) { From cbfe72577b71f91682cb820c7c8347d9b9335e4b Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 8 Apr 2022 18:21:14 +0200 Subject: [PATCH 2/4] [Linux] test FlViewAccessible --- ci/licenses_golden/licenses_flutter | 1 + shell/platform/linux/BUILD.gn | 1 + .../platform/linux/fl_view_accessible_test.cc | 157 ++++++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 shell/platform/linux/fl_view_accessible_test.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index e1dec7770725b..61237238ffa06 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2102,6 +2102,7 @@ FILE: ../../../flutter/shell/platform/linux/fl_value_test.cc 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/key_mapping.cc FILE: ../../../flutter/shell/platform/linux/key_mapping.h diff --git a/shell/platform/linux/BUILD.gn b/shell/platform/linux/BUILD.gn index 9d218cd186fe4..f3641151ff70b 100644 --- a/shell/platform/linux/BUILD.gn +++ b/shell/platform/linux/BUILD.gn @@ -203,6 +203,7 @@ executable("flutter_linux_unittests") { "fl_texture_gl_test.cc", "fl_texture_registrar_test.cc", "fl_value_test.cc", + "fl_view_accessible_test.cc", "testing/fl_test.cc", "testing/mock_binary_messenger.cc", "testing/mock_binary_messenger_response_handle.cc", diff --git a/shell/platform/linux/fl_view_accessible_test.cc b/shell/platform/linux/fl_view_accessible_test.cc new file mode 100644 index 0000000000000..adbe48dc14bf2 --- /dev/null +++ b/shell/platform/linux/fl_view_accessible_test.cc @@ -0,0 +1,157 @@ +// 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. + +// Included first as it collides with the X11 headers. +#include "gtest/gtest.h" + +#include "flutter/shell/platform/linux/fl_view_accessible.h" +#include "flutter/shell/platform/linux/public/flutter_linux/fl_engine.h" +#include "flutter/shell/platform/linux/testing/fl_test.h" +#include "flutter/shell/platform/linux/testing/mock_signal_handler.h" + +TEST(FlViewAccessibleTest, BuildTree) { + g_autoptr(FlEngine) engine = make_mock_engine(); + g_autoptr(FlViewAccessible) accessible = FL_VIEW_ACCESSIBLE( + g_object_new(fl_view_accessible_get_type(), "engine", engine, nullptr)); + + const int32_t children[] = {111, 222}; + const FlutterSemanticsNode root_node = { + .id = 0, + .label = "root", + .child_count = 2, + .children_in_traversal_order = children, + }; + fl_view_accessible_handle_update_semantics_node(accessible, &root_node); + + const FlutterSemanticsNode child1_node = {.id = 111, .label = "child 1"}; + fl_view_accessible_handle_update_semantics_node(accessible, &child1_node); + + const FlutterSemanticsNode child2_node = {.id = 222, .label = "child 2"}; + fl_view_accessible_handle_update_semantics_node(accessible, &child2_node); + + AtkObject* root_object = + atk_object_ref_accessible_child(ATK_OBJECT(accessible), 0); + EXPECT_STREQ(atk_object_get_name(root_object), "root"); + EXPECT_EQ(atk_object_get_index_in_parent(root_object), 0); + EXPECT_EQ(atk_object_get_n_accessible_children(root_object), 2); + + AtkObject* child1_object = atk_object_ref_accessible_child(root_object, 0); + EXPECT_STREQ(atk_object_get_name(child1_object), "child 1"); + EXPECT_EQ(atk_object_get_parent(child1_object), root_object); + EXPECT_EQ(atk_object_get_index_in_parent(child1_object), 0); + EXPECT_EQ(atk_object_get_n_accessible_children(child1_object), 0); + + AtkObject* child2_object = atk_object_ref_accessible_child(root_object, 1); + EXPECT_STREQ(atk_object_get_name(child2_object), "child 2"); + EXPECT_EQ(atk_object_get_parent(child2_object), root_object); + EXPECT_EQ(atk_object_get_index_in_parent(child2_object), 1); + EXPECT_EQ(atk_object_get_n_accessible_children(child2_object), 0); +} + +TEST(FlViewAccessibleTest, AddRemoveChildren) { + g_autoptr(FlEngine) engine = make_mock_engine(); + g_autoptr(FlViewAccessible) accessible = FL_VIEW_ACCESSIBLE( + g_object_new(fl_view_accessible_get_type(), "engine", engine, nullptr)); + + FlutterSemanticsNode root_node = { + .id = 0, + .label = "root", + .child_count = 0, + }; + fl_view_accessible_handle_update_semantics_node(accessible, &root_node); + + AtkObject* root_object = + atk_object_ref_accessible_child(ATK_OBJECT(accessible), 0); + EXPECT_EQ(atk_object_get_n_accessible_children(root_object), 0); + + // add child1 + AtkObject* child1_object = nullptr; + { + flutter::testing::MockSignalHandler2 child1_added( + root_object, "children-changed::add"); + EXPECT_SIGNAL2(child1_added, ::testing::Eq(0), ::testing::A()) + .WillOnce(::testing::SaveArg<1>(&child1_object)); + + const int32_t children[] = {111}; + root_node.child_count = 1; + root_node.children_in_traversal_order = children; + fl_view_accessible_handle_update_semantics_node(accessible, &root_node); + + const FlutterSemanticsNode child1_node = {.id = 111, .label = "child 1"}; + fl_view_accessible_handle_update_semantics_node(accessible, &child1_node); + } + + EXPECT_EQ(atk_object_get_n_accessible_children(root_object), 1); + EXPECT_EQ(atk_object_ref_accessible_child(root_object, 0), child1_object); + + EXPECT_STREQ(atk_object_get_name(child1_object), "child 1"); + EXPECT_EQ(atk_object_get_parent(child1_object), root_object); + EXPECT_EQ(atk_object_get_index_in_parent(child1_object), 0); + EXPECT_EQ(atk_object_get_n_accessible_children(child1_object), 0); + + // add child2 + AtkObject* child2_object = nullptr; + { + flutter::testing::MockSignalHandler2 child2_added( + root_object, "children-changed::add"); + EXPECT_SIGNAL2(child2_added, ::testing::Eq(1), ::testing::A()) + .WillOnce(::testing::SaveArg<1>(&child2_object)); + + const int32_t children[] = {111, 222}; + root_node.child_count = 2; + root_node.children_in_traversal_order = children; + fl_view_accessible_handle_update_semantics_node(accessible, &root_node); + + const FlutterSemanticsNode child2_node = {.id = 222, .label = "child 2"}; + fl_view_accessible_handle_update_semantics_node(accessible, &child2_node); + } + + EXPECT_EQ(atk_object_get_n_accessible_children(root_object), 2); + EXPECT_EQ(atk_object_ref_accessible_child(root_object, 0), child1_object); + EXPECT_EQ(atk_object_ref_accessible_child(root_object, 1), child2_object); + + EXPECT_STREQ(atk_object_get_name(child1_object), "child 1"); + EXPECT_EQ(atk_object_get_parent(child1_object), root_object); + EXPECT_EQ(atk_object_get_index_in_parent(child1_object), 0); + EXPECT_EQ(atk_object_get_n_accessible_children(child1_object), 0); + + EXPECT_STREQ(atk_object_get_name(child2_object), "child 2"); + EXPECT_EQ(atk_object_get_parent(child2_object), root_object); + EXPECT_EQ(atk_object_get_index_in_parent(child2_object), 1); + EXPECT_EQ(atk_object_get_n_accessible_children(child2_object), 0); + + // remove child1 + { + flutter::testing::MockSignalHandler2 child1_removed( + root_object, "children-changed::remove"); + EXPECT_SIGNAL2(child1_removed, ::testing::Eq(0), + ::testing::Eq(child1_object)); + + const int32_t children[] = {222}; + root_node.child_count = 1; + root_node.children_in_traversal_order = children; + fl_view_accessible_handle_update_semantics_node(accessible, &root_node); + } + + EXPECT_EQ(atk_object_get_n_accessible_children(root_object), 1); + EXPECT_EQ(atk_object_ref_accessible_child(root_object, 0), child2_object); + + EXPECT_STREQ(atk_object_get_name(child2_object), "child 2"); + EXPECT_EQ(atk_object_get_parent(child2_object), root_object); + EXPECT_EQ(atk_object_get_index_in_parent(child2_object), 0); + EXPECT_EQ(atk_object_get_n_accessible_children(child2_object), 0); + + // remove child2 + { + flutter::testing::MockSignalHandler2 child2_removed( + root_object, "children-changed::remove"); + EXPECT_SIGNAL2(child2_removed, ::testing::Eq(0), + ::testing::Eq(child2_object)); + + root_node.child_count = 0; + fl_view_accessible_handle_update_semantics_node(accessible, &root_node); + } + + EXPECT_EQ(atk_object_get_n_accessible_children(root_object), 0); +} From b771ce8c1d711b7880288557a00b70a437092e9b Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Tue, 10 May 2022 07:11:16 +0200 Subject: [PATCH 3/4] Assert one time init of engine --- shell/platform/linux/fl_view_accessible.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/platform/linux/fl_view_accessible.cc b/shell/platform/linux/fl_view_accessible.cc index da99aaaa0cc09..7bb9da606353a 100644 --- a/shell/platform/linux/fl_view_accessible.cc +++ b/shell/platform/linux/fl_view_accessible.cc @@ -23,6 +23,7 @@ G_DEFINE_TYPE(FlViewAccessible, static void fl_view_accessible_set_engine(FlViewAccessible* self, FlEngine* engine) { + g_assert(self->engine == nullptr); self->engine = engine; g_object_add_weak_pointer(G_OBJECT(self), reinterpret_cast(&self->engine)); From 4429dc3bbaac98cff92275dcc40af41c57b925f2 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Tue, 10 May 2022 07:12:25 +0200 Subject: [PATCH 4/4] Rename static functions - drop fl_view_accessible prefix for non-methods - set_engine -> init_engine to make the intention clear --- shell/platform/linux/fl_view_accessible.cc | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/shell/platform/linux/fl_view_accessible.cc b/shell/platform/linux/fl_view_accessible.cc index 7bb9da606353a..0bc0e44f489c6 100644 --- a/shell/platform/linux/fl_view_accessible.cc +++ b/shell/platform/linux/fl_view_accessible.cc @@ -21,18 +21,17 @@ G_DEFINE_TYPE(FlViewAccessible, fl_view_accessible, GTK_TYPE_CONTAINER_ACCESSIBLE) -static void fl_view_accessible_set_engine(FlViewAccessible* self, - FlEngine* engine) { +static void init_engine(FlViewAccessible* self, FlEngine* engine) { g_assert(self->engine == nullptr); self->engine = engine; g_object_add_weak_pointer(G_OBJECT(self), reinterpret_cast(&self->engine)); } -static FlEngine* fl_view_accessible_get_engine(FlViewAccessible* self) { +static FlEngine* get_engine(FlViewAccessible* self) { if (self->engine == nullptr) { FlView* view = FL_VIEW(gtk_accessible_get_widget(GTK_ACCESSIBLE(self))); - fl_view_accessible_set_engine(self, fl_view_get_engine(view)); + init_engine(self, fl_view_get_engine(view)); } return self->engine; } @@ -46,7 +45,7 @@ static FlAccessibleNode* get_node(FlViewAccessible* self, int32_t id) { return node; } - FlEngine* engine = fl_view_accessible_get_engine(self); + FlEngine* engine = get_engine(self); node = fl_accessible_node_new(engine, id); if (id == 0) { fl_accessible_node_set_parent(node, ATK_OBJECT(self), 0); @@ -87,7 +86,7 @@ static void fl_view_accessible_set_property(GObject* object, FlViewAccessible* self = FL_VIEW_ACCESSIBLE(object); switch (prop_id) { case PROP_ENGINE: - fl_view_accessible_set_engine(self, FL_ENGINE(g_value_get_object(value))); + init_engine(self, FL_ENGINE(g_value_get_object(value))); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);