views/bookmarks: Fix memory leak in BookmarkEditorView.

Allocate |url_label_| on the stack instead of in the free store(heap), and set
set_parent_owned to false.

Thus, we manage its memory when we create a unittest with EditDetails::NEW_FOLDER.

BUG=92159
TEST=None

R=pkasting@chromium.org

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

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

3 files changed, 54 insertions, 45 deletions

diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
index df017a9..a0d5601 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc
@@ -31,20 +31,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,
@@ -67,6 +67,7 @@ BookmarkEditorView::BookmarkEditorView(
       tree_view_(NULL),
       new_folder_button_(NULL),
       url_label_(NULL),
+      url_tf_(NULL),
       title_label_(NULL),
       parent_(parent),
       details_(details),
@@ -110,9 +111,11 @@ std::wstring BookmarkEditorView::GetWindowTitle() const {
 
 bool BookmarkEditorView::Accept() {
   if (!IsDialogButtonEnabled(MessageBoxFlags::DIALOGBUTTON_OK)) {
-    // The url is invalid, focus the url field.
-    url_tf_.SelectAll();
-    url_tf_.RequestFocus();
+    if (details_.type != EditDetails::NEW_FOLDER) {
+      // The url is invalid, focus the url field.
+      url_tf_->SelectAll();
+      url_tf_->RequestFocus();
+    }
     return false;
   }
   // Otherwise save changes and close the dialog box.
@@ -179,13 +182,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:
@@ -288,7 +291,6 @@ void BookmarkEditorView::Init() {
   DCHECK(bb_model_);
   bb_model_->AddObserver(this);
 
-  url_tf_.set_parent_owned(false);
   title_tf_.set_parent_owned(false);
 
   std::wstring title;
@@ -312,25 +314,6 @@ void BookmarkEditorView::Init() {
       UTF16ToWide(l10n_util::GetStringUTF16(IDS_BOOMARK_EDITOR_NAME_LABEL)));
   title_tf_.SetAccessibleName(WideToUTF16Hack(title_label_->GetText()));
 
-  string16 url_text;
-  if (details_.type != EditDetails::NEW_FOLDER) {
-    std::string languages = profile_
-        ? profile_->GetPrefs()->GetString(prefs::kAcceptLanguages)
-        : std::string();
-    // Because this gets parsed by FixupURL(), it's safe to omit the scheme or
-    // trailing slash, and unescape most characters, but we need to not drop any
-    // username/password, or unescape anything that changes the meaning.
-    url_text = net::FormatUrl(url, languages,
-        net::kFormatUrlOmitAll & ~net::kFormatUrlOmitUsernamePassword,
-        UnescapeRule::SPACES, NULL, NULL, NULL);
-  }
-  url_tf_.SetText(UTF16ToWide(url_text));
-  url_tf_.SetController(this);
-
-  url_label_ = new views::Label(
-      UTF16ToWide(l10n_util::GetStringUTF16(IDS_BOOMARK_EDITOR_URL_LABEL)));
-  url_tf_.SetAccessibleName(WideToUTF16Hack(url_label_->GetText()));
-
   if (show_tree_) {
     tree_view_ = new views::TreeView();
     tree_view_->set_lines_at_root(true);
@@ -354,7 +337,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);
@@ -382,11 +365,29 @@ void BookmarkEditorView::Init() {
   layout->AddView(&title_tf_);
 
   if (details_.type != EditDetails::NEW_FOLDER) {
+    url_label_ = new views::Label(
+      UTF16ToWide(l10n_util::GetStringUTF16(IDS_BOOMARK_EDITOR_URL_LABEL)));
+
+    std::string languages =
+        profile_ ? profile_->GetPrefs()->GetString(prefs::kAcceptLanguages)
+                 : std::string();
+    // Because this gets parsed by FixupURL(), it's safe to omit the scheme or
+    // trailing slash, and unescape most characters, but we need to not drop any
+    // username/password, or unescape anything that changes the meaning.
+    string16 url_text = net::FormatUrl(url, languages,
+        net::kFormatUrlOmitAll & ~net::kFormatUrlOmitUsernamePassword,
+        UnescapeRule::SPACES, NULL, NULL, NULL);
+
+    url_tf_ = new views::Textfield;
+    url_tf_->SetText(UTF16ToWide(url_text));
+    url_tf_->SetController(this);
+    url_tf_->SetAccessibleName(WideToUTF16Hack(url_label_->GetText()));
+
     layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
 
     layout->StartRow(0, labels_column_set_id);
     layout->AddView(url_label_);
-    layout->AddView(&url_tf_);
+    layout->AddView(url_tf_);
   }
 
   if (show_tree_) {
@@ -458,8 +459,11 @@ void BookmarkEditorView::Reset() {
   if (parent())
     ExpandAndSelect();
 }
+
 GURL BookmarkEditorView::GetInputURL() const {
-  return URLFixerUpper::FixupURL(UTF16ToUTF8(url_tf_.text()), std::string());
+  if (details_.type == EditDetails::NEW_FOLDER)
+    return GURL();
+  return URLFixerUpper::FixupURL(UTF16ToUTF8(url_tf_->text()), std::string());
 }
 
 std::wstring BookmarkEditorView::GetInputTitle() const {
@@ -467,11 +471,13 @@ std::wstring BookmarkEditorView::GetInputTitle() const {
 }
 
 void BookmarkEditorView::UserInputChanged() {
-  const GURL url(GetInputURL());
-  if (!url.is_valid())
-    url_tf_.SetBackgroundColor(kErrorColor);
-  else
-    url_tf_.UseDefaultBackgroundColor();
+  if (details_.type != EditDetails::NEW_FOLDER) {
+    const GURL url(GetInputURL());
+    if (!url.is_valid())
+      url_tf_->SetBackgroundColor(kErrorColor);
+    else
+      url_tf_->UseDefaultBackgroundColor();
+  }
   GetDialogClientView()->UpdateDialogButtons();
 }
 
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.h b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.h
index 5e06e19..b5a07e3 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view.h
+++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view.h
@@ -239,8 +239,8 @@ class BookmarkEditorView : public BookmarkEditor,
   // The label for the url text field.
   views::Label* url_label_;
 
-  // Used for editing the URL.
-  views::Textfield url_tf_;
+  // The text field used for editing the URL.
+  views::Textfield* url_tf_;
 
   // The label for the title text field.
   views::Label* title_label_;
diff --git a/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc b/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc
index 8ec3a59..cf481ff 100644
--- a/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc
+++ b/chrome/browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc
@@ -69,7 +69,8 @@ class BookmarkEditorViewTest : public TestingBrowserProcessTest {
   }
 
   void SetURLText(const std::wstring& text) {
-    editor_->url_tf_.SetText(text);
+    if (editor_->details_.type != BookmarkEditor::EditDetails::NEW_FOLDER)
+      editor_->url_tf_->SetText(text);
   }
 
   void ApplyEdits(BookmarkEditorView::EditorNode* node) {
@@ -82,7 +83,9 @@ class BookmarkEditorViewTest : public TestingBrowserProcessTest {
   }
 
   bool URLTFHasParent() {
-    return editor_->url_tf_.parent();
+    if (editor_->details_.type == BookmarkEditor::EditDetails::NEW_FOLDER)
+      return false;
+    return editor_->url_tf_->parent();
   }
 
   MessageLoopForUI message_loop_;