Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(99)

Unified Diff: content/browser/frame_host/render_frame_host_impl.cc

Issue 625443002: Reset accessibility if it gets out of sync. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Add reset token, test killing renderer too Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/browser/frame_host/render_frame_host_impl.h ('k') | content/common/accessibility_messages.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 8c06e812ab8d1d2075e3a1634b102a8921cf07d9..91b3cd6500c0e0e3bc90cb2670ea8d8a2fadc1a7 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -10,6 +10,7 @@
#include "base/lazy_instance.h"
#include "base/metrics/histogram.h"
#include "base/metrics/user_metrics_action.h"
+#include "base/rand_util.h"
#include "base/time/time.h"
#include "content/browser/accessibility/accessibility_mode_helper.h"
#include "content/browser/accessibility/browser_accessibility_manager.h"
@@ -68,6 +69,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*>
@@ -187,6 +193,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),
+ disallow_browser_accessibility_manager_for_testing_(false),
weak_ptr_factory_(this) {
frame_tree_->RegisterRenderFrameHost(this);
GetProcess()->AddRoute(routing_id_, this);
@@ -472,8 +481,20 @@ 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 {
+ // Generate a random reset token and ignore accessibility until we get a
+ // response from the renderer with this token.
+ accessibility_reset_token_ = base::RandInt(1, INT_MAX);
Tom Sepez 2014/10/02 21:56:49 This is probably overkill, and there is always the
dmazzoni 2014/10/02 22:01:24 No problem, replaced with sequential.
+ UMA_HISTOGRAM_COUNTS("Accessibility.FrameResetCount", 1);
+ Send(new AccessibilityMsg_Reset(routing_id_, accessibility_reset_token_));
+ }
}
gfx::AcceleratedWidget
@@ -645,6 +666,7 @@ void RenderFrameHostImpl::OnDidStartProvisionalLoadForFrame(
bool is_transition_navigation) {
frame_tree_node_->navigator()->DidStartProvisionalLoad(
this, url, is_transition_navigation);
+ accessibility_reset_count_ = 0;
}
void RenderFrameHostImpl::OnDidFailProvisionalLoadWithError(
@@ -1061,7 +1083,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());
@@ -1136,6 +1168,9 @@ 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())) {
@@ -1446,12 +1481,18 @@ 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 &&
- !browser_accessibility_manager_) {
+ if (view && !browser_accessibility_manager_) {
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();
}
« no previous file with comments | « content/browser/frame_host/render_frame_host_impl.h ('k') | content/common/accessibility_messages.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698