Improve Android accessible hit testing.

content/browser/accessibility/browser_accessibility_manager_android.cc

Index: content/browser/accessibility/browser_accessibility_manager_android.cc
diff --git a/content/browser/accessibility/browser_accessibility_manager_android.cc b/content/browser/accessibility/browser_accessibility_manager_android.cc
index 4a772dd885885f2e2df792d62e186d2d35a3b874..f9a3b3107b7216cbdc417e1876f5b6c68c7309b7 100644
--- a/content/browser/accessibility/browser_accessibility_manager_android.cc
+++ b/content/browser/accessibility/browser_accessibility_manager_android.cc
@@ -56,20 +56,9 @@ bool SectionPredicate(
 
 bool AllInterestingNodesPredicate(
     BrowserAccessibility* start, BrowserAccessibility* node) {
-  // Focusable nodes should never be skipped. Note that IsFocusable()
-  // already skips over things like iframes and child frames that are
-  // technically focusable but shouldn't be exposed as focusable on Android.
   BrowserAccessibilityAndroid* android_node =
       static_cast<BrowserAccessibilityAndroid*>(node);
-  if (android_node->IsFocusable())
-    return true;
-
-  // If it's not focusable but has a control role, then it's interesting.
-  if (android_node->IsControl())
-    return true;
-
-  // Otherwise, the interesting nodes are leaf nodes with text.
-  return node->PlatformIsLeaf() && !node->GetText().empty();
+  return android_node->IsInterestingOnAndroid();
 }
 
 void AddToPredicateMap(const char* search_key_ascii,
@@ -710,19 +699,30 @@ void BrowserAccessibilityManagerAndroid::HandleHoverEvent(
   if (obj.is_null())
     return;
 
-  BrowserAccessibilityAndroid* ancestor =
-      static_cast<BrowserAccessibilityAndroid*>(node->GetParent());
-  while (ancestor && ancestor != GetRoot()) {
-    if (ancestor->PlatformIsLeaf() ||
-        (ancestor->IsFocusable() && !ancestor->HasFocusableChild())) {
-      node = ancestor;
-      // Don't break - we want the highest ancestor that's focusable or a
-      // leaf node.
-    }
-    ancestor = static_cast<BrowserAccessibilityAndroid*>(ancestor->GetParent());
+  // First walk up to the nearest platform node, in case this node isn't
+  // even exposed on the platform.
+  node = node->GetClosestPlatformObject();
+
+  // If this node is uninteresting and just a wrapper around an interesting
+  // child, prefer that child instead.
+  const BrowserAccessibilityAndroid* android_node =
+      static_cast<BrowserAccessibilityAndroid*>(node);
+  const BrowserAccessibilityAndroid* interesting_child =

[aboxhall, 2016/10/18 21:03:24]

Do we need to make sure the child matches the hit test?

[dmazzoni, 2016/10/18 22:09:36]

Good thought, but I don't think so - the only place we'd use
this is when we've already decided that the node that does
match the hit test is uninteresting (like a wrapper) so it
seems okay to land on the child even if the child is not hit.

An example of where this works well is if you have a <div>
containing text but with a lot of padding - previously,
clicking on the text would read the text, but clicking on
the padding would land on the div and TalkBack wouldn't read
anything. Now it would focus the text when you click on the
padding around it, but if there are multiple text boxes
inside it would instead focus an ancestor that wraps all
of them that actually has a role or label

+      android_node->GetInterestingChild();
+  if (interesting_child)
+    android_node = interesting_child;
+
+  // Finally, if this node is still uninteresting, try to walk up to
+  // find an interesting parent.
+  while (android_node && !android_node->IsInterestingOnAndroid()) {
+    android_node = static_cast<BrowserAccessibilityAndroid*>(
+        android_node->GetParent());
   }
 
-  Java_BrowserAccessibilityManager_handleHover(env, obj, node->unique_id());
+  if (android_node) {
+    Java_BrowserAccessibilityManager_handleHover(
+        env, obj, android_node->unique_id());
+  }
 }
 
 jint BrowserAccessibilityManagerAndroid::FindElementType(