Revert "Reland "[A11yPerformance] Clean up BrowserAccessibilityState""

This reverts commit 717eefc40c.

Reason for revert: broke deactivation when VoiceOver is stopped

Original change's description:
> Reland "[A11yPerformance] Clean up BrowserAccessibilityState"
>
> Problem and fix:
> The test NewTabPageAppA11yTest.Click failed as mentioned in
> crbug.com/404576872 failed because there were
> two call sites setting the process accessibility mode directly,
> basically competing with each other. This only happens after
> the CL landed where we are removing outdated
> OnScreenReaderDetected/Stopped methods and replacing them
> with close equivalents. However, these two methods were not
> symmetrical in the old code. One enabled accessibility, and one
> potentially disabled accessibility after a short delay.
>
> The modern way to set an accessibility mode, which fixes this issue,
> is to use ScopedAccessibilityMode. When it goes out of scope it
> doesn't destroy the a11y mode set by any other piece of code.
> A follow-up is to move all code setting a process accessibility mode
> to ScopedAccessibilityMode.
>
> Original change's description:
> > Revert "[A11yPerformance] Clean up BrowserAccessibilityState"
> >
> > This reverts commit 8c794f38d3.
> >
> > Reason for revert: breaks test on multiple Mac bots crbug.com/404576872
> >
> > Original change's description:
> > > [A11yPerformance] Clean up BrowserAccessibilityState
> > >
> > > 1. Clarify which methods/variables are related to Auto Disable as
> > >    opposed to just Disable (manually disabling of a11y), so that
> > >    it's no longer confusing what belongs to what intention.
> > >
> > > 2. Rename some methods to accurately reflect what they do,
> > >   and add comments:
> > > * IsRendererAccessibilityEnabled() -> IsAccessibilityAllowed().
> > >   This was not specific to renderers, or whether a11y was enabled.
> > >   It returned true if a11y was not disallowed via the command line.
> > > * EnableAccessibility() -> EnableCompleteAccessibility().
> > >   This turns on kAXModeComplete.
> > >
> > > 3. Remove some poorly named and redundant methods that are not
> > >    actually needed:
> > > * ResetAccessibilityMode() -- the same as DisableAccessibility().
> > > * OnScreenReaderDetected() -- did not actually reflect screen
> > >   reader detection and was just a way to EnableAccessibility(), so
> > >   is redundant with that. Any remaining methods and modes with
> > >   "screen reader" in the name will now make sense.
> > > * OnScreenReaderStopped() -- did not actually reflect the stoppage
> > >   of a screen reader and callers are better off using
> > >   DisableAccessibility().
> > >
> > > 4. The new UMA Accessibility.EngineUse.TimeUntilStart that was just
> > >    added was not working on all platforms, because some platforms
> > >    use AddAccessibilityModeFlags() and others use
> > >    EnableAccessibility(). Move the UMA code to
> > >    OnAccessibilityAPIUsage() where it will always be hit.
> > >
> > > This relates to a11y performance because it helps build the
> > > foundation for the auto disable refresh by clarifying what
> > > different pieces do and cleaning up code paths.
> > >
> > > Follow-ups:
> > > - Audit calls to OnAccessibilityAPIUsed(), such as from
> > > AXPlatformNodeDelegate::GetRole(). If we don't change that, we'll
> > > always think that there's new API usage every time we Unserialize
> > > from a renderer, which breaks the auto-disable heuristic.
> > > - Use ScopedAccessibilityMode for chrome/browser/apps/app_shim/app_shim_host_mac.cc.
> > >
> > > Bug: 401232343
> > > Change-Id: Id386c378812484aae01e2a93dac90336c0aa7ae3
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6333854
> > > Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
> > > Reviewed-by: Greg Thompson <grt@chromium.org>
> > > Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
> > > Reviewed-by: Nasko Oskov <nasko@chromium.org>
> > > Cr-Commit-Position: refs/heads/main@{#1434483}
> >
> > Bug: 401232343,404576872
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Change-Id: I6d43b0fa9d36b7ee0adeaf38b2b04e2b044eea60
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6367795
> > Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
> > Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
> > Commit-Queue: Ming-Ying Chung <mych@chromium.org>
> > Owners-Override: Ming-Ying Chung <mych@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1434613}
>
> Bug: 401232343,404576872,404576872
> Change-Id: I1192fe2310402a84f47e4c89114bba8d30ee733f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6373060
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Reviewed-by: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#1435606}

Bug: 401232343,404576872,404576872
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: I565a21249c59a07cecec635a6249a369b6529ccb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6380873
Reviewed-by: Ming-Ying Chung <mych@chromium.org>
Reviewed-by: Jiacheng Guo <gjc@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Peter Pakkenberg <pbirk@chromium.org>
Auto-Submit: Greg Thompson <grt@chromium.org>
Owners-Override: Peter Pakkenberg <pbirk@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1435995}

chrome/browser/accessibility/accessibility_extension_api_ash.cc

@@ -948,11 +948,9 @@ AccessibilityPrivateSetNativeAccessibilityEnabledFunction::Run() {
   EXTENSION_FUNCTION_VALIDATE(args()[0].is_bool());
   bool enabled = args()[0].GetBool();
   if (enabled) {
-    content::BrowserAccessibilityState::GetInstance()
-        ->EnableProcessAccessibility();
+    content::BrowserAccessibilityState::GetInstance()->EnableAccessibility();
   } else {
-    content::BrowserAccessibilityState::GetInstance()
-        ->DisableProcessAccessibility();
+    content::BrowserAccessibilityState::GetInstance()->DisableAccessibility();
   }
   return RespondNow(NoArguments());
 }

chrome/browser/apps/app_shim/app_shim_host_mac.cc

@@ -265,13 +265,14 @@ void AppShimHost::EnableAccessibilitySupport(
       content::BrowserAccessibilityState::GetInstance();
   switch (mode) {
     case chrome::mojom::AppShimScreenReaderSupportMode::kComplete: {
-      process_accessibility_mode_ =
-          accessibility_state->CreateScopedModeForProcess(ui::kAXModeComplete);
+      accessibility_state->OnScreenReaderDetected();
       break;
     }
     case chrome::mojom::AppShimScreenReaderSupportMode::kPartial: {
-      process_accessibility_mode_ =
-          accessibility_state->CreateScopedModeForProcess(ui::kAXModeBasic);
+      if (!accessibility_state->GetAccessibilityMode().has_mode(
+              ui::kAXModeBasic.flags())) {
+        accessibility_state->AddAccessibilityModeFlags(ui::kAXModeBasic);
+      }
       break;
     }
   }

chrome/browser/apps/app_shim/app_shim_host_mac.h

@@ -17,7 +17,6 @@
 #include "chrome/browser/web_applications/os_integration/mac/app_shim_launch.h"
 #include "chrome/common/mac/app_shim.mojom.h"
 #include "components/metrics/histogram_child_process.h"
-#include "content/public/browser/scoped_accessibility_mode.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
@@ -206,10 +205,6 @@ class AppShimHost : public chrome::mojom::AppShimHost,
   // This class is only ever to be used on the UI thread.
   THREAD_CHECKER(thread_checker_);
 
-  // Will be created if accessibility APIs are needed, e.g. if the VoiceOver
-  // screen reader is enabled.
-  std::unique_ptr<content::ScopedAccessibilityMode> process_accessibility_mode_;
-
   // This weak factory is used for launch callbacks only.
   base::WeakPtrFactory<AppShimHost> launch_weak_factory_;
 };

chrome/browser/chrome_browser_application_mac.mm

@@ -23,9 +23,7 @@
 #include "content/public/browser/browser_accessibility_state.h"
 #include "content/public/browser/native_event_processor_mac.h"
 #include "content/public/browser/native_event_processor_observer_mac.h"
-#include "content/public/browser/scoped_accessibility_mode.h"
 #include "content/public/common/content_features.h"
-#include "ui/accessibility/ax_mode.h"
 #include "ui/base/cocoa/accessibility_focus_overrider.h"
 
 namespace chrome_browser_application_mac {
@@ -147,7 +145,6 @@ std::string DescriptionForNSEvent(NSEvent* event) {
   base::ObserverList<content::NativeEventProcessorObserver>::Unchecked
       _observers;
   BOOL _handlingSendEvent;
-  std::unique_ptr<content::ScopedAccessibilityMode> _scoped_accessibility_mode;
 }
 
 + (void)initialize {
@@ -459,14 +456,13 @@ std::string DescriptionForNSEvent(NSEvent* event) {
 // Accessibility Support
 
 - (void)enableScreenReaderCompleteMode:(BOOL)enable {
+  content::BrowserAccessibilityState* accessibility_state =
+      content::BrowserAccessibilityState::GetInstance();
+
   if (enable) {
-    if (!_scoped_accessibility_mode) {
-      _scoped_accessibility_mode =
-          content::BrowserAccessibilityState::GetInstance()
-              ->CreateScopedModeForProcess(ui::kAXModeComplete);
-    }
+    accessibility_state->OnScreenReaderDetected();
   } else {
-    _scoped_accessibility_mode.reset();
+    accessibility_state->OnScreenReaderStopped();
   }
 }
 

chrome/browser/ui/webui/accessibility/accessibility_ui.cc

@@ -195,8 +195,8 @@ void HandleAccessibilityRequestCallback(
   PrefService* pref = Profile::FromBrowserContext(current_context)->GetPrefs();
   ui::AXMode mode =
       content::BrowserAccessibilityState::GetInstance()->GetAccessibilityMode();
-  bool is_a11y_allowed = content::BrowserAccessibilityState::GetInstance()
-                             ->IsAccessibilityAllowed();
+  bool is_native_enabled = content::BrowserAccessibilityState::GetInstance()
+                               ->IsRendererAccessibilityEnabled();
   bool native = mode.has_mode(ui::AXMode::kNativeAPIs);
   bool web = mode.has_mode(ui::AXMode::kWebContents);
   bool text = mode.has_mode(ui::AXMode::kInlineTextBoxes);
@@ -206,12 +206,12 @@ void HandleAccessibilityRequestCallback(
 
   // The "native" and "web" flags are disabled if
   // --disable-renderer-accessibility is set.
-  data.Set(kNative, is_a11y_allowed ? (native ? kOn : kOff) : kDisabled);
-  data.Set(kWeb, is_a11y_allowed ? (web ? kOn : kOff) : kDisabled);
+  data.Set(kNative, is_native_enabled ? (native ? kOn : kOff) : kDisabled);
+  data.Set(kWeb, is_native_enabled ? (web ? kOn : kOff) : kDisabled);
 
   // The "text", "extendedProperties" and "html" flags are only
   // meaningful if "web" is enabled.
-  bool is_web_enabled = is_a11y_allowed && web;
+  bool is_web_enabled = is_native_enabled && web;
   data.Set(kText, is_web_enabled ? (text ? kOn : kOff) : kDisabled);
   data.Set(kExtendedProperties,
            is_web_enabled ? (extendedProperties ? kOn : kOff) : kDisabled);
@@ -280,7 +280,7 @@ void HandleAccessibilityRequestCallback(
     }
 
     base::Value::Dict descriptor = BuildTargetDescriptor(rvh);
-    descriptor.Set(kNative, is_a11y_allowed);
+    descriptor.Set(kNative, is_native_enabled);
     descriptor.Set(kExtendedProperties, is_web_enabled && extendedProperties);
     descriptor.Set(kWeb, is_web_enabled);
     page_list.Append(std::move(descriptor));

content/browser/accessibility/accessibility_mode_browsertest.cc

@@ -335,7 +335,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityModeTest, ReEnablingDoesNotAlterUniqueIds) {
   int32_t unique_id_2 = button_2->GetAXPlatformNode()->GetUniqueId();
 
   // Turn accessibility off again.
-  BrowserAccessibilityState::GetInstance()->DisableProcessAccessibility();
+  BrowserAccessibilityState::GetInstance()->ResetAccessibilityMode();
   accessibility_mode = web_contents()->GetAccessibilityMode();
   ASSERT_TRUE(accessibility_mode.is_mode_off());
   EXPECT_EQ(nullptr, GetManager());

content/browser/accessibility/accessibility_win_browsertest.cc

@@ -2342,8 +2342,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
   EXPECT_TRUE(found);
 
   // Remove all accessibility modes.
-  content::BrowserAccessibilityState::GetInstance()
-      ->DisableProcessAccessibility();
+  content::BrowserAccessibilityState::GetInstance()->ResetAccessibilityMode();
 
   // Ensure accessibility is not enabled before we begin the test.
   EXPECT_TRUE(content::BrowserAccessibilityStateImpl::GetInstance()
@@ -6556,7 +6555,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
 
   WebContentsImpl* web_contents =
       static_cast<WebContentsImpl*>(shell()->web_contents());
-  BrowserAccessibilityState::GetInstance()->DisableProcessAccessibility();
+  BrowserAccessibilityState::GetInstance()->ResetAccessibilityMode();
   auto accessibility_mode = web_contents->GetAccessibilityMode();
   ASSERT_TRUE(accessibility_mode.is_mode_off());
   EXPECT_EQ(nullptr, GetManager());
@@ -6584,7 +6583,7 @@ IN_PROC_BROWSER_TEST_F(AccessibilityWinBrowserTest,
   int32_t unique_id_2 = button_2->GetAXPlatformNode()->GetUniqueId();
 
   // Turn accessibility off again.
-  BrowserAccessibilityState::GetInstance()->DisableProcessAccessibility();
+  BrowserAccessibilityState::GetInstance()->ResetAccessibilityMode();
   accessibility_mode = web_contents->GetAccessibilityMode();
   ASSERT_TRUE(accessibility_mode.is_mode_off());
   EXPECT_EQ(nullptr, GetManager());

content/browser/accessibility/browser_accessibility_state_impl.cc

@@ -50,6 +50,13 @@ constexpr int kAutoDisableAccessibilityEventCount = 3;
 // good for perf. Instead, delay the update task.
 constexpr int kOnAccessibilityUsageUpdateDelaySecs = 5;
 
+// How long to wait after `OnScreenReaderStopped` was called before actually
+// disabling accessibility support. The main use case is when a screen reader
+// or other client is toggled off and on in rapid succession. We don't want to
+// destroy the full accessibility tree only to immediately recreate it because
+// doing so is bad for performance.
+constexpr int kDisableAccessibilitySupportDelaySecs = 2;
+
 // Used for validating the 'basic' bundle parameter for
 // --force-renderer-accessibility.
 const char kAXModeBundleBasic[] = "basic";
@@ -267,6 +274,32 @@ BrowserAccessibilityStateImpl::~BrowserAccessibilityStateImpl() {
   g_instance = nullptr;
 }
 
+void BrowserAccessibilityStateImpl::OnScreenReaderDetected() {
+  // Clear any previous, now obsolete, request to disable support.
+  disable_accessibility_request_time_ = base::TimeTicks();
+
+  if (!allow_ax_mode_changes_) {
+    return;
+  }
+  EnableAccessibility();
+}
+
+void BrowserAccessibilityStateImpl::OnScreenReaderStopped() {
+  disable_accessibility_request_time_ = ui::EventTimeForNow();
+
+  // If a screen reader or other client using accessibility API is toggled off
+  // and on in short succession, we risk destroying and recreating large
+  // accessibility trees unnecessarily which is bad for performance. So we post
+  // a delayed task here, and only reset accessibility mode if nothing has
+  // requested accessibility support be re-enabled after that delay has passed.
+  GetUIThreadTaskRunner({})->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(
+          &BrowserAccessibilityStateImpl::MaybeResetAccessibilityMode,
+          weak_factory_.GetWeakPtr()),
+      base::Seconds(kDisableAccessibilitySupportDelaySecs));
+}
+
 void BrowserAccessibilityStateImpl::OnAssistiveTechFound(
     ui::AssistiveTech assistive_tech) {
   ax_platform_.NotifyAssistiveTechChanged(assistive_tech);
@@ -281,16 +314,66 @@ ui::AssistiveTech BrowserAccessibilityStateImpl::ActiveAssistiveTech() const {
   return ui::AXPlatform::GetInstance().active_assistive_tech();
 }
 
-void BrowserAccessibilityStateImpl::EnableProcessAccessibility() {
-  SetProcessMode(ui::kAXModeComplete);
+void BrowserAccessibilityStateImpl::EnableAccessibility() {
+  if (!allow_ax_mode_changes_) {
+    return;
+  }
+
+  // Track the time since start-up before the kWebContents mode was enabled,
+  // ensuring we record this value only one time.
+  if (!has_enabled_accessibility_in_session_ &&
+      GetAccessibilityMode().has_mode(ui::AXMode::kWebContents)) {
+    has_enabled_accessibility_in_session_ = true;
+    UMA_HISTOGRAM_LONG_TIMES_100("Accessibility.EngineUse.TimeUntilStart",
+                                 timer_.Elapsed());
+  }
+
+  // Enabling accessibility is generally the result of an accessibility API
+  // call, so we should also reset the auto-disable accessibility code. The only
+  // exception is in tests or when a user manually toggles accessibility flags
+  // in chrome://accessibility.
+  OnAccessibilityApiUsage();
+
+  const ui::AXMode previous_mode = process_accessibility_mode_->mode();
+
+  // First disable any non-additive modes that restrict or filter the
+  // information available in the tree.
+  const ui::AXMode new_mode =
+      (previous_mode & ~ui::kAXModeFormControls) | ui::kAXModeComplete;
+
+  process_accessibility_mode_ = CreateScopedModeForProcess(new_mode);
 }
 
-bool BrowserAccessibilityStateImpl::IsAccessibilityAllowed() {
+void BrowserAccessibilityStateImpl::DisableAccessibility() {
+  ResetAccessibilityMode();
+}
+
+bool BrowserAccessibilityStateImpl::IsRendererAccessibilityEnabled() {
   return !base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kDisableRendererAccessibility);
 }
 
-void BrowserAccessibilityStateImpl::DisableProcessAccessibility() {
+void BrowserAccessibilityStateImpl::MaybeResetAccessibilityMode() {
+  // `OnScreenReaderStopped` sets `disable_accessibility_request_time_`, and
+  // `OnScreenReaderDetected` clears it. If we no longer have a request time
+  // to disable accessibility, this delayed task is obsolete.
+  if (disable_accessibility_request_time_.is_null()) {
+    return;
+  }
+
+  // `OnScreenReaderStopped` could be called multiple times prior to the delay
+  // expiring. The value of `disable_accessibility_request_time_` is updated
+  // for every call. If we're running this task prior to the delay expiring,
+  // this request time to disable accessibility is obsolete.
+  if ((base::TimeTicks::Now() - disable_accessibility_request_time_) <
+      base::Seconds(kDisableAccessibilitySupportDelaySecs)) {
+    return;
+  }
+
+  ResetAccessibilityMode();
+}
+
+void BrowserAccessibilityStateImpl::ResetAccessibilityMode() {
   SetProcessMode(ui::AXMode());
 }
 
@@ -322,6 +405,10 @@ void BrowserAccessibilityStateImpl::UpdateHistogramsOnUIThread() {
   }
   ui_thread_histogram_callbacks_.clear();
 
+  UMA_HISTOGRAM_BOOLEAN(
+      "Accessibility.ManuallyEnabled",
+      !GetAccessibilityMode().is_mode_off() && !allow_ax_mode_changes_);
+
   ui_thread_done_ = true;
   if (background_thread_done_callback_) {
     std::move(background_thread_done_callback_).Run();
@@ -434,7 +521,7 @@ void BrowserAccessibilityStateImpl::OnUserInputEvent() {
                                   now - accessibility_enabled_time_);
 
       accessibility_disabled_time_ = now;
-      DisableProcessAccessibility();
+      DisableAccessibility();
     }
   }
 }
@@ -456,6 +543,16 @@ void BrowserAccessibilityStateImpl::NotifyWebContentsPreferencesChanged()
 }
 
 void BrowserAccessibilityStateImpl::AddAccessibilityModeFlags(ui::AXMode mode) {
+  if (!allow_ax_mode_changes_) {
+    return;
+  }
+
+  // Adding an accessibility mode flag is generally the result of an
+  // accessibility API call, so we should also reset the auto-disable
+  // accessibility code. The only exception is in tests or when a user manually
+  // toggles accessibility flags in chrome://accessibility.
+  OnAccessibilityApiUsage();
+
   // Update process_accessibility_mode_ via SetProcessMode so that the remainder
   // of processing is identical to when AXPlatformNode::NotifyAddAXModeFlags()
   // is called -- it will defer to AXPlatform::SetMode() to update the global
@@ -469,7 +566,15 @@ void BrowserAccessibilityStateImpl::AddAccessibilityModeFlags(ui::AXMode mode) {
 
 void BrowserAccessibilityStateImpl::RemoveAccessibilityModeFlags(
     ui::AXMode mode) {
-  SetProcessMode(process_accessibility_mode_->mode() & ~mode);
+  // Turning off accessibility or changing the mode will not be allowed if the
+  // --force-renderer-accessibility or --disable-renderer-accessibility command
+  // line flags are present, or during testing
+  if (!allow_ax_mode_changes_) {
+    return;
+  }
+
+  process_accessibility_mode_ =
+      CreateScopedModeForProcess(process_accessibility_mode_->mode() & ~mode);
 }
 
 base::CallbackListSubscription
@@ -491,13 +596,6 @@ void BrowserAccessibilityStateImpl::SetProcessMode(ui::AXMode new_mode) {
     return;
   }
 
-  if (!new_mode.is_mode_off()) {
-    // Unless the mode is being turned off, setting accessibility flags is
-    // generally caused by accessibility API call, so we should also reset the
-    // auto-disable accessibility code.
-    OnAccessibilityApiUsage();
-  }
-
   const ui::AXMode previous_mode = GetAccessibilityMode();
   if (new_mode == previous_mode) {
     return;
@@ -527,15 +625,6 @@ void BrowserAccessibilityStateImpl::OnAccessibilityApiUsage() {
             base::Unretained(this)),
         base::Seconds(kOnAccessibilityUsageUpdateDelaySecs));
   }
-
-  // Track the time since start-up before the kWebContents mode was enabled,
-  // ensuring we record this value only one time.
-  if (!has_enabled_accessibility_in_session_ &&
-      GetAccessibilityMode().has_mode(ui::AXMode::kWebContents)) {
-    has_enabled_accessibility_in_session_ = true;
-    UMA_HISTOGRAM_LONG_TIMES_100("Accessibility.EngineUse.TimeUntilStart",
-                                 timer_.Elapsed());
-  }
 }
 
 void BrowserAccessibilityStateImpl::OnInputEvent(

content/browser/accessibility/browser_accessibility_state_impl.h

@@ -68,15 +68,22 @@ class CONTENT_EXPORT BrowserAccessibilityStateImpl
   virtual void InitBackgroundTasks();
 
   // BrowserAccessibilityState implementation.
-  void EnableProcessAccessibility() override;
-  void DisableProcessAccessibility() override;
-  bool IsAccessibilityAllowed() override;
+  void EnableAccessibility() override;
+  void DisableAccessibility() override;
+  bool IsRendererAccessibilityEnabled() override;
   ui::AXMode GetAccessibilityMode() override;
   ui::AXMode GetAccessibilityModeForBrowserContext(
       BrowserContext* browser_context) override;
-  // TODO(aleventhal): Rename this to Add/RemoveProcessAccessibilityFlags()
   void AddAccessibilityModeFlags(ui::AXMode mode) override;
   void RemoveAccessibilityModeFlags(ui::AXMode mode) override;
+  void ResetAccessibilityMode() override;
+  // These methods indicate the presence of AXMode::kAllProperties, which is
+  // a misnomer because it is used by many clients, and not just screen readers.
+  // Methods with "AssistiveTech" in the name deal with actual
+  // screen reader usage.
+  // TODO(accessibility) Rename these methods to fix the misnomer.
+  void OnScreenReaderDetected() override;
+  void OnScreenReaderStopped() override;
   // Some platforms have a strong signal indicating the presence of a
   // screen reader and can call in to let us know when one has
   // been enabled/disabled.
@@ -244,6 +251,8 @@ class CONTENT_EXPORT BrowserAccessibilityStateImpl
   // ResetAccessibilityMode(); and applies them to all WebContentses in the
   // process. Guaranteed to hold at least an instance with no mode flags set.
   std::unique_ptr<ScopedAccessibilityMode> process_accessibility_mode_;
+
+  base::WeakPtrFactory<BrowserAccessibilityStateImpl> weak_factory_{this};
 };
 
 }  // namespace content

content/browser/accessibility/browser_accessibility_state_impl_unittest.cc

@@ -42,7 +42,7 @@ class BrowserAccessibilityStateImplTest : public ::testing::Test {
 
   void TearDown() override {
     // Disable accessibility so that it does not impact subsequent tests.
-    state_->DisableProcessAccessibility();
+    state_->DisableAccessibility();
   }
 
   base::test::ScopedFeatureList scoped_feature_list_;
@@ -63,7 +63,7 @@ TEST_F(BrowserAccessibilityStateImplTest,
   EXPECT_EQ(ui::AXPlatform::GetInstance().GetMode(), ui::AXMode());
 
   // Enable accessibility based on usage of accessibility APIs.
-  state_->EnableProcessAccessibility();
+  state_->EnableAccessibility();
   // Indicate that an actual screen reader is not running (a screen reader
   // will prevent auto-disable from taking place).
   state_->SetScreenReaderAppActive(false);
@@ -98,7 +98,7 @@ TEST_F(BrowserAccessibilityStateImplTest,
   EXPECT_EQ(ui::AXPlatform::GetInstance().GetMode(), ui::AXMode());
 
   // Enable accessibility based on usage of accessibility APIs.
-  state_->EnableProcessAccessibility();
+  state_->OnScreenReaderDetected();
   // Indicate that an actual screen reader is not running (a screen reader
   // will prevent auto-disable from taking place).
   state_->SetScreenReaderAppActive(false);
@@ -144,10 +144,7 @@ TEST_F(BrowserAccessibilityStateImplTest,
   EXPECT_EQ(ui::AXPlatform::GetInstance().GetMode(), ui::AXMode());
 
   // Enable accessibility.
-  state_->EnableProcessAccessibility();
-  // Indicate that an actual screen reader is not running (a screen reader
-  // will prevent auto-disable from taking place).
-  state_->SetScreenReaderAppActive(false);
+  state_->OnScreenReaderDetected();
   EXPECT_TRUE(state_->IsAccessibleBrowser());
   EXPECT_EQ(ui::AXPlatform::GetInstance().GetMode(), ui::kAXModeComplete);
 
@@ -193,7 +190,7 @@ TEST_F(BrowserAccessibilityStateImplTest,
   EXPECT_EQ(ui::AXPlatform::GetInstance().GetMode(), ui::AXMode());
 
   // Enable accessibility.
-  state_->EnableProcessAccessibility();
+  state_->OnScreenReaderDetected();
   EXPECT_TRUE(state_->IsAccessibleBrowser());
   EXPECT_EQ(ui::AXPlatform::GetInstance().GetMode(), ui::kAXModeComplete);
 
@@ -210,6 +207,55 @@ TEST_F(BrowserAccessibilityStateImplTest,
   EXPECT_EQ(ui::AXPlatform::GetInstance().GetMode(), ui::kAXModeComplete);
 }
 
+TEST_F(BrowserAccessibilityStateImplTest, DisableAccessibilityHasADelay) {
+  // Initially accessibility should be disabled.
+  EXPECT_FALSE(state_->IsAccessibleBrowser());
+
+  // Enable accessibility.
+  state_->OnScreenReaderDetected();
+  EXPECT_TRUE(state_->IsAccessibleBrowser());
+
+  // After 10 seconds, disable accessibility in response to client being quit.
+  task_environment_.FastForwardBy(base::Seconds(10));
+  state_->OnScreenReaderStopped();
+
+  // After one second, accessibility support should still be enabled. This is
+  // because we delay disabling accessibility support in response to the client
+  // being quit just in case it is about to be toggled back on.
+  task_environment_.FastForwardBy(base::Seconds(1));
+  EXPECT_TRUE(state_->IsAccessibleBrowser());
+
+  // After the delay has passed without support being re-enabled, accessibility
+  // should now be disabled.
+  task_environment_.FastForwardBy(base::Seconds(10));
+  EXPECT_FALSE(state_->IsAccessibleBrowser());
+}
+
+TEST_F(BrowserAccessibilityStateImplTest,
+       EnableImmediatelyAfterDisablePreventsDisable) {
+  // Initially accessibility should be disabled.
+  EXPECT_FALSE(state_->IsAccessibleBrowser());
+
+  // Enable accessibility.
+  state_->OnScreenReaderDetected();
+  EXPECT_TRUE(state_->IsAccessibleBrowser());
+
+  // After 10 seconds, disable accessibility in response to client being quit.
+  // Then re-enable it immediately. Accessibility support should never get
+  // disabled because it was re-enabled before the delay to disable support
+  // had passed.
+  task_environment_.FastForwardBy(base::Seconds(10));
+  state_->OnScreenReaderStopped();
+  EXPECT_TRUE(state_->IsAccessibleBrowser());
+
+  task_environment_.FastForwardBy(base::Milliseconds(10));
+  state_->OnScreenReaderDetected();
+  for (int i = 0; i < 10; i++) {
+    task_environment_.FastForwardBy(base::Seconds(i));
+    EXPECT_TRUE(state_->IsAccessibleBrowser());
+  }
+}
+
 namespace {
 using ::testing::_;
 
@@ -232,11 +278,11 @@ TEST_F(BrowserAccessibilityStateImplTest,
 
   // Enable accessibility.
   EXPECT_CALL(mock_observer, OnAXModeAdded(ui::kAXModeComplete));
-  state_->EnableProcessAccessibility();
+  state_->OnScreenReaderDetected();
   ::testing::Mock::VerifyAndClearExpectations(&mock_observer);
 
   // A second call should be a no-op.
-  state_->EnableProcessAccessibility();
+  state_->OnScreenReaderDetected();
   ::testing::Mock::VerifyAndClearExpectations(&mock_observer);
 }
 

content/browser/accessibility/dump_accessibility_browsertest_base.cc

@@ -481,7 +481,7 @@ void DumpAccessibilityTestBase::RunTestForPlatform(
   // Start with no AXMode, so that in case the test was run with
   // --force-renderer-accessibility, we can still set the correct mode for the
   // test, e.g. form controls mode.
-  BrowserAccessibilityState::GetInstance()->DisableProcessAccessibility();
+  BrowserAccessibilityState::GetInstance()->DisableAccessibility();
 
   if (enable_accessibility_after_navigating_ &&
       web_contents->GetAccessibilityMode().is_mode_off()) {

content/browser/accessibility/web_contents_accessibility_android.cc

@@ -586,7 +586,7 @@ void WebContentsAccessibilityAndroid::DisableRendererAccessibility(
   // Turn off accessibility on the renderer side by resetting the AXMode.
   BrowserAccessibilityStateImpl* accessibility_state =
       BrowserAccessibilityStateImpl::GetInstance();
-  accessibility_state->DisableProcessAccessibility();
+  accessibility_state->ResetAccessibilityMode();
 }
 
 void WebContentsAccessibilityAndroid::ReEnableRendererAccessibility(
@@ -2209,7 +2209,7 @@ void JNI_WebContentsAccessibilityImpl_SetBrowserAXMode(
     // When the browser is not yet accessible, then set the AXMode to
     // |ui::kAXModeComplete| for all web contents.
     if (!accessibility_state->IsAccessibleBrowser()) {
-      accessibility_state->EnableProcessAccessibility();
+      accessibility_state->OnScreenReaderDetected();
     }
     return;
   }

content/public/browser/browser_accessibility_state.h

@@ -32,24 +32,15 @@ class CONTENT_EXPORT BrowserAccessibilityState {
   static BrowserAccessibilityState* GetInstance();
 
   // Enables accessibility for all running tabs.
-  // Called when an accessibility client is detected.
-  // It is often preferable to use ScopedAccessibilityMode.
-  // The process AXMode is the default used for new WebContents and pages.
-  // When no mode argument is passed, ui::kAXModeComplete is assumed.
-  virtual void EnableProcessAccessibility() = 0;
+  virtual void EnableAccessibility() = 0;
 
   // Disables accessibility for all running tabs. (Only if accessibility is not
   // required by a command line flag or by a platform requirement.)
-  // Called when all AXModes should be turned off.
-  // By default, new WebContents and pages will not have accessibility on.
-  virtual void DisableProcessAccessibility() = 0;
+  virtual void DisableAccessibility() = 0;
 
-  // Returns true if accessibility is not disallowed via
+  // Returns true if renderer accessibility is not disabled via
   // --disable-renderer-accessibility on the process's command line.
-  // Note: the command line flag --disable-renderer-accessibility is a misnomer
-  // because it also disables accessibility in non-renderer contexts, such as
-  // UI.
-  virtual bool IsAccessibilityAllowed() = 0;
+  virtual bool IsRendererAccessibilityEnabled() = 0;
 
   // Returns the effective accessibility mode for the process. Individual
   // WebContentses may have an effective mode that is a superset of this as a
@@ -91,6 +82,22 @@ class CONTENT_EXPORT BrowserAccessibilityState {
   // accessibility mode bitmap.
   virtual void RemoveAccessibilityModeFlags(ui::AXMode mode) = 0;
 
+  // DEPRECATED. Resets accessibility to the platform default for all running
+  // tabs. This is probably off, but may be on, if
+  // --force_renderer_accessibility is passed, or EditableTextOnly if this is
+  // Win7.
+  virtual void ResetAccessibilityMode() = 0;
+
+  // Called when an accessibility client is detected, using a heuristic.
+  // These methods indicate the presence of AXMode::kExtendedProperties.
+  // The current method name is a misnomer because it is used by many clients,
+  // and not just screen readers.
+  // TODO(accessibility) Remove this method.
+  virtual void OnScreenReaderDetected() = 0;
+
+  // Called when kExtendedProperties mode should be turned off.
+  virtual void OnScreenReaderStopped() = 0;
+
   // Some platforms have a strong signal indicating the presence of a
   // screen reader and can call in to let us know when one has
   // been enabled/disabled. This should be called for screen readers only.

content/shell/browser/shell_platform_delegate_ios.mm

@@ -237,8 +237,7 @@ static const char kAllTracingCategories[] = "*";
 
   // Enable Accessibility if VoiceOver is already running.
   if (UIAccessibilityIsVoiceOverRunning()) {
-    content::BrowserAccessibilityState::GetInstance()
-        ->EnableProcessAccessibility();
+    content::BrowserAccessibilityState::GetInstance()->OnScreenReaderDetected();
   }
 
   // Register for VoiceOver notifications.
@@ -422,11 +421,9 @@ static const char kAllTracingCategories[] = "*";
   content::BrowserAccessibilityState* accessibility_state =
       content::BrowserAccessibilityState::GetInstance();
   if (UIAccessibilityIsVoiceOverRunning()) {
-    accessibility_state->EnableProcessAccessibility();
-    accessibility_state->SetScreenReaderAppActive(true);
+    accessibility_state->OnScreenReaderDetected();
   } else {
-    accessibility_state->DisableProcessAccessibility();
-    accessibility_state->SetScreenReaderAppActive(false);
+    accessibility_state->OnScreenReaderStopped();
   }
 }
 @end