From 6b26d546338439d4433fd439d56db4143add6d05 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Thu, 9 Dec 2021 19:46:20 -0800 Subject: [PATCH] Override FlutterPlatformNodeDelegate::GetUniqueId The default implementation of GetUniqueId on ui::AXPlatformNodeDelegate always returns ID 1. We had previously implemented this on the windows platform node delegate, but for consistency's sake, and because the default implementation is surprising, we're promoting this to the FlutterPlatformNodeDelegate base class. Issue: https://github.com/flutter/flutter/issues/77838 --- .../common/flutter_platform_node_delegate.h | 4 +++ ...lutter_platform_node_delegate_unittests.cc | 28 +++++++++++++++++++ ...ibility_bridge_delegate_win32_unittests.cc | 15 ---------- .../flutter_platform_node_delegate_win32.h | 3 -- 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/shell/platform/common/flutter_platform_node_delegate.h b/shell/platform/common/flutter_platform_node_delegate.h index f28cd9181e3bc..2878fe45afe92 100644 --- a/shell/platform/common/flutter_platform_node_delegate.h +++ b/shell/platform/common/flutter_platform_node_delegate.h @@ -98,6 +98,9 @@ class FlutterPlatformNodeDelegate : public ui::AXPlatformNodeDelegateBase { // |ui::AXPlatformNodeDelegateBase| virtual ~FlutterPlatformNodeDelegate() override; + // |ui::AXPlatformNodeDelegateBase| + const ui::AXUniqueId& GetUniqueId() const override { return unique_id_; } + // |ui::AXPlatformNodeDelegateBase| const ui::AXNodeData& GetData() const override; @@ -144,6 +147,7 @@ class FlutterPlatformNodeDelegate : public ui::AXPlatformNodeDelegateBase { private: ui::AXNode* ax_node_; std::weak_ptr bridge_; + ui::AXUniqueId unique_id_; }; } // namespace flutter diff --git a/shell/platform/common/flutter_platform_node_delegate_unittests.cc b/shell/platform/common/flutter_platform_node_delegate_unittests.cc index d6aecb2c61cc0..62a1b22487849 100644 --- a/shell/platform/common/flutter_platform_node_delegate_unittests.cc +++ b/shell/platform/common/flutter_platform_node_delegate_unittests.cc @@ -12,6 +12,34 @@ namespace flutter { namespace testing { +TEST(FlutterPlatformNodeDelegateTest, NodeDelegateHasUniqueId) { + TestAccessibilityBridgeDelegate* delegate = + new TestAccessibilityBridgeDelegate(); + std::unique_ptr ptr(delegate); + std::shared_ptr bridge = + std::make_shared(std::move(ptr)); + + // Add node 0: root. + FlutterSemanticsNode node0{sizeof(FlutterSemanticsNode), 0}; + std::vector node0_children{1}; + node0.child_count = node0_children.size(); + node0.children_in_traversal_order = node0_children.data(); + node0.children_in_hit_test_order = node0_children.data(); + + // Add node 1: text child of node 0. + FlutterSemanticsNode node1{sizeof(FlutterSemanticsNode), 1}; + node1.label = "prefecture"; + node1.value = "Kyoto"; + + bridge->AddFlutterSemanticsNodeUpdate(&node0); + bridge->AddFlutterSemanticsNodeUpdate(&node1); + bridge->CommitUpdates(); + + auto node0_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(0).lock(); + auto node1_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(1).lock(); + EXPECT_TRUE(node0_delegate->GetUniqueId() != node1_delegate->GetUniqueId()); +} + TEST(FlutterPlatformNodeDelegateTest, canPerfomActions) { TestAccessibilityBridgeDelegate* delegate = new TestAccessibilityBridgeDelegate(); diff --git a/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc b/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc index 8afc6351de42d..78a4a528f522c 100644 --- a/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc +++ b/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc @@ -193,21 +193,6 @@ TEST(AccessibilityBridgeDelegateWin32, GetParentOnRootRetunsNullptr) { ASSERT_TRUE(node0_delegate->GetParent() == nullptr); } -TEST(AccessibilityBridgeDelegateWin32, NodeDelegateHasUniqueId) { - auto window_binding_handler = - std::make_unique<::testing::NiceMock>(); - FlutterWindowsView view(std::move(window_binding_handler)); - view.SetEngine(GetTestEngine()); - view.OnUpdateSemanticsEnabled(true); - - auto bridge = view.GetEngine()->accessibility_bridge().lock(); - PopulateAXTree(bridge); - - auto node0_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(0).lock(); - auto node1_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(1).lock(); - EXPECT_TRUE(node0_delegate->GetUniqueId() != node1_delegate->GetUniqueId()); -} - TEST(AccessibilityBridgeDelegateWin32, DispatchAccessibilityAction) { auto window_binding_handler = std::make_unique<::testing::NiceMock>(); diff --git a/shell/platform/windows/flutter_platform_node_delegate_win32.h b/shell/platform/windows/flutter_platform_node_delegate_win32.h index 2b36f7aff460d..3fd1aef71bdee 100644 --- a/shell/platform/windows/flutter_platform_node_delegate_win32.h +++ b/shell/platform/windows/flutter_platform_node_delegate_win32.h @@ -35,8 +35,6 @@ class FlutterPlatformNodeDelegateWin32 : public FlutterPlatformNodeDelegate { const ui::AXClippingBehavior clipping_behavior, ui::AXOffscreenResult* offscreen_result) const override; - const ui::AXUniqueId& GetUniqueId() const override { return unique_id_; } - // Dispatches a Windows accessibility event of the specified type, generated // by the accessibility node associated with this object. This is a // convenience wrapper around |NotifyWinEvent|. @@ -49,7 +47,6 @@ class FlutterPlatformNodeDelegateWin32 : public FlutterPlatformNodeDelegate { private: ui::AXPlatformNode* ax_platform_node_; FlutterWindowsEngine* engine_; - ui::AXUniqueId unique_id_; }; } // namespace flutter