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

Issue 733065 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug-Regression



Sign in to add a comment

Mac reload button doesn't show a menu (menu_button.mm Check failed: [self attachedMenu] when right-clicking "reload")

Project Member Reported by tapted@chromium.org, Jun 14 2017

Issue description

Chrome Version       : 61.0.3128.0
OS Version: OS X 10.12.5

What steps will reproduce the problem?
1a. On a Mac debug build, right-click the reload button
1b. On a Mac release, right-click the reload button


What is the expected result?

a) No crash
b) A menu appears


What happens instead of that?

a) DCHECKs
b) No menu appears


[ReloadButton awakeFromNib] calls [self setOpenMenuOnRightClick:YES]; And it has a -populateMenu: method that looks like it tries to add things, but the menu doesn't show up.

Did we ever show this menu? Other platforms don't seem to have it. Can we just delete this reload button menu code?


https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/toolbar/reload_button_cocoa.mm?type=cs&q=setopenmenuonrightclick+package:%5Echromium$&l=74


[98544:775:0614/132405.555057:FATAL:menu_button.mm(201)] Check failed: [self attachedMenu].
0   libbase.dylib                       0x00000001034dceee base::debug::StackTrace::StackTrace(unsigned long) + 174
1   libbase.dylib                       0x00000001034dcf8d base::debug::StackTrace::StackTrace(unsigned long) + 29
2   libbase.dylib                       0x00000001034db20c base::debug::StackTrace::StackTrace() + 28
3   libbase.dylib                       0x000000010357b18f logging::LogMessage::~LogMessage() + 479
4   libbase.dylib                       0x0000000103578b05 logging::LogMessage::~LogMessage() + 21
5   libchrome_dll.dylib                 0x0000000116bf2483 -[MenuButton(Private) clickShowMenu:] + 259
6   libchrome_dll.dylib                 0x0000000116bf14ad -[MenuButton rightMouseDown:] + 125
7   AppKit                              0x00007fffb22da7a9 forwardMethod + 133
8   AppKit                              0x00007fffb2538eea -[NSView rightMouseDown:] + 152
9   AppKit                              0x00007fffb2a52c10 -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] + 6458
10  AppKit                              0x00007fffb2a50f0a -[NSWindow(NSEventRouting) sendEvent:] + 541
11  libchrome_dll.dylib                 0x0000000116ade54a -[ChromeEventProcessingWindow sendEvent:] + 122
12  AppKit                              0x00007fffb28d5e7e -[NSApplication(NSEvent) sendEvent:] + 3190
13  libchrome_dll.dylib                 0x000000011104d043 __34-[BrowserCrApplication sendEvent:]_block_invoke + 259
14  libbase.dylib                       0x00000001035837da base::mac::CallWithEHFrame(void () block_pointer) + 10
15  libchrome_dll.dylib                 0x000000011104cdcf -[BrowserCrApplication sendEvent:] + 575
16  AppKit                              0x00007fffb2150427 -[NSApplication run] + 1002
 

Comment 1 by rsesek@chromium.org, Jun 14 2017

Owner: rsesek@chromium.org
Status: Started (was: Unconfirmed)
Other platforms seem to have the same menu items: https://cs.chromium.org/chromium/src/chrome/browser/ui/views/toolbar/reload_button.cc?type=cs&q=IDS_RELOAD_MENU_EMPTY_AND_HARD_RELOAD_ITEM&sq=package:chromium&l=31

But it only show sup if DevTools is open.

The bug is that -[ReloadButton setMenuEnabled:] doesn't disable -setOpenMenuOnRightClick: properly.
Screen Shot 2017-06-14 at 7.33.00 PM.png
99 KB View Download
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 15 2017

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

commit f7fd12c1dedd498830bc2e9d049687749dbadcf9
Author: Robert Sesek <rsesek@chromium.org>
Date: Thu Jun 15 20:21:05 2017

[Mac] Fix a DCHECK when right-clicking the reload button.

The reload button has an associated context menu that is only shown if DevTools
is open. The -[ReloadButton setMenuEnabled:] did not disable the menu in the
appropriate way, resulting in a DCHECK.

This change also refactors out a new MenuTestObserver class to assist with
testing NSMenu open/close behavior.

Bug:  733065 
Change-Id: I321a640e5e8e85f849dbd1fe64317dba9286a1c8
Reviewed-on: https://chromium-review.googlesource.com/536513
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479808}
[modify] https://crrev.com/f7fd12c1dedd498830bc2e9d049687749dbadcf9/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/f7fd12c1dedd498830bc2e9d049687749dbadcf9/chrome/browser/ui/cocoa/menu_button_unittest.mm
[add] https://crrev.com/f7fd12c1dedd498830bc2e9d049687749dbadcf9/chrome/browser/ui/cocoa/test/menu_test_observer.h
[add] https://crrev.com/f7fd12c1dedd498830bc2e9d049687749dbadcf9/chrome/browser/ui/cocoa/test/menu_test_observer.mm
[modify] https://crrev.com/f7fd12c1dedd498830bc2e9d049687749dbadcf9/chrome/browser/ui/cocoa/toolbar/reload_button_cocoa.mm
[modify] https://crrev.com/f7fd12c1dedd498830bc2e9d049687749dbadcf9/chrome/browser/ui/cocoa/toolbar/reload_button_unittest.mm

Comment 3 by rsesek@chromium.org, Jun 15 2017

Status: Fixed (was: Started)

Sign in to add a comment