New issue
Advanced search Search tips

Issue 810454 link

Starred by 1 user
Status: WontFix
Owner:
tommycli@chromium.org
Closed: Jun 2018
Cc:
jdonnelly@chromium.org
pkasting@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Crash when defocusing the Omnibox via Shift-Tab and there's a RevertAll call in OnBlur

Project Member Reported by tommycli@chromium.org, Feb 8 2018 
Repro steps:

1. Use a build with DCHECK on.
2. Turn on chrome://flags/#omnibox-ui-hide-steady-state-url-scheme-and-subdomains
3. Navigate to "https://www.google.com/"
4. Observe that the scheme and www are hidden in the steady state Omnibox.
5. Click on the Omnibox and observe they are re-shown.
6. Press shift-tab.
7. Observe a DCHECK crash.

The root cause is:
1. In the OnBlur handler, we call RevertAll on the view
2. This fires an observer in the location bar, which re-layouts the location bar.
3. As part of the relayout, it initially hides the Secure icon.
4. Since pressing shift-tab focuses the Secure icon, this causes Views to advance the focus.
5. But now we are re-focusing what we just blurred..., and causing re-entrant behavior into FocusManager::SetFocusedViewWithReason.
6. FocusManager::SetFocusedViewWithReason really does not like reentrancy - it hits a DCHECK.

Some possible solutions (any one of these should work):
1. Maybe Views should defer post OnBlur, OnFocus event handlers onto the message loop...
2. During LocationBarView::Layout(), it doesn't seem like we should unconditionally hide the Secure icon control.
3. Maybe the OmniboxEditModel should post the RevertAll call onto the message loop instead of doing it directly within the OnBlur handler.

--

Note -- this bug likely existed previously too, since ZeroSuggest can trigger a RevertAll call during the OnBlur call. It's just much easier to trigger now that we have steady state elisions causing a RevertAll call on any defocus event.
RootCause.PNG
159 KB View Download
CrashStack.PNG
137 KB View Download

Comment 1 by tommycli@chromium.org, Feb 8 2018 

Here's the Linux crash stack in text form...

[166830:166830:0208/101240.067302:FATAL:observer_list.h(236)] Check failed: false. Observers can only be added once!
#0 0x7fbf2987563c base::debug::StackTrace::StackTrace()
#1 0x7fbf2989fd1c logging::LogMessage::~LogMessage()
#2 0x7fbf249830e9 base::ObserverList<>::AddObserver()
#3 0x7fbf24a0e2c4 views::View::AddObserver()
#4 0x7fbf249f7af0 views::FocusManager::SetFocusedViewWithReason()
#5 0x7fbf249f75c0 views::FocusManager::OnKeyEvent()
#6 0x7fbf24a34896 views::FocusManagerEventHandler::OnKeyEvent()
#7 0x7fbf25532427 ui::EventDispatcher::DispatchEventToEventHandlers()
#8 0x7fbf255320ee ui::EventDispatcher::ProcessEvent()
#9 0x7fbf25531ef6 ui::EventDispatcherDelegate::DispatchEvent()
#10 0x7fbf255338fe ui::EventProcessor::OnEventFromSource()
#11 0x7fbf2543983c aura::WindowTreeHost::DispatchKeyEventPostIME()
#12 0x7fbf2550f883 ui::InputMethodBase::DispatchKeyEventPostIME()
#13 0x7fbf2550d6f8 ui::InputMethodAuraLinux::ProcessKeyEventDone()
#14 0x7fbf2550d028 ui::InputMethodAuraLinux::DispatchKeyEvent()
#15 0x7fbf25431a9e aura::WindowEventDispatcher::PreDispatchEvent()
#16 0x7fbf25531e94 ui::EventDispatcherDelegate::DispatchEvent()
#17 0x7fbf255338fe ui::EventProcessor::OnEventFromSource()
#18 0x7fbf25533eab ui::EventSource::SendEventToSink()
#19 0x7fbf24a46d69 views::DesktopWindowTreeHostX11::DispatchEvent()
#20 0x7fbf24a47a20 views::DesktopWindowTreeHostX11::DispatchEvent()
#21 0x7fbf28ab9bab ui::PlatformEventSource::DispatchEvent()
#22 0x7fbf1d09c6c0 ui::X11EventSource::ExtractCookieDataDispatchEvent()
#23 0x7fbf24572cf6 libgtkui::Gtk2EventLoop::ProcessGdkEventKey()
#24 0x7fbf24572bfb libgtkui::Gtk2EventLoop::DispatchGdkEvent()
#25 0x7fbf18aa9405 <unknown>
#26 0x7fbf18ada7d2 <unknown>
#27 0x7fbf1ede97f7 g_main_context_dispatch
#28 0x7fbf1ede9a60 <unknown>
#29 0x7fbf1ede9b0c g_main_context_iteration
#30 0x7fbf298b0c12 base::MessagePumpGlib::Run()
#31 0x7fbf298ae32c base::MessageLoop::Run()
#32 0x7fbf298e6ef6 base::RunLoop::Run()
#33 0x55b7565591a7 ChromeBrowserMainParts::MainMessageLoopRun()
#34 0x7fbf26ddb607 content::BrowserMainLoop::RunMainMessageLoopParts()
#35 0x7fbf26dded23 content::BrowserMainRunnerImpl::Run()
#36 0x7fbf26dd742a content::BrowserMain()
#37 0x7fbf27842abb content::RunNamedProcessTypeMain()
#38 0x7fbf27843c07 content::ContentMainRunnerImpl::Run()
#39 0x7fbf29d85614 service_manager::Main()
#40 0x7fbf27841f51 content::ContentMain()
#41 0x55b755ffa1cb ChromeMain
#42 0x7fbf1d4e62b1 __libc_start_main
#43 0x55b755ffa02a _start

Comment 2 by tommycli@chromium.org, Jun 26 2018

Status: WontFix (was: Assigned)
This bug doesn't happen anymore because the FocusManager seems to be considering the a different View focused now, so it doesn't double-add the observer.

I'm marking WONTFIX for now, but we can revisit if it crashes again.
►

Sign in to add a comment