Temporarily disable extra mac nodes to fix table crashes

This is temporary until we have a proper fix.

Bug: 1052334,1050598
Change-Id: I6e9370dbac191acea5543d3d015b19f75554e48b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062878
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742806}

content/browser/accessibility/browser_accessibility_mac_unittest.mm

@@ -241,6 +241,9 @@ TEST_F(BrowserAccessibilityMacTest, TestComputeTextEdit) {
 
 // Test Mac-specific table APIs.
 TEST_F(BrowserAccessibilityMacTest, TableAPIs) {
+  // This line is needed until we turn extra mac nodes back on by default.
+  BrowserAccessibilityManager::AllowExtraMacNodesForTesting();
+
   ui::AXTreeUpdate initial_state;
   initial_state.root_id = 1;
   initial_state.nodes.resize(7);

content/browser/accessibility/browser_accessibility_manager.cc

@@ -224,6 +224,9 @@ void BrowserAccessibilityManager::Initialize(
 bool BrowserAccessibilityManager::never_suppress_or_delay_events_for_testing_ =
     false;
 
+// A flag for use in tests to indicate that extra mac nodes are allowed.
+bool BrowserAccessibilityManager::allow_extra_mac_nodes_for_testing_ = false;
+
 // static
 base::Optional<int32_t> BrowserAccessibilityManager::last_focused_node_id_ = {};
 
@@ -691,6 +694,16 @@ void BrowserAccessibilityManager::NeverSuppressOrDelayEventsForTesting() {
   never_suppress_or_delay_events_for_testing_ = true;
 }
 
+// static
+void BrowserAccessibilityManager::AllowExtraMacNodesForTesting() {
+  allow_extra_mac_nodes_for_testing_ = true;
+}
+
+// static
+bool BrowserAccessibilityManager::GetExtraMacNodesAllowed() {
+  return allow_extra_mac_nodes_for_testing_;
+}
+
 void BrowserAccessibilityManager::Decrement(const BrowserAccessibility& node) {
   if (!delegate_)
     return;

content/browser/accessibility/browser_accessibility_manager.h

@@ -237,6 +237,12 @@ class CONTENT_EXPORT BrowserAccessibilityManager : public ui::AXTreeObserver,
   // had focus.
   static void NeverSuppressOrDelayEventsForTesting();
 
+  // Extra mac nodes are temporarily disabled, except for in tests.
+  static void AllowExtraMacNodesForTesting();
+  // Are extra mac nodes allowed at all? Currently only allowed in tests.
+  // Even when returning true, platforms other than Mac OS do not enable them.
+  static bool GetExtraMacNodesAllowed();
+
   // Accessibility actions. All of these are implemented asynchronously
   // by sending a message to the renderer to perform the respective action
   // on the given node.  See the definition of |ui::AXActionData| for more
@@ -533,6 +539,9 @@ class CONTENT_EXPORT BrowserAccessibilityManager : public ui::AXTreeObserver,
   // flakiness. See NeverSuppressOrDelayEventsForTesting() for details.
   static bool never_suppress_or_delay_events_for_testing_;
 
+  // Extra mac nodes are disabled even on mac currently, except for in tests.
+  static bool allow_extra_mac_nodes_for_testing_;
+
   const ui::AXEventGenerator& event_generator() const {
     return event_generator_;
   }

content/browser/accessibility/browser_accessibility_manager_mac.mm

@@ -123,7 +123,10 @@ BrowserAccessibilityManagerMac::BrowserAccessibilityManagerMac(
     BrowserAccessibilityFactory* factory)
     : BrowserAccessibilityManager(delegate, factory) {
   Initialize(initial_tree);
-  ax_tree()->SetEnableExtraMacNodes(true);
+  // Temporary fix. Disable extra mac nodes, which only affects column
+  // navigation but fixes a number of crash bugs seen only with VoiceOver.
+  // This does not affect verbalization of columns headers in cell navigation.
+  ax_tree()->SetEnableExtraMacNodes(GetExtraMacNodesAllowed());
 }
 
 BrowserAccessibilityManagerMac::~BrowserAccessibilityManagerMac() {}

content/browser/accessibility/dump_accessibility_browsertest_base.cc

@@ -241,6 +241,10 @@ void DumpAccessibilityTestBase::RunTestForPlatform(
   // flaky.
   BrowserAccessibilityManager::NeverSuppressOrDelayEventsForTesting();
 
+  // Extra mac nodes are disabled temporarily for stability purposes, but keep
+  // them on for tests.
+  BrowserAccessibilityManager::AllowExtraMacNodesForTesting();
+
   EXPECT_TRUE(NavigateToURL(shell(), GURL(url::kAboutBlankURL)));
 
   // Exit without running the test if we can't find an expectation file.