arc-a11y: prevent crash with a wrong source node id

Previously we had an assumption that the event source from Android is
serialized in event node data. But it can be incorrect in some rare
cases.
This change prevents Chrome crash on such a case.

AX-Relnotes: n/a.
Bug: b:159861631
Test: AXTreeSourceArcTest
Change-Id: I22308ba29e23ff1de9271661443ceb7f67e79275
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2265895
Commit-Queue: Hiroki Sato <hirokisato@chromium.org>
Reviewed-by: Sara Kato <sarakato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782784}

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

@@ -50,9 +50,7 @@ ax::mojom::Event ToAXEvent(
     case mojom::AccessibilityEventType::VIEW_TEXT_SELECTION_CHANGED:
       return ax::mojom::Event::kTextSelectionChanged;
     case mojom::AccessibilityEventType::WINDOW_STATE_CHANGED: {
-      if (arc_content_change_types.has_value()) {
-        DCHECK(source_node);  // Should be not null because it's dropped in
-                              // Android side.
+      if (source_node && arc_content_change_types.has_value()) {
         const base::Optional<ax::mojom::Event> event_or_null =
             FromContentChangeTypesToAXEvent(arc_content_change_types.value(),
                                             *source_node);
@@ -68,9 +66,7 @@ ax::mojom::Event ToAXEvent(
     case mojom::AccessibilityEventType::NOTIFICATION_STATE_CHANGED:
       return ax::mojom::Event::kLayoutComplete;
     case mojom::AccessibilityEventType::WINDOW_CONTENT_CHANGED:
-      if (arc_content_change_types.has_value()) {
-        DCHECK(source_node);  // Should be not null because it's dropped in
-                              // Android side.
+      if (source_node && arc_content_change_types.has_value()) {
         const base::Optional<ax::mojom::Event> event_or_null =
             FromContentChangeTypesToAXEvent(arc_content_change_types.value(),
                                             *source_node);

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

@@ -124,7 +124,8 @@ void AXTreeSourceArc::NotifyAccessibilityEvent(AXEventData* event_data) {
       event_data->event_text) {
     AccessibilityInfoDataWrapper* source_node =
         GetFromId(event_data->source_id);
-    UpdateAXNameCache(source_node, *event_data->event_text);
+    if (IsValid(source_node))
+      UpdateAXNameCache(source_node, *event_data->event_text);
   }
 
   ApplyCachedProperties();
@@ -371,12 +372,11 @@ AXTreeSourceArc::GetSelectedNodeInfoFromAdapterView(
 
 bool AXTreeSourceArc::UpdateAndroidFocusedId(const AXEventData& event_data) {
   if (event_data.event_type == AXEventType::VIEW_FOCUSED) {
-    AccessibilityInfoDataWrapper* focused_node =
-        GetFromId(event_data.source_id);
-    if (focused_node && focused_node->IsVisibleToUser()) {
+    AccessibilityInfoDataWrapper* source_node = GetFromId(event_data.source_id);
+    if (source_node && source_node->IsVisibleToUser()) {
       // Sometimes Android sets focus on unfocusable node, e.g. ListView.
       AccessibilityInfoDataWrapper* adjusted_node =
-          FindFirstFocusableNode(focused_node);
+          FindFirstFocusableNode(source_node);
       android_focused_id_ = IsValid(adjusted_node) ? adjusted_node->GetId()
                                                    : event_data.source_id;
     }
@@ -385,8 +385,9 @@ bool AXTreeSourceArc::UpdateAndroidFocusedId(const AXEventData& event_data) {
     // 1. Changing a value in ProgressBar or TimePicker in ARC P.
     // 2. Selecting an item in the context of an AdapterView.
     AccessibilityInfoDataWrapper* source_node = GetFromId(event_data.source_id);
-    DCHECK(source_node);
-    DCHECK(source_node->IsNode());
+    if (!source_node || !source_node->IsNode())
+      return false;
+
     AXNodeInfoData* node_info = source_node->GetNode();
     DCHECK(node_info);
 
@@ -436,16 +437,16 @@ bool AXTreeSourceArc::UpdateAndroidFocusedId(const AXEventData& event_data) {
 }
 
 void AXTreeSourceArc::UpdateAXNameCache(
-    AccessibilityInfoDataWrapper* focused_node,
+    AccessibilityInfoDataWrapper* source_node,
     const std::vector<std::string>& event_text) {
-  if (IsDrawerLayout(focused_node->GetNode())) {
+  if (IsDrawerLayout(source_node->GetNode())) {
     // When drawer menu opened, make the menu title announced.
     // When focus is changed, ChromeVox computes the diff in ancestry between
     // the previously focused and new focused node.
     // As the DrawerLayout is LCA of them, set the new title to be the first
     // visible child node (which is usually drawer menu).
     std::vector<AccessibilityInfoDataWrapper*> children;
-    focused_node->GetChildren(&children);
+    source_node->GetChildren(&children);
     for (auto* child : children) {
       if (child->IsNode() && child->IsVisibleToUser() &&
           GetBooleanProperty(child->GetNode(), AXBooleanProperty::IMPORTANCE)) {

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

@@ -123,7 +123,7 @@ class AXTreeSourceArc : public ui::AXTreeSource<AccessibilityInfoDataWrapper*,
   // automation.
   bool UpdateAndroidFocusedId(const mojom::AccessibilityEventData& event_data);
 
-  void UpdateAXNameCache(AccessibilityInfoDataWrapper* focused_node,
+  void UpdateAXNameCache(AccessibilityInfoDataWrapper* source_node,
                          const std::vector<std::string>& event_text);
 
   void ApplyCachedProperties();

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

@@ -1178,4 +1178,37 @@ TEST_F(AXTreeSourceArcTest, StateDescriptionChangedEvent) {
   // TODO(sahok): add test when source_node is not a range widget.
 }
 
+TEST_F(AXTreeSourceArcTest, EventWithWrongSourceId) {
+  auto event = AXEventData::New();
+  event->source_id = 99999;  // This doesn't exist in serialized nodes.
+  event->task_id = 1;
+
+  event->window_data = std::vector<mojom::AccessibilityWindowInfoDataPtr>();
+  event->window_data->push_back(AXWindowInfoData::New());
+  AXWindowInfoData* root_window = event->window_data->back().get();
+  root_window->window_id = 100;
+  root_window->root_node_id = 10;
+
+  event->node_data.push_back(AXNodeInfoData::New());
+  AXNodeInfoData* node = event->node_data.back().get();
+  node->id = 10;
+
+  // This test only verifies that wrong source id won't make Chrome crash.
+
+  event->event_type = AXEventType::VIEW_FOCUSED;
+  CallNotifyAccessibilityEvent(event.get());
+
+  event->event_type = AXEventType::VIEW_SELECTED;
+  CallNotifyAccessibilityEvent(event.get());
+
+  event->event_type = AXEventType::WINDOW_STATE_CHANGED;
+  event->event_text = std::vector<std::string>({"test text."});
+  SetProperty(event.get(), AXEventIntListProperty::CONTENT_CHANGE_TYPES,
+              {static_cast<int>(mojom::ContentChangeType::STATE_DESCRIPTION)});
+  CallNotifyAccessibilityEvent(event.get());
+
+  event->event_type = AXEventType::WINDOW_CONTENT_CHANGED;
+  CallNotifyAccessibilityEvent(event.get());
+}
+
 }  // namespace arc