Reset accessibility if it gets out of sync.

content/browser/accessibility/accessibility_ipc_error_browsertest.cc

+// Copyright (c) 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/browser/web_contents/web_contents_impl.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/content_browser_test.h"
+#include "content/public/test/content_browser_test_utils.h"
+#include "content/shell/browser/shell.h"
+#include "content/test/accessibility_browser_test_utils.h"
+#include "ui/accessibility/ax_node.h"
+#include "ui/accessibility/ax_tree.h"
+
+namespace content {
+
+class AccessibilityIpcErrorBrowserTest : public ContentBrowserTest {
+ public:
+  AccessibilityIpcErrorBrowserTest() {}
+
+ protected:
+  // Convenience method to get the value of a particular AXNode
+  // attribute as a UTF-8 string.
+  std::string GetAttr(const ui::AXNode* node,
+                      const ui::AXStringAttribute attr) {
+    const ui::AXNodeData& data = node->data();
+    for (size_t i = 0; i < data.string_attributes.size(); ++i) {
+      if (data.string_attributes[i].first == attr)
+        return data.string_attributes[i].second;
+    }
+    return std::string();
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(AccessibilityIpcErrorBrowserTest);
+};
+
+IN_PROC_BROWSER_TEST_F(AccessibilityIpcErrorBrowserTest,
+                       ResetBrowserAccessibilityManager) {
+  // Create a data url and load it.
+  const char url_str[] =
+      "data:text/html,"
+      "<div aria-live='polite'>"
+      "  <p id='p1'>Paragraph One</p>"
+      "  <p id='p2'>Paragraph Two</p>"
+      "</div>"
+      "<button id='button'>Button</button>";
+  GURL url(url_str);
+  NavigateToURL(shell(), url);
+
+  // Simulate a condition where the RFH can't create a
+  // BrowserAccessibilityManager - like if there's no view.
+  RenderFrameHostImpl* frame = static_cast<RenderFrameHostImpl*>(
+      shell()->web_contents()->GetMainFrame());
+  frame->set_disallow_browser_accessibility_manager_for_testing(true);
+  ASSERT_EQ(NULL, frame->GetOrCreateBrowserAccessibilityManager());
+
+  // Enable accessibility (passing AccessibilityModeComplete to
+  // AccessibilityNotificationWaiter does this automatically) and wait for
+  // the first event.
+  AccessibilityNotificationWaiter waiter(
+      shell(), AccessibilityModeComplete, ui::AX_EVENT_LAYOUT_COMPLETE);
+  waiter.WaitForNotification();
+
+  // Make sure we still didn't create a BrowserAccessibilityManager.
+  // This means that at least one accessibility IPC was lost.
+  ASSERT_EQ(NULL, frame->GetOrCreateBrowserAccessibilityManager());
+
+  // Now allow a BrowserAccessibilityManager, simulating what would happen
+  // if the RFH's view is created now.
+  frame->set_disallow_browser_accessibility_manager_for_testing(false);
+  BrowserAccessibilityManager* manager =
+      frame->GetOrCreateBrowserAccessibilityManager();
+  ASSERT_TRUE(manager != NULL);
+
+  // Hide one of the elements on the page, and wait for an accessibility
+  // notification triggered by the hide.
+  AccessibilityNotificationWaiter waiter2(

[nasko, 2014/10/02 16:38:47]

Since you don't preserve any state in each of those wait blocks, why not enclose
them in local scope (between {}) that way you don't need to have waiterX as you
have more of these blocks. It also makes it easier to read/follow, since it is
clear that this is a unit of code.

[dmazzoni, 2014/10/02 21:51:12]

Good idea, done.

+      shell(), AccessibilityModeComplete, ui::AX_EVENT_LIVE_REGION_CHANGED);
+  ASSERT_TRUE(ExecuteScript(shell()->web_contents(),
+                  "document.getElementById('p1').style.display = 'none';"));
+  waiter2.WaitForNotification();
+
+  // Show that accessibility was reset because we have a new
+  // BrowserAccessibilityManager object.
+  ASSERT_TRUE(manager != frame->GetOrCreateBrowserAccessibilityManager());
+
+  // Ensure that we didn't kill the renderer; we can still send it messages.

[nasko, 2014/10/02 16:38:48]

Do you plan to test that the process kill functionality works?

[dmazzoni, 2014/10/02 21:51:12]

Sure, makes sense. I added a new test.

+  AccessibilityNotificationWaiter waiter3(
+      shell(), AccessibilityModeComplete, ui::AX_EVENT_FOCUS);
+  ASSERT_TRUE(ExecuteScript(shell()->web_contents(),
+                  "document.getElementById('button').focus();"));
+  waiter3.WaitForNotification();
+
+  // Get the accessibility tree, ensure it reflects the final state of the
+  // document.
+  const ui::AXTree& tree = waiter3.GetAXTree();
+  const ui::AXNode* root = tree.GetRoot();
+
+  // Use this for debugging if the test fails.
+  VLOG(1) << tree.ToString();
+
+  EXPECT_EQ(ui::AX_ROLE_ROOT_WEB_AREA, root->data().role);
+  ASSERT_EQ(2, root->child_count());
+
+  const ui::AXNode* live_region = root->ChildAtIndex(0);
+  ASSERT_EQ(1, live_region->child_count());
+  EXPECT_EQ(ui::AX_ROLE_DIV, live_region->data().role);
+
+  const ui::AXNode* para = live_region->ChildAtIndex(0);
+  EXPECT_EQ(ui::AX_ROLE_PARAGRAPH, para->data().role);
+
+  const ui::AXNode* button_container = root->ChildAtIndex(1);
+  EXPECT_EQ(ui::AX_ROLE_GROUP, button_container->data().role);
+  ASSERT_EQ(1, button_container->child_count());
+
+  const ui::AXNode* button = button_container->ChildAtIndex(0);
+  EXPECT_EQ(ui::AX_ROLE_BUTTON, button->data().role);
+  EXPECT_TRUE(button->data().state >> ui::AX_STATE_FOCUSED & 1);
+}
+
+}  // namespace content