Properly parent transient window dialogs for the ATK API

Similar to what happens for the Windows port, the AuraLinux
accessibility implementation should walk up the transient window chain
when returning parents of dialogs in transient windows. This fixes an
issue where bubbles show up twice in the accessibility tree and where
bubbles did not return their frame parents when calling
atk_object_get_parent.

Bug: 1024342
Change-Id: I15fc2db6c6d21a7368eae92798127e597ba8b0c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1919930
Commit-Queue: Martin Robinson <mrobinson@igalia.com>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719072}

ui/views/BUILD.gn

@@ -814,6 +814,12 @@ jumbo_component("views") {
       "accessibility/view_ax_platform_node_delegate_win.cc",
       "accessibility/view_ax_platform_node_delegate_win.h",
     ]
+    if (use_aura) {
+      sources += [
+        "accessibility/views_utilities_aura.cc",
+        "accessibility/views_utilities_aura.h",
+      ]
+    }
   }
 
   if (is_fuchsia) {

ui/views/accessibility/view_ax_platform_node_delegate_auralinux.cc

@@ -17,15 +17,45 @@
 #include "ui/accessibility/platform/ax_platform_node_delegate_base.h"
 #include "ui/aura/window.h"
 #include "ui/gfx/native_widget_types.h"
+#include "ui/views/accessibility/views_utilities_aura.h"
 #include "ui/views/view.h"
 #include "ui/views/views_delegate.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_observer.h"
+#include "ui/wm/core/window_util.h"
 
 namespace views {
 
 namespace {
 
+// Return the widget of any parent window of |widget|, first checking for
+// transient parent windows.
+Widget* GetWidgetOfParentWindowIncludingTransient(Widget* widget) {
+  if (!widget)
+    return nullptr;
+
+  aura::Window* window = widget->GetNativeWindow();
+  if (!window)
+    return nullptr;
+
+  // Look for an ancestor window with a Widget, and if found, return
+  // the NativeViewAccessible for its RootView.
+  aura::Window* ancestor_window = GetWindowParentIncludingTransient(window);
+  if (!ancestor_window)
+    return nullptr;
+
+  return Widget::GetWidgetForNativeView(ancestor_window);
+}
+
+// Return the toplevel widget ancestor of |widget|, including widgets of
+// parents of transient windows.
+Widget* GetToplevelWidgetIncludingTransientWindows(Widget* widget) {
+  widget = widget = widget->GetTopLevelWidget();
+  if (Widget* parent_widget = GetWidgetOfParentWindowIncludingTransient(widget))
+    return GetToplevelWidgetIncludingTransientWindows(parent_widget);
+  return widget;
+}
+
 // ATK requires that we have a single root "application" object that's the
 // owner of all other windows. This is a simple class that implements the
 // AXPlatformNodeDelegate interface so we can create such an application
@@ -51,7 +81,7 @@ class AuraLinuxApplication : public ui::AXPlatformNodeDelegateBase,
     if (!widget)
       return;
 
-    widget = widget->GetTopLevelWidget();
+    widget = GetToplevelWidgetIncludingTransientWindows(widget);
     if (base::Contains(widgets_, widget))
       return;
 
@@ -149,10 +179,16 @@ ViewAXPlatformNodeDelegateAuraLinux::~ViewAXPlatformNodeDelegateAuraLinux() =
     default;
 
 gfx::NativeViewAccessible ViewAXPlatformNodeDelegateAuraLinux::GetParent() {
-  gfx::NativeViewAccessible parent = ViewAXPlatformNodeDelegate::GetParent();
-  if (!parent)
-    parent = AuraLinuxApplication::GetInstance()->GetNativeViewAccessible();
-  return parent;
+  if (gfx::NativeViewAccessible parent =
+          ViewAXPlatformNodeDelegate::GetParent())
+    return parent;
+
+  Widget* parent_widget =
+      GetWidgetOfParentWindowIncludingTransient(view()->GetWidget());
+  if (parent_widget)
+    return parent_widget->GetRootView()->GetNativeViewAccessible();
+
+  return AuraLinuxApplication::GetInstance()->GetNativeViewAccessible();
 }
 
 void ViewAXPlatformNodeDelegateAuraLinux::OnViewHierarchyChanged(

ui/views/accessibility/view_ax_platform_node_delegate_auralinux_unittest.cc

@@ -97,5 +97,49 @@ TEST_F(ViewAXPlatformNodeDelegateAuraLinuxTest, TextfieldAccessibility) {
   text_insert_events.clear();
 }
 
+TEST_F(ViewAXPlatformNodeDelegateAuraLinuxTest, AuraChildWidgets) {
+  // Create the parent widget.
+  Widget widget;
+  Widget::InitParams init_params =
+      CreateParams(Widget::InitParams::TYPE_WINDOW);
+  init_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  init_params.bounds = gfx::Rect(0, 0, 400, 200);
+  widget.Init(std::move(init_params));
+  widget.Show();
+
+  // Initially it has 1 child.
+  AtkObject* root_view_accessible =
+      widget.GetRootView()->GetNativeViewAccessible();
+  ASSERT_EQ(1, atk_object_get_n_accessible_children(root_view_accessible));
+
+  // Create the child widget, one of two ways (see below).
+  Widget child_widget;
+  Widget::InitParams child_init_params =
+      CreateParams(Widget::InitParams::TYPE_BUBBLE);
+  child_init_params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  child_init_params.parent = widget.GetNativeView();
+  child_init_params.bounds = gfx::Rect(30, 40, 100, 50);
+  child_init_params.child = false;
+  child_widget.Init(std::move(child_init_params));
+  child_widget.Show();
+
+  // Now the AtkObject for the parent widget should have 2 children.
+  ASSERT_EQ(2, atk_object_get_n_accessible_children(root_view_accessible));
+
+  // Make sure that querying the parent of the child gets us back to
+  // the original parent.
+  AtkObject* child_widget_accessible =
+      child_widget.GetRootView()->GetNativeViewAccessible();
+  ASSERT_EQ(atk_object_get_parent(child_widget_accessible),
+            root_view_accessible);
+
+  // Make sure that querying the second child of the parent is the child widget
+  // accessible as well.
+  AtkObject* second_child =
+      atk_object_ref_accessible_child(root_view_accessible, 1);
+  ASSERT_EQ(second_child, child_widget_accessible);
+  g_object_unref(second_child);
+}
+
 }  // namespace test
 }  // namespace views

ui/views/accessibility/view_ax_platform_node_delegate_win.cc

@@ -25,30 +25,14 @@
 #include "ui/base/win/accessibility_misc_utils.h"
 #include "ui/base/win/atl_module.h"
 #include "ui/display/win/screen_win.h"
+#include "ui/views/accessibility/views_utilities_aura.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/win/hwnd_util.h"
-#include "ui/wm/core/window_util.h"
 
 namespace views {
 
-namespace {
-
-// Return the parent of |window|, first checking to see if it has a
-// transient parent. This allows us to walk up the aura::Window
-// hierarchy when it spans multiple window tree hosts, each with
-// their own HWND.
-aura::Window* GetWindowParentIncludingTransient(aura::Window* window) {
-  aura::Window* transient_parent = wm::GetTransientParent(window);
-  if (transient_parent)
-    return transient_parent;
-
-  return window->parent();
-}
-
-}  // namespace
-
 // static
 std::unique_ptr<ViewAccessibility> ViewAccessibility::Create(View* view) {
   return std::make_unique<ViewAXPlatformNodeDelegateWin>(view);

ui/views/accessibility/views_utilities_aura.cc

+// Copyright 2019 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/views/accessibility/views_utilities_aura.h"
+
+#include "ui/aura/window.h"
+#include "ui/views/widget/widget.h"
+#include "ui/wm/core/window_util.h"
+
+namespace views {
+
+aura::Window* GetWindowParentIncludingTransient(aura::Window* window) {
+  aura::Window* transient_parent = wm::GetTransientParent(window);
+  if (transient_parent)
+    return transient_parent;
+
+  return window->parent();
+}
+
+}  // namespace views

ui/views/accessibility/views_utilities_aura.h

+// Copyright 2019 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_VIEWS_ACCESSIBILITY_VIEWS_UTILITIES_AURA_H_
+#define UI_VIEWS_ACCESSIBILITY_VIEWS_UTILITIES_AURA_H_
+
+namespace aura {
+class Window;
+}
+
+namespace views {
+
+// Return the parent of |window|, first checking to see if it has a
+// transient parent. This allows us to walk up the aura::Window
+// hierarchy when it spans multiple window tree hosts, each with
+// their own native window.
+aura::Window* GetWindowParentIncludingTransient(aura::Window* window);
+
+}  // namespace views
+
+#endif  // UI_VIEWS_ACCESSIBILITY_VIEWS_UTILITIES_AURA_H_