New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 823193 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Chrome_Mac: Crash Report - -[MenuControllerCocoa menuDidClose:]

Project Member Reported by cr...@system.gserviceaccount.com, Mar 19 2018

Issue description

reporter:brajkumar@google.com

Magic Signature: -[MenuControllerCocoa menuDidClose:]

Crash link: https://crash.corp.google.com/browse?q=product.name%3D'Chrome_Mac'%20AND%20product.version%3D'66.0.3359.33'%20AND%20expanded_custom_data.ChromeCrashProto.channel%3D'beta'%20AND%20expanded_custom_data.ChromeCrashProto.ptype%3D'browser'%20AND%20expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D'-%5BMenuControllerCocoa%20menuDidClose%3A%5D'&stbtiq=&reportid=&index=0

-------------------------------------------------------------------------------
Sample Report
-------------------------------------------------------------------------------
Product name: Chrome_Mac
Magic Signature : -[MenuControllerCocoa menuDidClose:]
Product Version: 66.0.3359.33
Process type: browser
Report ID: 127da355eeac3097
Report Url: https://crash.corp.google.com/127da355eeac3097
Report Time: 2018-03-17T02:22:22-07:00
Upload Time: 2018-03-17T02:22:23.98-07:00
Uptime: 24000 ms
CumulativeProductUptime: 0 ms
OS Name: Mac OS X
OS Version: 10.12.6 16G1212
CPU Architecture: amd64
CPU Info: family 6 model 58 stepping 9

-------------------------------------------------------------------------------
Crashing thread: Thread index: 0. Stack Quality: 84%. Thread id: 124397.
-------------------------------------------------------------------------------
0x0000000106ac1e86 (Google Chrome Framework - menu_controller.mm: 345)	-[MenuControllerCocoa menuDidClose:]
0x00007fff7e8f8e12 (AppKit + 0x003d7e12)	-[NSCarbonMenuImpl _carbonCloseEvent:handlerCallRef:]
0x00007fff7e6f85ea (AppKit + 0x001d75ea)	NSSLMMenuEventHandler
0x00007fff7ffa8d84 (HIToolbox + 0x00008d84)	DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*)
0x00007fff7ffa7ff5 (HIToolbox + 0x00007ff5)	SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*)
0x00007fff7ffa7e3e (HIToolbox + 0x00007e3e)	SendEventToEventTargetWithOptions
0x00007fff8002fe51 (HIToolbox + 0x0008fe51)	SendMenuClosed(MenuData*, unsigned int, unsigned char)
0x00007fff8002fbec (HIToolbox + 0x0008fbec)	HideMenus(MenuSelectData*, __CFArray*)
0x00007fff80150e3c (HIToolbox + 0x001b0e3c)	SelectItemAndRestoreAllMenuBits(MenuSelectData&, SelectionData*, MenuResult*, MenuResult*)
0x00007fff80151e42 (HIToolbox + 0x001b1e42)	TrackMenuCommon(MenuSelectData&, unsigned char*, SelectionData*, MenuResult*, MenuResult*)
0x00007fff80014abf (HIToolbox + 0x00074abf)	PopUpMenuSelectCore(MenuData*, Point, double, Point, unsigned short, unsigned int, Rect const*, unsigned short, unsigned int, Rect const*, Rect const*, __CFDictionary const*, __CFString const*, OpaqueMenuRef**, unsigned short*)
0x00007fff80013bd2 (HIToolbox + 0x00073bd2)	_HandlePopUpMenuSelection8(OpaqueMenuRef*, OpaqueEventRef*, unsigned int, Point, unsigned short, unsigned int, Rect const*, unsigned short, Rect const*, Rect const*, __CFDictionary const*, __CFString const*, OpaqueMenuRef**, unsigned short*)
0x00007fff800137aa (HIToolbox + 0x000737aa)	_HandlePopUpMenuSelectionWithDictionary
0x00007fff7e8385a6 (AppKit + 0x003175a6)	_NSSLMPopUpCarbonMenu3
0x00007fff7e94b137 (AppKit + 0x0042a137)	-[NSCarbonMenuImpl _popUpContextMenu:withEvent:forView:withFont:]
0x00007fff7e94af81 (AppKit + 0x00429f81)	-[NSMenu _popUpContextMenu:withEvent:forView:withFont:]
0x000000010785ebea (Google Chrome Framework - menu_runner_impl_cocoa.mm: 173)	views::internal::MenuRunnerImplCocoa::RunMenuAt(views::Widget*, views::MenuButton*, gfx::Rect const&, views::MenuAnchorPosition, int)
0x000000010787c4b2 (Google Chrome Framework - textfield.cc: 1119)	non-virtual thunk to views::Textfield::ShowContextMenuForView(views::View*, gfx::Point const&, ui::MenuSourceType)
0x0000000107898339 (Google Chrome Framework - view.cc: 2536)	views::View::ProcessMousePressed(ui::MouseEvent const&)
0x0000000107898066 (Google Chrome Framework - view.cc: 1090)	views::View::OnMouseEvent(ui::MouseEvent*)
0x0000000106aeb693 (Google Chrome Framework - event_dispatcher.cc: 191)	ui::EventDispatcher::ProcessEvent(ui::EventTarget*, ui::Event*)
0x0000000106aeb4cd (Google Chrome Framework - event_dispatcher.cc: 86)	ui::EventDispatcherDelegate::DispatchEvent(ui::EventTarget*, ui::Event*)
0x00000001078a0c2f (Google Chrome Framework - root_view.cc: 378)	views::internal::RootView::OnMousePressed(ui::MouseEvent const&)
0x00000001078a6012 (Google Chrome Framework - widget.cc: 1198)	views::Widget::OnMouseEvent(ui::MouseEvent*)
0x0000000107836025 (Google Chrome Framework - bridged_content_view.mm: 607)	-[BridgedContentView mouseEvent:]
0x00007fff7ee5ec0f (AppKit + 0x0093dc0f)	-[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:]
0x00007fff7ee5cf09 (AppKit + 0x0093bf09)	-[NSWindow(NSEventRouting) sendEvent:]
0x000000010783e8d6 (Google Chrome Framework - native_widget_mac_nswindow.mm: 148)	-[NativeWidgetMacNSWindow sendEvent:]
0x00007fff7ece1e7d (AppKit + 0x007c0e7d)	-[NSApplication(NSEvent) sendEvent:]
0x0000000105d3879b (Google Chrome Framework - chrome_browser_application_mac.mm: 268)	__34-[BrowserCrApplication sendEvent:]_block_invoke
0x00000001060f2049 (Google Chrome Framework + 0x02005049)	base::mac::CallWithEHFrame(void () block_pointer)
0x0000000105d38575 (Google Chrome Framework - chrome_browser_application_mac.mm: 251)	-[BrowserCrApplication sendEvent:]
0x00007fff7e55c426 (AppKit + 0x0003b426)	-[NSApplication run]
0x0000000106100ebb (Google Chrome Framework - message_pump_mac.mm: 815)	base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*)
0x00000001060ffa3d (Google Chrome Framework - message_pump_mac.mm: 189)	base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*)
0x00000001061246d4 (Google Chrome Framework - run_loop.cc: 133)	<name omitted>
0x0000000105d3e087 (Google Chrome Framework - chrome_browser_main.cc: 2174)	ChromeBrowserMainParts::MainMessageLoopRun(int*)
0x0000000104a8bbf3 (Google Chrome Framework - browser_main_loop.cc: 1104)	content::BrowserMainLoop::RunMainMessageLoopParts()
0x0000000104a8e391 (Google Chrome Framework - browser_main_runner.cc: 160)	content::BrowserMainRunnerImpl::Run()
0x0000000104a8837b (Google Chrome Framework - browser_main.cc: 46)	content::BrowserMain(content::MainFunctionParams const&)
0x0000000105cf0758 (Google Chrome Framework - content_main_runner.cc: 703)	content::ContentMainRunnerImpl::Run()
0x0000000107492c71 (Google Chrome Framework - main.cc: 453)	service_manager::Main(service_manager::MainParams const&)
0x0000000105cefca3 (Google Chrome Framework - content_main.cc: 19)	content::ContentMain(content::ContentMainParams const&)
0x00000001040f0c52 (Google Chrome Framework - chrome_main.cc: 101)	ChromeMain
0x000000010405cdd3 (Google Chrome - chrome_exe_main_mac.cc: 165)	main
0x00007fff9663b234 (libdyld.dylib + 0x00005234)	start
0x00007fff9663b234 (libdyld.dylib + 0x00005234)	start

-------------------------------------------------------------------------------
Manual regression range finder link
-------------------------------------------------------------------------------
https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D'-%5BMenuControllerCocoa%20menuDidClose%3A%5D'%20AND%20expanded_custom_data.ChromeCrashProto.ptype%3D'browser'#-property-selector,-samplereports,+productname,+productversion:1000,+directory,-clientid,+operatingsystem,+url,+simplifiedurl,+extensions

 
Cc: a...@chromium.org brajkumar@chromium.org
Components: -UI UI>Browser
Labels: -Type-Bug -Pri-2 Target-67 RegressedIn-66 TE-CrashTriage Target-66 FoundIn-66 FoundIn-67 Pri-1 Type-Bug-Regression
Owner: brat...@opera.com
Status: Assigned (was: Untriaged)
This issue is seen from M66-66.0.3359.33, Below link gives in details of the number of instances in which the crash has occurred for associated builds:

https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_Mac%27%20%20AND%20expanded_custom_data.ChromeCrashProto.ptype%3D%27browser%27%20AND%20expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27-%5BMenuControllerCocoa%20menuDidClose%3A%5D%27

Note:
=====
1) This is browser crash listed under beta build 66.0.3359.33 for Mac build on 2 different client ID's.
2) Currently this crash is ranked as number #6 with 2 instances
3) This crash is observed only on Mac OS
4) Crash instances are seen on chrome latest dev M67 #67.0.3371.0 with 3 instances and canary #67.0.3374.0 with 2 instances
5) Not adding any blocker label due to less crash instances, if any unusual spike is observed please feel free to add it it

Used Code Search for the file "menu_controller.mm" and suspecting the below change might have caused this issue.
https://chromium.googlesource.com/chromium/src/+/fcceef5d30a36ed29063cb6782e9f6c6006d5f3a

bratell@ -- Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner.

Thanks!
Project Member

Comment 2 by sheriffbot@chromium.org, Mar 19 2018

Labels: Fracas
Users experienced this crash on the following builds:

Mac Beta 66.0.3359.33 -  0.22 CPM, 4 reports, 4 clients (signature -[MenuControllerCocoa menuDidClose:])

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Comment 3 by rsesek@chromium.org, Apr 20 2018

Cc: brat...@opera.com
Labels: Proj-MacViews
Owner: ellyjo...@chromium.org
This is the #1 browser crash on M66 stable right now. Appears to be NSMenu+Views related. Over to Elly for triage.
Labels: -Pri-1 Pri-0
Owner: spqc...@chromium.org
Thanks rsesek@.

I got a 100% repro for this: right-click in any Views textfield and hit Paste. :\. It repros for example using the bookmark editor with "foobar" on the clipboard.

Assigning this to spqchan@, but since this is the #1 stable crasher and it's already end of week in Sydney I'll start looking now.
Status: Started (was: Assigned)
I have a root cause:

This crash is caused by this code in Textfield::OnCaretBoundsChanged():

#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

interacting poorly with this:

- (void)menuDidClose:(NSMenu*)menu {
  if (isMenuOpen_) {
    model_->MenuWillClose();
    isMenuOpen_ = NO;
  }
  ...
}

The following things happen when using MenuRunnerCocoa:

1) The user clicks the "paste" item
2) Control flows: [MenuControllerCocoa itemWillBeSelected:] -> Textfield::ExecuteTextEditCommand() -> Textfield::UpdateAfterChange() -> Textfield::OnCaretBoundsChanged()
3) Textfield::OnCaretBoundsChanged() resets context_menu_contents_, which is the model for the involved menu. This deallocates the model.
4) Control returns to MenuRunnerImplCocoa::RunMenuAt (indirectly)
5) Control flows from there through AppKit and HIToolBox to [NSCarbonMenuImpl _carbonCloseEvent:handlerCallRef:] and thence [MenuControllerCocoa menuDidClose:]
6) [MenuControllerCocoa menuDidClose:] does model_->MenuWillClose() on the model that was destroyed at step 3.
Probably the best fix is to revert the CL that added that code, which is 645bd3e8c31c73258b236e37a3f8eea41ba2d3d5 from spqchan@. The involved code path can't happen on M67 or later, since we dropped Cocoa menus for MacViews controls in M67, but our safest play is still to:

1) Revert the CL on trunk
2) Merge the revert to M66

Unfortunately, the CL has a merge conflict against trunk when reverting, probably because other work has built on top of it. Probably a better approach is to *just* revert it on the M66 branch. I will take care of that.
Labels: -Restrict-View-EditIssue
Stripping R-V-EditIssue from this - while it is definitely a crash it is not security-sensitive.
I just validated on my M66 build that reverting 645bd3e8c31c73258b236e37a3f8eea41ba2d3d5 (as https://chromium-review.googlesource.com/c/chromium/src/+/1021911):

a) Fixes the repro case in #4
b) Removes the "Lookup" item, which is what I'd expect from the revert
c) Doesn't leave the browser totally broken :)

Comment 9 by brat...@opera.com, Apr 20 2018

Sorry for not responding. I completely missed the mail. Thanks @ellyjones for handling it!
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 20 2018

Labels: merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6256f3e509d353aa667d82706ae64e9baccd7e3a

commit 6256f3e509d353aa667d82706ae64e9baccd7e3a
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Fri Apr 20 14:41:43 2018

Revert "[MacViews] Add Lookup in the Textfield Context Menu"

This reverts commit 645bd3e8c31c73258b236e37a3f8eea41ba2d3d5.

Bug:  823193 
Change-Id: I0d9e8c7f7079d1e5dccd782c02f0dab4f8ac48a2
Reviewed-on: https://chromium-review.googlesource.com/1021911
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#743}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/gfx/BUILD.gn
[delete] https://crrev.com/1ba8d15bf716e397766c79078ec52b0761895c7b/ui/gfx/decorated_text_mac.h
[delete] https://crrev.com/1ba8d15bf716e397766c79078ec52b0761895c7b/ui/gfx/decorated_text_mac.mm
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/gfx/render_text.cc
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/gfx/render_text.h
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/gfx/render_text_unittest.cc
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/views/cocoa/bridged_content_view.mm
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/views/controls/label.cc
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/views/controls/label.h
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/views/controls/textfield/textfield.cc
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/views/controls/textfield/textfield.h
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/views/controls/textfield/textfield_unittest.cc
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/views/controls/views_text_services_context_menu.h
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/views/controls/views_text_services_context_menu_mac.mm
[modify] https://crrev.com/6256f3e509d353aa667d82706ae64e9baccd7e3a/ui/views/word_lookup_client.h

Cc: manoranj...@chromium.org abdulsyed@chromium.org
Labels: ReleaseBlock-Stable
+ abdulsyed@  (M66 release TPM). This is #1 browser crash on M66 stable.
Labels: M-67 M-66
ellyjones@, this will also need a merge to M67. Please request a merge to M67. Thank you.
Status:

I reverted the involved CL on M66, so the bug is gone on M66 head now.
For M67 onward, the bug doesn't manifest because the involved code is dead.
Labels: -M-67
Removing "M-67" per comment #13.
Labels: -FoundIn-67 -Target-67
Also removing "FoundIn-67" * "Target-67" per comment #13.
Labels: -Pri-0 -ReleaseBlock-Stable Pri-1
Dropping pri and stripping RBS: since this is now reverted in M66, the next M66 release should not have this bug.
Status: Fixed (was: Started)

Sign in to add a comment