From 247448202134220169c56d72d94fdcaa73d7d19e Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Fri, 10 Dec 2021 10:05:00 -0800 Subject: [PATCH] Override FlutterPlatformNodeDelegate::GetUniqueId (flutter/engine#30261) 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/engine/src/flutter/shell/platform/common/flutter_platform_node_delegate.h b/engine/src/flutter/shell/platform/common/flutter_platform_node_delegate.h index f28cd9181e..2878fe45af 100644 --- a/engine/src/flutter/shell/platform/common/flutter_platform_node_delegate.h +++ b/engine/src/flutter/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/engine/src/flutter/shell/platform/common/flutter_platform_node_delegate_unittests.cc b/engine/src/flutter/shell/platform/common/flutter_platform_node_delegate_unittests.cc index d6aecb2c61..62a1b22487 100644 --- a/engine/src/flutter/shell/platform/common/flutter_platform_node_delegate_unittests.cc +++ b/engine/src/flutter/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/engine/src/flutter/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc b/engine/src/flutter/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc index 8afc6351de..78a4a528f5 100644 --- a/engine/src/flutter/shell/platform/windows/accessibility_bridge_delegate_win32_unittests.cc +++ b/engine/src/flutter/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/engine/src/flutter/shell/platform/windows/flutter_platform_node_delegate_win32.h b/engine/src/flutter/shell/platform/windows/flutter_platform_node_delegate_win32.h index 2b36f7aff4..3fd1aef71b 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_platform_node_delegate_win32.h +++ b/engine/src/flutter/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