Add DumpWithoutCrashing to internal a11y errors in serialization

Use DumpWithoutCrashing() to get reports for problematic content.
Also, add information to the crash reports about objects that are
involved, and include information about commonly problematic properties.

Note: DumpWithoutCrashing() once added to AXTreeSerializer once before
but was reverted due to too many reports.

AX-RelNotes: n/a
Bug: None
Change-Id: Iac5f0ba377e03881efdb6734a954fbb9f1f3125c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2480624
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Daniel Libby <dlibby@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#819640}

content/browser/renderer_host/render_frame_host_impl.cc

@@ -1906,8 +1906,15 @@ void RenderFrameHostImpl::AccessibilityFatalError() {
 
   accessibility_reset_count_++;
   if (accessibility_reset_count_ > max_accessibility_resets_) {
+    // This will both create an "Aw Snap..." and generate a second crash report
+    // in addition to the DumpWithoutCrashing() for the first reset.
     render_accessibility_->FatalError();
   } else {
+    // Crash keys set in BrowserAccessibilityManager::Unserialize().
+    if (accessibility_reset_count_ == 1) {
+      // Only send crash report first time -- don't skew crash stats too much.
+      base::debug::DumpWithoutCrashing();
+    }
     AccessibilityReset();
   }
 }

third_party/blink/renderer/modules/accessibility/ax_object.cc

@@ -4612,6 +4612,32 @@ String AXObject::ToString(bool verbose) const {
       string_builder = string_builder + ">";
     }
 
+    // Add properties of interest that often contribute to errors:
+    if (SupportsARIAOwns())
+      string_builder = string_builder + " @aria-owns";
+    if (GetAOMPropertyOrARIAAttribute(AOMRelationProperty::kActiveDescendant))
+      string_builder = string_builder + " @aria-activedescendant";
+    if (IsFocused())
+      string_builder = string_builder + " focused";
+    if (AXObjectCache().IsAriaOwned(this))
+      string_builder = string_builder + " isAriaOwned";
+    if (AccessibilityIsIgnored()) {
+      string_builder = string_builder + " isIgnored";
+      if (!AccessibilityIsIncludedInTree())
+        string_builder = string_builder + " isRemovedFromTree";
+    }
+    if (GetNode() &&
+        DisplayLockUtilities::ShouldIgnoreNodeDueToDisplayLock(
+            *GetNode(), DisplayLockActivationReason::kAccessibility)) {
+      string_builder = string_builder + " isDisplayLocked";
+    }
+    if (AriaHiddenRoot())
+      string_builder = string_builder + " isAriaHidden";
+    if (IsHiddenViaStyle())
+      string_builder = string_builder + " isHiddenViaCSS";
+    if (GetNode() && GetNode()->IsInert())
+      string_builder = string_builder + " isInert";
+
     string_builder = string_builder + " name=";
   } else {
     string_builder = string_builder + ": ";

ui/accessibility/ax_tree_serializer.h

@@ -8,10 +8,13 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <ostream>
 #include <unordered_map>
 #include <unordered_set>
 #include <vector>
 
+#include "base/debug/crash_logging.h"
+#include "base/debug/dump_without_crashing.h"
 #include "base/logging.h"
 #include "ui/accessibility/ax_common.h"
 #include "ui/accessibility/ax_export.h"
@@ -393,11 +396,25 @@ ClientTreeNode*
 AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::GetClientTreeNodeParent(
     ClientTreeNode* obj) {
   ClientTreeNode* parent = obj->parent;
-#if defined(AX_FAIL_FAST_BUILD)
   if (!parent)
     return nullptr;
-  DCHECK(ClientTreeNodeById(parent->id)) << "Parent not in id map.";
+  if (!ClientTreeNodeById(parent->id)) {
+    std::ostringstream error;
+    error << "Parent not in id map. Child was: "
+          << tree_->GetDebugString(tree_->GetFromId(obj->id))
+          << "\n  Parent was: "
+          << tree_->GetDebugString(tree_->GetFromId(parent->id));
+    static auto* ax_tree_serializer_missing_parent_id_error =
+        base::debug::AllocateCrashKeyString(
+            "ax_tree_serializer_missing_parent_id_error",
+            base::debug::CrashKeySize::Size64);
+    base::debug::SetCrashKeyString(ax_tree_serializer_missing_parent_id_error,
+                                   error.str());
+#if defined(AX_FAIL_FAST_BUILD)
+    CHECK(false);
 #endif  // defined(AX_FAIL_FAST_BUILD)
+    base::debug::DumpWithoutCrashing();
+  }
   return parent;
 }
 
@@ -580,21 +597,26 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
 
     ClientTreeNode* client_child = ClientTreeNodeById(new_child_id);
     if (client_child && GetClientTreeNodeParent(client_child) != client_node) {
-#if defined(AX_FAIL_FAST_BUILD)
-      // TODO(accessibility) Remove all cases where this occurs and re-add
-      // NOTREACHED(), or add a histogram.
       // This condition leads to performance problems. It will
       // also reset virtual buffers, causing users to lose their place.
-      NOTREACHED()
-#else
-      LOG(ERROR)
+      std::ostringstream error;
+      error << "Illegal reparenting detected: "
+            << "\nPassed-in parent: "
+            << tree_->GetDebugString(tree_->GetFromId(client_node->id))
+            << "\nChild: " << tree_->GetDebugString(child)
+            << "\nChild's parent: "
+            << tree_->GetDebugString(tree_->GetFromId(client_child->parent->id))
+            << "\n-----------------------------------------\n\n\n";
+      static auto* ax_tree_serializer_illegal_reparenting_error =
+          base::debug::AllocateCrashKeyString(
+              "ax_tree_serializer_illegal_reparenting_error",
+              base::debug::CrashKeySize::Size64);
+      base::debug::SetCrashKeyString(
+          ax_tree_serializer_illegal_reparenting_error, error.str());
+#if defined(AX_FAIL_FAST_BUILD)
+      CHECK(false);
 #endif  // defined(AX_FAIL_FAST_BUILD)
-          << "Illegal reparenting detected: "
-          << "\nPassed-in parent: "
-          << tree_->GetDebugString(tree_->GetFromId(client_node->id))
-          << "\nChild: " << tree_->GetDebugString(child) << "\nChild's parent: "
-          << tree_->GetDebugString(tree_->GetFromId(client_child->parent->id))
-          << "\n-----------------------------------------\n\n\n";
+      base::debug::DumpWithoutCrashing();
       Reset();
       return false;
     }
@@ -674,22 +696,27 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
       new_child->invalid = false;
       client_node->children.push_back(new_child);
       if (ClientTreeNodeById(child_id)) {
-#if defined(AX_FAIL_FAST_BUILD)
         // TODO(accessibility) Remove all cases where this occurs and re-add
-        // NOTREACHED(), or add a histogram.
         // This condition leads to performance problems. It will
         // also reset virtual buffers, causing users to lose their place.
-        NOTREACHED()
-#else
-        LOG(ERROR)
+        std::ostringstream error;
+        error << "Child id " << child_id << " already exists in map."
+              << "\nChild is "
+              << tree_->GetDebugString(tree_->GetFromId(child_id))
+              << "\nWanted as child of parent " << tree_->GetDebugString(node)
+              << "\nAlready had parent "
+              << tree_->GetDebugString(tree_->GetFromId(
+                     ClientTreeNodeById(child_id)->parent->id));
+        static auto* ax_tree_serializer_duplicate_id_error =
+            base::debug::AllocateCrashKeyString(
+                "ax_tree_serializer_duplicate_id_error",
+                base::debug::CrashKeySize::Size64);
+        base::debug::SetCrashKeyString(ax_tree_serializer_duplicate_id_error,
+                                       error.str());
+#if defined(AX_FAIL_FAST_BUILD)
+        CHECK(false);
 #endif  // defined(AX_FAIL_FAST_BUILD)
-            << "Child id " << child_id << " already exists in map."
-            << "\nChild is "
-            << tree_->GetDebugString(tree_->GetFromId(child_id))
-            << "\nWanted as child of parent " << tree_->GetDebugString(node)
-            << "\nAlready had parent "
-            << tree_->GetDebugString(
-                   tree_->GetFromId(ClientTreeNodeById(child_id)->parent->id));
+        base::debug::DumpWithoutCrashing();
         Reset();
         return false;
       }