Re-land: Implement NativeViewAccessibilityWin using AXPlatformNodeWin.

ui/accessibility/platform/ax_platform_node_base.cc

Index: ui/accessibility/platform/ax_platform_node_base.cc
diff --git a/ui/accessibility/platform/ax_platform_node_base.cc b/ui/accessibility/platform/ax_platform_node_base.cc
index 3479af14ab5d1daa5204100454b551f02c3e67d9..7210f329df05854040e6f715737953f999b19335 100644
--- a/ui/accessibility/platform/ax_platform_node_base.cc
+++ b/ui/accessibility/platform/ax_platform_node_base.cc
@@ -4,6 +4,7 @@
 
 #include "ui/accessibility/platform/ax_platform_node_base.h"
 
+#include "base/strings/utf_string_conversions.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/accessibility/platform/ax_platform_node_delegate.h"
 
@@ -19,8 +20,12 @@ void AXPlatformNodeBase::Init(AXPlatformNodeDelegate* delegate) {
   delegate_ = delegate;
 }
 
-AXRole AXPlatformNodeBase::GetRole() const {
-  return delegate_ ? delegate_->GetData()->role : AX_ROLE_UNKNOWN;
+const AXNodeData& AXPlatformNodeBase::GetData() const {
+  CR_DEFINE_STATIC_LOCAL(ui::AXNodeData, empty_data, ());
+  if (!delegate_)
+    return empty_data;
+  const AXNodeData* data = delegate_->GetData();
+  return data ? *data : empty_data;
 }
 
 gfx::Rect AXPlatformNodeBase::GetBoundsInScreen() const {
@@ -43,7 +48,11 @@ gfx::NativeViewAccessible AXPlatformNodeBase::ChildAtIndex(int index) {
   return delegate_ ? delegate_->ChildAtIndex(index) : NULL;
 }
 
-// AXPlatformNode
+int AXPlatformNodeBase::GetIndexInParent() {
+  return delegate_ ? delegate_->GetIndexInParent() : 0;

[David Tseng, 2015/02/13 20:10:10]

Is this index 0-based?

[nektarios, 2015/02/13 22:44:29]

Yes.

[David Tseng, 2015/02/13 23:34:25]

Concern is 0 is also being used as the "error" value when delegate_ is NULL.

[dmazzoni, 2015/02/17 22:48:05]

Good point. Thinking about this some more, trying to provide values when
delegate_ is NULL is just delaying any potential errors. I stuck a
CHECK(delegate_) at the top of every function instead. All of the public API
functions like get_accName should be checking whether delegate_ is null first
thing and doing nothing if so, so the CHECK would only get hit if there's a bug
in our code and we're trying to actually use an object that's not valid anymore.

+}
+
+// AXPlatformNode.

[David Tseng, 2015/02/13 20:10:10]

?

[dmazzoni, 2015/02/17 22:48:05]

Changed to "AXPlatformNode overrides". Some Chrome code just says the name of
the superclass.

 
 void AXPlatformNodeBase::Destroy() {
   delegate_ = NULL;
@@ -54,5 +63,208 @@ gfx::NativeViewAccessible AXPlatformNodeBase::GetNativeViewAccessible() {
   return NULL;
 }
 
+// Helpers.
+
+AXPlatformNodeBase* AXPlatformNodeBase::GetPreviousSibling() {
+  gfx::NativeViewAccessible parent_accessible = GetParent();
+  AXPlatformNodeBase* parent = static_cast<AXPlatformNodeBase*>(

[nektarios, 2015/02/13 22:44:29]

Are all the static_casts in this file really necessary?

[dmazzoni, 2015/02/17 22:48:04]

Good point - I added an additional helper function to this file that returns an
AXPlatformNodeBase* so now there are fewer casts.

+      AXPlatformNode::FromNativeViewAccessible(parent_accessible));
+  if (!parent)
+    return NULL;
+
+  int index_in_parent = GetIndexInParent();
+  if (index_in_parent > 0 &&

[nektarios, 2015/02/13 22:44:29]

Use >= 1 and <= .count instead. Clearer.

[dmazzoni, 2015/02/17 22:48:05]

Agreed about the first, but I think that the second is clearer as-is because
it's highlighting the fact that we're checking that the previous sibling index
is valid.

+      index_in_parent - 1 < parent->GetChildCount()) {
+    return static_cast<AXPlatformNodeBase*>(
+        AXPlatformNode::FromNativeViewAccessible(
+            parent->ChildAtIndex(index_in_parent - 1)));
+  }
+  return NULL;
+}
+
+AXPlatformNodeBase* AXPlatformNodeBase::GetNextSibling() {
+  gfx::NativeViewAccessible parent_accessible = GetParent();
+  AXPlatformNodeBase* parent = static_cast<AXPlatformNodeBase*>(
+      AXPlatformNode::FromNativeViewAccessible(parent_accessible));
+  if (!parent)
+    return NULL;
+
+  int index_in_parent = GetIndexInParent();

[nektarios, 2015/02/13 22:44:29]

Do the plus one here. Clearer.

[dmazzoni, 2015/02/17 22:48:05]

Great idea.

+  if (index_in_parent + 1 < parent->GetChildCount()) {

[David Tseng, 2015/02/13 20:10:10]

No lower bound?

[nektarios, 2015/02/13 22:44:29]

Non-user provided. Supposed to be error-free.

[dmazzoni, 2015/02/17 22:48:05]

Done.

[dmazzoni, 2015/02/17 22:48:05]

Done.

+    return static_cast<AXPlatformNodeBase*>(
+        AXPlatformNode::FromNativeViewAccessible(
+            parent->ChildAtIndex(index_in_parent + 1)));
+  }
+  return NULL;
+}
+
+bool AXPlatformNodeBase::IsDescendant(AXPlatformNodeBase* descendant) {

[David Tseng, 2015/02/13 20:10:10]

I find this param name confusing. Perhaps just node?

+  if (!descendant)
+    return false;
+  if (descendant == this)
+    return true;
+  AXPlatformNodeBase* parent = static_cast<AXPlatformNodeBase*>(
+      AXPlatformNode::FromNativeViewAccessible(descendant->GetParent()));

[David Tseng, 2015/02/13 20:10:10]

Suggestion: refactor this static_cast... pattern into a separate function.

[dmazzoni, 2015/02/17 22:48:05]

Done.

+  return IsDescendant(parent);
+}
+
+bool AXPlatformNodeBase::HasBoolAttribute(
+    ui::AXBoolAttribute attribute) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.bool_attributes.size(); ++i) {

[nektarios, 2015/02/13 22:44:29]

Instead of these for loops I prefer a function that returns an iterator to the
first element whose ->first matches. Perhaps std::find(attributes.begin(),
attributes.end(), IsFirstEqual(...));

[dmazzoni, 2015/02/17 22:48:05]

OK. I created a templated helper function FindInVectorOfPairs because then the
compiler can infer the types. If I tried to do it just using std::find_if
directly I'd need to specify the templated types each time and it'd be far less
readable. Does this look okay?

+    if (data.bool_attributes[i].first == attribute)
+      return true;
+  }
+
+  return false;
+}
+
+

[David Tseng, 2015/02/13 20:10:10]

nit: extra line

[dmazzoni, 2015/02/17 22:48:05]

Done.

+bool AXPlatformNodeBase::GetBoolAttribute(
+    ui::AXBoolAttribute attribute) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.bool_attributes.size(); ++i) {
+    if (data.bool_attributes[i].first == attribute)
+      return data.bool_attributes[i].second;
+  }
+
+  return false;
+}

[David Tseng, 2015/02/13 20:10:10]

The below method is a generalization of this one. Just call it in this impl.

[dmazzoni, 2015/02/17 22:48:05]

Sure. Done for bool, int, and float. I kept the GetStringAttribute functions
separate because one form returns a const reference and the other copies.

+
+bool AXPlatformNodeBase::GetBoolAttribute(
+    ui::AXBoolAttribute attribute, bool* value) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.bool_attributes.size(); ++i) {
+    if (data.bool_attributes[i].first == attribute) {
+      *value = data.bool_attributes[i].second;
+      return true;
+    }
+  }
+
+  return false;
+}
+
+bool AXPlatformNodeBase::HasFloatAttribute(
+    ui::AXFloatAttribute attribute) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.float_attributes.size(); ++i) {
+    if (data.float_attributes[i].first == attribute)
+      return true;
+  }
+
+  return false;
+}

[David Tseng, 2015/02/13 20:10:10]

Same for this one. Just use the return value of the second overload.

[dmazzoni, 2015/02/17 22:48:04]

Done.

+
+float AXPlatformNodeBase::GetFloatAttribute(
+    ui::AXFloatAttribute attribute) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.float_attributes.size(); ++i) {
+    if (data.float_attributes[i].first == attribute)
+      return data.float_attributes[i].second;
+  }
+
+  return 0.0;
+}

[David Tseng, 2015/02/13 20:10:10]

ditto.

+
+bool AXPlatformNodeBase::GetFloatAttribute(
+    ui::AXFloatAttribute attribute, float* value) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.float_attributes.size(); ++i) {
+    if (data.float_attributes[i].first == attribute) {
+      *value = data.float_attributes[i].second;
+      return true;
+    }
+  }
+
+  return false;
+}

[David Tseng, 2015/02/13 20:10:10]

ditto

+
+bool AXPlatformNodeBase::HasIntAttribute(
+    ui::AXIntAttribute attribute) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.int_attributes.size(); ++i) {
+    if (data.int_attributes[i].first == attribute)
+      return true;
+  }
+
+  return false;
+}

[David Tseng, 2015/02/13 20:10:10]

ditto

+
+int AXPlatformNodeBase::GetIntAttribute(ui::AXIntAttribute attribute) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.int_attributes.size(); ++i) {
+    if (data.int_attributes[i].first == attribute)
+      return data.int_attributes[i].second;
+  }
+
+  return 0;
+}

[David Tseng, 2015/02/13 20:10:10]

ditto

+
+bool AXPlatformNodeBase::GetIntAttribute(
+    ui::AXIntAttribute attribute, int* value) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.int_attributes.size(); ++i) {
+    if (data.int_attributes[i].first == attribute) {
+      *value = data.int_attributes[i].second;
+      return true;
+    }
+  }
+
+  return false;
+}
+
+bool AXPlatformNodeBase::HasStringAttribute(
+    ui::AXStringAttribute attribute) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.string_attributes.size(); ++i) {
+    if (data.string_attributes[i].first == attribute)
+      return true;
+  }
+
+  return false;
+}

[David Tseng, 2015/02/13 20:10:11]

ditto

+
+const std::string& AXPlatformNodeBase::GetStringAttribute(
+    ui::AXStringAttribute attribute) const {
+  const ui::AXNodeData& data = GetData();
+  CR_DEFINE_STATIC_LOCAL(std::string, empty_string, ());
+  for (size_t i = 0; i < data.string_attributes.size(); ++i) {
+    if (data.string_attributes[i].first == attribute)
+      return data.string_attributes[i].second;
+  }
+
+  return empty_string;
+}

[David Tseng, 2015/02/13 20:10:11]

ditto

+
+bool AXPlatformNodeBase::GetStringAttribute(
+    ui::AXStringAttribute attribute, std::string* value) const {
+  const ui::AXNodeData& data = GetData();
+  for (size_t i = 0; i < data.string_attributes.size(); ++i) {
+    if (data.string_attributes[i].first == attribute) {
+      *value = data.string_attributes[i].second;
+      return true;
+    }
+  }
+
+  return false;
+}
+
+base::string16 AXPlatformNodeBase::GetString16Attribute(
+    ui::AXStringAttribute attribute) const {
+  std::string value_utf8;
+  if (!GetStringAttribute(attribute, &value_utf8))
+    return base::string16();
+  return base::UTF8ToUTF16(value_utf8);
+}
+
+bool AXPlatformNodeBase::GetString16Attribute(
+    ui::AXStringAttribute attribute,
+    base::string16* value) const {
+  std::string value_utf8;
+  if (!GetStringAttribute(attribute, &value_utf8))
+    return false;
+  *value = base::UTF8ToUTF16(value_utf8);
+  return true;
+}
 
 }  // namespace ui