Tests for BookmarkEditorView.

BUG=1323423
TEST=this is some tests.



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

4 files changed, 249 insertions, 5 deletions

diff --git a/chrome/browser/views/bookmark_editor_view.cc b/chrome/browser/views/bookmark_editor_view.cc
index 8f6ae2c..8ae30ed 100644
--- a/chrome/browser/views/bookmark_editor_view.cc
+++ b/chrome/browser/views/bookmark_editor_view.cc
@@ -403,13 +403,17 @@ void BookmarkEditorView::NewGroup() {
     return;
   }
 
+  tree_view_.StartEditing(AddNewGroup(parent));
+}
+
+BookmarkEditorView::BookmarkNode* BookmarkEditorView::AddNewGroup(
+    BookmarkNode* parent) {
   BookmarkNode* new_node = new BookmarkNode();
   new_node->SetTitle(l10n_util::GetString(IDS_BOOMARK_EDITOR_NEW_FOLDER_NAME));
   new_node->value = 0;
   // new_node is now owned by parent.
   tree_model_->Add(parent, parent->GetChildCount(), new_node);
-  // Edit the new node.
-  tree_view_.StartEditing(new_node);
+  return new_node;
 }
 
 void BookmarkEditorView::ExpandAndSelect() {
@@ -473,6 +477,11 @@ void BookmarkEditorView::ApplyEdits() {
     NOTREACHED();
     return;
   }
+  ApplyEdits(tree_model_->AsNode(tree_view_.GetSelectedNode()));
+}
+
+void BookmarkEditorView::ApplyEdits(BookmarkNode* parent) {
+  DCHECK(parent);
 
   // We're going to apply edits to the bookmark bar model, which will call us
   // back. Normally when a structural edit occurs we reset the tree model.
@@ -494,8 +503,7 @@ void BookmarkEditorView::ApplyEdits() {
   // Create the new groups and update the titles.
   BookmarkBarNode* new_parent = NULL;
   ApplyNameChangesAndCreateNewGroups(
-      bb_model_->root_node(), tree_model_->GetRoot(),
-      tree_model_->AsNode(tree_view_.GetSelectedNode()), &new_parent);
+      bb_model_->root_node(), tree_model_->GetRoot(), parent, &new_parent);
 
   if (!new_parent) {
     // Bookmarks must be parented.
diff --git a/chrome/browser/views/bookmark_editor_view.h b/chrome/browser/views/bookmark_editor_view.h
index 5a2b9d6..8662b07 100644
--- a/chrome/browser/views/bookmark_editor_view.h
+++ b/chrome/browser/views/bookmark_editor_view.h
@@ -64,6 +64,12 @@ class BookmarkEditorView : public ChromeViews::View,
                            public ChromeViews::ContextMenuController,
                            public Menu::Delegate,
                            public BookmarkBarModelObserver {
+  FRIEND_TEST(BookmarkEditorViewTest, ChangeParent);
+  FRIEND_TEST(BookmarkEditorViewTest, ChangeURLToExistingURL);
+  FRIEND_TEST(BookmarkEditorViewTest, EditTitleKeepsPosition);
+  FRIEND_TEST(BookmarkEditorViewTest, EditURLKeepsPosition);
+  FRIEND_TEST(BookmarkEditorViewTest, ModelsMatch);
+  FRIEND_TEST(BookmarkEditorViewTest, MoveToNewParent);
  public:
   // Shows the BookmarkEditorView editing the specified entry.
   static void Show(HWND parent_window,
@@ -194,9 +200,13 @@ class BookmarkEditorView : public ChromeViews::View,
   BookmarkNode* FindNodeWithID(BookmarkEditorView::BookmarkNode* node,
                                history::UIStarID id);
 
-  // Applies the edits done by the user.
+  // Invokes ApplyEdits with the selected node.
   void ApplyEdits();
 
+  // Applies the edits done by the user. |parent| gives the parent of the URL
+  // being edited.
+  void ApplyEdits(BookmarkNode* parent);
+
   // Recursively adds newly created groups and sets the title of nodes to
   // match the user edited title.
   //
@@ -227,6 +237,11 @@ class BookmarkEditorView : public ChromeViews::View,
   // editing on the new gorup as well.
   void NewGroup();
 
+  // Creates a new BookmarkNode as the last child of parent. The new node is
+  // added to the model and returned. This does NOT start editing. This is used
+  // internally by NewGroup and broken into a separate method for testing.
+  BookmarkNode* AddNewGroup(BookmarkNode* parent);
+
   // Profile the entry is from.
   Profile* profile_;
 
diff --git a/chrome/browser/views/bookmark_editor_view_unittest.cc b/chrome/browser/views/bookmark_editor_view_unittest.cc
new file mode 100644
index 0000000..4ea79b6
--- /dev/null
+++ b/chrome/browser/views/bookmark_editor_view_unittest.cc
@@ -0,0 +1,213 @@
+// Copyright 2008, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//    * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//    * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//    * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include "base/string_util.h"
+#include "chrome/browser/bookmark_bar_model.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/views/bookmark_editor_view.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/common/pref_service.h"
+#include "chrome/test/testing_profile.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// Base class for bookmark editor tests. Creates a BookmarkBarModel and
+// populates it with test data.
+class BookmarkEditorViewTest : public testing::Test {
+ public:
+  BookmarkEditorViewTest() : model_(NULL) {
+  }
+
+  virtual void SetUp() {
+    profile_.reset(new TestingProfile());
+    profile_->set_has_history_service(true);
+    profile_->CreateBookmarkBarModel();
+
+    model_ = profile_->GetBookmarkBarModel();
+
+    AddTestData();
+  }
+
+  virtual void TearDown() {
+  }
+
+ protected:
+  BookmarkBarModel* model_;
+  scoped_ptr<TestingProfile> profile_;
+
+  std::string base_path() const { return "file:///c:/tmp/"; }
+
+ private:
+  // Creates the following structure:
+  // bookmark bar node
+  //   a
+  //   F1
+  //    f1a
+  //    F11
+  //     f11a
+  //   F2
+  // other node
+  //   oa
+  //   OF1
+  //     of1a
+  void AddTestData() {
+    std::string test_base = base_path();
+
+    model_->AddURL(model_->GetBookmarkBarNode(), 0, L"a",
+                   GURL(test_base + "a"));
+    BookmarkBarNode* f1 =
+        model_->AddGroup(model_->GetBookmarkBarNode(), 1, L"F1");
+    model_->AddURL(f1, 0, L"f1a", GURL(test_base + "f1a"));
+    BookmarkBarNode* f11 = model_->AddGroup(f1, 1, L"F11");
+    model_->AddURL(f11, 0, L"f11a", GURL(test_base + "f11a"));
+    model_->AddGroup(model_->GetBookmarkBarNode(), 2, L"F2");
+
+    // Children of the other node.
+    model_->AddURL(model_->other_node(), 0, L"oa",
+                   GURL(test_base + "oa"));
+    BookmarkBarNode* of1 =
+        model_->AddGroup(model_->other_node(), 1, L"OF1");
+    model_->AddURL(of1, 0, L"of1a", GURL(test_base + "of1a"));
+  }
+};
+
+// Makes sure the tree model matches that of the bookmark bar model.
+TEST_F(BookmarkEditorViewTest, ModelsMatch) {
+  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "xxx"),
+                            L"xxx");
+  BookmarkEditorView::BookmarkNode* editor_root = editor.tree_model_->GetRoot();
+  // The root should have two children, one for the bookmark bar node,
+  // the other for the 'other bookmarks' folder.
+  ASSERT_EQ(2, editor_root->GetChildCount());
+
+  BookmarkEditorView::BookmarkNode* bb_node = editor_root->GetChild(0);
+  // The root should have 2 nodes: folder F1 and F2.
+  ASSERT_EQ(2, bb_node->GetChildCount());
+  ASSERT_EQ(L"F1", bb_node->GetChild(0)->GetTitle());
+  ASSERT_EQ(L"F2", bb_node->GetChild(1)->GetTitle());
+
+  // F1 should have one child, F11
+  ASSERT_EQ(1, bb_node->GetChild(0)->GetChildCount());
+  ASSERT_EQ(L"F11", bb_node->GetChild(0)->GetChild(0)->GetTitle());
+
+  BookmarkEditorView::BookmarkNode* other_node = editor_root->GetChild(1);
+  // Other node should have one child (OF1).
+  ASSERT_EQ(1, other_node->GetChildCount());
+  ASSERT_EQ(L"OF1", other_node->GetChild(0)->GetTitle());
+}
+
+// Changes the title and makes sure parent/visual order doesn't change.
+TEST_F(BookmarkEditorViewTest, EditTitleKeepsPosition) {
+  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"new_a");
+
+  editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(0));
+
+  BookmarkBarNode* bb_node =
+      profile_->GetBookmarkBarModel()->GetBookmarkBarNode();
+  ASSERT_EQ(L"new_a", bb_node->GetChild(0)->GetTitle());
+  // The URL shouldn't have changed.
+  ASSERT_TRUE(GURL(base_path() + "a") == bb_node->GetChild(0)->GetURL());
+}
+
+// Changes the url and makes sure parent/visual order doesn't change.
+TEST_F(BookmarkEditorViewTest, EditURLKeepsPosition) {
+  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
+
+  editor.url_tf_.SetText(UTF8ToWide(GURL(base_path() + "new_a").spec()));
+
+  editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(0));
+
+  BookmarkBarNode* bb_node =
+      profile_->GetBookmarkBarModel()->GetBookmarkBarNode();
+  ASSERT_EQ(L"a", bb_node->GetChild(0)->GetTitle());
+  // The URL should have changed.
+  ASSERT_TRUE(GURL(base_path() + "new_a") == bb_node->GetChild(0)->GetURL());
+}
+
+// Moves 'a' to be a child of the other node.
+TEST_F(BookmarkEditorViewTest, ChangeParent) {
+  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
+
+  editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(1));
+
+  BookmarkBarNode* other_node = profile_->GetBookmarkBarModel()->other_node();
+  ASSERT_EQ(L"a", other_node->GetChild(2)->GetTitle());
+  ASSERT_TRUE(GURL(base_path() + "a") == other_node->GetChild(2)->GetURL());
+}
+
+// Changes the URL to a URL that is already starred.
+TEST_F(BookmarkEditorViewTest, ChangeURLToExistingURL) {
+  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
+
+  editor.url_tf_.SetText(UTF8ToWide(GURL(base_path() + "f1a").spec()));
+
+  editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(0));
+
+  // Position shouldn't have changed.
+  BookmarkBarNode* bb_node =
+      profile_->GetBookmarkBarModel()->GetBookmarkBarNode();
+  ASSERT_EQ(L"a", bb_node->GetChild(0)->GetTitle());
+  // The URL should have changed.
+  ASSERT_TRUE(GURL(base_path() + "f1a") == bb_node->GetChild(0)->GetURL());
+
+  // And F1 should have one last child (f1a was removed from it).
+  ASSERT_EQ(1, bb_node->GetChild(1)->GetChildCount());
+  ASSERT_NE(L"f1a", bb_node->GetChild(1)->GetChild(0)->GetTitle());
+}
+
+// Creates a new folder and moves a node to it.
+TEST_F(BookmarkEditorViewTest, MoveToNewParent) {
+  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
+
+  // Create two nodes: "F21" as a child of "F2" and "F211" as a child of "F21".
+  BookmarkEditorView::BookmarkNode* f2 =
+      editor.tree_model_->GetRoot()->GetChild(0)->GetChild(1);
+  BookmarkEditorView::BookmarkNode* f21 = editor.AddNewGroup(f2);
+  f21->SetTitle(L"F21");
+  BookmarkEditorView::BookmarkNode* f211 = editor.AddNewGroup(f21);
+  f211->SetTitle(L"F211");
+
+  // Parent the node to "F21".
+  editor.ApplyEdits(f2);
+
+  BookmarkBarNode* bb_node =
+      profile_->GetBookmarkBarModel()->GetBookmarkBarNode();
+  BookmarkBarNode* mf2 = bb_node->GetChild(1);
+
+  // F2 in the model should have two children now: F21 and the node edited.
+  ASSERT_EQ(2, mf2->GetChildCount());
+  // F21 should be first.
+  ASSERT_EQ(L"F21", mf2->GetChild(0)->GetTitle());
+  // Then a.
+  ASSERT_EQ(L"a", mf2->GetChild(1)->GetTitle());
+
+  // F21 should have one child, F211.
+  BookmarkBarNode* mf21 = mf2->GetChild(0);
+  ASSERT_EQ(1, mf21->GetChildCount());
+  ASSERT_EQ(L"F211", mf21->GetChild(0)->GetTitle());
+}
diff --git a/chrome/test/unit/unittests.vcproj b/chrome/test/unit/unittests.vcproj
index 245996e..095d594 100644
--- a/chrome/test/unit/unittests.vcproj
+++ b/chrome/test/unit/unittests.vcproj
@@ -227,6 +227,14 @@
 			</File>
 		</Filter>
 		<Filter
+			Name="TestBookmarkEditorView"
+			>
+			<File
+				RelativePath="..\..\browser\views\bookmark_editor_view_unittest.cc"
+				>
+			</File>
+		</Filter>
+		<Filter
 			Name="TestGoogleURLTracker"
 			>
 			<File