Fix leaks found by valgrind (which now works on the Mac!)

http://code.google.com/p/chromium/issues/detail?id=9376

Review URL: http://codereview.chromium.org/53116

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@12629 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 15 insertions, 8 deletions

diff --git a/chrome/browser/cocoa/bookmark_menu_bridge.h b/chrome/browser/cocoa/bookmark_menu_bridge.h
index 23c1d47..7c4565c 100644
--- a/chrome/browser/cocoa/bookmark_menu_bridge.h
+++ b/chrome/browser/cocoa/bookmark_menu_bridge.h
@@ -76,6 +76,7 @@ class BookmarkMenuBridge : public BookmarkModelObserver,
   friend class BookmarkMenuBridgeTest;
 
   BookmarkMenuCocoaController* controller_;  // strong
+  bool observing_;  // are we observing the browser list?
 };
 
 #endif  // CHROME_BROWSER_COCOA_BOOKMARK_MENU_BRIDGE_H_
diff --git a/chrome/browser/cocoa/bookmark_menu_bridge.mm b/chrome/browser/cocoa/bookmark_menu_bridge.mm
index 8d32150..f2754a0 100644
--- a/chrome/browser/cocoa/bookmark_menu_bridge.mm
+++ b/chrome/browser/cocoa/bookmark_menu_bridge.mm
@@ -14,12 +14,17 @@
 #include "chrome/browser/profile_manager.h"
 
 BookmarkMenuBridge::BookmarkMenuBridge()
-    : controller_([[BookmarkMenuCocoaController alloc] initWithBridge:this]) {
+    : controller_([[BookmarkMenuCocoaController alloc] initWithBridge:this]),
+      observing_(true) {
   BrowserList::AddObserver(this);
 }
 
 BookmarkMenuBridge::~BookmarkMenuBridge() {
-  GetBookmarkModel()->RemoveObserver(this);
+  if (observing_)
+    BrowserList::RemoveObserver(this);
+  BookmarkModel *model = GetBookmarkModel();
+  if (model)
+    model->RemoveObserver(this);
   [controller_ release];
 }
 
@@ -98,6 +103,7 @@ void BookmarkMenuBridge::OnBrowserRemoving(const Browser* browser) {
 // complete.
 void BookmarkMenuBridge::OnBrowserSetLastActive(const Browser* browser) {
   BrowserList::RemoveObserver(this);
+  observing_ = false;
   BookmarkModel* model = GetBookmarkModel();
   model->AddObserver(this);
   if (model->IsLoaded())
diff --git a/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm b/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm
index 48a6dc4..ac37a72 100644
--- a/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_menu_bridge_unittest.mm
@@ -41,8 +41,8 @@ class BookmarkMenuBridgeTest : public testing::Test {
 
 // Test that ClearBookmarkMenu() removes all bookmark menus.
 TEST_F(BookmarkMenuBridgeTest, TestClearBookmarkMenu) {
-  BookmarkMenuBridge* bridge = new BookmarkMenuBridge();
-  EXPECT_TRUE(bridge);
+  scoped_ptr<BookmarkMenuBridge> bridge(new BookmarkMenuBridge());
+  EXPECT_TRUE(bridge.get());
 
   NSMenu* menu = [[[NSMenu alloc] initWithTitle:@"foo"] autorelease];
 
@@ -53,7 +53,7 @@ TEST_F(BookmarkMenuBridgeTest, TestClearBookmarkMenu) {
   AddItemToMenu(menu, @"not", @selector(openBookmarkMenuItem:));
   AddItemToMenu(menu, @"zippy", @selector(length));
 
-  ClearBookmarkMenu(bridge, menu);
+  ClearBookmarkMenu(bridge.get(), menu);
 
   // Make sure all bookmark items are removed, and all items with
   // submenus removed.
@@ -68,8 +68,8 @@ TEST_F(BookmarkMenuBridgeTest, TestClearBookmarkMenu) {
 TEST_F(BookmarkMenuBridgeTest, TestAddNodeToMenu) {
   Profile* profile = browser_test_helper_.GetProfile();
 
-  BookmarkMenuBridge *bridge = new BookmarkMenuBridge();
-  EXPECT_TRUE(bridge);
+  scoped_ptr<BookmarkMenuBridge> bridge(new BookmarkMenuBridge());
+  EXPECT_TRUE(bridge.get());
 
   NSMenu* menu = [[[NSMenu alloc] initWithTitle:@"foo"] autorelease];
 
@@ -87,7 +87,7 @@ TEST_F(BookmarkMenuBridgeTest, TestAddNodeToMenu) {
   root->GetChild(1)->Add(0, node);
 
   // Add to the NSMenu, then confirm it looks good
-  AddNodeToMenu(bridge, root, menu);
+  AddNodeToMenu(bridge.get(), root, menu);
 
   EXPECT_EQ(3, [menu numberOfItems]);
   for (int x=0; x < 3; x++) {