From b0de5cc2f4b3f0792f1f7bfdf58a52348200a5fd Mon Sep 17 00:00:00 2001 From: Aaron Leventhal Date: Thu, 11 Jan 2018 01:18:53 +0000 Subject: [PATCH] Consistent, unified Unique IDs for accessibility objects in browser process Summary: - Needed to form relationships between objects - Unique IDs for all objects in Chrome via AXUniqueID - Windows will now use the same IDs for firing events, instead of creating its own in ax_platform_node_win.cc - Separate Chrome unique IDs from Blink IDs by consistently using the name "UniqueId" and by making them different types. - AXPlatformNodeDelegate is now responsible for providing IDs Every accessibility node Chrome needs a unique ids so that events can be fired for it in Windows, and so that it can be the target of relationships. Unlike the ids provided by Blink, these ids must be unique for the entire Chrome browser process. Because there are these two types of ids (Blink IDs which are only unique within a frame, and must be created by Blink, and Chrome unique IDs that are unique within the entire Chrome process), it is best to make this fool proof and not allow the two to be mixed. This is accomplished via a UniqueId class for the Chrome IDs, which cannot be initialized with a Blink ID (int32_t). Bug: 797992 Change-Id: I60d2dfeca8fe107b15838cfcc17dc07a90c29cc9 Reviewed-on: https://chromium-review.googlesource.com/840480 Commit-Queue: Aaron Leventhal Reviewed-by: Dominic Mazzoni Reviewed-by: David Tseng Cr-Commit-Position: refs/heads/master@{#528518} --- .../ax_tree_source_aura_unittest.cc | 12 ++-- .../accessibility/automation_manager_aura.cc | 2 +- .../aura/accessibility/ax_root_obj_wrapper.cc | 9 ++- .../aura/accessibility/ax_root_obj_wrapper.h | 7 +- .../aura/accessibility/ax_tree_source_aura.cc | 16 ++--- .../accessibility/browser_accessibility.cc | 8 +++ .../accessibility/browser_accessibility.h | 10 ++- .../browser_accessibility_android.cc | 11 ++- .../browser_accessibility_android.h | 2 +- .../browser_accessibility_com_win.cc | 4 +- .../browser_accessibility_manager_win.cc | 2 +- .../browser_accessibility_win_unittest.cc | 2 +- ui/accessibility/BUILD.gn | 5 +- ui/accessibility/platform/ax_platform_node.cc | 5 ++ ui/accessibility/platform/ax_platform_node.h | 5 ++ .../platform/ax_platform_node_base.h | 2 +- .../platform/ax_platform_node_delegate.h | 3 + .../platform/ax_platform_node_win.cc | 28 ++++---- .../platform/ax_platform_node_win.h | 5 +- .../platform/ax_platform_relation_win.cc | 2 +- .../platform/ax_platform_unique_id.cc | 20 ------ .../platform/ax_platform_unique_id.h | 23 ------- .../platform/ax_system_caret_win.cc | 11 +-- .../platform/ax_system_caret_win.h | 4 ++ ui/accessibility/platform/ax_unique_id.cc | 63 +++++++++++++++++ ui/accessibility/platform/ax_unique_id.h | 51 ++++++++++++++ .../platform/ax_unique_id_unittest.cc | 67 +++++++++++++++++++ .../platform/test_ax_node_wrapper.cc | 4 ++ .../platform/test_ax_node_wrapper.h | 2 + ui/views/accessibility/ax_aura_obj_cache.cc | 6 +- ui/views/accessibility/ax_aura_obj_cache.h | 5 -- ui/views/accessibility/ax_aura_obj_wrapper.h | 3 +- ui/views/accessibility/ax_view_obj_wrapper.cc | 6 +- ui/views/accessibility/ax_view_obj_wrapper.h | 2 +- .../accessibility/ax_widget_obj_wrapper.cc | 7 +- .../accessibility/ax_widget_obj_wrapper.h | 5 +- .../accessibility/ax_window_obj_wrapper.cc | 7 +- .../accessibility/ax_window_obj_wrapper.h | 5 +- .../native_view_accessibility_auralinux.cc | 3 + .../native_view_accessibility_base.cc | 4 ++ .../native_view_accessibility_base.h | 5 +- ui/views/accessibility/view_accessibility.cc | 4 ++ ui/views/accessibility/view_accessibility.h | 5 ++ 43 files changed, 326 insertions(+), 126 deletions(-) delete mode 100644 ui/accessibility/platform/ax_platform_unique_id.cc delete mode 100644 ui/accessibility/platform/ax_platform_unique_id.h create mode 100644 ui/accessibility/platform/ax_unique_id.cc create mode 100644 ui/accessibility/platform/ax_unique_id.h create mode 100644 ui/accessibility/platform/ax_unique_id_unittest.cc diff --git a/chrome/browser/ui/ash/accessibility/ax_tree_source_aura_unittest.cc b/chrome/browser/ui/ash/accessibility/ax_tree_source_aura_unittest.cc index 3704424aede052..424a7028b3af16 100644 --- a/chrome/browser/ui/ash/accessibility/ax_tree_source_aura_unittest.cc +++ b/chrome/browser/ui/ash/accessibility/ax_tree_source_aura_unittest.cc @@ -84,8 +84,8 @@ TEST_F(AXTreeSourceAuraTest, Accessors) { AXTreeSourceAura ax_tree; ASSERT_TRUE(ax_tree.GetRoot()); - // ID's should start at 1 and there should be a root. - ASSERT_EQ(1, ax_tree.GetRoot()->GetID()); + // ID's should be > 0. + ASSERT_GE(ax_tree.GetRoot()->GetUniqueId().Get(), 1); // Grab the content view directly from cache to avoid walking down the tree. AXAuraObjWrapper* content = @@ -105,6 +105,8 @@ TEST_F(AXTreeSourceAuraTest, Accessors) { ASSERT_EQ(content, textfield->GetParent()); + ASSERT_NE(textfield->GetUniqueId(), ax_tree.GetRoot()->GetUniqueId()); + // Try walking up the tree to the root. AXAuraObjWrapper* test_root = NULL; for (AXAuraObjWrapper* root_finder = ax_tree.GetParent(content); root_finder; @@ -124,7 +126,7 @@ TEST_F(AXTreeSourceAuraTest, DoDefault) { ASSERT_FALSE(textfield_->HasFocus()); ui::AXActionData action_data; action_data.action = ui::AX_ACTION_DO_DEFAULT; - action_data.target_node_id = textfield_wrapper->GetID(); + action_data.target_node_id = textfield_wrapper->GetUniqueId().Get(); textfield_wrapper->HandleAccessibleAction(action_data); ASSERT_TRUE(textfield_->HasFocus()); } @@ -140,7 +142,7 @@ TEST_F(AXTreeSourceAuraTest, Focus) { ASSERT_FALSE(textfield_->HasFocus()); ui::AXActionData action_data; action_data.action = ui::AX_ACTION_FOCUS; - action_data.target_node_id = textfield_wrapper->GetID(); + action_data.target_node_id = textfield_wrapper->GetUniqueId().Get(); textfield_wrapper->HandleAccessibleAction(action_data); ASSERT_TRUE(textfield_->HasFocus()); } @@ -177,7 +179,7 @@ TEST_F(AXTreeSourceAuraTest, Serialize) { int text_field_update_index = -1; for (size_t i = 0; i < node_count; ++i) { - if (textfield_wrapper->GetID() == out_update2.nodes[i].id) + if (textfield_wrapper->GetUniqueId().Get() == out_update2.nodes[i].id) text_field_update_index = i; } diff --git a/chrome/browser/ui/aura/accessibility/automation_manager_aura.cc b/chrome/browser/ui/aura/accessibility/automation_manager_aura.cc index 3f9b6666daec51..109267ce414ba8 100644 --- a/chrome/browser/ui/aura/accessibility/automation_manager_aura.cc +++ b/chrome/browser/ui/aura/accessibility/automation_manager_aura.cc @@ -202,7 +202,7 @@ void AutomationManagerAura::SendEvent(BrowserContext* context, current_tree_serializer_->SerializeChanges(focus, ¶ms.update); params.tree_id = 0; - params.id = aura_obj->GetID(); + params.id = aura_obj->GetUniqueId().Get(); params.event_type = event_type; params.mouse_location = aura::Env::GetInstance()->last_mouse_location(); AutomationEventRouter* router = AutomationEventRouter::GetInstance(); diff --git a/chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.cc b/chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.cc index b723c3b10ea0a3..dd03af7c280332 100644 --- a/chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.cc +++ b/chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.cc @@ -12,8 +12,7 @@ #include "ui/views/accessibility/ax_aura_obj_cache.h" #include "ui/views/accessibility/ax_window_obj_wrapper.h" -AXRootObjWrapper::AXRootObjWrapper(int32_t id) - : id_(id), alert_window_(new aura::Window(NULL)) { +AXRootObjWrapper::AXRootObjWrapper() : alert_window_(new aura::Window(NULL)) { alert_window_->Init(ui::LAYER_NOT_DRAWN); } @@ -52,12 +51,12 @@ void AXRootObjWrapper::GetChildren( } void AXRootObjWrapper::Serialize(ui::AXNodeData* out_node_data) { - out_node_data->id = id_; + out_node_data->id = unique_id_.Get(); out_node_data->role = ui::AX_ROLE_DESKTOP; out_node_data->AddStringAttribute(ui::AX_ATTR_CHROME_CHANNEL, chrome::GetChannelString()); } -int32_t AXRootObjWrapper::GetID() { - return id_; +const ui::AXUniqueId& AXRootObjWrapper::GetUniqueId() const { + return unique_id_; } diff --git a/chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.h b/chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.h index fab9cedef9dfff..189c6c501b12c5 100644 --- a/chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.h +++ b/chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.h @@ -10,6 +10,7 @@ #include #include "base/macros.h" +#include "ui/accessibility/platform/ax_unique_id.h" #include "ui/views/accessibility/ax_aura_obj_wrapper.h" namespace aura { @@ -18,7 +19,7 @@ class Window; class AXRootObjWrapper : public views::AXAuraObjWrapper { public: - explicit AXRootObjWrapper(int32_t id); + AXRootObjWrapper(); ~AXRootObjWrapper() override; // Returns an AXAuraObjWrapper for an alert window with title set to |text|. @@ -32,10 +33,10 @@ class AXRootObjWrapper : public views::AXAuraObjWrapper { void GetChildren( std::vector* out_children) override; void Serialize(ui::AXNodeData* out_node_data) override; - int32_t GetID() override; + const ui::AXUniqueId& GetUniqueId() const override; private: - int32_t id_; + ui::AXUniqueId unique_id_; aura::Window* alert_window_; diff --git a/chrome/browser/ui/aura/accessibility/ax_tree_source_aura.cc b/chrome/browser/ui/aura/accessibility/ax_tree_source_aura.cc index a153282948b008..e6cca883581b41 100644 --- a/chrome/browser/ui/aura/accessibility/ax_tree_source_aura.cc +++ b/chrome/browser/ui/aura/accessibility/ax_tree_source_aura.cc @@ -24,7 +24,7 @@ using views::AXAuraObjCache; using views::AXAuraObjWrapper; AXTreeSourceAura::AXTreeSourceAura() { - root_.reset(new AXRootObjWrapper(AXAuraObjCache::GetInstance()->GetNextID())); + root_.reset(new AXRootObjWrapper()); } AXTreeSourceAura::~AXTreeSourceAura() { @@ -57,7 +57,7 @@ bool AXTreeSourceAura::GetTreeData(ui::AXTreeData* tree_data) const { tree_data->loading_progress = 1.0; AXAuraObjWrapper* focus = AXAuraObjCache::GetInstance()->GetFocus(); if (focus) - tree_data->focus_id = focus->GetID(); + tree_data->focus_id = focus->GetUniqueId().Get(); return true; } @@ -66,13 +66,13 @@ AXAuraObjWrapper* AXTreeSourceAura::GetRoot() const { } AXAuraObjWrapper* AXTreeSourceAura::GetFromId(int32_t id) const { - if (id == root_->GetID()) + if (id == root_->GetUniqueId().Get()) return root_.get(); return AXAuraObjCache::GetInstance()->Get(id); } int32_t AXTreeSourceAura::GetId(AXAuraObjWrapper* node) const { - return node->GetID(); + return node->GetUniqueId().Get(); } void AXTreeSourceAura::GetChildren( @@ -83,13 +83,13 @@ void AXTreeSourceAura::GetChildren( AXAuraObjWrapper* AXTreeSourceAura::GetParent(AXAuraObjWrapper* node) const { AXAuraObjWrapper* parent = node->GetParent(); - if (!parent && node->GetID() != root_->GetID()) + if (!parent && node->GetUniqueId() != root_->GetUniqueId()) parent = root_.get(); return parent; } bool AXTreeSourceAura::IsValid(AXAuraObjWrapper* node) const { - return node && node->GetID() != -1; + return node != nullptr; } bool AXTreeSourceAura::IsEqual(AXAuraObjWrapper* node1, @@ -97,7 +97,7 @@ bool AXTreeSourceAura::IsEqual(AXAuraObjWrapper* node1, if (!node1 || !node2) return false; - return node1->GetID() == node2->GetID() && node1->GetID() != -1; + return node1->GetUniqueId() == node2->GetUniqueId(); } AXAuraObjWrapper* AXTreeSourceAura::GetNull() const { @@ -118,7 +118,7 @@ void AXTreeSourceAura::SerializeNode(AXAuraObjWrapper* node, ui::AXNodeData parent_data; parent->Serialize(&parent_data); out_data->location.Offset(-parent_data.location.OffsetFromOrigin()); - out_data->offset_container_id = parent->GetID(); + out_data->offset_container_id = parent->GetUniqueId().Get(); } if (out_data->role == ui::AX_ROLE_WEB_VIEW) { diff --git a/content/browser/accessibility/browser_accessibility.cc b/content/browser/accessibility/browser_accessibility.cc index 09fcfc800b1813..7e9b1231ec5de4 100644 --- a/content/browser/accessibility/browser_accessibility.cc +++ b/content/browser/accessibility/browser_accessibility.cc @@ -17,6 +17,7 @@ #include "content/common/accessibility_messages.h" #include "ui/accessibility/ax_role_properties.h" #include "ui/accessibility/ax_text_utils.h" +#include "ui/accessibility/platform/ax_unique_id.h" #include "ui/gfx/geometry/rect_conversions.h" #include "ui/gfx/geometry/rect_f.h" @@ -868,6 +869,13 @@ std::set BrowserAccessibility::GetReverseRelations( return manager_->ax_tree()->GetReverseRelations(attr, dst_id); } +const ui::AXUniqueId& BrowserAccessibility::GetUniqueId() const { + // This is not the same as GetData().id which comes from Blink, because + // those ids are only unique within the Blink process. We need one that is + // unique for the browser process. + return unique_id_; +} + gfx::NativeViewAccessible BrowserAccessibility::GetNativeViewAccessible() { // TODO(703369) On Windows, where we have started to migrate to an // AXPlatformNode implementation, the BrowserAccessibilityWin subclass has diff --git a/content/browser/accessibility/browser_accessibility.h b/content/browser/accessibility/browser_accessibility.h index c86fa30350bac6..222a1912faa468 100644 --- a/content/browser/accessibility/browser_accessibility.h +++ b/content/browser/accessibility/browser_accessibility.h @@ -366,8 +366,11 @@ class CONTENT_EXPORT BrowserAccessibility : public ui::AXPlatformNodeDelegate { // The underlying node. ui::AXNode* node_; - // A unique ID, since node IDs are frame-local. - int32_t unique_id_; + // Protected so that it can't be called directly on a BrowserAccessibility + // where it could be confused with an id that comes from the node data, + // which is only unique to the Blink process. + // Does need to be called by subclasses such as BrowserAccessibilityAndroid. + const ui::AXUniqueId& GetUniqueId() const override; private: // |GetInnerText| recursively includes all the text from descendants such as @@ -376,6 +379,9 @@ class CONTENT_EXPORT BrowserAccessibility : public ui::AXPlatformNodeDelegate { // text, depending on the platform. base::string16 GetInnerText() const; + // A unique ID, since node IDs are frame-local. + ui::AXUniqueId unique_id_; + DISALLOW_COPY_AND_ASSIGN(BrowserAccessibility); }; diff --git a/content/browser/accessibility/browser_accessibility_android.cc b/content/browser/accessibility/browser_accessibility_android.cc index b1f7586b20d36d..667dfab9b89a19 100644 --- a/content/browser/accessibility/browser_accessibility_android.cc +++ b/content/browser/accessibility/browser_accessibility_android.cc @@ -18,8 +18,8 @@ #include "third_party/skia/include/core/SkColor.h" #include "ui/accessibility/ax_role_properties.h" #include "ui/accessibility/platform/ax_android_constants.h" -#include "ui/accessibility/platform/ax_platform_unique_id.h" #include "ui/accessibility/platform/ax_snapshot_node_android_platform.h" +#include "ui/accessibility/platform/ax_unique_id.h" namespace { @@ -76,14 +76,13 @@ BrowserAccessibilityAndroid* BrowserAccessibilityAndroid::GetFromUniqueId( return nullptr; } -BrowserAccessibilityAndroid::BrowserAccessibilityAndroid() - : unique_id_(ui::GetNextAXPlatformNodeUniqueId()) { - g_unique_id_map.Get()[unique_id_] = this; +BrowserAccessibilityAndroid::BrowserAccessibilityAndroid() { + g_unique_id_map.Get()[unique_id()] = this; } BrowserAccessibilityAndroid::~BrowserAccessibilityAndroid() { - if (unique_id_) - g_unique_id_map.Get().erase(unique_id_); + if (unique_id()) + g_unique_id_map.Get().erase(unique_id()); } bool BrowserAccessibilityAndroid::IsNative() const { diff --git a/content/browser/accessibility/browser_accessibility_android.h b/content/browser/accessibility/browser_accessibility_android.h index e30bb699a4fd29..8ac237a8a4a9f9 100644 --- a/content/browser/accessibility/browser_accessibility_android.h +++ b/content/browser/accessibility/browser_accessibility_android.h @@ -18,7 +18,7 @@ namespace content { class CONTENT_EXPORT BrowserAccessibilityAndroid : public BrowserAccessibility { public: static BrowserAccessibilityAndroid* GetFromUniqueId(int32_t unique_id); - int32_t unique_id() const { return unique_id_; } + int32_t unique_id() const { return GetUniqueId().Get(); } // Overrides from BrowserAccessibility. void OnDataChanged() override; diff --git a/content/browser/accessibility/browser_accessibility_com_win.cc b/content/browser/accessibility/browser_accessibility_com_win.cc index 56be8f7ab57503..a9288ce0987c55 100644 --- a/content/browser/accessibility/browser_accessibility_com_win.cc +++ b/content/browser/accessibility/browser_accessibility_com_win.cc @@ -1109,7 +1109,7 @@ STDMETHODIMP BrowserAccessibilityComWin::get_nodeInfo( *name_space_id = 0; *node_value = SysAllocString(value().c_str()); *num_children = owner()->PlatformChildCount(); - *unique_id = -AXPlatformNodeWin::unique_id(); + *unique_id = -AXPlatformNodeWin::GetUniqueId(); if (owner()->IsDocument()) { *node_type = NODETYPE_DOCUMENT; @@ -1914,7 +1914,7 @@ void BrowserAccessibilityComWin::Destroy() { void BrowserAccessibilityComWin::Init(ui::AXPlatformNodeDelegate* delegate) { owner_ = static_cast(delegate); - AXPlatformNodeBase::Init(delegate); + AXPlatformNodeWin::Init(delegate); } std::vector BrowserAccessibilityComWin::ComputeTextAttributes() diff --git a/content/browser/accessibility/browser_accessibility_manager_win.cc b/content/browser/accessibility/browser_accessibility_manager_win.cc index 4cbb1b33625a2a..e5a1c931547e7c 100644 --- a/content/browser/accessibility/browser_accessibility_manager_win.cc +++ b/content/browser/accessibility/browser_accessibility_manager_win.cc @@ -220,7 +220,7 @@ void BrowserAccessibilityManagerWin::FireWinAccessibilityEvent( // argument to NotifyWinEvent; the AT client will then call get_accChild // on the HWND's accessibility object and pass it that same id, which // we can use to retrieve the IAccessible for this node. - LONG child_id = -(ToBrowserAccessibilityWin(node)->GetCOM()->unique_id()); + LONG child_id = -(ToBrowserAccessibilityWin(node)->GetCOM()->GetUniqueId()); ::NotifyWinEvent(win_event_type, hwnd, OBJID_CLIENT, child_id); } diff --git a/content/browser/accessibility/browser_accessibility_win_unittest.cc b/content/browser/accessibility/browser_accessibility_win_unittest.cc index ab7238f08cd5bc..d0fa26b4f60089 100644 --- a/content/browser/accessibility/browser_accessibility_win_unittest.cc +++ b/content/browser/accessibility/browser_accessibility_win_unittest.cc @@ -711,7 +711,7 @@ TEST_F(BrowserAccessibilityTest, TestCreateEmptyDocument) { int32_t GetUniqueId(BrowserAccessibility* accessibility) { BrowserAccessibilityWin* win_root = ToBrowserAccessibilityWin(accessibility); - return win_root->GetCOM()->unique_id(); + return win_root->GetCOM()->GetUniqueId(); } // This is a regression test for a bug where the initial empty document diff --git a/ui/accessibility/BUILD.gn b/ui/accessibility/BUILD.gn index 495756cb93514d..b5a6c772be0de3 100644 --- a/ui/accessibility/BUILD.gn +++ b/ui/accessibility/BUILD.gn @@ -56,10 +56,10 @@ component("accessibility") { "platform/ax_android_constants.h", "platform/ax_platform_node.cc", "platform/ax_platform_node.h", - "platform/ax_platform_unique_id.cc", - "platform/ax_platform_unique_id.h", "platform/ax_snapshot_node_android_platform.cc", "platform/ax_snapshot_node_android_platform.h", + "platform/ax_unique_id.cc", + "platform/ax_unique_id.h", ] if (has_native_accessibility) { @@ -181,6 +181,7 @@ test("accessibility_unittests") { "platform/ax_platform_node_unittest.cc", "platform/ax_platform_node_unittest.h", "platform/ax_platform_node_win_unittest.cc", + "platform/ax_unique_id_unittest.cc", ] deps = [ diff --git a/ui/accessibility/platform/ax_platform_node.cc b/ui/accessibility/platform/ax_platform_node.cc index 18bc0be25bf062..f0f00d69bd134e 100644 --- a/ui/accessibility/platform/ax_platform_node.cc +++ b/ui/accessibility/platform/ax_platform_node.cc @@ -51,6 +51,11 @@ AXPlatformNode::~AXPlatformNode() { void AXPlatformNode::Destroy() { } +int32_t AXPlatformNode::GetUniqueId() const { + DCHECK(GetDelegate()); // Must be called after Init() + return GetDelegate() ? GetDelegate()->GetUniqueId().Get() : -1; +} + // static void AXPlatformNode::AddAXModeObserver(AXModeObserver* observer) { ax_mode_observers_.Get().AddObserver(observer); diff --git a/ui/accessibility/platform/ax_platform_node.h b/ui/accessibility/platform/ax_platform_node.h index ca126036a441fa..f2e7e7a05567e9 100644 --- a/ui/accessibility/platform/ax_platform_node.h +++ b/ui/accessibility/platform/ax_platform_node.h @@ -69,6 +69,9 @@ class AX_EXPORT AXPlatformNode { // Return this object's delegate. virtual AXPlatformNodeDelegate* GetDelegate() const = 0; + // Return the unique ID + int32_t GetUniqueId() const; + protected: AXPlatformNode(); virtual ~AXPlatformNode(); @@ -80,6 +83,8 @@ class AX_EXPORT AXPlatformNode { static base::LazyInstance::Leaky native_window_handler_; + + DISALLOW_COPY_AND_ASSIGN(AXPlatformNode); }; } // namespace ui diff --git a/ui/accessibility/platform/ax_platform_node_base.h b/ui/accessibility/platform/ax_platform_node_base.h index 1099e6446e2341..7a3a93f7cc5607 100644 --- a/ui/accessibility/platform/ax_platform_node_base.h +++ b/ui/accessibility/platform/ax_platform_node_base.h @@ -18,7 +18,7 @@ class AXPlatformNodeDelegate; class AX_EXPORT AXPlatformNodeBase : public AXPlatformNode { public: - virtual void Init(AXPlatformNodeDelegate* delegate); + virtual void Init(AXPlatformNodeDelegate* delegate); // These are simple wrappers to our delegate. const AXNodeData& GetData() const; diff --git a/ui/accessibility/platform/ax_platform_node_delegate.h b/ui/accessibility/platform/ax_platform_node_delegate.h index b3a0eca4154283..704bc5e9a4c5c6 100644 --- a/ui/accessibility/platform/ax_platform_node_delegate.h +++ b/ui/accessibility/platform/ax_platform_node_delegate.h @@ -9,6 +9,7 @@ #include "ui/accessibility/ax_enums.h" #include "ui/accessibility/ax_export.h" +#include "ui/accessibility/platform/ax_unique_id.h" #include "ui/gfx/geometry/vector2d.h" #include "ui/gfx/native_widget_types.h" @@ -96,6 +97,8 @@ class AX_EXPORT AXPlatformNodeDelegate { virtual std::set GetReverseRelations(AXIntListAttribute attr, int32_t dst_id) = 0; + virtual const AXUniqueId& GetUniqueId() const = 0; + // // Events. // diff --git a/ui/accessibility/platform/ax_platform_node_win.cc b/ui/accessibility/platform/ax_platform_node_win.cc index 721c1d89becc34..05b390f3fc4c15 100644 --- a/ui/accessibility/platform/ax_platform_node_win.cc +++ b/ui/accessibility/platform/ax_platform_node_win.cc @@ -26,7 +26,6 @@ #include "ui/accessibility/ax_tree_data.h" #include "ui/accessibility/platform/ax_platform_node_delegate.h" #include "ui/accessibility/platform/ax_platform_relation_win.h" -#include "ui/accessibility/platform/ax_platform_unique_id.h" #include "ui/base/win/atl_module.h" #include "ui/gfx/geometry/rect_conversions.h" @@ -244,15 +243,15 @@ AXPlatformNode* AXPlatformNodeWin::GetFromUniqueId(int32_t unique_id) { // AXPlatformNodeWin // -AXPlatformNodeWin::AXPlatformNodeWin() - : unique_id_(GetNextAXPlatformNodeUniqueId()) { - g_unique_id_map.Get()[unique_id_] = this; -} +AXPlatformNodeWin::AXPlatformNodeWin() {} AXPlatformNodeWin::~AXPlatformNodeWin() { ClearOwnRelations(); - if (unique_id_) - g_unique_id_map.Get().erase(unique_id_); +} + +void AXPlatformNodeWin::Init(AXPlatformNodeDelegate* delegate) { + AXPlatformNodeBase::Init(delegate); + g_unique_id_map.Get()[GetUniqueId()] = this; } // static @@ -325,8 +324,7 @@ void AXPlatformNodeWin::Dispose() { } void AXPlatformNodeWin::Destroy() { - g_unique_id_map.Get().erase(unique_id_); - unique_id_ = 0; + g_unique_id_map.Get().erase(GetUniqueId()); RemoveAlertTarget(); @@ -357,7 +355,7 @@ void AXPlatformNodeWin::NotifyAccessibilityEvent(AXEvent event_type) { if (native_event < EVENT_MIN) return; - ::NotifyWinEvent(native_event, hwnd, OBJID_CLIENT, -unique_id_); + ::NotifyWinEvent(native_event, hwnd, OBJID_CLIENT, -GetUniqueId()); // Keep track of objects that are a target of an alert event. if (event_type == AX_EVENT_ALERT) @@ -932,10 +930,10 @@ STDMETHODIMP AXPlatformNodeWin::get_states(AccessibleStates* states) { return S_OK; } -STDMETHODIMP AXPlatformNodeWin::get_uniqueID(LONG* unique_id) { +STDMETHODIMP AXPlatformNodeWin::get_uniqueID(LONG* id) { WIN_ACCESSIBILITY_API_HISTOGRAM(UMA_API_GET_UNIQUE_ID); - COM_OBJECT_VALIDATE_1_ARG(unique_id); - *unique_id = -unique_id_; + COM_OBJECT_VALIDATE_1_ARG(id); + *id = -GetUniqueId(); return S_OK; } @@ -3266,7 +3264,7 @@ AXHypertext AXPlatformNodeWin::ComputeHypertext() { hypertext += child->GetString16Attribute(ui::AX_ATTR_NAME); } else { int32_t char_offset = static_cast(hypertext.size()); - int32_t child_unique_id = child->unique_id(); + int32_t child_unique_id = child->GetUniqueId(); int32_t index = static_cast(result.hyperlinks.size()); result.hyperlink_offset_to_index[char_offset] = index; result.hyperlinks.push_back(child_unique_id); @@ -3690,7 +3688,7 @@ int32_t AXPlatformNodeWin::GetHyperlinkIndexFromChild( return -1; auto iterator = std::find(hypertext_.hyperlinks.begin(), - hypertext_.hyperlinks.end(), child->unique_id()); + hypertext_.hyperlinks.end(), child->GetUniqueId()); if (iterator == hypertext_.hyperlinks.end()) return -1; diff --git a/ui/accessibility/platform/ax_platform_node_win.h b/ui/accessibility/platform/ax_platform_node_win.h index 9238fa9fc09034..7b49fa9b316c89 100644 --- a/ui/accessibility/platform/ax_platform_node_win.h +++ b/ui/accessibility/platform/ax_platform_node_win.h @@ -257,6 +257,8 @@ class AX_EXPORT __declspec(uuid("26f5641a-246d-457b-a96d-07f3fae6acf2")) // Return the number of instances of AXPlatformNodeWin, for leak testing. static size_t GetInstanceCountForTesting(); + void Init(AXPlatformNodeDelegate* delegate) override; + // Represents a non-static text node in IAccessibleHypertext. This character // is embedded in the response to IAccessibleText::get_text, indicating the // position where a non-static text child object appears. @@ -265,7 +267,6 @@ class AX_EXPORT __declspec(uuid("26f5641a-246d-457b-a96d-07f3fae6acf2")) // Clear any AXPlatformRelationWin nodes owned by this node. void ClearOwnRelations(); static AXPlatformNode* GetFromUniqueId(int32_t unique_id); - int32_t unique_id() const { return unique_id_; } // AXPlatformNode overrides. gfx::NativeViewAccessible GetNativeViewAccessible() override; @@ -707,8 +708,6 @@ class AX_EXPORT __declspec(uuid("26f5641a-246d-457b-a96d-07f3fae6acf2")) TextBoundaryType IA2TextBoundaryToTextBoundary(IA2TextBoundaryType type); private: - int32_t unique_id_; - int MSAAEvent(AXEvent event); bool IsWebAreaForPresentationalIframe(); bool ShouldNodeHaveReadonlyStateByDefault(const AXNodeData& data) const; diff --git a/ui/accessibility/platform/ax_platform_relation_win.cc b/ui/accessibility/platform/ax_platform_relation_win.cc index 8395ead4633a49..83480eba1d1edf 100644 --- a/ui/accessibility/platform/ax_platform_relation_win.cc +++ b/ui/accessibility/platform/ax_platform_relation_win.cc @@ -25,7 +25,7 @@ #include "ui/accessibility/ax_text_utils.h" #include "ui/accessibility/ax_tree_data.h" #include "ui/accessibility/platform/ax_platform_node_delegate.h" -#include "ui/accessibility/platform/ax_platform_unique_id.h" +#include "ui/accessibility/platform/ax_unique_id.h" #include "ui/base/win/atl_module.h" #include "ui/gfx/geometry/rect_conversions.h" diff --git a/ui/accessibility/platform/ax_platform_unique_id.cc b/ui/accessibility/platform/ax_platform_unique_id.cc deleted file mode 100644 index ece23eb421e54f..00000000000000 --- a/ui/accessibility/platform/ax_platform_unique_id.cc +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright 2017 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "ui/accessibility/platform/ax_platform_unique_id.h" - -namespace ui { - -int32_t GetNextAXPlatformNodeUniqueId() { - static int32_t next_unique_id = 1; - int32_t unique_id = next_unique_id; - if (next_unique_id == INT32_MAX) - next_unique_id = 1; - else - next_unique_id++; - - return unique_id; -} - -} // namespace ui diff --git a/ui/accessibility/platform/ax_platform_unique_id.h b/ui/accessibility/platform/ax_platform_unique_id.h deleted file mode 100644 index 1525d60770e361..00000000000000 --- a/ui/accessibility/platform/ax_platform_unique_id.h +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright 2017 The Chromium 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 UI_ACCESSIBILITY_PLATFORM_AX_PLATFORM_UNIQUE_ID_H_ -#define UI_ACCESSIBILITY_PLATFORM_AX_PLATFORM_UNIQUE_ID_H_ - -#include - -#include "ui/accessibility/ax_export.h" - -namespace ui { - -// Each platform accessibility object has a unique id that's guaranteed to be a -// positive number. (It's stored in an int32_t as opposed to uint32_t because -// some platforms want to negate it, so we want to ensure the range is below the -// signed int max.) This can be used when the id has to be unique across -// multiple frames, since node ids are only unique within one tree. -int32_t AX_EXPORT GetNextAXPlatformNodeUniqueId(); - -} // namespace ui - -#endif // UI_ACCESSIBILITY_PLATFORM_AX_PLATFORM_UNIQUE_ID_H_ diff --git a/ui/accessibility/platform/ax_system_caret_win.cc b/ui/accessibility/platform/ax_system_caret_win.cc index 2b146bd84c2244..7547cd669a9c52 100644 --- a/ui/accessibility/platform/ax_system_caret_win.cc +++ b/ui/accessibility/platform/ax_system_caret_win.cc @@ -9,7 +9,6 @@ #include "base/logging.h" #include "ui/accessibility/ax_enums.h" #include "ui/accessibility/platform/ax_platform_node_win.h" -#include "ui/accessibility/platform/ax_platform_unique_id.h" #include "ui/gfx/geometry/rect_conversions.h" #include "ui/gfx/geometry/rect_f.h" @@ -31,14 +30,14 @@ AXSystemCaretWin::AXSystemCaretWin(gfx::AcceleratedWidget event_target) if (event_target_) { ::NotifyWinEvent(EVENT_OBJECT_CREATE, event_target_, OBJID_CARET, - -caret_->unique_id()); + -caret_->GetUniqueId()); } } AXSystemCaretWin::~AXSystemCaretWin() { if (event_target_) { ::NotifyWinEvent(EVENT_OBJECT_DESTROY, event_target_, OBJID_CARET, - -caret_->unique_id()); + -caret_->GetUniqueId()); } caret_->Destroy(); } @@ -58,7 +57,7 @@ void AXSystemCaretWin::MoveCaretTo(const gfx::Rect& bounds) { data_.location = gfx::RectF(bounds); if (event_target_) { ::NotifyWinEvent(EVENT_OBJECT_LOCATIONCHANGE, event_target_, OBJID_CARET, - -caret_->unique_id()); + -caret_->GetUniqueId()); } } @@ -144,4 +143,8 @@ std::set AXSystemCaretWin::GetReverseRelations(AXIntListAttribute attr, return std::set(); } +const ui::AXUniqueId& AXSystemCaretWin::GetUniqueId() const { + return unique_id_; +} + } // namespace ui diff --git a/ui/accessibility/platform/ax_system_caret_win.h b/ui/accessibility/platform/ax_system_caret_win.h index 02c2eb2d2d6182..6a4ec9686ee8fb 100644 --- a/ui/accessibility/platform/ax_system_caret_win.h +++ b/ui/accessibility/platform/ax_system_caret_win.h @@ -49,6 +49,7 @@ class AX_EXPORT AXSystemCaretWin : private AXPlatformNodeDelegate { bool AccessibilityPerformAction(const AXActionData& data) override; bool ShouldIgnoreHoveredStateForTesting() override; bool IsOffscreen() const override; + const ui::AXUniqueId& GetUniqueId() const override; std::set GetReverseRelations(AXIntAttribute attr, int32_t dst_id) override; std::set GetReverseRelations(AXIntListAttribute attr, @@ -60,6 +61,9 @@ class AX_EXPORT AXSystemCaretWin : private AXPlatformNodeDelegate { friend class AXPlatformNodeWin; DISALLOW_COPY_AND_ASSIGN(AXSystemCaretWin); + + private: + ui::AXUniqueId unique_id_; }; } // namespace ui diff --git a/ui/accessibility/platform/ax_unique_id.cc b/ui/accessibility/platform/ax_unique_id.cc new file mode 100644 index 00000000000000..645d3f40b5b70e --- /dev/null +++ b/ui/accessibility/platform/ax_unique_id.cc @@ -0,0 +1,63 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include +#include + +#include "base/lazy_instance.h" +#include "ui/accessibility/platform/ax_unique_id.h" + +namespace ui { + +namespace { + +base::LazyInstance>::Leaky g_assigned_ids = + LAZY_INSTANCE_INITIALIZER; + +} // namespace + +AXUniqueId::AXUniqueId(const int32_t max_id) : id_(GetNextAXUniqueId(max_id)) {} + +AXUniqueId::~AXUniqueId() { + g_assigned_ids.Get().erase(id_); +} + +bool AXUniqueId::operator==(const AXUniqueId& other) const { + return Get() == other.Get(); +} + +bool AXUniqueId::operator!=(const AXUniqueId& other) const { + return !(*this == other); +} + +bool AXUniqueId::IsAssigned(int32_t id) const { + auto id_map = g_assigned_ids.Get(); + return id_map.find(id) != id_map.end(); +} + +int32_t AXUniqueId::GetNextAXUniqueId(const int32_t max_id) { + static int32_t current_id = 0; + static bool has_wrapped = false; + + const int32_t prev_id = current_id; + + while (true) { + if (current_id == max_id) { + current_id = 1; + has_wrapped = true; + } else { + current_id++; + } + if (current_id == prev_id) + LOG(FATAL) << "Over 2 billion active ids, something is wrong."; + if (!has_wrapped || !IsAssigned(current_id)) + break; + } + + g_assigned_ids.Get().insert(current_id); + + return current_id; +} + +} // namespace ui diff --git a/ui/accessibility/platform/ax_unique_id.h b/ui/accessibility/platform/ax_unique_id.h new file mode 100644 index 00000000000000..f0c20fcba0cf34 --- /dev/null +++ b/ui/accessibility/platform/ax_unique_id.h @@ -0,0 +1,51 @@ +// Copyright 2017 The Chromium 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 UI_ACCESSIBILITY_PLATFORM_AX_UNIQUE_ID_H_ +#define UI_ACCESSIBILITY_PLATFORM_AX_UNIQUE_ID_H_ + +#include + +#include "base/macros.h" +#include "ui/accessibility/ax_export.h" + +namespace ui { + +// AXUniqueID provides IDs for accessibility objects that are guaranteed to be +// unique for the entire Chrome instance. Instantiating the class is all that +// is required to generate the ID, and the ID is freed when the AXUniqueID is +// destroyed. +// +// The unique id that's guaranteed to be a positive number. Becase some +// platforms want to negate it, we ensure the range is below the signed int max. +// +// These ids must not be conflated with the int id, that comes with web node +// data, which are only unique within their source frame. +class AX_EXPORT AXUniqueId { + public: + AXUniqueId() : AXUniqueId(INT32_MAX) {} + ~AXUniqueId(); + + int32_t Get() const { return id_; } + + bool operator==(const AXUniqueId& other) const; + bool operator!=(const AXUniqueId& other) const; + + protected: + // Passing the max id is necessary for testing. + explicit AXUniqueId(const int32_t max_id); + + private: + int32_t GetNextAXUniqueId(const int32_t max_id); + + bool IsAssigned(int32_t) const; + + int32_t id_; + + DISALLOW_COPY_AND_ASSIGN(AXUniqueId); +}; + +} // namespace ui + +#endif // UI_ACCESSIBILITY_PLATFORM_AX_UNIQUE_ID_H_ diff --git a/ui/accessibility/platform/ax_unique_id_unittest.cc b/ui/accessibility/platform/ax_unique_id_unittest.cc new file mode 100644 index 00000000000000..0d7fda8f706a5e --- /dev/null +++ b/ui/accessibility/platform/ax_unique_id_unittest.cc @@ -0,0 +1,67 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "ui/accessibility/platform/ax_unique_id.h" + +#include "testing/gtest/include/gtest/gtest.h" + +namespace ui { + +class AXPlatformUniqueIdTest : public testing::Test { + public: + AXPlatformUniqueIdTest() {} + ~AXPlatformUniqueIdTest() override {} + + void SetUp() override {} + + void TearDown() override {} +}; + +TEST_F(AXPlatformUniqueIdTest, TestIdsUnique) { + AXUniqueId id1, id2; + EXPECT_FALSE(id1 == id2); + EXPECT_GT(id2.Get(), id1.Get()); +} + +static const int32_t kMaxId = 100; + +class AXTestSmallBankUniqueId : public AXUniqueId { + public: + AXTestSmallBankUniqueId(); + + friend class AXUniqueId; +}; + +AXTestSmallBankUniqueId::AXTestSmallBankUniqueId() : AXUniqueId(kMaxId) {} + +TEST_F(AXPlatformUniqueIdTest, TestIdsNotReused) { + // Create a bank of ids that uses up all available ids. + // Then remove an id and replace with a new one. Since it's the only + // slot available, the id will end up having the same value, rather than + // starting over at 1. + AXTestSmallBankUniqueId* ids[kMaxId]; + + for (int i = 0; i < kMaxId; i++) { + ids[i] = new AXTestSmallBankUniqueId(); + } + + static int kIdToReplace = 10; + + // IDs are 1-based. + EXPECT_EQ(ids[kIdToReplace]->Get(), kIdToReplace + 1); + + // Delete one of the ids and replace with a new one. + delete ids[kIdToReplace]; + ids[kIdToReplace] = new AXTestSmallBankUniqueId(); + + // IDs are 1-based. + EXPECT_EQ(ids[kIdToReplace]->Get(), kIdToReplace + 1); + + // Clean up. + for (int i = 0; i < kMaxId; i++) { + delete ids[i]; + } +} + +} // namespace ui diff --git a/ui/accessibility/platform/test_ax_node_wrapper.cc b/ui/accessibility/platform/test_ax_node_wrapper.cc index 762bf30f9ec281..045e04cef2bd85 100644 --- a/ui/accessibility/platform/test_ax_node_wrapper.cc +++ b/ui/accessibility/platform/test_ax_node_wrapper.cc @@ -241,6 +241,10 @@ std::set TestAXNodeWrapper::GetReverseRelations( return tree_->GetReverseRelations(attr, dst_id); } +const ui::AXUniqueId& TestAXNodeWrapper::GetUniqueId() const { + return unique_id_; +} + TestAXNodeWrapper::TestAXNodeWrapper(AXTree* tree, AXNode* node) : tree_(tree), node_(node), diff --git a/ui/accessibility/platform/test_ax_node_wrapper.h b/ui/accessibility/platform/test_ax_node_wrapper.h index 5b4496d5d3fbca..4327748de82c3c 100644 --- a/ui/accessibility/platform/test_ax_node_wrapper.h +++ b/ui/accessibility/platform/test_ax_node_wrapper.h @@ -46,6 +46,7 @@ class TestAXNodeWrapper : public AXPlatformNodeDelegate { bool AccessibilityPerformAction(const AXActionData& data) override; bool ShouldIgnoreHoveredStateForTesting() override; bool IsOffscreen() const override; + const ui::AXUniqueId& GetUniqueId() const override; std::set GetReverseRelations(AXIntAttribute attr, int32_t dst_id) override; std::set GetReverseRelations(AXIntListAttribute attr, @@ -61,6 +62,7 @@ class TestAXNodeWrapper : public AXPlatformNodeDelegate { AXTree* tree_; AXNode* node_; + ui::AXUniqueId unique_id_; AXPlatformNode* platform_node_; }; diff --git a/ui/views/accessibility/ax_aura_obj_cache.cc b/ui/views/accessibility/ax_aura_obj_cache.cc index 22e14648639eac..1372dd28de59bd 100644 --- a/ui/views/accessibility/ax_aura_obj_cache.cc +++ b/ui/views/accessibility/ax_aura_obj_cache.cc @@ -205,9 +205,9 @@ AXAuraObjWrapper* AXAuraObjCache::CreateInternal( return Get(it->second); AXAuraObjWrapper* wrapper = new AuraViewWrapper(aura_view); - aura_view_to_id_map[aura_view] = current_id_; - cache_[current_id_] = base::WrapUnique(wrapper); - current_id_++; + int32_t id = wrapper->GetUniqueId().Get(); + aura_view_to_id_map[aura_view] = id; + cache_[id] = base::WrapUnique(wrapper); return wrapper; } diff --git a/ui/views/accessibility/ax_aura_obj_cache.h b/ui/views/accessibility/ax_aura_obj_cache.h index bb40bc59d7ff5e..69958c3d0483c5 100644 --- a/ui/views/accessibility/ax_aura_obj_cache.h +++ b/ui/views/accessibility/ax_aura_obj_cache.h @@ -53,10 +53,6 @@ class VIEWS_EXPORT AXAuraObjCache : public aura::client::FocusChangeObserver { int32_t GetID(Widget* widget) const; int32_t GetID(aura::Window* window) const; - // Gets the next unique id for this cache. Useful for non-Aura view backed - // views. - int32_t GetNextID() { return current_id_++; } - // Removes an entry from this cache based on an Aura view. void Remove(View* view); void Remove(Widget* widget); @@ -128,7 +124,6 @@ class VIEWS_EXPORT AXAuraObjCache : public aura::client::FocusChangeObserver { std::map window_to_id_map_; std::map> cache_; - int32_t current_id_ = 1; // True immediately when entering this object's destructor. bool is_destroying_ = false; diff --git a/ui/views/accessibility/ax_aura_obj_wrapper.h b/ui/views/accessibility/ax_aura_obj_wrapper.h index 5101edd05cbf3c..74ffaacc3b0ea4 100644 --- a/ui/views/accessibility/ax_aura_obj_wrapper.h +++ b/ui/views/accessibility/ax_aura_obj_wrapper.h @@ -17,6 +17,7 @@ namespace ui { struct AXActionData; struct AXNodeData; +class AXUniqueId; } // namespace ui namespace views { @@ -32,7 +33,7 @@ class VIEWS_EXPORT AXAuraObjWrapper { virtual void GetChildren( std::vector* out_children) = 0; virtual void Serialize(ui::AXNodeData* out_node_data) = 0; - virtual int32_t GetID() = 0; + virtual const ui::AXUniqueId& GetUniqueId() const = 0; // Actions. virtual bool HandleAccessibleAction(const ui::AXActionData& action); diff --git a/ui/views/accessibility/ax_view_obj_wrapper.cc b/ui/views/accessibility/ax_view_obj_wrapper.cc index f27df732c28401..2c0cb81523b72b 100644 --- a/ui/views/accessibility/ax_view_obj_wrapper.cc +++ b/ui/views/accessibility/ax_view_obj_wrapper.cc @@ -48,11 +48,11 @@ void AXViewObjWrapper::GetChildren( void AXViewObjWrapper::Serialize(ui::AXNodeData* out_node_data) { view_->GetViewAccessibility().GetAccessibleNodeData(out_node_data); - out_node_data->id = GetID(); + out_node_data->id = GetUniqueId().Get(); } -int32_t AXViewObjWrapper::GetID() { - return AXAuraObjCache::GetInstance()->GetID(view_); +const ui::AXUniqueId& AXViewObjWrapper::GetUniqueId() const { + return view_->GetViewAccessibility().GetUniqueId(); } bool AXViewObjWrapper::HandleAccessibleAction(const ui::AXActionData& action) { diff --git a/ui/views/accessibility/ax_view_obj_wrapper.h b/ui/views/accessibility/ax_view_obj_wrapper.h index c42bdf5526bcda..c14058a497cf85 100644 --- a/ui/views/accessibility/ax_view_obj_wrapper.h +++ b/ui/views/accessibility/ax_view_obj_wrapper.h @@ -25,7 +25,7 @@ class AXViewObjWrapper : public AXAuraObjWrapper { AXAuraObjWrapper* GetParent() override; void GetChildren(std::vector* out_children) override; void Serialize(ui::AXNodeData* out_node_data) override; - int32_t GetID() override; + const ui::AXUniqueId& GetUniqueId() const final; bool HandleAccessibleAction(const ui::AXActionData& action) override; private: diff --git a/ui/views/accessibility/ax_widget_obj_wrapper.cc b/ui/views/accessibility/ax_widget_obj_wrapper.cc index 2d96f8c9400ae7..bfb5725e752295 100644 --- a/ui/views/accessibility/ax_widget_obj_wrapper.cc +++ b/ui/views/accessibility/ax_widget_obj_wrapper.cc @@ -8,6 +8,7 @@ #include "ui/accessibility/ax_node_data.h" #include "ui/views/accessibility/ax_aura_obj_cache.h" #include "ui/views/accessibility/ax_aura_obj_wrapper.h" +#include "ui/views/accessibility/view_accessibility.h" #include "ui/views/widget/widget.h" #include "ui/views/widget/widget_delegate.h" @@ -42,7 +43,7 @@ void AXWidgetObjWrapper::GetChildren( } void AXWidgetObjWrapper::Serialize(ui::AXNodeData* out_node_data) { - out_node_data->id = GetID(); + out_node_data->id = GetUniqueId().Get(); out_node_data->role = widget_->widget_delegate()->GetAccessibleWindowRole(); out_node_data->AddStringAttribute( ui::AX_ATTR_NAME, @@ -52,8 +53,8 @@ void AXWidgetObjWrapper::Serialize(ui::AXNodeData* out_node_data) { out_node_data->state = 0; } -int32_t AXWidgetObjWrapper::GetID() { - return AXAuraObjCache::GetInstance()->GetID(widget_); +const ui::AXUniqueId& AXWidgetObjWrapper::GetUniqueId() const { + return unique_id_; } void AXWidgetObjWrapper::OnWidgetDestroying(Widget* widget) { diff --git a/ui/views/accessibility/ax_widget_obj_wrapper.h b/ui/views/accessibility/ax_widget_obj_wrapper.h index 88512cdcc6de88..6cc11bc27aaad0 100644 --- a/ui/views/accessibility/ax_widget_obj_wrapper.h +++ b/ui/views/accessibility/ax_widget_obj_wrapper.h @@ -8,6 +8,7 @@ #include #include "base/macros.h" +#include "ui/accessibility/platform/ax_unique_id.h" #include "ui/views/accessibility/ax_aura_obj_wrapper.h" #include "ui/views/widget/widget_observer.h" #include "ui/views/widget/widget_removals_observer.h" @@ -27,7 +28,7 @@ class AXWidgetObjWrapper : public AXAuraObjWrapper, AXAuraObjWrapper* GetParent() override; void GetChildren(std::vector* out_children) override; void Serialize(ui::AXNodeData* out_node_data) override; - int32_t GetID() override; + const ui::AXUniqueId& GetUniqueId() const final; // WidgetObserver overrides. void OnWidgetDestroying(Widget* widget) override; @@ -40,6 +41,8 @@ class AXWidgetObjWrapper : public AXAuraObjWrapper, private: Widget* widget_; + const ui::AXUniqueId unique_id_; + DISALLOW_COPY_AND_ASSIGN(AXWidgetObjWrapper); }; diff --git a/ui/views/accessibility/ax_window_obj_wrapper.cc b/ui/views/accessibility/ax_window_obj_wrapper.cc index 422a2092ab5850..ab604049070678 100644 --- a/ui/views/accessibility/ax_window_obj_wrapper.cc +++ b/ui/views/accessibility/ax_window_obj_wrapper.cc @@ -10,6 +10,7 @@ #include "ui/accessibility/ax_enums.h" #include "ui/accessibility/ax_node_data.h" #include "ui/accessibility/platform/aura_window_properties.h" +#include "ui/accessibility/platform/ax_unique_id.h" #include "ui/aura/client/focus_client.h" #include "ui/aura/window.h" #include "ui/views/accessibility/ax_aura_obj_cache.h" @@ -57,7 +58,7 @@ void AXWindowObjWrapper::GetChildren( } void AXWindowObjWrapper::Serialize(ui::AXNodeData* out_node_data) { - out_node_data->id = GetID(); + out_node_data->id = GetUniqueId().Get(); ui::AXRole role = window_->GetProperty(ui::kAXRoleOverride); if (role != ui::AX_ROLE_NONE) out_node_data->role = role; @@ -92,8 +93,8 @@ void AXWindowObjWrapper::Serialize(ui::AXNodeData* out_node_data) { } } -int32_t AXWindowObjWrapper::GetID() { - return AXAuraObjCache::GetInstance()->GetID(window_); +const ui::AXUniqueId& AXWindowObjWrapper::GetUniqueId() const { + return unique_id_; } void AXWindowObjWrapper::OnWindowDestroyed(aura::Window* window) { diff --git a/ui/views/accessibility/ax_window_obj_wrapper.h b/ui/views/accessibility/ax_window_obj_wrapper.h index 5707a4348e9109..a1f5834f5d3fa3 100644 --- a/ui/views/accessibility/ax_window_obj_wrapper.h +++ b/ui/views/accessibility/ax_window_obj_wrapper.h @@ -8,6 +8,7 @@ #include #include "base/macros.h" +#include "ui/accessibility/platform/ax_unique_id.h" #include "ui/aura/window_observer.h" #include "ui/views/accessibility/ax_aura_obj_wrapper.h" @@ -34,7 +35,7 @@ class AXWindowObjWrapper : public AXAuraObjWrapper, AXAuraObjWrapper* GetParent() override; void GetChildren(std::vector* out_children) override; void Serialize(ui::AXNodeData* out_node_data) override; - int32_t GetID() override; + const ui::AXUniqueId& GetUniqueId() const final; // WindowObserver overrides. void OnWindowDestroyed(aura::Window* window) override; @@ -55,6 +56,8 @@ class AXWindowObjWrapper : public AXAuraObjWrapper, bool is_root_window_; + const ui::AXUniqueId unique_id_; + DISALLOW_COPY_AND_ASSIGN(AXWindowObjWrapper); }; diff --git a/ui/views/accessibility/native_view_accessibility_auralinux.cc b/ui/views/accessibility/native_view_accessibility_auralinux.cc index b9af1363578fe8..7fbbca936cfb92 100644 --- a/ui/views/accessibility/native_view_accessibility_auralinux.cc +++ b/ui/views/accessibility/native_view_accessibility_auralinux.cc @@ -60,6 +60,8 @@ class AuraLinuxApplication return platform_node_->GetNativeViewAccessible(); } + const ui::AXUniqueId& GetUniqueId() const override { return unique_id_; } + // WidgetObserver: void OnWidgetDestroying(Widget* widget) override { @@ -159,6 +161,7 @@ class AuraLinuxApplication ui::AXPlatformNode* platform_node_; ui::AXNodeData data_; + ui::AXUniqueId unique_id_; std::vector widgets_; DISALLOW_COPY_AND_ASSIGN(AuraLinuxApplication); diff --git a/ui/views/accessibility/native_view_accessibility_base.cc b/ui/views/accessibility/native_view_accessibility_base.cc index f06a463a7b09b6..d26a9ec8cee8eb 100644 --- a/ui/views/accessibility/native_view_accessibility_base.cc +++ b/ui/views/accessibility/native_view_accessibility_base.cc @@ -250,6 +250,10 @@ std::set NativeViewAccessibilityBase::GetReverseRelations( return std::set(); } +const ui::AXUniqueId& NativeViewAccessibilityBase::GetUniqueId() const { + return ViewAccessibility::GetUniqueId(); +} + gfx::RectF NativeViewAccessibilityBase::GetBoundsInScreen() const { return gfx::RectF(view()->GetBoundsInScreen()); } diff --git a/ui/views/accessibility/native_view_accessibility_base.h b/ui/views/accessibility/native_view_accessibility_base.h index d8c74002137432..413a3a32af9482 100644 --- a/ui/views/accessibility/native_view_accessibility_base.h +++ b/ui/views/accessibility/native_view_accessibility_base.h @@ -14,6 +14,7 @@ #include "ui/accessibility/ax_tree_data.h" #include "ui/accessibility/platform/ax_platform_node.h" #include "ui/accessibility/platform/ax_platform_node_delegate.h" +#include "ui/accessibility/platform/ax_unique_id.h" #include "ui/gfx/native_widget_types.h" #include "ui/views/accessibility/view_accessibility.h" #include "ui/views/views_export.h" @@ -33,7 +34,7 @@ class VIEWS_EXPORT NativeViewAccessibilityBase public: ~NativeViewAccessibilityBase() override; - // NativeViewAccessibility: + // ViewAccessibility: gfx::NativeViewAccessible GetNativeObject() override; void NotifyAccessibilityEvent(ui::AXEvent event_type) override; @@ -53,6 +54,8 @@ class VIEWS_EXPORT NativeViewAccessibilityBase bool AccessibilityPerformAction(const ui::AXActionData& data) override; bool ShouldIgnoreHoveredStateForTesting() override; bool IsOffscreen() const override; + const ui::AXUniqueId& GetUniqueId() + const override; // Also in ViewAccessibility std::set GetReverseRelations(ui::AXIntAttribute attr, int32_t dst_id) override; std::set GetReverseRelations(ui::AXIntListAttribute attr, diff --git a/ui/views/accessibility/view_accessibility.cc b/ui/views/accessibility/view_accessibility.cc index b8bd1c975c571b..577872cb4e78e6 100644 --- a/ui/views/accessibility/view_accessibility.cc +++ b/ui/views/accessibility/view_accessibility.cc @@ -43,6 +43,10 @@ ViewAccessibility::ViewAccessibility(View* view) : owner_view_(view) {} ViewAccessibility::~ViewAccessibility() {} +const ui::AXUniqueId& ViewAccessibility::GetUniqueId() const { + return unique_id_; +} + void ViewAccessibility::GetAccessibleNodeData(ui::AXNodeData* data) const { // Views may misbehave if their widget is closed; return an unknown role // rather than possibly crashing. diff --git a/ui/views/accessibility/view_accessibility.h b/ui/views/accessibility/view_accessibility.h index e26b34fa756527..b9c02c4fe3ff07 100644 --- a/ui/views/accessibility/view_accessibility.h +++ b/ui/views/accessibility/view_accessibility.h @@ -10,6 +10,7 @@ #include "base/macros.h" #include "ui/accessibility/ax_enums.h" #include "ui/accessibility/ax_node_data.h" +#include "ui/accessibility/platform/ax_unique_id.h" #include "ui/gfx/native_widget_types.h" #include "ui/views/views_export.h" @@ -49,6 +50,8 @@ class VIEWS_EXPORT ViewAccessibility { virtual gfx::NativeViewAccessible GetNativeObject(); virtual void NotifyAccessibilityEvent(ui::AXEvent event_type) {} + virtual const ui::AXUniqueId& GetUniqueId() const; + protected: explicit ViewAccessibility(View* view); @@ -58,6 +61,8 @@ class VIEWS_EXPORT ViewAccessibility { // Weak. Owns this. View* const owner_view_; + const ui::AXUniqueId unique_id_; + // Contains data set explicitly via SetRole, SetName, etc. that overrides // anything provided by GetAccessibleNodeData(). ui::AXNodeData custom_data_;