views/bookmarks: Fix memory leak in BookmarkEditorView.

chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc

Index: chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
index df017a94c75d257cad86e3a8c836f7712232a9bd..d4293da1f91c8ec0f30b444afd0ba21c57e14104 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
@@ -23,7 +23,6 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "views/background.h"
 #include "views/controls/button/text_button.h"
-#include "views/controls/label.h"
 #include "views/controls/menu/menu_2.h"
 #include "views/controls/textfield/textfield.h"
 #include "views/focus/focus_manager.h"
@@ -31,20 +30,20 @@
 #include "views/layout/layout_constants.h"
 #include "views/widget/widget.h"
 
-using views::Button;
-using views::ColumnSet;
 using views::GridLayout;
-using views::Label;
-using views::Textfield;
+
+namespace {
 
 // Background color of text field when URL is invalid.
-static const SkColor kErrorColor = SkColorSetRGB(0xFF, 0xBC, 0xBC);
+const SkColor kErrorColor = SkColorSetRGB(0xFF, 0xBC, 0xBC);
 
 // Preferred width of the tree.
-static const int kTreeWidth = 300;
+const int kTreeWidth = 300;
 
 // ID for various children.
-static const int kNewFolderButtonID = 1002;
+const int kNewFolderButtonID = 1002;
+
+}  // namespace
 
 // static
 void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
@@ -66,7 +65,6 @@ BookmarkEditorView::BookmarkEditorView(
     : profile_(profile),
       tree_view_(NULL),
       new_folder_button_(NULL),
-      url_label_(NULL),
       title_label_(NULL),
       parent_(parent),
       details_(details),
@@ -179,13 +177,13 @@ bool BookmarkEditorView::CanEdit(views::TreeView* tree_view,
   return (bb_node->parent() && bb_node->parent()->parent());
 }
 
-void BookmarkEditorView::ContentsChanged(Textfield* sender,
+void BookmarkEditorView::ContentsChanged(views::Textfield* sender,
                                          const std::wstring& new_contents) {
   UserInputChanged();
 }
 
-void BookmarkEditorView::ButtonPressed(
-    Button* sender, const views::Event& event) {
+void BookmarkEditorView::ButtonPressed(views::Button* sender,
+                                       const views::Event& event) {
   DCHECK(sender);
   switch (sender->id()) {
     case kNewFolderButtonID:
@@ -327,9 +325,10 @@ void BookmarkEditorView::Init() {
   url_tf_.SetText(UTF16ToWide(url_text));
   url_tf_.SetController(this);
 
-  url_label_ = new views::Label(
+  url_label_.set_parent_owned(false);

[Peter Kasting, 2011/08/11 18:59:42]

This whole system concerns me.

We have |url_tf_| and |url_label_| always existing, but only sometimes in the
view hierarchy.  Then various functions access |url_tf_|.

It seems like these objects ought to be created only when they're actually going
to be added to the tree.  In order to make this safe, we need to either verify
that the accessing functions will only be called in this case (and add some
DCHECKs to make that clear), or we should understand why they're accessing
invisible objects, which seems very wrong.

+  url_label_.SetText(
       UTF16ToWide(l10n_util::GetStringUTF16(IDS_BOOMARK_EDITOR_URL_LABEL)));
-  url_tf_.SetAccessibleName(WideToUTF16Hack(url_label_->GetText()));
+  url_tf_.SetAccessibleName(WideToUTF16Hack(url_label_.GetText()));
 
   if (show_tree_) {
     tree_view_ = new views::TreeView();
@@ -354,7 +353,7 @@ void BookmarkEditorView::Init() {
   const int single_column_view_set_id = 1;
   const int buttons_column_set_id = 2;
 
-  ColumnSet* column_set = layout->AddColumnSet(labels_column_set_id);
+  views::ColumnSet* column_set = layout->AddColumnSet(labels_column_set_id);
   column_set->AddColumn(GridLayout::LEADING, GridLayout::CENTER, 0,
                         GridLayout::USE_PREF, 0, 0);
   column_set->AddPaddingColumn(0, views::kRelatedControlHorizontalSpacing);
@@ -385,7 +384,7 @@ void BookmarkEditorView::Init() {
     layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
     layout->StartRow(0, labels_column_set_id);
-    layout->AddView(url_label_);
+    layout->AddView(&url_label_);
     layout->AddView(&url_tf_);
   }