Reset accessibility if it gets out of sync.

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 585fb225282f1448ba825bd80d6f1509fe1bc9d8..e3ff7cdfb1a729a322490c0ea0f8bce056a6ca8a 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -68,6 +68,11 @@ namespace content {
 
 namespace {
 
+// 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.
+const int kMaxAccessibilityResets = 5;
+
 // The (process id, routing id) pair that identifies one RenderFrame.
 typedef std::pair<int32, int32> RenderFrameHostID;
 typedef base::hash_map<RenderFrameHostID, RenderFrameHostImpl*>
@@ -180,6 +185,9 @@ RenderFrameHostImpl::RenderFrameHostImpl(RenderViewHostImpl* render_view_host,
       is_swapped_out_(is_swapped_out),
       render_frame_created_(false),
       navigations_suspended_(false),
+      waiting_on_accessibility_reset_(false),
+      accessibility_reset_count_(0),
+      disallow_browser_accessibility_manager_for_testing_(false),
       weak_ptr_factory_(this) {
   frame_tree_->RegisterRenderFrameHost(this);
   GetProcess()->AddRoute(routing_id_, this);
@@ -453,8 +461,17 @@ void RenderFrameHostImpl::AccessibilityHitTest(const gfx::Point& point) {
 }
 
 void RenderFrameHostImpl::AccessibilityFatalError() {
-  Send(new AccessibilityMsg_FatalError(routing_id_));
   browser_accessibility_manager_.reset(NULL);
+  if (waiting_on_accessibility_reset_)
+    return;
+
+  accessibility_reset_count_++;
+  if (accessibility_reset_count_ >= kMaxAccessibilityResets) {
+    Send(new AccessibilityMsg_FatalError(routing_id_));
+  } else {
+    Send(new AccessibilityMsg_Reset(routing_id_));
+    waiting_on_accessibility_reset_ = true;
+  }
 }
 
 gfx::AcceleratedWidget
@@ -1028,7 +1045,17 @@ void RenderFrameHostImpl::OnBeginNavigation(
 }
 
 void RenderFrameHostImpl::OnAccessibilityEvents(
-    const std::vector<AccessibilityHostMsg_EventParams>& params) {
+    const std::vector<AccessibilityHostMsg_EventParams>& params,
+    bool is_reset) {
+  // Don't process this IPC if either we're waiting on a reset and this
+  // isn't a reset, or if we're not waiting on a reset but this is a reset.
+  if (waiting_on_accessibility_reset_ != is_reset) {
+    LOG(ERROR) << " *** Browser skipping *** ";

[Mike West, 2014/10/02 06:27:48]

Perhaps you could consider crashing the renderer if you get a reset message
while you're not waiting for a reset. That sounds like a good way for the
browser and renderer to get out of sync.

Also a nit: The log message should probably contain detail about what the
browser is skipping.

[dmazzoni, 2014/10/02 06:40:17]

Well, part of the point of this change is to avoid crashing the renderer,
because when we do it's almost impossible to decipher the crash logs. I'm hoping
that just ignoring those IPCs is pretty safe, that prevents them from being used
as an attack vector.
Oops, I didn't mean to leave the log in at all.

+    Send(new AccessibilityMsg_Events_ACK(routing_id_));
+    return;
+  }
+  waiting_on_accessibility_reset_ = false;
+
   RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
       render_view_host_->GetView());
 
@@ -1103,6 +1130,9 @@ void RenderFrameHostImpl::OnAccessibilityEvents(
 
 void RenderFrameHostImpl::OnAccessibilityLocationChanges(
     const std::vector<AccessibilityHostMsg_LocationChangeParams>& params) {
+  if (waiting_on_accessibility_reset_)
+    return;
+
   RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
       render_view_host_->GetView());
   if (view &&
@@ -1372,6 +1402,9 @@ const ui::AXTree* RenderFrameHostImpl::GetAXTreeForTesting() {
 
 BrowserAccessibilityManager*
     RenderFrameHostImpl::GetOrCreateBrowserAccessibilityManager() {
+  if (disallow_browser_accessibility_manager_for_testing_)
+    return NULL;

[Mike West, 2014/10/02 06:27:48]

Nit: nullptr?

+
   RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
       render_view_host_->GetView());
   if (view &&