Consistent, unified Unique IDs for accessibility objects in browser p…

…rocess

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 <aleventhal@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528518}

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;
   }
 

chrome/browser/ui/aura/accessibility/automation_manager_aura.cc

@@ -202,7 +202,7 @@ void AutomationManagerAura::SendEvent(BrowserContext* context,
     current_tree_serializer_->SerializeChanges(focus, &params.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();

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_;
 }

chrome/browser/ui/aura/accessibility/ax_root_obj_wrapper.h

@@ -10,6 +10,7 @@
 #include <string>
 
 #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<views::AXAuraObjWrapper*>* 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_;
 

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,21 +83,21 @@ 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,
                                AXAuraObjWrapper* node2) const {
   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) {

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<int32_t> 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

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);
 };
 

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 {

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;

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<BrowserAccessibilityWin*>(delegate);
-  AXPlatformNodeBase::Init(delegate);
+  AXPlatformNodeWin::Init(delegate);
 }
 
 std::vector<base::string16> BrowserAccessibilityComWin::ComputeTextAttributes()

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);
 }
 

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

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 = [

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);

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<NativeWindowHandlerCallback>::Leaky
       native_window_handler_;
+
+  DISALLOW_COPY_AND_ASSIGN(AXPlatformNode);
 };
 
 }  // namespace ui

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;

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<int32_t> GetReverseRelations(AXIntListAttribute attr,
                                                 int32_t dst_id) = 0;
 
+  virtual const AXUniqueId& GetUniqueId() const = 0;
+
   //
   // Events.
   //