Various cleanups and improvements to Arc++ accessibility

- use base::Optional in all internal AXTreeSourceArc node id usages. This makes it clear where we are setting values and not, forcing the author to think about the id. This also catches various places where optional ids were hard-coded. This change is independent of the broader switch to use kInvalidAXNodeId as an invalid id (which still works), but is not a concern in code internal to AXTreeSourceArc.
- refactor common code from AccessibilityWindowInfoDataWrapper and AccessibilityNodeInfoDataWrapper into base class. I've also renamed it to AccessibilityInfoDataWrapper to align naming.
- align the ordering of function declarations (in .h) with their order in .cc. Also, declare non-overiddenn methods first in each visibility block in .h.
- remove some obsolete forward declares and functions (e.g. Focus()). Unsure why compiler did not catch this.
- it is fine to have some AXTreeSource methods as public and some as private, remove TODO and group methods together.

Bug: none
Test: unit_tests --gtest_filter=AXTreeSourceArc*.*
Change-Id: I39f2ae67a213df450327bc70ef540c0aa6f0510e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1860524
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: Sara Kato <sarakato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707481}

chrome/browser/chromeos/BUILD.gn

@@ -438,13 +438,14 @@ source_set("chromeos") {
     "apps/intent_helper/chromeos_apps_navigation_throttle.h",
     "apps/intent_helper/common_apps_navigation_throttle.cc",
     "apps/intent_helper/common_apps_navigation_throttle.h",
+    "arc/accessibility/accessibility_info_data_wrapper.cc",
+    "arc/accessibility/accessibility_info_data_wrapper.h",
     "arc/accessibility/accessibility_node_info_data_wrapper.cc",
     "arc/accessibility/accessibility_node_info_data_wrapper.h",
     "arc/accessibility/accessibility_window_info_data_wrapper.cc",
     "arc/accessibility/accessibility_window_info_data_wrapper.h",
     "arc/accessibility/arc_accessibility_helper_bridge.cc",
     "arc/accessibility/arc_accessibility_helper_bridge.h",
-    "arc/accessibility/arc_accessibility_info_data.h",
     "arc/accessibility/arc_accessibility_util.cc",
     "arc/accessibility/arc_accessibility_util.h",
     "arc/accessibility/ax_tree_source_arc.cc",

chrome/browser/chromeos/arc/accessibility/accessibility_info_data_wrapper.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 "chrome/browser/chromeos/arc/accessibility/accessibility_info_data_wrapper.h"
+
+#include "chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.h"
+#include "components/exo/wm_helper.h"
+
+namespace arc {
+
+AccessibilityInfoDataWrapper::AccessibilityInfoDataWrapper(
+    AXTreeSourceArc* tree_source)
+    : tree_source_(tree_source) {}
+
+void AccessibilityInfoDataWrapper::Serialize(ui::AXNodeData* out_data) const {
+  out_data->id = GetId();
+  PopulateAXRole(out_data);
+
+  exo::WMHelper* wm_helper =
+      exo::WMHelper::HasInstance() ? exo::WMHelper::GetInstance() : nullptr;
+
+  if (tree_source_->GetRoot() && wm_helper) {
+    // This is the computed bounds which relies upon the existence of an
+    // associated focused window and a root node.
+    aura::Window* active_window = (tree_source_->is_notification() ||
+                                   tree_source_->is_input_method_window())
+                                      ? nullptr
+                                      : wm_helper->GetActiveWindow();
+    const gfx::Rect& local_bounds = tree_source_->GetBounds(
+        tree_source_->GetFromId(GetId()), active_window);
+    out_data->relative_bounds.bounds.SetRect(local_bounds.x(), local_bounds.y(),
+                                             local_bounds.width(),
+                                             local_bounds.height());
+  } else {
+    // We cannot compute global bounds, so use the raw bounds.
+    const auto& bounds = GetBounds();
+    out_data->relative_bounds.bounds.SetRect(bounds.x(), bounds.y(),
+                                             bounds.width(), bounds.height());
+  }
+
+  // TODO(katie): Try using offset_container_id to make bounds calculations
+  // more efficient. If this is the child of the root, set the
+  // offset_container_id to be the root. Otherwise, set it to the first node
+  // child of the root.
+}
+
+}  // namespace arc

chrome/browser/chromeos/arc/accessibility/arc_accessibility_info_data.h → chrome/browser/chromeos/arc/accessibility/accessibility_info_data_wrapper.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_CHROMEOS_ARC_ACCESSIBILITY_ARC_ACCESSIBILITY_INFO_DATA_H_
-#define CHROME_BROWSER_CHROMEOS_ARC_ACCESSIBILITY_ARC_ACCESSIBILITY_INFO_DATA_H_
+#ifndef CHROME_BROWSER_CHROMEOS_ARC_ACCESSIBILITY_ACCESSIBILITY_INFO_DATA_WRAPPER_H_
+#define CHROME_BROWSER_CHROMEOS_ARC_ACCESSIBILITY_ACCESSIBILITY_INFO_DATA_WRAPPER_H_
 
 #include "components/arc/mojom/accessibility_helper.mojom.h"
 
@@ -12,15 +12,17 @@ struct AXNodeData;
 }  // namespace ui
 
 namespace arc {
+class AXTreeSourceArc;
 
-// ArcAccessibilityInfoData represents a single ARC++ node or window. This class
-// can be used by AXTreeSourceArc to encapsulate ARC-side information which maps
-// to a single AXNodeData.
-class ArcAccessibilityInfoData {
+// AccessibilityInfoDataWrapper represents a single ARC++ node or window. This
+// class can be used by AXTreeSourceArc to encapsulate ARC-side information
+// which maps to a single AXNodeData.
+class AccessibilityInfoDataWrapper {
  public:
-  virtual ~ArcAccessibilityInfoData() = default;
+  explicit AccessibilityInfoDataWrapper(AXTreeSourceArc* tree_source);
+  virtual ~AccessibilityInfoDataWrapper() = default;
 
-  // True if this ArcAccessibilityInfoData represents an Android node, false
+  // True if this AccessibilityInfoDataWrapper represents an Android node, false
   // if it represents an Android window.
   virtual bool IsNode() const = 0;
 
@@ -38,9 +40,12 @@ class ArcAccessibilityInfoData {
   virtual void PopulateAXState(ui::AXNodeData* out_data) const = 0;
   virtual void Serialize(ui::AXNodeData* out_data) const = 0;
   virtual void GetChildren(
-      std::vector<ArcAccessibilityInfoData*>* children) const = 0;
+      std::vector<AccessibilityInfoDataWrapper*>* children) const = 0;
+
+ protected:
+  AXTreeSourceArc* tree_source_;
 };
 
 }  // namespace arc
 
-#endif  // CHROME_BROWSER_CHROMEOS_ARC_ACCESSIBILITY_ARC_ACCESSIBILITY_INFO_DATA_H_
+#endif  // CHROME_BROWSER_CHROMEOS_ARC_ACCESSIBILITY_ACCESSIBILITY_INFO_DATA_WRAPPER_H_

chrome/browser/chromeos/arc/accessibility/accessibility_node_info_data_wrapper.cc

@@ -7,7 +7,6 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.h"
-#include "components/exo/wm_helper.h"
 #include "ui/accessibility/ax_enums.mojom.h"
 #include "ui/accessibility/ax_node.h"
 #include "ui/accessibility/platform/ax_android_constants.h"
@@ -30,7 +29,7 @@ using AXStringProperty = mojom::AccessibilityStringProperty;
 AccessibilityNodeInfoDataWrapper::AccessibilityNodeInfoDataWrapper(
     AXTreeSourceArc* tree_source,
     AXNodeInfoData* node)
-    : tree_source_(tree_source), node_ptr_(node) {}
+    : AccessibilityInfoDataWrapper(tree_source), node_ptr_(node) {}
 
 bool AccessibilityNodeInfoDataWrapper::IsNode() const {
   return true;
@@ -143,8 +142,8 @@ void AccessibilityNodeInfoDataWrapper::PopulateAXRole(
     // need additional information contained only in the CollectionInfo. The
     // CollectionInfo should be an ancestor of this node.
     AXCollectionInfoData* collection_info = nullptr;
-    for (const ArcAccessibilityInfoData* container =
-             static_cast<const ArcAccessibilityInfoData*>(this);
+    for (const AccessibilityInfoDataWrapper* container =
+             static_cast<const AccessibilityInfoDataWrapper*>(this);
          container;) {
       if (!container || !container->IsNode())
         break;
@@ -272,7 +271,7 @@ void AccessibilityNodeInfoDataWrapper::PopulateAXState(
 
 void AccessibilityNodeInfoDataWrapper::Serialize(
     ui::AXNodeData* out_data) const {
-  PopulateAXRole(out_data);
+  AccessibilityInfoDataWrapper::Serialize(out_data);
 
   // String properties.
   int labelled_by = -1;
@@ -287,7 +286,7 @@ void AccessibilityNodeInfoDataWrapper::Serialize(
   GetProperty(AXStringProperty::TEXT, &text);
 
   if (GetProperty(AXIntProperty::LABELED_BY, &labelled_by)) {
-    ArcAccessibilityInfoData* labelled_by_node =
+    AccessibilityInfoDataWrapper* labelled_by_node =
         tree_source_->GetFromId(labelled_by);
     if (labelled_by_node && labelled_by_node->IsNode()) {
       ui::AXNodeData labelled_by_data;
@@ -401,29 +400,6 @@ void AccessibilityNodeInfoDataWrapper::Serialize(
                                 range_info->max);
   }
 
-  exo::WMHelper* wm_helper =
-      exo::WMHelper::HasInstance() ? exo::WMHelper::GetInstance() : nullptr;
-
-  // To get bounds of a node which can be passed to AXNodeData.location,
-  // - Root node must exist.
-  // - Window where this tree is attached to need to be focused.
-  if (tree_source_->GetRoot()->GetId() != ui::AXNode::kInvalidAXID &&
-      wm_helper) {
-    aura::Window* active_window = (tree_source_->is_notification() ||
-                                   tree_source_->is_input_method_window())
-                                      ? nullptr
-                                      : wm_helper->GetActiveWindow();
-    const gfx::Rect& local_bounds = tree_source_->GetBounds(
-        tree_source_->GetFromId(GetId()), active_window);
-    out_data->relative_bounds.bounds.SetRect(local_bounds.x(), local_bounds.y(),
-                                             local_bounds.width(),
-                                             local_bounds.height());
-  }
-  // TODO(katie): Try using offset_container_id to make bounds calculations
-  // more efficient. If this is the child of the root, set the
-  // offset_container_id to be the root. Otherwise, set it to the first node
-  // child of the root.
-
   // Integer properties.
   int32_t val;
   if (GetProperty(AXIntProperty::TEXT_SELECTION_START, &val) && val >= 0)
@@ -470,7 +446,7 @@ void AccessibilityNodeInfoDataWrapper::Serialize(
 }
 
 void AccessibilityNodeInfoDataWrapper::GetChildren(
-    std::vector<ArcAccessibilityInfoData*>* children) const {
+    std::vector<AccessibilityInfoDataWrapper*>* children) const {
   if (!node_ptr_->int_list_properties)
     return;
   auto it =
@@ -598,9 +574,9 @@ void AccessibilityNodeInfoDataWrapper::ComputeNameFromContents(
   }
 
   // Otherwise, continue looking for a name in this subtree.
-  std::vector<ArcAccessibilityInfoData*> children;
+  std::vector<AccessibilityInfoDataWrapper*> children;
   data->GetChildren(&children);
-  for (ArcAccessibilityInfoData* child : children) {
+  for (AccessibilityInfoDataWrapper* child : children) {
     ComputeNameFromContents(
         static_cast<AccessibilityNodeInfoDataWrapper*>(child), names);
   }

chrome/browser/chromeos/arc/accessibility/accessibility_node_info_data_wrapper.h

@@ -5,7 +5,7 @@
 #ifndef CHROME_BROWSER_CHROMEOS_ARC_ACCESSIBILITY_ACCESSIBILITY_NODE_INFO_DATA_WRAPPER_H_
 #define CHROME_BROWSER_CHROMEOS_ARC_ACCESSIBILITY_ACCESSIBILITY_NODE_INFO_DATA_WRAPPER_H_
 
-#include "chrome/browser/chromeos/arc/accessibility/arc_accessibility_info_data.h"
+#include "chrome/browser/chromeos/arc/accessibility/accessibility_info_data_wrapper.h"
 #include "ui/accessibility/ax_enum_util.h"
 #include "ui/accessibility/ax_node_data.h"
 
@@ -14,13 +14,12 @@ namespace arc {
 class AXTreeSourceArc;
 
 // Wrapper class for an AccessibilityWindowInfoData.
-class AccessibilityNodeInfoDataWrapper : public ArcAccessibilityInfoData {
+class AccessibilityNodeInfoDataWrapper : public AccessibilityInfoDataWrapper {
  public:
-  explicit AccessibilityNodeInfoDataWrapper(
-      AXTreeSourceArc* tree_source,
+  AccessibilityNodeInfoDataWrapper(AXTreeSourceArc* tree_source,
                                    mojom::AccessibilityNodeInfoData* node);
 
-  // ArcAccessibilityInfoData overrides.
+  // AccessibilityInfoDataWrapper overrides.
   bool IsNode() const override;
   mojom::AccessibilityNodeInfoData* GetNode() const override;
   mojom::AccessibilityWindowInfoData* GetWindow() const override;
@@ -33,7 +32,7 @@ class AccessibilityNodeInfoDataWrapper : public ArcAccessibilityInfoData {
   void PopulateAXState(ui::AXNodeData* out_data) const override;
   void Serialize(ui::AXNodeData* out_data) const override;
   void GetChildren(
-      std::vector<ArcAccessibilityInfoData*>* children) const override;
+      std::vector<AccessibilityInfoDataWrapper*>* children) const override;
 
   mojom::AccessibilityNodeInfoData* node() { return node_ptr_; }
 
@@ -55,7 +54,6 @@ class AccessibilityNodeInfoDataWrapper : public ArcAccessibilityInfoData {
   void ComputeNameFromContents(const AccessibilityNodeInfoDataWrapper* data,
                                std::vector<std::string>* names) const;
 
-  AXTreeSourceArc* tree_source_ = nullptr;
   mojom::AccessibilityNodeInfoData* node_ptr_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(AccessibilityNodeInfoDataWrapper);

chrome/browser/chromeos/arc/accessibility/accessibility_window_info_data_wrapper.cc

@@ -15,7 +15,7 @@ namespace arc {
 AccessibilityWindowInfoDataWrapper::AccessibilityWindowInfoDataWrapper(
     AXTreeSourceArc* tree_source,
     mojom::AccessibilityWindowInfoData* window)
-    : tree_source_(tree_source), window_ptr_(window) {}
+    : AccessibilityInfoDataWrapper(tree_source), window_ptr_(window) {}
 
 bool AccessibilityWindowInfoDataWrapper::IsNode() const {
   return false;
@@ -93,7 +93,7 @@ void AccessibilityWindowInfoDataWrapper::Serialize(
   if (!tree_source_->GetRoot())
     return;
 
-  PopulateAXRole(out_data);
+  AccessibilityInfoDataWrapper::Serialize(out_data);
 
   // String properties.
   std::string title;
@@ -102,22 +102,6 @@ void AccessibilityWindowInfoDataWrapper::Serialize(
     out_data->SetNameFrom(ax::mojom::NameFrom::kTitle);
   }
 
-  // Bounds.
-  exo::WMHelper* wm_helper =
-      exo::WMHelper::HasInstance() ? exo::WMHelper::GetInstance() : nullptr;
-  if (tree_source_->GetRoot()->GetId() != ui::AXNode::kInvalidAXID &&
-      wm_helper) {
-    aura::Window* active_window = (tree_source_->is_notification() ||
-                                   tree_source_->is_input_method_window())
-                                      ? nullptr
-                                      : wm_helper->GetActiveWindow();
-    const gfx::Rect& local_bounds = tree_source_->GetBounds(
-        tree_source_->GetFromId(GetId()), active_window);
-    out_data->relative_bounds.bounds.SetRect(local_bounds.x(), local_bounds.y(),
-                                             local_bounds.width(),
-                                             local_bounds.height());
-  }
-
   // Not all properties are currently used in Chrome Accessibility.
 
   // Boolean properties:
@@ -132,7 +116,7 @@ void AccessibilityWindowInfoDataWrapper::Serialize(
 }
 
 void AccessibilityWindowInfoDataWrapper::GetChildren(
-    std::vector<ArcAccessibilityInfoData*>* children) const {
+    std::vector<AccessibilityInfoDataWrapper*>* children) const {
   // Populate the children vector by combining the child window IDs with the
   // root node ID.
   if (window_ptr_->int_list_properties) {

chrome/browser/chromeos/arc/accessibility/accessibility_window_info_data_wrapper.h

@@ -7,7 +7,7 @@
 
 #include <vector>
 
-#include "chrome/browser/chromeos/arc/accessibility/arc_accessibility_info_data.h"
+#include "chrome/browser/chromeos/arc/accessibility/accessibility_info_data_wrapper.h"
 #include "ui/accessibility/ax_node_data.h"
 
 namespace arc {
@@ -15,13 +15,13 @@ namespace arc {
 class AXTreeSourceArc;
 
 // Wrapper class for an AccessibilityWindowInfoData.
-class AccessibilityWindowInfoDataWrapper : public ArcAccessibilityInfoData {
+class AccessibilityWindowInfoDataWrapper : public AccessibilityInfoDataWrapper {
  public:
-  explicit AccessibilityWindowInfoDataWrapper(
+  AccessibilityWindowInfoDataWrapper(
       AXTreeSourceArc* tree_source,
       mojom::AccessibilityWindowInfoData* window);
 
-  // ArcAccessibilityInfoData overrides.
+  // AccessibilityInfoDataWrapper overrides.
   bool IsNode() const override;
   mojom::AccessibilityNodeInfoData* GetNode() const override;
   mojom::AccessibilityWindowInfoData* GetWindow() const override;
@@ -34,7 +34,7 @@ class AccessibilityWindowInfoDataWrapper : public ArcAccessibilityInfoData {
   void PopulateAXState(ui::AXNodeData* out_data) const override;
   void Serialize(ui::AXNodeData* out_data) const override;
   void GetChildren(
-      std::vector<ArcAccessibilityInfoData*>* children) const override;
+      std::vector<AccessibilityInfoDataWrapper*>* children) const override;
 
  private:
   bool GetProperty(mojom::AccessibilityWindowBooleanProperty prop) const;
@@ -46,7 +46,6 @@ class AccessibilityWindowInfoDataWrapper : public ArcAccessibilityInfoData {
   bool GetProperty(mojom::AccessibilityWindowIntListProperty prop,
                    std::vector<int32_t>* out_value) const;
 
-  AXTreeSourceArc* tree_source_ = nullptr;
   mojom::AccessibilityWindowInfoData* window_ptr_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(AccessibilityWindowInfoDataWrapper);

chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc

@@ -578,7 +578,7 @@ void ArcAccessibilityHelperBridge::OnAction(
   AXTreeSourceArc* tree_source = GetFromTreeId(data.target_tree_id);
   if (!tree_source)
     return;
-  action_data->window_id = tree_source->window_id();
+  action_data->window_id = tree_source->GetWindowId();
 
   switch (data.action) {
     case ax::mojom::Action::kDoDefault:

chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc.h

@@ -9,7 +9,7 @@
 #include <memory>
 #include <vector>
 
-#include "chrome/browser/chromeos/arc/accessibility/arc_accessibility_info_data.h"
+#include "chrome/browser/chromeos/arc/accessibility/accessibility_info_data_wrapper.h"
 #include "components/arc/mojom/accessibility_helper.mojom.h"
 #include "extensions/browser/api/automation_internal/automation_event_router.h"
 #include "ui/accessibility/ax_action_handler.h"
@@ -27,11 +27,12 @@ class Window;
 namespace arc {
 class AXTreeSourceArcTest;
 
-using AXTreeArcSerializer = ui::
-    AXTreeSerializer<ArcAccessibilityInfoData*, ui::AXNodeData, ui::AXTreeData>;
+using AXTreeArcSerializer = ui::AXTreeSerializer<AccessibilityInfoDataWrapper*,
+                                                 ui::AXNodeData,
+                                                 ui::AXTreeData>;
 
 // This class represents the accessibility tree from the focused ARC window.
-class AXTreeSourceArc : public ui::AXTreeSource<ArcAccessibilityInfoData*,
+class AXTreeSourceArc : public ui::AXTreeSource<AccessibilityInfoDataWrapper*,
                                                 ui::AXNodeData,
                                                 ui::AXTreeData>,
                         public ui::AXActionHandler {
@@ -44,9 +45,6 @@ class AXTreeSourceArc : public ui::AXTreeSource<ArcAccessibilityInfoData*,
   explicit AXTreeSourceArc(Delegate* delegate);
   ~AXTreeSourceArc() override;
 
-  // AXTreeSource overrides.
-  bool GetTreeData(ui::AXTreeData* data) const override;
-
   // Notify automation of an accessibility event.
   void NotifyAccessibilityEvent(mojom::AccessibilityEventData* event_data);
 
@@ -57,21 +55,6 @@ class AXTreeSourceArc : public ui::AXTreeSource<ArcAccessibilityInfoData*,
   void NotifyGetTextLocationDataResult(const ui::AXActionData& data,
                                        const base::Optional<gfx::Rect>& rect);
 
-  // Attaches tree to an aura window and gives it system focus.
-  void Focus(aura::Window* window);
-
-  // Gets the window id of this tree.
-  int32_t window_id() const { return window_id_; }
-
-  // AXTreeSource overrides used by ArcAccessibilityInfoData subclasses.
-  // TODO(katie): should these be "friended" or "protected" instead?
-  ArcAccessibilityInfoData* GetRoot() const override;
-  ArcAccessibilityInfoData* GetFromId(int32_t id) const override;
-  void SerializeNode(ArcAccessibilityInfoData* info_data,
-                     ui::AXNodeData* out_data) const override;
-  ArcAccessibilityInfoData* GetParent(
-      ArcAccessibilityInfoData* info_data) const override;
-
   // Returns bounds of a node which can be passed to AXNodeData.location. Bounds
   // are returned in the following coordinates depending on whether it's root or
   // not.
@@ -79,7 +62,7 @@ class AXTreeSourceArc : public ui::AXTreeSource<ArcAccessibilityInfoData*,
   // - Non-root node is relative to the root node of this tree.
   //
   // focused_window is nullptr for notification.
-  const gfx::Rect GetBounds(ArcAccessibilityInfoData* info_data,
+  const gfx::Rect GetBounds(AccessibilityInfoDataWrapper* info_data,
                             aura::Window* focused_window) const;
 
   // Invalidates the tree serializer.
@@ -89,53 +72,65 @@ class AXTreeSourceArc : public ui::AXTreeSource<ArcAccessibilityInfoData*,
   // parent window).
   bool IsRootOfNodeTree(int32_t id) const;
 
+  // Gets the window id of this tree.
+  int32_t GetWindowId() const;
+
+  // AXTreeSource:
+  bool GetTreeData(ui::AXTreeData* data) const override;
+  AccessibilityInfoDataWrapper* GetRoot() const override;
+  AccessibilityInfoDataWrapper* GetFromId(int32_t id) const override;
+  AccessibilityInfoDataWrapper* GetParent(
+      AccessibilityInfoDataWrapper* info_data) const override;
+  void SerializeNode(AccessibilityInfoDataWrapper* info_data,
+                     ui::AXNodeData* out_data) const override;
+
   bool is_notification() { return is_notification_; }
 
   bool is_input_method_window() { return is_input_method_window_; }
 
  private:
+  friend class arc::AXTreeSourceArcTest;
+
   // virtual for testing.
   virtual extensions::AutomationEventRouterInterface* GetAutomationEventRouter()
       const;
 
-  friend class arc::AXTreeSourceArcTest;
-  class FocusStealer;
-
-  // AXTreeSource overrides.
-  int32_t GetId(ArcAccessibilityInfoData* info_data) const override;
-  void GetChildren(
-      ArcAccessibilityInfoData* info_data,
-      std::vector<ArcAccessibilityInfoData*>* out_children) const override;
-  bool IsIgnored(ArcAccessibilityInfoData* info_data) const override;
-  bool IsValid(ArcAccessibilityInfoData* info_data) const override;
-  bool IsEqual(ArcAccessibilityInfoData* info_data1,
-               ArcAccessibilityInfoData* info_data2) const override;
-  ArcAccessibilityInfoData* GetNull() const override;
-
   // Computes the smallest rect that encloses all of the descendants of
   // |info_data|.
-  gfx::Rect ComputeEnclosingBounds(ArcAccessibilityInfoData* info_data) const;
+  gfx::Rect ComputeEnclosingBounds(
+      AccessibilityInfoDataWrapper* info_data) const;
 
   // Helper to recursively compute bounds for |info_data|. Returns true if
   // non-empty bounds were encountered.
-  void ComputeEnclosingBoundsInternal(ArcAccessibilityInfoData* info_data,
+  void ComputeEnclosingBoundsInternal(AccessibilityInfoDataWrapper* info_data,
                                       gfx::Rect& computed_bounds) const;
 
-  // AXActionHandler overrides.
-  void PerformAction(const ui::AXActionData& data) override;
-
   // Resets tree state.
   void Reset();
 
-  // Maps an ArcAccessibilityInfoData ID to its tree data.
-  std::map<int32_t, std::unique_ptr<ArcAccessibilityInfoData>> tree_map_;
+  // AXTreeSource:
+  int32_t GetId(AccessibilityInfoDataWrapper* info_data) const override;
+  void GetChildren(
+      AccessibilityInfoDataWrapper* info_data,
+      std::vector<AccessibilityInfoDataWrapper*>* out_children) const override;
+  bool IsIgnored(AccessibilityInfoDataWrapper* info_data) const override;
+  bool IsValid(AccessibilityInfoDataWrapper* info_data) const override;
+  bool IsEqual(AccessibilityInfoDataWrapper* info_data1,
+               AccessibilityInfoDataWrapper* info_data2) const override;
+  AccessibilityInfoDataWrapper* GetNull() const override;
+
+  // AXActionHandler:
+  void PerformAction(const ui::AXActionData& data) override;
+
+  // Maps an AccessibilityInfoDataWrapper ID to its tree data.
+  std::map<int32_t, std::unique_ptr<AccessibilityInfoDataWrapper>> tree_map_;
 
-  // Maps an ArcAccessibilityInfoData ID to its parent.
+  // Maps an AccessibilityInfoDataWrapper ID to its parent.
   std::map<int32_t, int32_t> parent_map_;
   std::unique_ptr<AXTreeArcSerializer> current_tree_serializer_;
-  int32_t root_id_;
-  int32_t window_id_;
-  int32_t focused_id_;
+  base::Optional<int32_t> root_id_;
+  base::Optional<int32_t> window_id_;
+  base::Optional<int32_t> focused_id_;
   bool is_notification_;
   bool is_input_method_window_;
 
@@ -144,7 +139,7 @@ class AXTreeSourceArc : public ui::AXTreeSource<ArcAccessibilityInfoData*,
   const Delegate* const delegate_;
   std::string package_name_;
 
-  // Mapping from ArcAccessibilityInfoData ID to its cached computed bounds.
+  // Mapping from AccessibilityInfoDataWrapper ID to its cached computed bounds.
   // This simplifies bounds calculations.
   std::map<int32_t, gfx::Rect> cached_computed_bounds_;
 

chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc_unittest.cc

@@ -163,7 +163,7 @@ class AXTreeSourceArcTest : public testing::Test,
 
   void CallGetChildren(
       AXNodeInfoData* node,
-      std::vector<ArcAccessibilityInfoData*>* out_children) const {
+      std::vector<AccessibilityInfoDataWrapper*>* out_children) const {
     AccessibilityNodeInfoDataWrapper node_data(tree_source_.get(), node);
     tree_source_->GetChildren(&node_data, out_children);
   }
@@ -184,7 +184,7 @@ class AXTreeSourceArcTest : public testing::Test,
     tree_source_->SerializeNode(&window_data, out_data->get());
   }
 
-  ArcAccessibilityInfoData* CallGetFromId(int32_t id) const {
+  AccessibilityInfoDataWrapper* CallGetFromId(int32_t id) const {
     return tree_source_->GetFromId(id);
   }
 
@@ -244,6 +244,7 @@ TEST_F(AXTreeSourceArcTest, ReorderChildrenByLayout) {
   SetProperty(button1, AXStringProperty::CLASS_NAME, ui::kAXButtonClassname);
   SetProperty(button1, AXBooleanProperty::VISIBLE_TO_USER, true);
   SetProperty(button1, AXBooleanProperty::FOCUSABLE, true);
+  SetProperty(button1, AXBooleanProperty::IMPORTANCE, true);
 
   // Add another child button.
   event->node_data.push_back(AXNodeInfoData::New());
@@ -252,6 +253,7 @@ TEST_F(AXTreeSourceArcTest, ReorderChildrenByLayout) {
   SetProperty(button2, AXStringProperty::CLASS_NAME, ui::kAXButtonClassname);
   SetProperty(button2, AXBooleanProperty::VISIBLE_TO_USER, true);
   SetProperty(button2, AXBooleanProperty::FOCUSABLE, true);
+  SetProperty(button2, AXBooleanProperty::IMPORTANCE, true);
 
   // Non-overlapping, bottom to top.
   button1->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
@@ -259,7 +261,7 @@ TEST_F(AXTreeSourceArcTest, ReorderChildrenByLayout) {
 
   // Trigger an update which refreshes the computed bounds used for reordering.
   CallNotifyAccessibilityEvent(event.get());
-  std::vector<ArcAccessibilityInfoData*> top_to_bottom;
+  std::vector<AccessibilityInfoDataWrapper*> top_to_bottom;
   CallGetChildren(root, &top_to_bottom);
   ASSERT_EQ(2U, top_to_bottom.size());
   EXPECT_EQ(2, top_to_bottom[0]->GetId());
@@ -279,7 +281,7 @@ TEST_F(AXTreeSourceArcTest, ReorderChildrenByLayout) {
   button1->bounds_in_screen = gfx::Rect(101, 100, 99, 100);
   button2->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
   CallNotifyAccessibilityEvent(event.get());
-  std::vector<ArcAccessibilityInfoData*> left_to_right;
+  std::vector<AccessibilityInfoDataWrapper*> left_to_right;
   CallGetChildren(root, &left_to_right);
   ASSERT_EQ(2U, left_to_right.size());
   EXPECT_EQ(2, left_to_right[0]->GetId());
@@ -319,7 +321,7 @@ TEST_F(AXTreeSourceArcTest, ReorderChildrenByLayout) {
   button1->bounds_in_screen = gfx::Rect(100, 100, 100, 10);
   button2->bounds_in_screen = gfx::Rect(100, 100, 100, 100);
   CallNotifyAccessibilityEvent(event.get());
-  std::vector<ArcAccessibilityInfoData*> dimension;
+  std::vector<AccessibilityInfoDataWrapper*> dimension;
   CallGetChildren(event->node_data[0].get(), &dimension);
   ASSERT_EQ(2U, dimension.size());
   EXPECT_EQ(2, dimension[0]->GetId());
@@ -354,6 +356,15 @@ TEST_F(AXTreeSourceArcTest, ReorderChildrenByLayout) {
   EXPECT_EQ(2, dimension[1]->GetId());
 
   EXPECT_EQ(10, GetDispatchedEventCount(ax::mojom::Event::kFocus));
+
+  // Sanity check tree output.
+  ExpectTree(
+      "id=0 genericContainer INVISIBLE (0, 0)-(0, 0) restriction=disabled "
+      "modal=true child_ids=1,2\n"
+      "  id=1 button FOCUSABLE (100, 100)-(100, 100) restriction=disabled "
+      "class_name=android.widget.Button\n"
+      "  id=2 button FOCUSABLE (100, 100)-(10, 100) restriction=disabled "
+      "class_name=android.widget.Button\n");
 }
 
 TEST_F(AXTreeSourceArcTest, AccessibleNameComputationTextField) {
@@ -692,7 +703,7 @@ TEST_F(AXTreeSourceArcTest, MultipleNodeSubtrees) {
     CallNotifyAccessibilityEvent(event.get());
 
     // Check that only the middle tree was added, and that it is correct.
-    std::vector<ArcAccessibilityInfoData*> children;
+    std::vector<AccessibilityInfoDataWrapper*> children;
     CallGetChildren(event->node_data.at(tree_size).get(), &children);
     ASSERT_EQ(1U, children.size());
     EXPECT_EQ(5, children[0]->GetId());
@@ -766,7 +777,7 @@ TEST_F(AXTreeSourceArcTest, ComplexTreeStructure) {
   CallNotifyAccessibilityEvent(event.get());
 
   // Check that each node subtree tree was added, and that it is correct.
-  std::vector<ArcAccessibilityInfoData*> children;
+  std::vector<AccessibilityInfoDataWrapper*> children;
   for (int i = 0; i < num_trees; i++) {
     CallGetChildren(event->node_data.at(i * tree_size).get(), &children);
     ASSERT_EQ(1U, children.size());