New issue
Advanced search Search tips

Issue 910224 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 29
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug
Team-Accessibility



Sign in to add a comment

KSV app: DCHECK toggling ChromeVox under some circumstances

Project Member Reported by msw@chromium.org, Nov 29

Issue description

KSV app: DCHECK toggling ChromeVox under some circumstances

On ToT @ #611900 running linux-chromeos (DCHECK not hit on devices with release build):
(1) Open the KSV app (Ctrl-Alt-/)
(2) Turn on ChromeVox (Ctrl-Alt-Z) while the KSV window is active
(3) Activate or open a browser window
(4) Disable ChromeVox while the browser window is active
(5) Re-enable ChromeVox while the browser window is active
Expected: KSV app handles ChromeVox toggling gracefully.
Actual: DCHECK crash, stack below.

[shortcut:169747:169747:1129/095623.021755:FATAL:ax_remote_host.cc(229)] Check failed: tree_source_. 
#0 0x7f3d5b462ebf base::debug::StackTrace::StackTrace()
#1 0x7f3d5b3928ab logging::LogMessage::~LogMessage()
#2 0x7f3d5432a3ab views::AXRemoteHost::SendEvent()
#3 0x7f3d567b8504 aura::Window::AfterPropertyChange()
#4 0x7f3d5b6409c5 ui::PropertyHandler::SetPropertyInternal()
#5 0x7f3d56783e68 ui::PropertyHandler::SetProperty<>()
#6 0x7f3d543311bb views::DesktopWindowTreeHostMus::OnWindowPropertyChanged()
#7 0x7f3d567b8504 aura::Window::AfterPropertyChange()
#8 0x7f3d5b6409c5 ui::PropertyHandler::SetPropertyInternal()
#9 0x7f3d56783e68 ui::PropertyHandler::SetProperty<>()
#10 0x7f3d54329df0 views::AXRemoteHost::StartMonitoringWidget()
#11 0x7f3d5432a6e9 views::AXRemoteHost::Enable()
#12 0x7f3d5433db11 ax::mojom::AXRemoteHostStubDispatch::Accept()
#13 0x7f3d5b725ea6 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#14 0x7f3d5b7257d6 mojo::FilterChain::Accept()
#15 0x7f3d5b727235 mojo::InterfaceEndpointClient::HandleIncomingMessage()
#16 0x7f3d5b72d83b mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#17 0x7f3d5b72cc60 mojo::internal::MultiplexRouter::Accept()
#18 0x7f3d5b7257d6 mojo::FilterChain::Accept()
#19 0x7f3d5b720639 mojo::Connector::ReadSingleMessage()
#20 0x7f3d5b721151 mojo::Connector::ReadAllAvailableMessages()
#21 0x7f3d5b720ff9 mojo::Connector::OnHandleReadyInternal()
#22 0x7f3d5b721847 mojo::SimpleWatcher::DiscardReadyState()
#23 0x7f3d5b6e6c74 mojo::SimpleWatcher::OnHandleReady()
#24 0x7f3d5b6e7181 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKNSt3__15tupleIJSB_ijS5_EEEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NSI_16integer_sequenceImJXspT1_EEEE
#25 0x7f3d5b375b41 base::debug::TaskAnnotator::RunTask()
#26 0x7f3d5b3a1a7f base::MessageLoopImpl::RunTask()
#27 0x7f3d5b3a20e3 base::MessageLoopImpl::DoWork()
#28 0x7f3d5b3a3f2a base::MessagePumpDefault::Run()
#29 0x7f3d5b3a1628 base::MessageLoopImpl::Run()
#30 0x7f3d5b3d2ed9 base::RunLoop::Run()
#31 0x7f3d58da79cc content::UtilityMain()
#32 0x7f3d58dafb97 content::ContentMainRunnerImpl::Run()
#33 0x7f3d4b766386 service_manager::Main()
#34 0x7f3d58dae104 content::ContentMain()
#35 0x55d44d3050f3 ChromeMain
#36 0x7f3d4bd312b1 __libc_start_main
#37 0x55d44d304f6a _start

Looks like |tree_source_| is created in AXRemoteHost::StartMonitoringWidget and destroyed in AXRemoteHost::StopMonitoringWidget.
I'll investigate further, but may pass this off if the defect/fix is not straightforward.
 
AXRemoteHost::Enable/StartMonitoringWidget itself sets a property that triggers SendEvent.
Enabling ChromeVox the first time doesn't hit this because SendEvent early-outs if |!enabled_|.
However, toggling ChromeVox off never triggers the disable callstack, so toggling back on encounters a semi-initialized/enabled state.
I will convert AXRemoteHost::SendEvent's DCHECKs to match AutomationManagerAura::SendEvent's early returns.

It would be very nice if toggling ChromeVox off actually disabled the AutomationManager, AXHostService, and hosts!
Dominic, is there a reason we never disable these objects in production?
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bfb0e5146ea4e8740c9ab32227d78178b067a251

commit bfb0e5146ea4e8740c9ab32227d78178b067a251
Author: Mike Wasserman <msw@chromium.org>
Date: Thu Nov 29 21:06:24 2018

Fix AXRemoteHost::SendEvent DCHECK when toggling ChromeVox with KSV app

Bug:  910224 
Change-Id: Ie1d13e6d2c41b4096ce7912ba494db2fe468c678
Reviewed-on: https://chromium-review.googlesource.com/c/1355196
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612350}
[modify] https://crrev.com/bfb0e5146ea4e8740c9ab32227d78178b067a251/ui/views/mus/ax_remote_host.cc

Status: Fixed (was: Started)
This is fixed, but this point from Comment #1 stands:
It would be very nice if toggling ChromeVox off actually disabled the AutomationManager, AXHostService, and hosts!

Sign in to add a comment