Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(125)

Issue 2836008: Make the HistoryMenuBridge::HistoryItem co-own the NSMenuItem. This hopefully fixes a top-crash. (Closed)

Created:
10 years, 6 months ago by Robert Sesek
Modified:
9 years, 7 months ago
Reviewers:
John Grabowski
CC:
chromium-reviews
Base URL:
http://src.chromium.org/git/chromium.git
Visibility:
Public.

Description

Make the HistoryMenuBridge::HistoryItem co-own the NSMenuItem. This hopefully fixes a top-crash. BUG=46289 TEST=Open 3 windows, with 10-12 tabs in each. Cmd+W rapidly. Chrome doesn't crash. See bug for details. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=50134

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -15 lines) Patch
M chrome/browser/cocoa/history_menu_bridge.h View 1 chunk +6 lines, -2 lines 1 comment Download
M chrome/browser/cocoa/history_menu_bridge.mm View 1 chunk +10 lines, -11 lines 2 comments Download
M chrome/browser/cocoa/history_menu_bridge_unittest.mm View 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Robert Sesek
P1 top-crasher plz.
10 years, 6 months ago (2010-06-17 15:50:12 UTC) #1
John Grabowski
http://codereview.chromium.org/2836008/diff/1/3 File chrome/browser/cocoa/history_menu_bridge.mm (left): http://codereview.chromium.org/2836008/diff/1/3#oldcode301 chrome/browser/cocoa/history_menu_bridge.mm:301: scoped_nsobject<NSMenuItem> menu_item( Perhaps there is an easier solution? If ...
10 years, 6 months ago (2010-06-17 18:53:55 UTC) #2
Robert Sesek
http://codereview.chromium.org/2836008/diff/1/3 File chrome/browser/cocoa/history_menu_bridge.mm (left): http://codereview.chromium.org/2836008/diff/1/3#oldcode301 chrome/browser/cocoa/history_menu_bridge.mm:301: scoped_nsobject<NSMenuItem> menu_item( On 2010/06/17 18:53:55, John Grabowski wrote: > ...
10 years, 6 months ago (2010-06-17 19:05:44 UTC) #3
John Grabowski
LGTM http://codereview.chromium.org/2836008/diff/1/2 File chrome/browser/cocoa/history_menu_bridge.h (right): http://codereview.chromium.org/2836008/diff/1/2#newcode92 chrome/browser/cocoa/history_menu_bridge.h:92: // the NSMenu, but during a rebuild flood ...
10 years, 6 months ago (2010-06-17 19:30:44 UTC) #4
John Grabowski
10 years, 6 months ago (2010-06-17 19:31:18 UTC) #5
Oh -- please add details in this CL description on how to repro for the testers
to verify properly.

Powered by Google App Engine
This is Rietveld 408576698