Fix AXEventGenerator::Iterator::begin() point to non-empty entry

AXEventGenerator::Iterator iterates over the generated events in
std::map<AXNode*, std::set<EventParams>> tree_events_. Often there
could be entry with empty event sets (std::set<EventParams) in
|tree_events_|, and if these empty event sets happen to be the first
entry when we iterate over |tree_events_|, AXEventGenerator::Iterator
::operator++ would increment AXEventGenerator::Iterator::set_iter_
past its end, causing an exception.

This CL fixes this issue by making AXEventGenerator::Iterator
::begin() to always point to a non-empty std::set<EventParams> entry
in |tree_events_|.

AX-Relnotes: n/a

Bug: 1120281
Change-Id: I8fbdbbc92eb007d3ce01bfa5cb61730c9f38c115
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2399560
Commit-Queue: Victor Fei <vicfei@microsoft.com>
Reviewed-by: Ian Prest <iapres@microsoft.com>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810485}

ui/accessibility/ax_event_generator.cc

@@ -132,17 +132,16 @@ AXEventGenerator::Iterator& AXEventGenerator::Iterator::operator++() {
   if (map_iter_ == map_.end())
     return *this;
 
-  DCHECK(set_iter_ != map_iter_->second.end())
-      << "The set of events should not be empty";
+  DCHECK(set_iter_ != map_iter_->second.end());
   set_iter_++;
 
-  if (set_iter_ == map_iter_->second.end()) {
+  // |map_| may contain empty sets of events in its entries (i.e. |set_iter_| is
+  // at the iterator's end). In this case, we want to increment |map_iter_| to
+  // point to the next entry of |map_| that contains non-empty set of events.
+  while (map_iter_ != map_.end() && set_iter_ == map_iter_->second.end()) {
     map_iter_++;
-    if (map_iter_ != map_.end()) {
+    if (map_iter_ != map_.end())
       set_iter_ = map_iter_->second.begin();
-      DCHECK(set_iter_ != map_iter_->second.end())
-          << "The set of events should not be empty";
-    }
   }
 
   return *this;
@@ -175,6 +174,30 @@ void AXEventGenerator::ReleaseTree() {
   tree_ = nullptr;
 }
 
+AXEventGenerator::Iterator AXEventGenerator::begin() const {
+  auto map_iter = tree_events_.begin();
+  if (map_iter != tree_events_.end()) {
+    auto set_iter = map_iter->second.begin();
+
+    // |tree_events_| may contain empty sets of events in its first entry
+    // (i.e. |set_iter| is at the iterator's end). In this case, we want to
+    // increment |map_iter| to point to the next entry of |tree_events_| that
+    // contains non-empty set of events.
+    while (map_iter != tree_events_.end() &&
+           set_iter == map_iter->second.end()) {
+      map_iter++;
+      if (map_iter != tree_events_.end())
+        set_iter = map_iter->second.begin();
+    }
+  }
+
+  return AXEventGenerator::Iterator(tree_events_, map_iter);
+}
+
+AXEventGenerator::Iterator AXEventGenerator::end() const {
+  return AXEventGenerator::Iterator(tree_events_, tree_events_.end());
+}
+
 void AXEventGenerator::ClearEvents() {
   tree_events_.clear();
 }
@@ -632,6 +655,12 @@ void AXEventGenerator::OnAtomicUpdateFinished(
   PostprocessEvents();
 }
 
+void AXEventGenerator::AddEventsForTesting(
+    AXNode* node,
+    const std::set<EventParams>& events) {
+  tree_events_[node] = events;
+}
+
 void AXEventGenerator::FireLiveRegionEvents(AXNode* node) {
   AXNode* live_root = node;
   while (live_root && !live_root->data().HasStringAttribute(

ui/accessibility/ax_event_generator.h

@@ -180,10 +180,8 @@ class AX_EXPORT AXEventGenerator : public AXTreeObserver {
   // Null |tree_| without accessing it or destroying it.
   void ReleaseTree();
 
-  Iterator begin() const {
-    return Iterator(tree_events_, tree_events_.begin());
-  }
-  Iterator end() const { return Iterator(tree_events_, tree_events_.end()); }
+  Iterator begin() const;
+  Iterator end() const;
 
   // Clear any previously added events.
   void ClearEvents();
@@ -201,6 +199,8 @@ class AX_EXPORT AXEventGenerator : public AXTreeObserver {
     always_fire_load_complete_ = val;
   }
 
+  void AddEventsForTesting(AXNode* node, const std::set<EventParams>& events);
+
  protected:
   // AXTreeObserver overrides.
   void OnNodeDataChanged(AXTree* tree,

ui/accessibility/ax_event_generator_unittest.cc

@@ -40,6 +40,133 @@ MATCHER_P2(HasEventAtNode,
 
 }  // namespace
 
+TEST(AXEventGeneratorTest, IterateThroughEmptyEventSets) {
+  // The event map contains the following:
+  // node1, <>
+  // node2, <>
+  // node3, <IGNORED_CHANGED, SUBTREE_CREATED, NAME_CHANGED>
+  // node4, <>
+  // node5, <>
+  // node6, <>
+  // node7, <IGNORED_CHANGED>
+  // node8, <>
+  // node9, <>
+  // Verify AXEventGenerator can iterate through empty event sets, and returning
+  // the correct events.
+  AXTreeUpdate initial_state;
+  initial_state.root_id = 1;
+  initial_state.nodes.resize(9);
+  initial_state.nodes[0].id = 1;
+  initial_state.nodes[0].child_ids.push_back(2);
+
+  initial_state.nodes[1].id = 2;
+  initial_state.nodes[1].child_ids.push_back(3);
+
+  initial_state.nodes[2].id = 3;
+  initial_state.nodes[2].child_ids.push_back(4);
+
+  initial_state.nodes[3].id = 4;
+  initial_state.nodes[3].child_ids.push_back(5);
+
+  initial_state.nodes[4].id = 5;
+  initial_state.nodes[4].child_ids.push_back(6);
+
+  initial_state.nodes[5].id = 6;
+  initial_state.nodes[5].child_ids.push_back(7);
+
+  initial_state.nodes[6].id = 7;
+  initial_state.nodes[6].child_ids.push_back(8);
+
+  initial_state.nodes[7].id = 8;
+  initial_state.nodes[7].child_ids.push_back(9);
+
+  initial_state.nodes[8].id = 9;
+  initial_state.has_tree_data = true;
+
+  AXTree tree(initial_state);
+  AXEventGenerator event_generator(&tree);
+  AXNode* node1 = tree.root();
+  AXNode* node2 = tree.GetFromId(2);
+  AXNode* node3 = tree.GetFromId(3);
+  AXNode* node4 = tree.GetFromId(4);
+  AXNode* node5 = tree.GetFromId(5);
+  AXNode* node6 = tree.GetFromId(6);
+  AXNode* node7 = tree.GetFromId(7);
+  AXNode* node8 = tree.GetFromId(8);
+  AXNode* node9 = tree.GetFromId(9);
+
+  // Node1 contains no event.
+  std::set<AXEventGenerator::EventParams> node1_events;
+  // Node2 contains no event.
+  std::set<AXEventGenerator::EventParams> node2_events;
+  // Node3 contains IGNORED_CHANGED, SUBTREE_CREATED, NAME_CHANGED.
+  std::set<AXEventGenerator::EventParams> node3_events;
+  node3_events.emplace(AXEventGenerator::Event::IGNORED_CHANGED,
+                       ax::mojom::EventFrom::kNone, tree.event_intents());
+  node3_events.emplace(AXEventGenerator::Event::SUBTREE_CREATED,
+                       ax::mojom::EventFrom::kNone, tree.event_intents());
+  node3_events.emplace(AXEventGenerator::Event::NAME_CHANGED,
+                       ax::mojom::EventFrom::kNone, tree.event_intents());
+  // Node4 contains no event.
+  std::set<AXEventGenerator::EventParams> node4_events;
+  // Node5 contains no event.
+  std::set<AXEventGenerator::EventParams> node5_events;
+  // Node6 contains no event.
+  std::set<AXEventGenerator::EventParams> node6_events;
+  // Node7 contains IGNORED_CHANGED.
+  std::set<AXEventGenerator::EventParams> node7_events;
+  node7_events.emplace(AXEventGenerator::Event::IGNORED_CHANGED,
+                       ax::mojom::EventFrom::kNone, tree.event_intents());
+  // Node8 contains no event.
+  std::set<AXEventGenerator::EventParams> node8_events;
+  // Node9 contains no event.
+  std::set<AXEventGenerator::EventParams> node9_events;
+
+  event_generator.AddEventsForTesting(node1, node1_events);
+  event_generator.AddEventsForTesting(node2, node2_events);
+  event_generator.AddEventsForTesting(node3, node3_events);
+  event_generator.AddEventsForTesting(node4, node4_events);
+  event_generator.AddEventsForTesting(node5, node5_events);
+  event_generator.AddEventsForTesting(node6, node6_events);
+  event_generator.AddEventsForTesting(node7, node7_events);
+  event_generator.AddEventsForTesting(node8, node8_events);
+  event_generator.AddEventsForTesting(node9, node9_events);
+
+  std::map<AXNode*, std::set<AXEventGenerator::Event>> expected_event_map;
+  expected_event_map[node3] = {AXEventGenerator::Event::IGNORED_CHANGED,
+                               AXEventGenerator::Event::SUBTREE_CREATED,
+                               AXEventGenerator::Event::NAME_CHANGED};
+  expected_event_map[node7] = {AXEventGenerator::Event::IGNORED_CHANGED};
+
+  for (const auto& targeted_event : event_generator) {
+    auto map_iter = expected_event_map.find(targeted_event.node);
+
+    ASSERT_NE(map_iter, expected_event_map.end())
+        << "|expected_event_map| contains node.id=" << targeted_event.node->id()
+        << "\nExpected: true"
+        << "\nActual: " << std::boolalpha
+        << (map_iter != expected_event_map.end());
+
+    std::set<AXEventGenerator::Event>& node_events = map_iter->second;
+    auto event_iter = node_events.find(targeted_event.event_params.event);
+
+    ASSERT_NE(event_iter, node_events.end())
+        << "Event=" << targeted_event.event_params.event
+        << ", on node.id=" << targeted_event.node->id()
+        << " NOT found in |expected_event_map|";
+
+    // If the event from |event_generator| is found in |expected_event_map|,
+    // we want to delete the corresponding entry in |expected_event_map|.
+    node_events.erase(event_iter);
+    if (node_events.empty())
+      expected_event_map.erase(map_iter);
+  }
+
+  // We should expect |expected_event_map_| to be empty, when all the generated
+  // events match expected events.
+  EXPECT_TRUE(expected_event_map.empty());
+}
+
 TEST(AXEventGeneratorTest, LoadCompleteSameTree) {
   AXTreeUpdate initial_state;
   initial_state.root_id = 1;