Add BookmarkUndoServiceTest.UndoRemoveFolderWithSubfolders

components/undo/bookmark_undo_service_test.cc

Index: components/undo/bookmark_undo_service_test.cc
diff --git a/components/undo/bookmark_undo_service_test.cc b/components/undo/bookmark_undo_service_test.cc
index 2e989fda25e2dec2febfbf8b2360d44cc9d80e98..b6cc2511211ad51f3cb86d72729d1f3c8ce09a5d 100644
--- a/components/undo/bookmark_undo_service_test.cc
+++ b/components/undo/bookmark_undo_service_test.cc
@@ -394,6 +394,49 @@ TEST_F(BookmarkUndoServiceTest, UndoRemoveFolderWithBookmarks) {
   EXPECT_EQ(node->url(), GURL("http://www.bar.com"));
 }
 
+TEST_F(BookmarkUndoServiceTest, UndoRemoveFolderWithSubfolders) {
+  BookmarkModel* model = GetModel();
+  BookmarkUndoService* undo_service = GetUndoService();
+
+  // Setup bookmarks in the Other Bookmarks with the following structure:
+  // folder
+  //    subfolder1
+  //    subfolder2
+  //        bar - http://www.bar.com
+  // This setup of multiple subfolders where the first subfolder has 0 children
+  // is designed specifically to ensure we do not crash in this scenario and
+  // that bookmarks are restored to the proper subfolder. See crbug.com/474123.
+  const BookmarkNode* new_folder;
+  const BookmarkNode* sub_folder1;
+  const BookmarkNode* sub_folder2;
+  const BookmarkNode* parent = model->other_node();
+  new_folder = model->AddFolder(parent, 0, ASCIIToUTF16("folder"));
+  sub_folder1 = model->AddFolder(new_folder, 0, ASCIIToUTF16("subfolder1"));
+  sub_folder2 = model->AddFolder(new_folder, 1, ASCIIToUTF16("subfolder2"));
+  model->AddURL(sub_folder2, 0, ASCIIToUTF16("bar"), GURL("http://www.bar.com"));

[sky, 2015/07/20 20:01:35]

nit: > 80.

[Theresa, 2015/07/21 00:42:54]

Done.

+
+  model->Remove(parent->GetChild(0));
+
+  // Test that the undo restores the subfolders and their contents.
+  undo_service->undo_manager()->Undo();
+
+  ASSERT_EQ(1, model->other_node()->child_count());
+  new_folder = model->other_node()->GetChild(0);

[sky, 2015/07/20 20:01:35]

I think this code would be clearer if you didn't reuse the existing variables.
For example, use restored_new_folder instead of new_folder.

[Theresa, 2015/07/21 00:42:54]

Done.

+  EXPECT_EQ(2, new_folder->child_count());
+
+  sub_folder1 = new_folder->GetChild(0);
+  EXPECT_EQ(ASCIIToUTF16("subfolder1"), sub_folder1->GetTitle());
+  EXPECT_EQ(0, sub_folder1->child_count());
+
+  sub_folder2 = new_folder->GetChild(1);
+  EXPECT_EQ(ASCIIToUTF16("subfolder2"), sub_folder2->GetTitle());
+  EXPECT_EQ(1, sub_folder2->child_count());
+
+  const BookmarkNode* node = sub_folder2->GetChild(0);
+  EXPECT_EQ(node->GetTitle(), ASCIIToUTF16("bar"));
+  EXPECT_EQ(node->url(), GURL("http://www.bar.com"));
+}
+
 TEST_F(BookmarkUndoServiceTest, TestUpperLimit) {
   BookmarkModel* model = GetModel();
   BookmarkUndoService* undo_service = GetUndoService();