Commit 8f88c4db authored by Ian Prest's avatar Ian Prest Committed by Commit Bot

A11y: Add crash-key to track down AXTree crash

The linked bug is a crash caused by a dangling pointer; we hypothesize
that it occurs in one of the few places where AXTree::Unserialize is
called without an error being treated as fatal.

This change adds a CHECK around one such call (which didn't have any
error handling), and logs some data with a crash-key in a second case
(which had existing error handling, but didn't treat it as fatal). This
will help us prove (or disprove) these scenarios as the culprit.

Bug: 1034755
Change-Id: I1b1164295b25e508c61ef97c1ea0ce093b52ba18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2025836Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarEthan Jimenez <ethavar@microsoft.com>
Reviewed-by: default avatarAaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Ian Prest <iapres@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#736955}
parent e7164eba
...@@ -141,7 +141,8 @@ bool AomContentAxTree::ComputeAccessibilityTree() { ...@@ -141,7 +141,8 @@ bool AomContentAxTree::ComputeAccessibilityTree() {
tree_update.root_id = content_tree_update.root_id; tree_update.root_id = content_tree_update.root_id;
tree_update.nodes.assign(content_tree_update.nodes.begin(), tree_update.nodes.assign(content_tree_update.nodes.begin(),
content_tree_update.nodes.end()); content_tree_update.nodes.end());
return tree_.Unserialize(tree_update); CHECK(tree_.Unserialize(tree_update)) << tree_.error();
return true;
} }
bool AomContentAxTree::GetBoolAttributeForAXNode( bool AomContentAxTree::GetBoolAttributeForAXNode(
......
...@@ -269,6 +269,9 @@ jumbo_source_set("renderer") { ...@@ -269,6 +269,9 @@ jumbo_source_set("renderer") {
"//third_party/zlib/google:compression_utils", "//third_party/zlib/google:compression_utils",
] ]
# Temporarily allow crash_key; see https://crbug.com/1034755
deps += [ "//components/crash/core/common:crash_key" ]
if (proprietary_codecs && enable_wifi_display) { if (proprietary_codecs && enable_wifi_display) {
sources += [ sources += [
"api/display_source/wifi_display/wifi_display_audio_encoder.cc", "api/display_source/wifi_display/wifi_display_audio_encoder.cc",
......
...@@ -42,4 +42,8 @@ specific_include_rules = { ...@@ -42,4 +42,8 @@ specific_include_rules = {
"extension_throttle_unittest.cc": [ "extension_throttle_unittest.cc": [
"+net/url_request/redirect_info.h", "+net/url_request/redirect_info.h",
], ],
# Temporarily allow crash_key; see https://crbug.com/1034755
"automation_ax_tree_wrapper.cc": [
"+components/crash/core/common/crash_key.h",
]
} }
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
#include "base/no_destructor.h" #include "base/no_destructor.h"
#include "components/crash/core/common/crash_key.h"
#include "extensions/common/extension_messages.h" #include "extensions/common/extension_messages.h"
#include "extensions/renderer/api/automation/automation_internal_custom_bindings.h" #include "extensions/renderer/api/automation/automation_internal_custom_bindings.h"
#include "ui/accessibility/ax_language_detection.h" #include "ui/accessibility/ax_language_detection.h"
...@@ -289,6 +290,9 @@ bool AutomationAXTreeWrapper::OnAccessibilityEvents( ...@@ -289,6 +290,9 @@ bool AutomationAXTreeWrapper::OnAccessibilityEvents(
did_send_tree_change_during_unserialization_ = false; did_send_tree_change_during_unserialization_ = false;
if (!tree_.Unserialize(update)) { if (!tree_.Unserialize(update)) {
static crash_reporter::CrashKeyString<4> crash_key(
"ax-tree-wrapper-unserialize-failed");
crash_key.Set("yes");
event_generator_.ClearEvents(); event_generator_.ClearEvents();
return false; return false;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment