Fix WebView accessibility

content/browser/accessibility/browser_accessibility_manager.cc

Index: content/browser/accessibility/browser_accessibility_manager.cc
diff --git a/content/browser/accessibility/browser_accessibility_manager.cc b/content/browser/accessibility/browser_accessibility_manager.cc
index f2d4da7d24d9600734b5d83f294b571ceab818e9..5f85c80a52081e6cef69648eeb6a2649c681c33a 100644
--- a/content/browser/accessibility/browser_accessibility_manager.cc
+++ b/content/browser/accessibility/browser_accessibility_manager.cc
@@ -43,6 +43,10 @@ using AXTreeIDMap =
     base::hash_map<AXTreeIDRegistry::AXTreeID, BrowserAccessibilityManager*>;
 base::LazyInstance<AXTreeIDMap> g_ax_tree_id_map = LAZY_INSTANCE_INITIALIZER;
 
+// A function to call when focus changes, for testing only.
+base::LazyInstance<base::Closure> g_focus_change_callback_for_testing =
+    LAZY_INSTANCE_INITIALIZER;
+
 ui::AXTreeUpdate MakeAXTreeUpdate(
     const ui::AXNodeData& node1,
     const ui::AXNodeData& node2 /* = ui::AXNodeData() */,
@@ -179,11 +183,17 @@ BrowserAccessibilityManager::GetEmptyDocument() {
 
 void BrowserAccessibilityManager::FireFocusEventsIfNeeded() {
   BrowserAccessibility* focus = GetFocus();
-  if (delegate_ && !delegate_->AccessibilityViewHasFocus())
-    focus = nullptr;
 
-  if (!CanFireEvents())
-    focus = nullptr;
+  // Don't fire focus events if the window itself doesn't have focus.
+  // Bypass this check if a global focus listener was set up for testing
+  // so that the test passes whether the window is active or not.
+  if (!g_focus_change_callback_for_testing.Pointer()) {

[dcheng, 2016/03/24 18:15:11]

I wonder if it'd be possible to snoop IPC messages in the browser test itself,
and listen for, say, AccessibilityHostMsg_Events to figure out when to check for
focus changes. Is there similar precedent for adding test-only callbacks like
this to test accessibility code?

[dmazzoni, 2016/03/25 05:02:22]

That's how most of the tests in content/ work, we use
AccessibilityNotificationWaiter,
but I didn't want to pull a ton of interfaces into content/public just to test
this one
thing. I posted some other alternatives to site-isolation-discuss, this seemed
like
the most logical first step.

I'm also worried that snooping on particular IPCs might be a bit fragile, since
we rely on the hook in FrameTree::SetFocusedFrame that depents on a RFH
IPC and may come totally independently of accessibility IPCs.

Either way, I think a test-only callback really does make sense in this case
because
there aren't supposed to be any other consumers of BrowserAccessibilityManager
in the Chrome codebase. It's the part of the code that directly implements
native APIs, and this test just really needs to make sure it's got the whole
frame
tree when it does it.

+    if (delegate_ &&!delegate_->AccessibilityViewHasFocus())
+      focus = nullptr;
+
+    if (!CanFireEvents())
+      focus = nullptr;
+  }
 
   // Don't allow the document to be focused if it has no children and
   // hasn't finished loading yet. Wait for at least a tiny bit of content,
@@ -209,6 +219,8 @@ bool BrowserAccessibilityManager::CanFireEvents() {
 
 void BrowserAccessibilityManager::FireFocusEvent(BrowserAccessibility* node) {
   NotifyAccessibilityEvent(ui::AX_EVENT_FOCUS, node);
+  if (g_focus_change_callback_for_testing.Pointer())
+    g_focus_change_callback_for_testing.Get().Run();
 }
 
 BrowserAccessibility* BrowserAccessibilityManager::GetRoot() {
@@ -456,17 +468,22 @@ BrowserAccessibility* BrowserAccessibilityManager::GetFocus() {
   if (!focused_manager)
     focused_manager = root_manager;
 
-  int32_t focus_id = focused_manager->GetTreeData().focus_id;
-  BrowserAccessibility* obj = focused_manager->GetFromID(focus_id);
+  return focused_manager->GetFocusFromThisOrDescendantFrame();
+}
+
+BrowserAccessibility*
+BrowserAccessibilityManager::GetFocusFromThisOrDescendantFrame() {
+  int32_t focus_id = GetTreeData().focus_id;
+  BrowserAccessibility* obj = GetFromID(focus_id);
   if (!obj)
-    return focused_manager->GetRoot();
+    return GetRoot();
 
   if (obj->HasIntAttribute(ui::AX_ATTR_CHILD_TREE_ID)) {
     BrowserAccessibilityManager* child_manager =
         BrowserAccessibilityManager::FromID(
             obj->GetIntAttribute(ui::AX_ATTR_CHILD_TREE_ID));
     if (child_manager)
-      return child_manager->GetFocus();
+      return child_manager->GetFocusFromThisOrDescendantFrame();
   }
 
   return obj;
@@ -484,6 +501,12 @@ void BrowserAccessibilityManager::SetFocusLocallyForTesting(
   tree_->UpdateData(data);
 }
 
+// static
+void BrowserAccessibilityManager::SetFocusChangeCallbackForTesting(
+    base::Closure callback) {
+  g_focus_change_callback_for_testing.Get() = callback;
+}
+
 void BrowserAccessibilityManager::DoDefaultAction(
     const BrowserAccessibility& node) {
   if (delegate_)