macviews: crash quitting with menu open via system menubar |
||||
Issue description1) Open a menu (omnibox context menu works well) 2) File -> Quit 3) Crash The stack trace reveals that: BrowserCloseManager::CloseBrowsers() -> BridgedNativeWidget::SetVisibilityState() -> ~CocoaMouseCapture() -> MenuHost::OnMouseCaptureLost() -> MenuController::Cancel(this == nullptr) so perhaps the menu is already destructed or partly so when we arrive here.
,
May 1 2018
The issue is a MenuModel being destroyed while still referenced by MenuModelAdapter. Specifically, the Textfield's MenuModel is destroyed like so: 0 libbase.dylib 0x000000010cc4032e base::debug::StackTrace::StackTrace(unsigned long) + 174 1 libbase.dylib 0x000000010cc403ed base::debug::StackTrace::StackTrace(unsigned long) + 29 2 libbase.dylib 0x000000010c83dd1c base::debug::StackTrace::StackTrace() + 28 3 libui_base.dylib 0x000000010d1d1bdb ui::MenuModel::~MenuModel() + 123 4 libui_base.dylib 0x000000010d1d2fdd ui::SimpleMenuModel::~SimpleMenuModel() + 93 5 libui_base.dylib 0x000000010d1d3045 ui::SimpleMenuModel::~SimpleMenuModel() + 21 6 libui_base.dylib 0x000000010d1d3069 ui::SimpleMenuModel::~SimpleMenuModel() + 25 7 libviews.dylib 0x0000000140d48654 views::Textfield::OnCaretBoundsChanged() + 372 8 libviews.dylib 0x0000000140d48cc1 views::Textfield::UpdateAfterChange(bool, bool) + 193 9 libviews.dylib 0x0000000140d49869 views::Textfield::SelectRange(gfx::Range const&) + 89 10 libchrome_dll.dylib 0x0000000121569fb9 OmniboxViewViews::OnBlur() + 3033 11 libviews.dylib 0x0000000140dcd6b9 views::View::Blur() + 25 12 libviews.dylib 0x0000000140d78d0f views::FocusManager::SetFocusedViewWithReason(views::View*, views::FocusManager::FocusChangeReason) + 511 13 libviews.dylib 0x0000000140bef542 views::FocusManager::SetFocusedView(views::View*) + 34 14 libviews.dylib 0x0000000140d7a10a views::FocusManager::ClearFocus() + 42 15 libviews.dylib 0x0000000140d7a31a views::FocusManager::StoreFocusedView(bool) + 106 16 libviews.dylib 0x0000000140c5e755 views::BridgedNativeWidget::OnWindowKeyStatusChangedTo(bool) + 293 17 libviews.dylib 0x0000000140c74cfc -[ViewsNSWindowDelegate windowDidResignKey:] + 44 18 CoreFoundation 0x00007fff32ca861c __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 12 19 CoreFoundation 0x00007fff32ca84ea _CFXRegistrationPost + 458 20 CoreFoundation 0x00007fff32ca8221 ___CFXNotificationPost_block_invoke + 225 21 CoreFoundation 0x00007fff32c66d72 -[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1826 22 CoreFoundation 0x00007fff32c65e03 _CFXNotificationPost + 659 23 Foundation 0x00007fff34e308c7 -[NSNotificationCenter postNotificationName:object:userInfo:] + 66 24 AppKit 0x00007fff303fe0e1 -[NSWindow resignKeyWindow] + 867 25 AppKit 0x00007fff303bf8b8 -[NSWindow _orderOutAndCalcKeyWithCounter:stillVisible:docWindow:] + 253 26 AppKit 0x00007fff30347c0c NSPerformVisuallyAtomicChange + 146 27 AppKit 0x00007fff30af5833 -[NSWindow _doWindowOrderOutWithWithKeyCalc:forCounter:orderingDone:docWindow:] + 85 28 AppKit 0x00007fff30af6d26 -[NSWindow _reallyDoOrderWindowOutRelativeTo:findKey:forCounter:force:isModal:] + 446 29 AppKit 0x00007fff3034f8ec -[NSWindow _reallyDoOrderWindow:relativeTo:findKey:forCounter:force:isModal:] + 172 30 AppKit 0x00007fff3034e765 -[NSWindow _doOrderWindow:relativeTo:findKey:forCounter:force:isModal:] + 287 31 AppKit 0x00007fff3034e5cb -[NSWindow orderWindow:relativeTo:] + 169 32 libviews.dylib 0x0000000140c733a1 -[NativeWidgetMacNSWindow orderWindow:relativeTo:] + 81 33 libchrome_dll.dylib 0x00000001212fc281 -[BrowserNativeWidgetWindow orderWindow:relativeTo:] + 81 34 libviews.dylib 0x0000000140c5a410 views::BridgedNativeWidget::SetVisibilityState(views::BridgedNativeWidget::WindowVisibilityState) + 352 35 libviews.dylib 0x0000000140de38ec views::NativeWidgetMac::Hide() + 108 36 libviews.dylib 0x0000000140df6d7d views::Widget::Hide() + 29 37 libchrome_dll.dylib 0x0000000121500440 BrowserView::CanClose() + 256 38 libviews.dylib 0x0000000140e17a10 views::NonClientView::CanClose() + 32 39 libviews.dylib 0x0000000140df62c2 views::Widget::Close() + 98 40 libchrome_dll.dylib 0x00000001214f3cbe BrowserView::Close() + 46 41 libchrome_dll.dylib 0x000000011bd70e7c BrowserCloseManager::CloseBrowsers() + 1996 but then it is used via: MenuHost::OnMouseCaptureLost() -> MenuController::Cancel() -> MenuController::SetSelection() -> MenuModelAdapter::WillHideMenu(), which tries to use the now-dead model :(
,
May 1 2018
In turn, the issue there is that MenuModelAdapter requires that: // The caller retains ownership of the ui::MenuModel instance and // must ensure it exists for the lifetime of the adapter. and (implicitly) MenuRunner has the same contract. Textfield context menus do: std::unique_ptr<ui::SimpleMenuModel> context_menu_contents_; std::unique_ptr<ViewsTextServicesContextMenu> text_services_context_menu_; std::unique_ptr<views::MenuRunner> context_menu_runner_; which ought to be fine, except that they also do: #if defined(OS_MACOSX) // On Mac, the context menu contains a look up item which displays the // selected text. As such, the menu needs to be updated if the selection has // changed. context_menu_contents_.reset(); #endif which breaks the contract of the context menu runner :(.
,
May 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b3aa2b657d09c332d88ed29183c3f1710ae7111c commit b3aa2b657d09c332d88ed29183c3f1710ae7111c Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Wed May 02 12:57:57 2018 macviews: fix textfield context menu closure crash When closing the window on Mac via the system menus, Textfields can end up with their caret bounds changed (through a roundabout route) while they have a context menu open. This causes a reset of the context menu's model with the runner still referencing the old model, which leads to chaos. To fix that, reset the runner as well when updating the bounds. Bug: 837767 Change-Id: Id955d84c525f8d71121414f2784f216b4d9e6a65 Reviewed-on: https://chromium-review.googlesource.com/1037404 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#555364} [modify] https://crrev.com/b3aa2b657d09c332d88ed29183c3f1710ae7111c/ui/views/controls/textfield/textfield.cc
,
May 2 2018
,
May 4 2018
Able to reproduce the issue on mac 10.13.3 using the build without fix. Verified the fix on Mac 10.13.3 using Chrome version #68.0.3419.0 as per the comment #0. Attaching screen shot for reference. Observed that quitting chrome with menu open via system menubar did not produce any crash. Hence, the fix is working as expected. Adding the verified labels. Thanks...!! |
||||
►
Sign in to add a comment |
||||
Comment 1 by ellyjo...@chromium.org
, May 1 2018