A11y: Fix fuzzer-identified infinite loop

This change fixes an infinite loop in `OneShotAccessibilityTreeSearch`.
If the search starts at the root of a tree, goes backwards, and wraps
around to the last node, it will loop forever in `SearchByWalkingTree`.

The problem is that `stop_node` is set to NULL, so it is never matched.

The fix is to ensure that stop_node gets set to the root element (i.e.,
the start_node) so that we don't loop through the tree more than once.

Bug: 1001200
Change-Id: I67d8abd5e6044b2a125a633d39c7581e45cc7411
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1790255Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Kevin Babbitt <kbabbitt@microsoft.com>
Commit-Queue: Ian Prest <iapres@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#695764}

content/browser/accessibility/browser_accessibility_manager.cc

@@ -896,8 +896,7 @@ BrowserAccessibility* BrowserAccessibilityManager::PreviousInTreeOrder(
 
   // For android, this needs to be handled carefully. If not, there is a chance
   // of getting into infinite loop.
-  if (can_wrap_to_last_element &&
-      object->GetRole() == ax::mojom::Role::kRootWebArea &&
+  if (can_wrap_to_last_element && object->manager()->GetRoot() == object &&
       object->PlatformChildCount() != 0) {
     return object->PlatformDeepestLastChild();
   }

content/browser/accessibility/browser_accessibility_manager_unittest.cc

@@ -1114,6 +1114,7 @@ TEST_F(BrowserAccessibilityManagerTest, TestNextPreviousInTreeOrder) {
             manager->PreviousInTreeOrder(node3_accessible, false));
   EXPECT_EQ(root_accessible,
             manager->PreviousInTreeOrder(node2_accessible, false));
+  EXPECT_EQ(nullptr, manager->PreviousInTreeOrder(root_accessible, false));
 
   EXPECT_EQ(nullptr, manager->PreviousInTreeOrder(nullptr, true));
   EXPECT_EQ(node4_accessible,
@@ -1124,6 +1125,8 @@ TEST_F(BrowserAccessibilityManagerTest, TestNextPreviousInTreeOrder) {
             manager->PreviousInTreeOrder(node3_accessible, true));
   EXPECT_EQ(root_accessible,
             manager->PreviousInTreeOrder(node2_accessible, true));
+  EXPECT_EQ(node5_accessible,
+            manager->PreviousInTreeOrder(root_accessible, true));
 
   EXPECT_EQ(ax::mojom::TreeOrder::kEqual,
             BrowserAccessibilityManager::CompareNodes(*root_accessible,

content/browser/accessibility/one_shot_accessibility_tree_search.cc

@@ -177,10 +177,17 @@ void OneShotAccessibilityTreeSearch::SearchByWalkingTree() {
     if (Matches(node))
       matches_.push_back(node);
 
-    if (direction_ == FORWARDS)
+    if (direction_ == FORWARDS) {
       node = tree_->NextInTreeOrder(node);
-    else
+    } else {
+      // This needs to be handled carefully. If not, there is a chance of
+      // getting into infinite loop.
+      if (can_wrap_to_last_element_ && !stop_node &&
+          node->manager()->GetRoot() == node) {
+        stop_node = node;
+      }
       node = tree_->PreviousInTreeOrder(node, can_wrap_to_last_element_);
+    }
   }
 }
 

content/browser/accessibility/one_shot_accessibility_tree_search_unittest.cc

@@ -153,6 +153,20 @@ TEST_F(MAYBE_OneShotAccessibilityTreeSearchTest,
   EXPECT_EQ(1, search.GetMatchAtIndex(2)->GetId());
 }
 
+TEST_F(MAYBE_OneShotAccessibilityTreeSearchTest, BackwardsWrapFromRoot) {
+  OneShotAccessibilityTreeSearch search(tree_->GetRoot());
+  search.SetDirection(OneShotAccessibilityTreeSearch::BACKWARDS);
+  search.SetResultLimit(100);
+  search.SetCanWrapToLastElement(true);
+  ASSERT_EQ(6U, search.CountMatches());
+  EXPECT_EQ(1, search.GetMatchAtIndex(0)->GetId());
+  EXPECT_EQ(6, search.GetMatchAtIndex(1)->GetId());
+  EXPECT_EQ(5, search.GetMatchAtIndex(2)->GetId());
+  EXPECT_EQ(4, search.GetMatchAtIndex(3)->GetId());
+  EXPECT_EQ(3, search.GetMatchAtIndex(4)->GetId());
+  EXPECT_EQ(2, search.GetMatchAtIndex(5)->GetId());
+}
+
 TEST_F(MAYBE_OneShotAccessibilityTreeSearchTest,
        ForwardsWithStartNodeAndScope) {
   OneShotAccessibilityTreeSearch search(tree_->GetFromID(4));