Fix accessibility with out-of-process iframes.

content/browser/accessibility/site_per_process_accessibility_browsertest.cc

Index: content/browser/accessibility/site_per_process_accessibility_browsertest.cc
diff --git a/content/browser/accessibility/site_per_process_accessibility_browsertest.cc b/content/browser/accessibility/site_per_process_accessibility_browsertest.cc
index 1a3ace113889ff7e56c7f20942fec580ba17d618..9cc671e64b8e8a331569a64911422421df0171ed 100644
--- a/content/browser/accessibility/site_per_process_accessibility_browsertest.cc
+++ b/content/browser/accessibility/site_per_process_accessibility_browsertest.cc
@@ -47,17 +47,17 @@ bool AccessibilityTreeIsLoaded(BrowserAccessibilityManager* manager) {
           root->GetStringAttribute(ui::AX_ATTR_DOC_URL) != url::kAboutBlankURL);
 }
 
-// Disabled, see https://crbug.com/490831.
 // Times out on Android, not clear if it's an actual bug or just slow.
+#if defined(OS_ANDROID)
+#define MAYBE_CrossSiteIframeAccessibility DISABLED_CrossSiteIframeAccessibility
+#else
+#define MAYBE_CrossSiteIframeAccessibility CrossSiteIframeAccessibility
+#endif
 IN_PROC_BROWSER_TEST_F(SitePerProcessAccessibilityBrowserTest,
-                       DISABLED_CrossSiteIframeAccessibility) {
+                       MAYBE_CrossSiteIframeAccessibility) {
   // Enable full accessibility for all current and future WebContents.
   BrowserAccessibilityState::GetInstance()->EnableAccessibility();
 
-  AccessibilityNotificationWaiter main_frame_accessibility_waiter(
-      shell(), AccessibilityModeComplete,
-      ui::AX_EVENT_LOAD_COMPLETE);
-
   host_resolver()->AddRule("*", "127.0.0.1");
   ASSERT_TRUE(test_server()->Start());
   GURL main_url(test_server()->GetURL("files/site_per_process_main.html"));
@@ -74,6 +74,9 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessAccessibilityBrowserTest,
   NavigateFrameToURL(child, http_url);
 
   // Load cross-site page into iframe.
+  RenderFrameHostImpl* child_rfh =
+      child->render_manager()->current_frame_host();
+  RenderFrameHostDeletedObserver rfhdo(child_rfh);
   GURL::Replacements replace_host;
   GURL cross_site_url(test_server()->GetURL("files/title2.html"));
   replace_host.SetHostStr("foo.com");
@@ -85,9 +88,14 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessAccessibilityBrowserTest,
   SiteInstance* site_instance = child->current_frame_host()->GetSiteInstance();
   EXPECT_NE(shell()->web_contents()->GetSiteInstance(), site_instance);
 
-  // Wait for the accessibility tree from the main frame to load.
-  // Because we created the AccessibilityNotificationWaiter before accessibility
-  // was enabled, we're guaranteed to get a LOAD_COMPLETE event.
+  AccessibilityNotificationWaiter main_frame_accessibility_waiter(

[dmazzoni, 2015/06/25 22:15:38]

This still doesn't look early enough to me. I think it should be allocated
before the line of code that actually does the swap (NavigateFrameToURL).

[lfg, 2015/06/25 22:48:24]

It can't be above because it'll get the wrong event instead.

However, on further inspection, I believe that this message loop is
unneccessary. There's already a wait below for the child frame to be loaded
(line 111) that takes care of everything.

[dmazzoni, 2015/06/25 22:57:13]

A side effect of creating an AccessibilityNotificationWaiter is that it sets the
accessibility mode for that frame. If that's not necessary because setting it on
the child frame is having that effect anyway I guess that's fine.

+      shell(), AccessibilityModeComplete, ui::AX_EVENT_CHILDREN_CHANGED);
+
+  // Wait until the iframe completes the swap.
+  rfhdo.WaitUntilDeleted();
+
+  // Waits for the event that modifies the iframe in the accessibility tree.
+  // This event is sent immediatelly after the swap.
   main_frame_accessibility_waiter.WaitForNotification();
 
   RenderFrameHostImpl* main_frame = static_cast<RenderFrameHostImpl*>(
@@ -130,12 +138,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessAccessibilityBrowserTest,
   EXPECT_EQ(ui::AX_ROLE_IFRAME, ax_iframe->GetRole());
   ASSERT_EQ(1U, ax_iframe->PlatformChildCount());
 
-  BrowserAccessibility* ax_scroll_area = ax_iframe->PlatformGetChild(0);
-  EXPECT_EQ(ui::AX_ROLE_SCROLL_AREA, ax_scroll_area->GetRole());
-  ASSERT_EQ(1U, ax_scroll_area->PlatformChildCount());
-
-  BrowserAccessibility* ax_child_frame_root =
-      ax_scroll_area->PlatformGetChild(0);
+  BrowserAccessibility* ax_child_frame_root = ax_iframe->PlatformGetChild(0);
   EXPECT_EQ(ui::AX_ROLE_ROOT_WEB_AREA, ax_child_frame_root->GetRole());
   ASSERT_EQ(1U, ax_child_frame_root->PlatformChildCount());
   EXPECT_EQ("Title Of Awesomeness",
@@ -152,7 +155,7 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessAccessibilityBrowserTest,
   ASSERT_EQ(0U, ax_child_frame_static_text->PlatformChildCount());
 
   // Last, check that the parent of the child frame root is correct.
-  EXPECT_EQ(ax_child_frame_root->GetParent(), ax_scroll_area);
+  EXPECT_EQ(ax_child_frame_root->GetParent(), ax_iframe);
 }
 
 }  // namespace content