Forward AXPlatformNodeWin calls to the right child.

ui/accessibility/platform/ax_platform_node_win.cc

Index: ui/accessibility/platform/ax_platform_node_win.cc
diff --git a/ui/accessibility/platform/ax_platform_node_win.cc b/ui/accessibility/platform/ax_platform_node_win.cc
index c35356135374000f9e009cf637f1b8c4e12bb47f..b9e2b4d299472c3ca849c1beafbc91424498ba87 100644
--- a/ui/accessibility/platform/ax_platform_node_win.cc
+++ b/ui/accessibility/platform/ax_platform_node_win.cc
@@ -255,7 +255,12 @@ HRESULT AXPlatformNodeWin::accDoDefaultAction(VARIANT var_id) {
   COM_OBJECT_VALIDATE_VAR_ID(var_id);
   AXActionData data;
   data.action = ui::AX_ACTION_DO_DEFAULT;
-  if (delegate_->AccessibilityPerformAction(data))
+
+  auto* target = GetTargetFromChildID(var_id);

[dmazzoni, 2017/04/10 07:20:19]

What do you think about putting GetTargetFromChildID in the
COM_OBJECT_VALIDATE_VAR_ID macro?

    auto* target = COM_OBJECT_VALIDATE_VAR_ID(var_id);

That cuts down on the boilerplate a lot.

+  if (!target)
+    return E_INVALIDARG;
+
+  if (target->delegate_->AccessibilityPerformAction(data))
     return S_OK;
   return E_FAIL;
 }
@@ -263,7 +268,12 @@ HRESULT AXPlatformNodeWin::accDoDefaultAction(VARIANT var_id) {
 STDMETHODIMP AXPlatformNodeWin::accLocation(
     LONG* x_left, LONG* y_top, LONG* width, LONG* height, VARIANT var_id) {
   COM_OBJECT_VALIDATE_VAR_ID_4_ARGS(var_id, x_left, y_top, width, height);
-  gfx::Rect bounds = delegate_->GetScreenBoundsRect();
+
+  auto* target = GetTargetFromChildID(var_id);
+  if (!target)
+    return E_INVALIDARG;
+
+  gfx::Rect bounds = target->delegate_->GetScreenBoundsRect();
   *x_left = bounds.x();
   *y_top = bounds.y();
   *width  = bounds.width();
@@ -332,40 +342,15 @@ STDMETHODIMP AXPlatformNodeWin::accNavigate(
 STDMETHODIMP AXPlatformNodeWin::get_accChild(VARIANT var_child,
                                              IDispatch** disp_child) {
   COM_OBJECT_VALIDATE_1_ARG(disp_child);
-  LONG child_id = V_I4(&var_child);
-  if (child_id == CHILDID_SELF) {
-    *disp_child = this;
-    (*disp_child)->AddRef();
-    return S_OK;
-  }
-
-  if (child_id >= 1 && child_id <= delegate_->GetChildCount()) {
-    // Positive child ids are a 1-based child index, used by clients
-    // that want to enumerate all immediate children.
-    *disp_child = delegate_->ChildAtIndex(child_id - 1);
-    if (!(*disp_child))
-      return E_INVALIDARG;
-    (*disp_child)->AddRef();
-    return S_OK;
-  }
-
-  if (child_id >= 0)
+  auto* target = GetTargetFromChildID(var_child);
+  if (!target) {
+    *disp_child = nullptr;
     return E_INVALIDARG;
-
-  // Negative child ids can be used to map to any descendant.
-  AXPlatformNodeWin* child = static_cast<AXPlatformNodeWin*>(
-      GetFromUniqueId(-child_id));
-  if (child && !IsDescendant(child))
-    child = nullptr;
-
-  if (child) {
-    *disp_child = child;
-    (*disp_child)->AddRef();
-    return S_OK;
   }
 
-  *disp_child = nullptr;
-  return E_INVALIDARG;
+  *disp_child = target;
+  (*disp_child)->AddRef();
+  return S_OK;
 }
 
 STDMETHODIMP AXPlatformNodeWin::get_accChildCount(LONG* child_count) {
@@ -377,8 +362,13 @@ STDMETHODIMP AXPlatformNodeWin::get_accChildCount(LONG* child_count) {
 STDMETHODIMP AXPlatformNodeWin::get_accDefaultAction(
     VARIANT var_id, BSTR* def_action) {
   COM_OBJECT_VALIDATE_VAR_ID_1_ARG(var_id, def_action);
+
+  auto* target = GetTargetFromChildID(var_id);
+  if (!target)
+    return E_INVALIDARG;
+
   int action;
-  if (!GetIntAttribute(AX_ATTR_ACTION, &action)) {
+  if (!target->GetIntAttribute(AX_ATTR_ACTION, &action)) {
     *def_action = nullptr;
     return S_FALSE;
   }
@@ -398,7 +388,12 @@ STDMETHODIMP AXPlatformNodeWin::get_accDefaultAction(
 STDMETHODIMP AXPlatformNodeWin::get_accDescription(
     VARIANT var_id, BSTR* desc) {
   COM_OBJECT_VALIDATE_VAR_ID_1_ARG(var_id, desc);
-  return GetStringAttributeAsBstr(ui::AX_ATTR_DESCRIPTION, desc);
+
+  auto* target = GetTargetFromChildID(var_id);
+  if (!target)
+    return E_INVALIDARG;
+
+  return target->GetStringAttributeAsBstr(ui::AX_ATTR_DESCRIPTION, desc);
 }
 
 STDMETHODIMP AXPlatformNodeWin::get_accFocus(VARIANT* focus_child) {
@@ -422,13 +417,22 @@ STDMETHODIMP AXPlatformNodeWin::get_accFocus(VARIANT* focus_child) {
 STDMETHODIMP AXPlatformNodeWin::get_accKeyboardShortcut(
     VARIANT var_id, BSTR* acc_key) {
   COM_OBJECT_VALIDATE_VAR_ID_1_ARG(var_id, acc_key);
-  return GetStringAttributeAsBstr(ui::AX_ATTR_SHORTCUT, acc_key);
+
+  auto* target = GetTargetFromChildID(var_id);
+  if (!target)
+    return E_INVALIDARG;
+
+  return target->GetStringAttributeAsBstr(ui::AX_ATTR_SHORTCUT, acc_key);
 }
 
 STDMETHODIMP AXPlatformNodeWin::get_accName(
     VARIANT var_id, BSTR* name) {
   COM_OBJECT_VALIDATE_VAR_ID_1_ARG(var_id, name);
-  return GetStringAttributeAsBstr(ui::AX_ATTR_NAME, name);
+  auto* target = GetTargetFromChildID(var_id);
+  if (!target)
+    return E_INVALIDARG;
+
+  return target->GetStringAttributeAsBstr(ui::AX_ATTR_NAME, name);
 }
 
 STDMETHODIMP AXPlatformNodeWin::get_accParent(
@@ -446,16 +450,24 @@ STDMETHODIMP AXPlatformNodeWin::get_accParent(
 STDMETHODIMP AXPlatformNodeWin::get_accRole(
     VARIANT var_id, VARIANT* role) {
   COM_OBJECT_VALIDATE_VAR_ID_1_ARG(var_id, role);
+  auto* target = GetTargetFromChildID(var_id);
+  if (!target)
+    return E_INVALIDARG;
+
   role->vt = VT_I4;
-  role->lVal = MSAARole();
+  role->lVal = target->MSAARole();
   return S_OK;
 }
 
 STDMETHODIMP AXPlatformNodeWin::get_accState(
     VARIANT var_id, VARIANT* state) {
   COM_OBJECT_VALIDATE_VAR_ID_1_ARG(var_id, state);
+  auto* target = GetTargetFromChildID(var_id);
+  if (!target)
+    return E_INVALIDARG;
+
   state->vt = VT_I4;
-  state->lVal = MSAAState();
+  state->lVal = target->MSAAState();
   return S_OK;
 }
 
@@ -467,7 +479,11 @@ STDMETHODIMP AXPlatformNodeWin::get_accHelp(
 
 STDMETHODIMP AXPlatformNodeWin::get_accValue(VARIANT var_id, BSTR* value) {
   COM_OBJECT_VALIDATE_VAR_ID_1_ARG(var_id, value);
-  return GetStringAttributeAsBstr(ui::AX_ATTR_VALUE, value);
+
+  auto* target = GetTargetFromChildID(var_id);
+  if (!target)
+    return E_INVALIDARG;
+  return target->GetStringAttributeAsBstr(ui::AX_ATTR_VALUE, value);
 }
 
 STDMETHODIMP AXPlatformNodeWin::put_accValue(VARIANT var_id,
@@ -476,7 +492,12 @@ STDMETHODIMP AXPlatformNodeWin::put_accValue(VARIANT var_id,
   data.action = ui::AX_ACTION_SET_VALUE;
   data.value = new_value;
   COM_OBJECT_VALIDATE_VAR_ID(var_id);
-  if (delegate_->AccessibilityPerformAction(data))
+
+  auto* target = GetTargetFromChildID(var_id);
+  if (!target)
+    return E_INVALIDARG;
+
+  if (target->delegate_->AccessibilityPerformAction(data))
     return S_OK;
   return E_FAIL;
 }
@@ -1200,4 +1221,29 @@ LONG AXPlatformNodeWin::FindBoundary(
       AX_TEXT_AFFINITY_DOWNSTREAM));
 }
 
+AXPlatformNodeWin* AXPlatformNodeWin::GetTargetFromChildID(
+    const VARIANT& var_id) {
+  LONG child_id = V_I4(&var_id);
+  if (child_id == CHILDID_SELF) {
+    return this;
+  }
+
+  if (child_id >= 1 && child_id <= delegate_->GetChildCount()) {
+    // Positive child ids are a 1-based child index, used by clients
+    // that want to enumerate all immediate children.
+    return static_cast<AXPlatformNodeWin*>(

[dmazzoni, 2017/04/10 07:20:19]

Note that a child may not necessarily be an AXPlatformNodeWin, so this
cast isn't safe. In particular, right now the child of an AXPlatformNodeWin
may be a BrowserAccessibilityWin.

Instead, use FromNativeViewAccessible(), which will check if the object
is the right type and cast it if so.

If you get a AXPlatformNodeBase, it is safe to cast that to an
AXPlatformNodeWIn.

+        delegate_->ChildAtIndex(child_id - 1));
+  }
+
+  if (child_id >= 0)
+    return nullptr;
+
+  // Negative child ids can be used to map to any descendant.
+  auto* child = static_cast<AXPlatformNodeWin*>(GetFromUniqueId(-child_id));
+  if (child && !IsDescendant(child))
+    child = nullptr;
+
+  return child;
+}
+
 }  // namespace ui