Variation in setSelectionRanges for cross-tree selections

This change is a follow-up to
https://chromium-review.googlesource.com/c/chromium/src/+/2340291.

After that change went in, we are still seeing crashes in
BrowserAccessibility::AccessibilityPerformAction where the anchor
and/or focus nodes are not found, stemming from setSelectionRanges.

Based on code inspection, I was able to come up with this variation,
where the anchor and focus nodes are in the same tree as each other,
but in a different tree than the caller.

The crash dumps I have available not have enough data to prove that
this is the root cause of those specific crashes, but the unit test
added here is sufficient to show that this crash is possible.

Rather than returning an error in this scenario, when the anchor and
focus nodes are in in the same tree as each other, but a different
tree than the node calling setSelectionRanges, the tree that the anchor
and focus nodes reside in is used for looking up the manager and setting
the selection.

AX-Relnotes: n/a

Bug: 1110522
Change-Id: I28cb18e87bcd0cfa2abd3220878d4b04ac70f96a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2504630
Commit-Queue: Kurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829866}

content/browser/accessibility/accessibility_win_browsertest.cc

@@ -2766,20 +2766,63 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
       text_after_iframe.As(&text_after_iframe_iaccessible2_4));
 
   LONG n_ranges = 1;
-  IA2Range* ranges =
+  IA2Range* cross_tree_ranges =
       reinterpret_cast<IA2Range*>(CoTaskMemAlloc(sizeof(IA2Range)));
-  ranges[0].anchor = text_in_iframe.Get();
-  ranges[0].anchorOffset = 0;
-  ranges[0].active = text_after_iframe.Get();
-  ranges[0].activeOffset = 2;
+  cross_tree_ranges[0].anchor = text_in_iframe.Get();
+  cross_tree_ranges[0].anchorOffset = 0;
+  cross_tree_ranges[0].active = text_after_iframe.Get();
+  cross_tree_ranges[0].activeOffset = 2;
 
   // This is expected to fail because the anchor and focus nodes are in
   // different trees, which Blink doesn't support.
-  EXPECT_HRESULT_FAILED(
-      text_after_iframe_iaccessible2_4->setSelectionRanges(n_ranges, ranges));
+  EXPECT_HRESULT_FAILED(text_after_iframe_iaccessible2_4->setSelectionRanges(
+      n_ranges, cross_tree_ranges));
 
-  CoTaskMemFree(ranges);
-  ranges = nullptr;
+  CoTaskMemFree(cross_tree_ranges);
+  cross_tree_ranges = nullptr;
+
+  // Now test a variation where the selection start and end are in the same
+  // tree as each other, but a different tree than the caller.
+  IA2Range* same_tree_ranges =
+      reinterpret_cast<IA2Range*>(CoTaskMemAlloc(sizeof(IA2Range)));
+  same_tree_ranges[0].anchor = text_in_iframe.Get();
+  same_tree_ranges[0].anchorOffset = 0;
+  same_tree_ranges[0].active = text_in_iframe.Get();
+  same_tree_ranges[0].activeOffset = 1;
+
+  // This should succeed, however the selection will need to be queried from
+  // a node in the iframe tree.
+  AccessibilityNotificationWaiter selection_waiter(
+      shell()->web_contents(), ui::kAXModeComplete,
+      ax::mojom::Event::kTextSelectionChanged);
+  ASSERT_HRESULT_SUCCEEDED(text_after_iframe_iaccessible2_4->setSelectionRanges(
+      n_ranges, same_tree_ranges));
+  selection_waiter.WaitForNotification();
+
+  Microsoft::WRL::ComPtr<IAccessible2_4> text_in_iframe_iaccessible2_4;
+  ASSERT_HRESULT_SUCCEEDED(text_in_iframe.As(&text_in_iframe_iaccessible2_4));
+
+  IA2Range* result_ranges =
+      reinterpret_cast<IA2Range*>(CoTaskMemAlloc(sizeof(IA2Range)));
+  HRESULT hr = text_in_iframe_iaccessible2_4->get_selectionRanges(
+      &result_ranges, &n_ranges);
+  EXPECT_EQ(S_OK, hr);
+  EXPECT_EQ(1, n_ranges);
+  ASSERT_NE(nullptr, result_ranges);
+  ASSERT_NE(nullptr, result_ranges[0].anchor);
+  EXPECT_EQ(text_in_iframe.Get(), result_ranges[0].anchor);
+  EXPECT_EQ(0, result_ranges[0].anchorOffset);
+  ASSERT_NE(nullptr, result_ranges[0].active);
+  EXPECT_EQ(text_in_iframe.Get(), result_ranges[0].active);
+  EXPECT_EQ(1, result_ranges[0].activeOffset);
+
+  same_tree_ranges[0].anchor->Release();
+  same_tree_ranges[0].active->Release();
+  CoTaskMemFree(same_tree_ranges);
+  same_tree_ranges = nullptr;
+
+  CoTaskMemFree(result_ranges);
+  result_ranges = nullptr;
 }
 
 IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest, TestMultiLineSetSelection) {

content/browser/accessibility/browser_accessibility.cc

@@ -1820,11 +1820,25 @@ bool BrowserAccessibility::AccessibilityPerformAction(
       manager_->SetScrollOffset(*this, data.target_point);
       return true;
     case ax::mojom::Action::kSetSelection: {
-      // "data.anchor_offset" and "data.focus_ofset" might need to be adjusted
-      // if the anchor or the focus nodes include ignored children.
       ui::AXActionData selection = data;
+
+      // Prioritize target_tree_id if it was provided, as it is possible on
+      // some platforms (such as IAccessible2) to initiate a selection in a
+      // different tree than the current node resides in, as long as the nodes
+      // being selected share an AXTree with each other.
+      BrowserAccessibilityManager* selection_manager = nullptr;
+      if (selection.target_tree_id != ui::AXTreeIDUnknown()) {
+        selection_manager =
+            BrowserAccessibilityManager::FromID(selection.target_tree_id);
+      } else {
+        selection_manager = manager_;
+      }
+      DCHECK(selection_manager);
+
+      // "data.anchor_offset" and "data.focus_offset" might need to be adjusted
+      // if the anchor or the focus nodes include ignored children.
       const BrowserAccessibility* anchor_object =
-          manager()->GetFromID(selection.anchor_node_id);
+          selection_manager->GetFromID(selection.anchor_node_id);
       DCHECK(anchor_object);
       if (!anchor_object->PlatformIsLeaf()) {
         DCHECK_GE(selection.anchor_offset, 0);
@@ -1843,8 +1857,12 @@ bool BrowserAccessibility::AccessibilityPerformAction(
       }
 
       const BrowserAccessibility* focus_object =
-          manager()->GetFromID(selection.focus_node_id);
+          selection_manager->GetFromID(selection.focus_node_id);
       DCHECK(focus_object);
+
+      // Blink only supports selections between two nodes in the same tree.
+      DCHECK_EQ(anchor_object->GetTreeData().tree_id,
+                focus_object->GetTreeData().tree_id);
       if (!focus_object->PlatformIsLeaf()) {
         DCHECK_GE(selection.focus_offset, 0);
         const BrowserAccessibility* focus_child =
@@ -1859,7 +1877,7 @@ bool BrowserAccessibility::AccessibilityPerformAction(
         }
       }
 
-      manager_->SetSelection(selection);
+      selection_manager->SetSelection(selection);
       return true;
     }
     case ax::mojom::Action::kSetValue:

ui/accessibility/platform/ax_platform_node_win.cc

@@ -1682,8 +1682,9 @@ IFACEMETHODIMP AXPlatformNodeWin::setSelectionRanges(LONG nRanges,
     return E_INVALIDARG;
 
   // Blink only supports selections within a single tree.
-  if (anchor_node->GetDelegate()->GetTreeData().tree_id !=
-      focus_node->GetDelegate()->GetTreeData().tree_id) {
+  AXTreeID anchor_tree_id = anchor_node->GetDelegate()->GetTreeData().tree_id;
+  AXTreeID focus_tree_id = focus_node->GetDelegate()->GetTreeData().tree_id;
+  if (anchor_tree_id != focus_tree_id) {
     return E_INVALIDARG;
   }
 
@@ -1710,6 +1711,7 @@ IFACEMETHODIMP AXPlatformNodeWin::setSelectionRanges(LONG nRanges,
   AXActionData action_data;
   action_data.action = ax::mojom::Action::kSetSelection;
   action_data.anchor_node_id = anchor_node->GetData().id;
+  action_data.target_tree_id = anchor_tree_id;
   action_data.anchor_offset = int32_t{ranges->anchorOffset};
   action_data.focus_node_id = focus_node->GetData().id;
   action_data.focus_offset = int32_t{ranges->activeOffset};