Reset accessibility if it gets out of sync.

Previously if the browser got an accessibility IPC it was unable to
interpret, it killed the renderer. This change makes it reset the
accessibility state instead so we're tolerant of corner cases that
are very difficult to completely eliminate in practice.

The accessibility reset is designed to be safe, it throws away all
accessibility state on the browser side and waits for an IPC from
the renderer acknowledging the renderer reset as well.

BUG=372478

Committed: https://crrev.com/0c5e8d16661df7e08c708997f24c0a3070e58080
Cr-Commit-Position: refs/heads/master@{#298297}

Review URL: https://codereview.chromium.org/625443002

Cr-Commit-Position: refs/heads/master@{#301106}

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/common/accessibility_messages.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_no_create_browser_accessibility_manager_for_testing(true);
+  ASSERT_EQ(nullptr, 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(nullptr, frame->GetOrCreateBrowserAccessibilityManager());
+
+  // Now create a BrowserAccessibilityManager, simulating what would happen
+  // if the RFH's view is created now - but then disallow recreating the
+  // BrowserAccessibilityManager so that we can test that this one gets
+  // destroyed.
+  frame->set_no_create_browser_accessibility_manager_for_testing(false);
+  ASSERT_TRUE(frame->GetOrCreateBrowserAccessibilityManager() != nullptr);
+  frame->set_no_create_browser_accessibility_manager_for_testing(true);
+
+  {
+    // Hide one of the elements on the page, and wait for an accessibility
+    // notification triggered by the hide.
+    AccessibilityNotificationWaiter waiter(
+        shell(), AccessibilityModeComplete, ui::AX_EVENT_LIVE_REGION_CHANGED);
+    ASSERT_TRUE(ExecuteScript(
+        shell()->web_contents(),
+        "document.getElementById('p1').style.display = 'none';"));
+    waiter.WaitForNotification();
+  }
+
+  // Show that accessibility was reset because the frame doesn't have a
+  // BrowserAccessibilityManager anymore.
+  ASSERT_EQ(nullptr, frame->browser_accessibility_manager());
+
+  // Finally, allow creating a new accessibility manager and
+  // ensure that we didn't kill the renderer; we can still send it messages.
+  frame->set_no_create_browser_accessibility_manager_for_testing(false);
+  const ui::AXTree* tree = nullptr;
+  {
+    AccessibilityNotificationWaiter waiter(
+        shell(), AccessibilityModeComplete, ui::AX_EVENT_FOCUS);
+    ASSERT_TRUE(ExecuteScript(
+        shell()->web_contents(),
+        "document.getElementById('button').focus();"));
+    waiter.WaitForNotification();
+    tree = &waiter.GetAXTree();
+  }
+
+  // Get the accessibility tree, ensure it reflects the final state of the
+  // document.
+  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);
+}
+
+IN_PROC_BROWSER_TEST_F(AccessibilityIpcErrorBrowserTest,
+                       MultipleBadAccessibilityIPCsKillsRenderer) {
+  // Create a data url and load it.
+  const char url_str[] =
+      "data:text/html,"
+      "<button id='button'>Button</button>";
+  GURL url(url_str);
+  NavigateToURL(shell(), url);
+  RenderFrameHostImpl* frame = static_cast<RenderFrameHostImpl*>(
+      shell()->web_contents()->GetMainFrame());
+
+  {
+    // 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();
+  }
+
+  // Construct a bad accessibility message that BrowserAccessibilityManager
+  // will reject.
+  std::vector<AccessibilityHostMsg_EventParams> bad_accessibility_event_list;
+  bad_accessibility_event_list.push_back(AccessibilityHostMsg_EventParams());
+  bad_accessibility_event_list[0].update.node_id_to_clear = -2;
+
+  // We should be able to reset accessibility |max_iterations-1| times
+  // (see render_frame_host_impl.cc - kMaxAccessibilityResets),
+  // but the subsequent time the renderer should be killed.
+  int max_iterations = RenderFrameHostImpl::kMaxAccessibilityResets;
+
+  for (int iteration = 0; iteration < max_iterations; iteration++) {
+    // Send the browser accessibility the bad message.
+    BrowserAccessibilityManager* manager =
+        frame->GetOrCreateBrowserAccessibilityManager();
+    manager->OnAccessibilityEvents(bad_accessibility_event_list);
+
+    // Now the frame should have deleted the BrowserAccessibilityManager.
+    ASSERT_EQ(nullptr, frame->browser_accessibility_manager());
+
+    if (iteration == max_iterations - 1)
+      break;
+
+    AccessibilityNotificationWaiter waiter(
+        shell(), AccessibilityModeComplete, ui::AX_EVENT_LAYOUT_COMPLETE);
+    waiter.WaitForNotification();
+  }
+
+  // Wait for the renderer to be killed.
+  if (frame->IsRenderFrameLive()) {
+    RenderProcessHostWatcher render_process_watcher(
+        frame->GetProcess(), RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+    render_process_watcher.Wait();
+  }
+  ASSERT_FALSE(frame->IsRenderFrameLive());
+}
+
+}  // namespace content

content/browser/frame_host/render_frame_host_impl.cc

@@ -70,6 +70,9 @@ namespace content {
 
 namespace {
 
+// The next value to use for the accessibility reset token.
+int g_next_accessibility_reset_token = 1;
+
 // The (process id, routing id) pair that identifies one RenderFrame.
 typedef std::pair<int32, int32> RenderFrameHostID;
 typedef base::hash_map<RenderFrameHostID, RenderFrameHostImpl*>
@@ -189,6 +192,9 @@ RenderFrameHostImpl::RenderFrameHostImpl(RenderViewHostImpl* render_view_host,
       navigations_suspended_(false),
       is_waiting_for_beforeunload_ack_(false),
       unload_ack_is_for_cross_site_transition_(false),
+      accessibility_reset_token_(0),
+      accessibility_reset_count_(0),
+      no_create_browser_accessibility_manager_for_testing_(false),
       weak_ptr_factory_(this) {
   frame_tree_->RegisterRenderFrameHost(this);
   GetProcess()->AddRoute(routing_id_, this);
@@ -479,8 +485,18 @@ void RenderFrameHostImpl::AccessibilityHitTest(const gfx::Point& point) {
 }
 
 void RenderFrameHostImpl::AccessibilityFatalError() {
-  Send(new AccessibilityMsg_FatalError(routing_id_));
   browser_accessibility_manager_.reset(NULL);
+  if (accessibility_reset_token_)
+    return;
+
+  accessibility_reset_count_++;
+  if (accessibility_reset_count_ >= kMaxAccessibilityResets) {
+    Send(new AccessibilityMsg_FatalError(routing_id_));
+  } else {
+    accessibility_reset_token_ = g_next_accessibility_reset_token++;
+    UMA_HISTOGRAM_COUNTS("Accessibility.FrameResetCount", 1);
+    Send(new AccessibilityMsg_Reset(routing_id_, accessibility_reset_token_));
+  }
 }
 
 gfx::AcceleratedWidget
@@ -756,6 +772,7 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) {
     return;
   }
 
+  accessibility_reset_count_ = 0;
   frame_tree_node()->navigator()->DidNavigate(this, validated_params);
 }
 
@@ -1081,7 +1098,17 @@ void RenderFrameHostImpl::OnBeginNavigation(
 }
 
 void RenderFrameHostImpl::OnAccessibilityEvents(
-    const std::vector<AccessibilityHostMsg_EventParams>& params) {
+    const std::vector<AccessibilityHostMsg_EventParams>& params,
+    int reset_token) {
+  // Don't process this IPC if either we're waiting on a reset and this
+  // IPC doesn't have the matching token ID, or if we're not waiting on a
+  // reset but this message includes a reset token.
+  if (accessibility_reset_token_ != reset_token) {
+    Send(new AccessibilityMsg_Events_ACK(routing_id_));
+    return;
+  }
+  accessibility_reset_token_ = 0;
+
   RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
       render_view_host_->GetView());
 
@@ -1156,17 +1183,18 @@ void RenderFrameHostImpl::OnAccessibilityEvents(
 
 void RenderFrameHostImpl::OnAccessibilityLocationChanges(
     const std::vector<AccessibilityHostMsg_LocationChangeParams>& params) {
+  if (accessibility_reset_token_)
+    return;
+
   RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
       render_view_host_->GetView());
   if (view && RenderFrameHostImpl::IsRFHStateActive(rfh_state())) {
     AccessibilityMode accessibility_mode = delegate_->GetAccessibilityMode();
     if (accessibility_mode & AccessibilityModeFlagPlatform) {
-      if (!browser_accessibility_manager_) {
-        browser_accessibility_manager_.reset(
-            view->CreateBrowserAccessibilityManager(this));
-      }
-      if (browser_accessibility_manager_)
-        browser_accessibility_manager_->OnLocationChanges(params);
+      BrowserAccessibilityManager* manager =
+          GetOrCreateBrowserAccessibilityManager();
+      if (manager)
+        manager->OnLocationChanges(params);
     }
     // TODO(aboxhall): send location change events to web contents observers too
   }
@@ -1474,9 +1502,14 @@ BrowserAccessibilityManager*
   RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
       render_view_host_->GetView());
   if (view &&
-      !browser_accessibility_manager_) {
+      !browser_accessibility_manager_ &&
+      !no_create_browser_accessibility_manager_for_testing_) {
     browser_accessibility_manager_.reset(
         view->CreateBrowserAccessibilityManager(this));
+    if (browser_accessibility_manager_)
+      UMA_HISTOGRAM_COUNTS("Accessibility.FrameEnabledCount", 1);
+    else
+      UMA_HISTOGRAM_COUNTS("Accessibility.FrameDidNotEnableCount", 1);
   }
   return browser_accessibility_manager_.get();
 }

content/browser/frame_host/render_frame_host_impl.h

@@ -93,6 +93,11 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // number of active frames of a SiteInstance or not.
   static bool IsRFHStateActive(RenderFrameHostImplState rfh_state);
 
+  // An accessibility reset is only allowed to prevent very rare corner cases
+  // or race conditions where the browser and renderer get out of sync. If
+  // this happens more than this many times, kill the renderer.
+  static const int kMaxAccessibilityResets = 5;
+
   static RenderFrameHostImpl* FromID(int process_id, int routing_id);
 
   ~RenderFrameHostImpl() override;
@@ -334,6 +339,10 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // NULL.
   BrowserAccessibilityManager* GetOrCreateBrowserAccessibilityManager();
 
+  void set_no_create_browser_accessibility_manager_for_testing(bool flag) {
+    no_create_browser_accessibility_manager_for_testing_ = flag;
+  }
+
 #if defined(OS_WIN)
   void SetParentNativeViewAccessible(
       gfx::NativeViewAccessible accessible_parent);
@@ -426,7 +435,8 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void OnBeginNavigation(const FrameHostMsg_BeginNavigation_Params& params,
                          const CommonNavigationParams& common_params);
   void OnAccessibilityEvents(
-      const std::vector<AccessibilityHostMsg_EventParams>& params);
+      const std::vector<AccessibilityHostMsg_EventParams>& params,
+      int reset_token);
   void OnAccessibilityLocationChanges(
       const std::vector<AccessibilityHostMsg_LocationChangeParams>& params);
 
@@ -554,12 +564,25 @@ class CONTENT_EXPORT RenderFrameHostImpl
   scoped_ptr<ServiceRegistryAndroid> service_registry_android_;
 #endif
 
+  // The object managing the accessibility tree for this frame.
   scoped_ptr<BrowserAccessibilityManager> browser_accessibility_manager_;
 
+  // This is nonzero if we sent an accessibility reset to the renderer and
+  // we're waiting for an IPC containing this reset token (sequentially
+  // assigned) and a complete replacement accessibility tree.
+  int accessibility_reset_token_;
+
+  // A count of the number of times we needed to reset accessibility, so
+  // we don't keep trying to reset forever.
+  int accessibility_reset_count_;
+
   // Callback when an event is received, for testing.
   base::Callback<void(ui::AXEvent, int)> accessibility_testing_callback_;
   // The most recently received accessibility tree - for testing only.
   scoped_ptr<ui::AXTree> ax_tree_for_testing_;
+  // Flag to not create a BrowserAccessibilityManager, for testing. If one
+  // already exists it will still be used.
+  bool no_create_browser_accessibility_manager_for_testing_;
 
   // PlzNavigate: Owns the stream used in navigations to store the body of the
   // response once it has started.

content/common/accessibility_messages.h

@@ -131,16 +131,32 @@ IPC_MESSAGE_ROUTED1(AccessibilityMsg_HitTest,
 // message was processed and it can send addition events.
 IPC_MESSAGE_ROUTED0(AccessibilityMsg_Events_ACK)
 
-// Kill the renderer because we got a fatal error in the accessibility tree.
+// Tell the renderer to reset and send a new accessibility tree from
+// scratch because the browser is out of sync. It passes a sequential
+// reset token. This should be rare, and if we need reset the same renderer
+// too many times we just kill it. After sending a reset, the browser ignores
+// incoming accessibility IPCs until it receives one with the matching reset
+// token. Conversely, it ignores IPCs with a reset token if it was not
+// expecting a reset.
+IPC_MESSAGE_ROUTED1(AccessibilityMsg_Reset,
+                    int /* reset token */);
+
+// Kill the renderer because we got a fatal error in the accessibility tree
+// and we've already reset too many times.
 IPC_MESSAGE_ROUTED0(AccessibilityMsg_FatalError)
 
 // Messages sent from the renderer to the browser.
 
 // Sent to notify the browser about renderer accessibility events.
 // The browser responds with a AccessibilityMsg_Events_ACK.
-IPC_MESSAGE_ROUTED1(
+// The second parameter, reset_token, is set if this IPC was sent in response
+// to a reset request from the browser. When the browser requests a reset,
+// it ignores incoming IPCs until it sees one with the correct reset token.
+// Any other time, it ignores IPCs with a reset token.
+IPC_MESSAGE_ROUTED2(
     AccessibilityHostMsg_Events,
-    std::vector<AccessibilityHostMsg_EventParams>)
+    std::vector<AccessibilityHostMsg_EventParams> /* events */,
+    int /* reset_token */)
 
 // Sent to update the browser of the location of accessibility objects.
 IPC_MESSAGE_ROUTED1(

content/content_tests.gypi

@@ -1180,6 +1180,7 @@
           ],
           'sources': [
             'app/mojo/mojo_browsertest.cc',
+            'browser/accessibility/accessibility_ipc_error_browsertest.cc',
             'browser/accessibility/accessibility_mode_browsertest.cc',
             'browser/accessibility/accessibility_win_browsertest.cc',
             'browser/accessibility/android_hit_testing_browsertest.cc',

content/renderer/accessibility/renderer_accessibility_browsertest.cc

@@ -59,7 +59,7 @@ class RendererAccessibilityTest : public RenderViewTest {
     const IPC::Message* message =
         sink_->GetUniqueMessageMatching(AccessibilityHostMsg_Events::ID);
     ASSERT_TRUE(message);
-    Tuple1<std::vector<AccessibilityHostMsg_EventParams> > param;
+    Tuple2<std::vector<AccessibilityHostMsg_EventParams>, int> param;
     AccessibilityHostMsg_Events::Read(message, &param);
     ASSERT_GE(param.a.size(), 1U);
     *params = param.a[0];
@@ -406,7 +406,7 @@ TEST_F(RendererAccessibilityTest, EventOnObjectNotInTree) {
   const IPC::Message* message =
       sink_->GetUniqueMessageMatching(AccessibilityHostMsg_Events::ID);
   ASSERT_TRUE(message);
-  Tuple1<std::vector<AccessibilityHostMsg_EventParams> > param;
+  Tuple2<std::vector<AccessibilityHostMsg_EventParams>, int> param;
   AccessibilityHostMsg_Events::Read(message, &param);
   ASSERT_EQ(0U, param.a.size());
 }

content/renderer/accessibility/renderer_accessibility_complete.cc

@@ -39,6 +39,7 @@ RendererAccessibilityComplete::RendererAccessibilityComplete(
       serializer_(&tree_source_),
       last_scroll_offset_(gfx::Size()),
       ack_pending_(false),
+      reset_token_(0),
       weak_factory_(this) {
   WebView* web_view = render_frame_->GetRenderView()->GetWebView();
   WebSettings* settings = web_view->settings();
@@ -80,6 +81,7 @@ bool RendererAccessibilityComplete::OnMessageReceived(
     IPC_MESSAGE_HANDLER(AccessibilityMsg_SetTextSelection,
                         OnSetTextSelection)
     IPC_MESSAGE_HANDLER(AccessibilityMsg_HitTest, OnHitTest)
+    IPC_MESSAGE_HANDLER(AccessibilityMsg_Reset, OnReset)
     IPC_MESSAGE_HANDLER(AccessibilityMsg_FatalError, OnFatalError)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
@@ -247,7 +249,8 @@ void RendererAccessibilityComplete::SendPendingAccessibilityEvents() {
             << "\n" << event_msg.update.ToString();
   }
 
-  Send(new AccessibilityHostMsg_Events(routing_id(), event_msgs));
+  Send(new AccessibilityHostMsg_Events(routing_id(), event_msgs, reset_token_));
+  reset_token_ = 0;
 
   if (had_layout_complete_messages)
     SendLocationChanges();
@@ -434,6 +437,16 @@ void RendererAccessibilityComplete::OnSetFocus(int acc_obj_id) {
     obj.setFocused(true);
 }
 
+void RendererAccessibilityComplete::OnReset(int reset_token) {
+  reset_token_ = reset_token;
+  serializer_.Reset();
+  pending_events_.clear();
+
+  const WebDocument& document = GetMainDocument();
+  if (!document.isNull())
+    HandleAXEvent(document.accessibilityObject(), ui::AX_EVENT_LAYOUT_COMPLETE);
+}
+
 void RendererAccessibilityComplete::OnFatalError() {
   CHECK(false) << "Invalid accessibility tree.";
 }

content/renderer/accessibility/renderer_accessibility_complete.h

@@ -70,6 +70,7 @@ class CONTENT_EXPORT RendererAccessibilityComplete
   void OnSetFocus(int acc_obj_id);
   void OnSetTextSelection(int acc_obj_id, int start_offset, int end_offset);
   void OnHitTest(gfx::Point point);
+  void OnReset(int reset_token);
   void OnFatalError();
 
   // Events from Blink are collected until they are ready to be
@@ -93,6 +94,10 @@ class CONTENT_EXPORT RendererAccessibilityComplete
   // Set if we are waiting for an accessibility event ack.
   bool ack_pending_;
 
+  // Nonzero if the browser requested we reset the accessibility state.
+  // We need to return this token in the next IPC.
+  int reset_token_;
+
   // So we can queue up tasks to be executed later.
   base::WeakPtrFactory<RendererAccessibilityComplete> weak_factory_;