Improve accessible hit testing heuristic.

content/browser/accessibility/browser_accessibility.cc

Index: content/browser/accessibility/browser_accessibility.cc
diff --git a/content/browser/accessibility/browser_accessibility.cc b/content/browser/accessibility/browser_accessibility.cc
index ff57121c89d66d7f00d43e9b0edb3f5b23bad9e9..6c4d9afdb686f0868ca575b9864bf1554bdd2822 100644
--- a/content/browser/accessibility/browser_accessibility.cc
+++ b/content/browser/accessibility/browser_accessibility.cc
@@ -316,6 +316,12 @@ BrowserAccessibility* BrowserAccessibility::BrowserAccessibilityForPoint(
     const gfx::Point& point) {
   // Walk the children recursively looking for the BrowserAccessibility that

[aboxhall, 2014/03/27 23:28:27]

Suggest moving this comment above the for loop.

[dmazzoni, 2014/03/28 05:34:15]

Done.

   // most tightly encloses the specified point.
+
+  // The best result found that's a child of this object.
+  BrowserAccessibility* child_result = NULL;
+  // The best result that's an indirect descendant like grandchild, etc.
+  BrowserAccessibility* descendant_result = NULL;
+
   for (int i = static_cast<int>(PlatformChildCount()) - 1; i >= 0; --i) {
     BrowserAccessibility* child = PlatformGetChild(i);
 
@@ -324,9 +330,20 @@ BrowserAccessibility* BrowserAccessibility::BrowserAccessibilityForPoint(
     if (child->role() == ui::AX_ROLE_COLUMN)
       continue;
 
-    if (child->GetGlobalBoundsRect().Contains(point))
-      return child->BrowserAccessibilityForPoint(point);
+    if (child->GetGlobalBoundsRect().Contains(point)) {
+      BrowserAccessibility* result = child->BrowserAccessibilityForPoint(point);
+      if (result == child && !child_result)
+        child_result = result;
+      if (result != child && !descendant_result)

[aboxhall, 2014/03/27 23:28:27]

I thought you mentioned the descendant being a control like a button, input etc?
Did I misunderstand?

[dmazzoni, 2014/03/28 05:34:15]

I used that as an example, but I don't think that should be part of the
heuristic - when doing touch exploration, you're just as likely to want to touch
static text as you are a button or link.

Here's another explanation of why the heuristic helps: suppose there are two
rows of buttons, none of which overlap. However, the containers for each row of
buttons overlap a fair amount just because of the way the page was laid out.

Without this heuristic, the algorithm would greedily pick the container that
encloses the point, even if the button in question was in the other container.
That's what we want to prevent.

[aboxhall, 2014/03/28 16:13:32]

Ok. We should probably add a comment here explaining this preference, then. It
sounds to me like we basically want to pick the deepest descendant because
actual content is always going to be in leaf nodes.

+        descendant_result = result;
+    }
   }
+
+  if (descendant_result)
+    return descendant_result;
+  if (child_result)
+    return child_result;
+
   return this;
 }