Issue metadata
Sign in to add a comment
|
Tabbing on lock screen user menu results in infinite recursion |
||||||||||||||||||||||
Issue descriptionCaused by https://chromium-review.googlesource.com/c/chromium/src/+/1297595 The infinite loop happens at this line, when the LoginUserMenuView tries to shift focus away. https://cs.chromium.org/chromium/src/ash/login/ui/login_bubble.cc?l=337&rcl=ea50c8023c93be84d04978a473c31d0cd257301c The stack looks like this: #0 0x5603c2165d6f base::debug::StackTrace::StackTrace() #1 0x5603c47a4160 ash::(anonymous namespace)::LoginUserMenuView::AboutToRequestFocusFromTabTraversal() #2 0x5603c3bc2126 views::FocusManager::AdvanceFocus() .... .... #3 0x5603c47a419f ash::(anonymous namespace)::LoginUserMenuView::AboutToRequestFocusFromTabTraversal() #4 0x5603c3bc2126 views::FocusManager::AdvanceFocus() #5 0x5603c47a419f ash::(anonymous namespace)::LoginUserMenuView::AboutToRequestFocusFromTabTraversal() #6 0x5603c3bc2126 views::FocusManager::AdvanceFocus() #7 0x5603c47a419f ash::(anonymous namespace)::LoginUserMenuView::AboutToRequestFocusFromTabTraversal() #8 0x5603c3bc2126 views::FocusManager::AdvanceFocus() #9 0x5603c47a419f ash::(anonymous namespace)::LoginUserMenuView::AboutToRequestFocusFromTabTraversal() #10 0x5603c3bc2126 views::FocusManager::AdvanceFocus() #11 0x5603c3bc1fd2 views::FocusManager::OnKeyEvent() #12 0x5603c3bdb5d4 views::Widget::OnKeyEvent() #13 0x5603c3be8662 views::NativeWidgetAura::OnKeyEvent() #14 0x5603c2bc8ff3 ui::EventDispatcher::ProcessEvent() #15 0x5603c2bc8e15 ui::EventDispatcherDelegate::DispatchEventToTarget() #16 0x5603c2bc8d86 ui::EventDispatcherDelegate::DispatchEvent() #17 0x5603c6027832 ui::EventProcessor::OnEventFromSource() #18 0x5603c3790b4f aura::WindowTreeHost::DispatchKeyEventPostIME() #19 0x5603c45d8cf2 ash::WindowTreeHostManager::DispatchKeyEventPostIME() #20 0x5603c618adb1 ui::InputMethodBase::DispatchKeyEventPostIME() #21 0x5603c618bf68 ui::InputMethodChromeOS::ProcessUnfilteredKeyPressEvent() #22 0x5603c618bc9b ui::InputMethodChromeOS::DispatchKeyEvent() #23 0x5603c618c0ef ui::InputMethodChromeOS::DispatchKeyEvent() #24 0x5603c378963c aura::WindowEventDispatcher::PreDispatchEvent() #25 0x5603c2bc8d66 ui::EventDispatcherDelegate::DispatchEvent() #26 0x5603c6027832 ui::EventProcessor::OnEventFromSource() #27 0x5603c6027dee ui::EventSource::SendEventToSinkFromRewriter() #28 0x5603c45e4b84 ash::AshWindowTreeHostPlatform::DispatchEventFromQueue() #29 0x5603c4809d15 ws::HostEventQueue::DispatchOrQueueEvent() #30 0x5603c2bcc717 ui::DispatchEventFromNativeUiEvent() #31 0x5603bf74e98e ui::X11WindowOzone::DispatchEvent() #32 0x5603c2813253 ui::PlatformEventSource::DispatchEvent() #33 0x5603c33d8d39 ui::X11EventSourceLibevent::DispatchPlatformEvent() #34 0x5603c33d87e1 ui::X11EventSourceLibevent::ProcessXEvent() #35 0x5603c33d9621 ui::X11EventSource::DispatchXEvents() #36 0x5603c21844b2 base::MessagePumpLibevent::OnLibeventNotification() #37 0x5603c21a6d4d event_base_loop #38 0x5603c21847cb base::MessagePumpLibevent::Run() #39 0x5603c20d4d54 base::MessageLoop::Run() #40 0x5603c20fc979 base::RunLoop::Run() #41 0x5603c1c8cc5d ChromeBrowserMainParts::MainMessageLoopRun() #42 0x5603bfc8bdb7 content::BrowserMainLoop::RunMainMessageLoopParts() #43 0x5603bfc8e856 content::BrowserMainRunnerImpl::Run() #44 0x5603bfc883a9 content::BrowserMain() #45 0x5603c1c539ee content::ContentMainRunnerImpl::Run() #46 0x5603c1c84975 service_manager::Main() #47 0x5603c1c51d34 content::ContentMain() #48 0x5603bec01053 ChromeMain #49 0x7fa47b3b22b1 __libc_start_main #50 0x5603bec00eca _start
,
Nov 2
To repro: 1. do target_os = "chromeos" build 2. $ ./out/Release/chrome 3. lock the screen (meta+L) 4. hit tab, focus should be on dropdown arrow 5. hit enter, user menu will appear 6. hit tab, crash
,
Nov 5
We have a Desktop convergence so I can first take a look on Wednesday. +cc tapted@ if you want to take a look, I think focus goes into something anchored (as this key is now set for all dialogs that have an anchor) which for some reason breaks here. I hope there's no A->B, B->A anchor loop. Otherwise I think RBB is reasonable, I'll take a look asap.
,
Nov 15
@pbos can you please provide an update?
,
Nov 15
CL in the commit queue, should hopefully land soon.
,
Nov 15
There are some test failures that look real here. I've gotten repro steps from jdufault and hope to have time to take a look next week. CL for posterity: https://crrev.com/1338222
,
Nov 27
pbos@, any update for this issue? Thanks
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6b73dc9fad128a353cf2ea70920fc0bd66a888be commit 6b73dc9fad128a353cf2ea70920fc0bd66a888be Author: Peter Boström <pbos@chromium.org> Date: Tue Dec 04 01:06:08 2018 Fix escaping focus traversal from LoginUserMenu Makes use of SetFocusTraversableParent to declare where focus should go after the view has been fully traversed. Fixes infinite recursion where LoginUserMenu would redirect focus to its anchor and then forward focus (which takes it inside LoginUserMenu as the dialog is anchored). Bug: chromium:901456 Change-Id: I138be71866760de84980f5e4ca0c1b42d6d1793f Reviewed-on: https://chromium-review.googlesource.com/c/1338222 Commit-Queue: Peter Boström <pbos@chromium.org> Reviewed-by: Jacob Dufault <jdufault@chromium.org> Cr-Commit-Position: refs/heads/master@{#613367} [modify] https://crrev.com/6b73dc9fad128a353cf2ea70920fc0bd66a888be/ash/login/ui/login_bubble.cc
,
Dec 4
Verified on tip of tree, focus continues to "Shut down" button which is next in focus order.
,
Dec 4
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3c37c040ef3ba0761727aa68fbf481a2b229e8cf commit 3c37c040ef3ba0761727aa68fbf481a2b229e8cf Author: Peter Boström <pbos@chromium.org> Date: Tue Dec 04 21:55:17 2018 Fix escaping focus traversal from LoginUserMenu Makes use of SetFocusTraversableParent to declare where focus should go after the view has been fully traversed. Fixes infinite recursion where LoginUserMenu would redirect focus to its anchor and then forward focus (which takes it inside LoginUserMenu as the dialog is anchored). Bug: chromium:901456 Change-Id: I138be71866760de84980f5e4ca0c1b42d6d1793f Reviewed-on: https://chromium-review.googlesource.com/c/1338222 Commit-Queue: Peter Boström <pbos@chromium.org> Reviewed-by: Jacob Dufault <jdufault@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613367}(cherry picked from commit 6b73dc9fad128a353cf2ea70920fc0bd66a888be) Reviewed-on: https://chromium-review.googlesource.com/c/1362221 Reviewed-by: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#43} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/3c37c040ef3ba0761727aa68fbf481a2b229e8cf/ash/login/ui/login_bubble.cc
,
Dec 4
,
Dec 5
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 5
Already merged.
,
Dec 7
Issue 912481 has been merged into this issue.
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3c37c040ef3ba0761727aa68fbf481a2b229e8cf Commit: 3c37c040ef3ba0761727aa68fbf481a2b229e8cf Author: pbos@chromium.org Commiter: pbos@chromium.org Date: 2018-12-04 21:55:17 +0000 UTC Fix escaping focus traversal from LoginUserMenu Makes use of SetFocusTraversableParent to declare where focus should go after the view has been fully traversed. Fixes infinite recursion where LoginUserMenu would redirect focus to its anchor and then forward focus (which takes it inside LoginUserMenu as the dialog is anchored). Bug: chromium:901456 Change-Id: I138be71866760de84980f5e4ca0c1b42d6d1793f Reviewed-on: https://chromium-review.googlesource.com/c/1338222 Commit-Queue: Peter Boström <pbos@chromium.org> Reviewed-by: Jacob Dufault <jdufault@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613367}(cherry picked from commit 6b73dc9fad128a353cf2ea70920fc0bd66a888be) Reviewed-on: https://chromium-review.googlesource.com/c/1362221 Reviewed-by: Peter Boström <pbos@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#43} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
,
Jan 7
Verified the issue on ChromeOS 11316.66.0, 72.0.3626.49 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by jdufault@chromium.org
, Nov 2