New issue
Advanced search Search tips

Issue 889121 link

Starred by 3 users
Status: Fixed
Owner:
jamescook@chromium.org
Closed: Sep 25
Cc:
dtseng@chromium.org
dmazz...@chromium.org
msw@chromium.org
jamescook@chromium.org
mustash-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug
Team-Accessibility



Sign in to add a comment

Keyboard shortcut viewer crashes on open if select-to-speak or ChromeVox is enabled

Project Member Reported by jamescook@chromium.org, Sep 25 
chrome ToT e228478b745b9bc4dbcca4cbcf45468d2aa8cfed on linux-chromeos

1. Turn on Select-to-speak or ChromeVox
2. Hit Ctrl-Alt-/
3. Hover the mouse over the shortcut viewer window (or maybe just wait)

DCHECK failure:
[shortcut:137191:137191:0925/112212.424714:FATAL:ax_remote_host.cc(211)] Check failed: aura_obj. 
#0 0x7ff447e748ad base::debug::StackTrace::StackTrace()
#1 0x7ff447b8673c base::debug::StackTrace::StackTrace()
#2 0x7ff447bf07bd logging::LogMessage::~LogMessage()
#3 0x7ff42dc3bf56 views::AXRemoteHost::SendEvent()
#4 0x7ff42dc3c7bc views::AXRemoteHost::HandleEvent()
#5 0x7ff42dc6ed76 views::MusViewsDelegate::NotifyAccessibilityEvent()
#6 0x7ff4332df3e2 views::View::NotifyAccessibilityEvent()
#7 0x7ff4332de14e views::View::BoundsChanged()
#8 0x7ff4332ddb83 views::View::SetBoundsRect()
#9 0x7ff4332011f0 views::FocusRing::Layout()
#10 0x7ff433200eb1 views::FocusRing::Install()
#11 0x7ff433245570 views::ScrollView::ScrollView()
#12 0x556d79f20c10 keyboard_shortcut_viewer::(anonymous namespace)::CreateScrollView()
#13 0x556d79f1d5b2 keyboard_shortcut_viewer::KeyboardShortcutView::InitCategoriesTabbedPane()
#14 0x556d79f26170 _ZN4base8internal13FunctorTraitsIMN24keyboard_shortcut_viewer20KeyboardShortcutViewEFvNS_8OptionalINS2_16ShortcutCategoryEEEEvE6InvokeIS8_NS_7WeakPtrIS3_EEJNS_9nullopt_tEEEEvT_OT0_DpOT1_
#15 0x556d79f260c5 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIMN24keyboard_shortcut_viewer20KeyboardShortcutViewEFvNS_8OptionalINS4_16ShortcutCategoryEEEENS_7WeakPtrIS5_EEJNS_9nullopt_tEEEEvOT_OT0_DpOT1_
#16 0x556d79f2603c _ZN4base8internal7InvokerINS0_9BindStateIMN24keyboard_shortcut_viewer20KeyboardShortcutViewEFvNS_8OptionalINS3_16ShortcutCategoryEEEEJNS_7WeakPtrIS4_EENS_9nullopt_tEEEEFvvEE7RunImplIS9_NSt3__15tupleIJSB_SC_EEEJLm0ELm1EEEEvOT_OT0_NSH_16integer_sequenceImJXspT1_EEEE
#17 0x556d79f25f49 _ZN4base8internal7InvokerINS0_9BindStateIMN24keyboard_shortcut_viewer20KeyboardShortcutViewEFvNS_8OptionalINS3_16ShortcutCategoryEEEEJNS_7WeakPtrIS4_EENS_9nullopt_tEEEEFvvEE7RunOnceEPNS0_13BindStateBaseE

When I comment out the DCHECK this is a crash, so I suspect this happens in production.

I think the cause is dtseng's CL that makes AXAuraObjCache::GetOrCreate() sometimes return null:
https://chromium-review.googlesource.com/1227672

The code in AXRemoteHosts assumes it never returns null.

Comment 1 by jamescook@chromium.org, Sep 25 

Actually the original CL for transient focus issues was this one from dmazzoni:
https://chromium-review.googlesource.com/c/chromium/src/+/1176207

I said it was OK to delete one of the AXRemoteHost unit tests in that code review. I was wrong - I think it might have caught this case.

Fix up shortly.

Comment 2 by jamescook@chromium.org, Sep 25

Cc: msw@chromium.org jamescook@chromium.org
 Issue 889172  has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 25 

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

commit dc815dcb0ea051693aa5d71260932d0085430871
Author: James Cook <jamescook@chromium.org>
Date: Tue Sep 25 21:34:03 2018

chromeos: Fix crash in shortcut viewer app with ChromeVox enabled

AXAuraObjCache::GetOrCreate(View* view) can return null if the View
is not yet associated with a Widget. The shortcut viewer creates a
ScrollView that creates a FocusRing before the ScrollView is attached
to the Widget.

Do what AutomationManagerAura does in this case and just don't send
accessibility event updates for these unattached views.

Bug:  889121 
Test: added to views_mus_unittests
Change-Id: Ica14908778e45b9b4ee06077bd31c92fb7dfca62
Reviewed-on: https://chromium-review.googlesource.com/1244456
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594113}
[modify] https://crrev.com/dc815dcb0ea051693aa5d71260932d0085430871/ui/views/accessibility/ax_aura_obj_cache.cc
[modify] https://crrev.com/dc815dcb0ea051693aa5d71260932d0085430871/ui/views/accessibility/ax_aura_obj_cache.h
[modify] https://crrev.com/dc815dcb0ea051693aa5d71260932d0085430871/ui/views/mus/ax_remote_host.cc
[modify] https://crrev.com/dc815dcb0ea051693aa5d71260932d0085430871/ui/views/mus/ax_remote_host_unittest.cc

Comment 4 by msw@chromium.org, Sep 25 

Nice! Think we should merge this to M-70?

Comment 5 by jamescook@chromium.org, Sep 25

Labels: M-71
Status: Fixed (was: Started)
I think we're OK. The CL that caused the regression landed in M71.

Also, I don't see any crashes with this stack in go/crash.

Comment 6 by jamescook@chromium.org, Oct 5 

Issue 892661 has been merged into this issue.
►

Sign in to add a comment