Too many accessible nodes were marked as clickable on Android

content/browser/accessibility/browser_accessibility_android.cc

Index: content/browser/accessibility/browser_accessibility_android.cc
diff --git a/content/browser/accessibility/browser_accessibility_android.cc b/content/browser/accessibility/browser_accessibility_android.cc
index 991cf010d1fdacd439e518233aeabe27b3dd6c25..7ab3be80ee37daad9f763510671329638aeadc70 100644
--- a/content/browser/accessibility/browser_accessibility_android.cc
+++ b/content/browser/accessibility/browser_accessibility_android.cc
@@ -154,12 +154,8 @@ bool BrowserAccessibilityAndroid::IsChecked() const {
 }
 
 bool BrowserAccessibilityAndroid::IsClickable() const {
-  if (!PlatformIsLeaf())
-    return false;
-
-  // We are very aggressive about returning true with IsClickable on Android
-  // because there is no way to know for sure what might have a click handler.

[David Tseng, 2016/10/12 20:28:35]

Is this no longer a concern or do you have a more robust way to detect the
associated dom node has a click handler (e.g. onclick set)?

[dmazzoni, 2016/10/12 22:41:58]

An older version of TalkBack was skipping over anything
that wasn't clickable, so we returned "clickable" for more
than what was actually clickable.

Now TalkBack is fixed, so we can revert to returning
clickable when we think a node is clickable. Same as other
platforms, it's not bad but not 100% perfect. It works
correctly for onclick and for real event handlers, but
doesn't handle event delegation where the listener is on
the ancestor.

-  return IsFocusable() || !GetText().empty();
+  // A node is clickable if it's focusable or if it has a default action.
+  return IsFocusable() || HasStringAttribute(ui::AX_ATTR_ACTION);
 }
 
 bool BrowserAccessibilityAndroid::IsCollapsed() const {
@@ -207,12 +203,15 @@ bool BrowserAccessibilityAndroid::IsExpanded() const {
 }
 
 bool BrowserAccessibilityAndroid::IsFocusable() const {
-  bool focusable = HasState(ui::AX_STATE_FOCUSABLE);
-  if (IsIframe() ||
-      GetRole() == ui::AX_ROLE_WEB_AREA) {
-    focusable = false;
-  }
-  return focusable;
+  // Don't count iframes as focusable.
+  if (IsIframe())
+    return false;
+
+  // Only the root of the tree, and not child frames, are focusable.

[David Tseng, 2016/10/12 20:28:35]

Seems strange to have this restriction; what if a page author sets an aria role
on body like application? This code would allow focus in that case. Also, if an
iframe has an aria label, wouldn't it be interesting?

[dmazzoni, 2016/10/12 22:41:58]

Agreed. I changed it so that we skip iframes and child frame
root notes if they don't have a name, but we include them if
they have a name.

I don't think there's an issue with an aria role on the body,
the root of our tree is actually the parent of the body element.

+  if (GetRole() == ui::AX_ROLE_ROOT_WEB_AREA)
+    return !GetParent();
+
+  return HasState(ui::AX_STATE_FOCUSABLE);
 }
 
 bool BrowserAccessibilityAndroid::IsFocused() const {