Re-land (2): Fix loading accessibility tree for child frame that's already loaded.

Original issue: http://crrev.com/2299673002
Second attempt: http://crrev.com/2299283002

Fix the flakiness by not trying to toggle accessibility off and on between the
two runs of the same page within each test.

Code leftover from the pre-OOPIF days was causing us to exit early from
the RenderAccessibilityImpl constructor for some child frames that were
already loaded. Everything worked fine if accessibility was already enabled
when loading the frame, but if the frame was already loaded and then
accessibility was enabled, this could cause it to fail to create an
accessibility tree.

The code in RenderAccessibilityImpl is no longer needed because now we
have exactly one accessibility tree per frame.

This wasn't caught by tests because we didn't cover both scenarios, we
always enabled accessibility first.

Added two variants of existing tests that load the page first and then
enable accessibility.

BUG=640231
TBR=dtseng@chromium.org

Review-Url: https://codereview.chromium.org/2317323002
Cr-Commit-Position: refs/heads/master@{#417194}

content/browser/accessibility/dump_accessibility_browsertest_base.cc

@@ -72,7 +72,9 @@ bool AccessibilityTreeContainsLoadedDocWithUrl(BrowserAccessibility* node,
 
 typedef AccessibilityTreeFormatter::Filter Filter;
 
-DumpAccessibilityTestBase::DumpAccessibilityTestBase() {
+DumpAccessibilityTestBase::DumpAccessibilityTestBase()
+    : is_blink_pass_(false),
+      enable_accessibility_after_navigating_(false) {
 }
 
 DumpAccessibilityTestBase::~DumpAccessibilityTestBase() {
@@ -242,19 +244,33 @@ void DumpAccessibilityTestBase::RunTestForPlatform(
   AddDefaultFilters(&filters_);
   ParseHtmlForExtraDirectives(html_contents, &filters_, &wait_for);
 
-  // Load the test html and wait for the "load complete" AX event.
+  // Get the test URL.
   GURL url(embedded_test_server()->GetURL(
       "/" + std::string(file_dir) + "/" + file_path.BaseName().MaybeAsASCII()));
-  AccessibilityNotificationWaiter accessibility_waiter(
-      shell()->web_contents(),
-      AccessibilityModeComplete,
-      ui::AX_EVENT_LOAD_COMPLETE);
-  NavigateToURL(shell(), url);
-  accessibility_waiter.WaitForNotification();
-
-  // Get the url of every frame in the frame tree.
   WebContentsImpl* web_contents = static_cast<WebContentsImpl*>(
       shell()->web_contents());
+
+  if (enable_accessibility_after_navigating_ &&
+      web_contents->GetAccessibilityMode() == AccessibilityModeOff) {
+    // Load the url, then enable accessibility.
+    NavigateToURL(shell(), url);
+    AccessibilityNotificationWaiter accessibility_waiter(
+        web_contents,
+        AccessibilityModeComplete,
+        ui::AX_EVENT_NONE);
+    accessibility_waiter.WaitForNotification();
+  } else {
+    // Enable accessibility, then load the test html and wait for the
+    // "load complete" AX event.
+    AccessibilityNotificationWaiter accessibility_waiter(
+        web_contents,
+        AccessibilityModeComplete,
+        ui::AX_EVENT_LOAD_COMPLETE);
+    NavigateToURL(shell(), url);
+    accessibility_waiter.WaitForNotification();
+  }
+
+  // Get the url of every frame in the frame tree.
   FrameTree* frame_tree = web_contents->GetFrameTree();
   std::vector<std::string> all_frame_urls;
   for (FrameTreeNode* node : frame_tree->Nodes()) {

content/browser/accessibility/dump_accessibility_browsertest_base.h

@@ -107,6 +107,10 @@ class DumpAccessibilityTestBase : public ContentBrowserTest {
 
   // Whether we're doing a native pass or internal/blink tree pass.
   bool is_blink_pass_;
+
+  // Whether we should enable accessibility after navigating to the page,
+  // otherwise we enable it first.
+  bool enable_accessibility_after_navigating_;
 };
 
 }  // namespace content

content/browser/accessibility/dump_accessibility_tree_browsertest.cc

@@ -848,6 +848,12 @@ IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityFrameset) {
   RunHtmlTest(FILE_PATH_LITERAL("frameset.html"));
 }
 
+IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,
+                       AccessibilityFramesetPostEnable) {
+  enable_accessibility_after_navigating_ = true;
+  RunHtmlTest(FILE_PATH_LITERAL("frameset.html"));
+}
+
 IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityHead) {
   RunHtmlTest(FILE_PATH_LITERAL("head.html"));
 }
@@ -881,6 +887,12 @@ IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityIframe) {
   RunHtmlTest(FILE_PATH_LITERAL("iframe.html"));
 }
 
+IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,
+                       AccessibilityIframePostEnable) {
+  enable_accessibility_after_navigating_ = true;
+  RunHtmlTest(FILE_PATH_LITERAL("iframe.html"));
+}
+
 IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,
                        AccessibilityIframeCrossProcess) {
   RunHtmlTest(FILE_PATH_LITERAL("iframe-cross-process.html"));

content/renderer/accessibility/render_accessibility_impl.cc

@@ -79,15 +79,6 @@ RenderAccessibilityImpl::RenderAccessibilityImpl(RenderFrameImpl* render_frame)
       ack_pending_(false),
       reset_token_(0),
       weak_factory_(this) {
-  // There's only one AXObjectCache for the root of a local frame tree,
-  // so if this frame's parent is local we can safely do nothing.
-  if (render_frame_ &&
-      render_frame_->GetWebFrame() &&
-      render_frame_->GetWebFrame()->parent() &&
-      render_frame_->GetWebFrame()->parent()->isWebLocalFrame()) {
-    return;
-  }
-
   WebView* web_view = render_frame_->GetRenderView()->GetWebView();
   WebSettings* settings = web_view->settings();
   settings->setAccessibilityEnabled(true);