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

Issue 192031: [Mac] Add testing code to expose an NSColor memory leak. (Closed)

Created:
11 years, 3 months ago by Scott Hess - ex-Googler
Modified:
9 years, 6 months ago
Reviewers:
John Grabowski
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

[Mac] Add testing code to expose an NSColor memory leak. AutocompleteTextFieldCell calls -keyboardFocusIndicatorColor when drawing the focus ring, which valgrind considers a leak. While we were testing drawing when not focussed, unit tests could not test drawing when focussed without the window being made key, which is bad for many reasons. This change adds some code to allow faking the key window, then adds tests for drawing when focussed which exposes the memory leak, then adds a suppression for that memory leak. http://crbug.com/21137 TEST=valgrind AutocompleteTextFieldCellTest.Display

Patch Set 1 #

Patch Set 2 : Adjust suppression to work for keyword-drawing, too. #

Total comments: 1

Patch Set 3 : Remove egregious s/private/public/ leftover from prototyping. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -7 lines) Patch
M chrome/browser/cocoa/autocomplete_text_field_cell_unittest.mm View 2 chunks +7 lines, -1 line 0 comments Download
M chrome/browser/cocoa/autocomplete_text_field_unittest.mm View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/cocoa_test_helper.h View 1 2 3 chunks +37 lines, -6 lines 0 comments Download
A chrome/browser/cocoa/cocoa_test_helper.mm View 1 chunk +28 lines, -0 lines 0 comments Download
M chrome/chrome.gyp View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M tools/valgrind/memcheck/suppressions_mac.txt View 1 2 1 chunk +9 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Scott Hess - ex-Googler
John, Unfortunately, this didn't lead to a suppression which will work for the other leak, ...
11 years, 3 months ago (2009-09-04 23:18:47 UTC) #1
Scott Hess - ex-Googler
On 2009/09/04 23:18:47, shess wrote: > John, > > Unfortunately, this didn't lead to a ...
11 years, 3 months ago (2009-09-04 23:37:51 UTC) #2
John Grabowski
11 years, 3 months ago (2009-09-04 23:50:39 UTC) #3
LGTM

http://codereview.chromium.org/192031/diff/3001/2004
File chrome/browser/cocoa/cocoa_test_helper.h (right):

http://codereview.chromium.org/192031/diff/3001/2004#newcode21
Line 21: @interface CocoaTestHelperWindow : NSWindow {
Real nice how you added this in a way others can reuse.

Powered by Google App Engine
This is Rietveld 408576698