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..1b3617b22ffcd6914d6f7a195fe426aa30a60324 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_++;

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

This is never reset to 0, even when the renderer is killed. The user can reload
a tab with killed renderer, but RFH will not be recreated for the top-level
frame, so this will just keep on killing the renderer.

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

Good point. I reset it in OnDidStartProvisionalLoadForFrame, in manual testing
that seemed to work if I reload a page but not otherwise. Does that look good or
can you think of a better place?

+  if (accessibility_reset_count_ >= kMaxAccessibilityResets) {
+    Send(new AccessibilityMsg_FatalError(routing_id_));
+  } else {
+    Send(new AccessibilityMsg_Reset(routing_id_));

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

Do you want to UMA stat how often this happens? According to the comments, it
should be very rare, so some concrete confirmation through data might be useful.

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

Great idea. I added three counts that should be enough to give us a good sense
of how often this is happening.

+    waiting_on_accessibility_reset_ = true;
+  }
 }
 
 gfx::AcceleratedWidget
@@ -1028,7 +1045,16 @@ 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) {
+    Send(new AccessibilityMsg_Events_ACK(routing_id_));
+    return;
+  }
+  waiting_on_accessibility_reset_ = false;
+
   RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
       render_view_host_->GetView());
 
@@ -1103,6 +1129,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 +1401,9 @@ const ui::AXTree* RenderFrameHostImpl::GetAXTreeForTesting() {
 
 BrowserAccessibilityManager*
     RenderFrameHostImpl::GetOrCreateBrowserAccessibilityManager() {
+  if (disallow_browser_accessibility_manager_for_testing_)
+    return NULL;
+
   RenderWidgetHostViewBase* view = static_cast<RenderWidgetHostViewBase*>(
       render_view_host_->GetView());
   if (view &&