Add BookmarkUndoServiceTest.UndoRemoveFolderWithSubfolders

components/undo/bookmark_undo_service_test.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/undo/bookmark_undo_service.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/bookmarks/test/bookmark_test_helpers.h"
 #include "components/bookmarks/test/test_bookmark_client.h"
@@ skipping 376 matching lines @@
   undo_service->undo_manager()->Undo();
 
   ASSERT_EQ(1, model->other_node()->child_count());
   new_folder = model->other_node()->GetChild(0);
   EXPECT_EQ(1, new_folder->child_count());
   node = new_folder->GetChild(0);
   EXPECT_EQ(node->GetTitle(), ASCIIToUTF16("bar"));
   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();
 
   // This maximum is set in undo_manager.cc
   const size_t kMaxUndoGroups = 100;
 
   const BookmarkNode* parent = model->other_node();
   model->AddURL(parent, 0, ASCIIToUTF16("foo"), GURL("http://www.foo.com"));
   for (size_t i = 1; i < kMaxUndoGroups + 1; ++i)
     model->AddURL(parent, i, ASCIIToUTF16("bar"), GURL("http://www.bar.com"));
 
   EXPECT_EQ(kMaxUndoGroups, undo_service->undo_manager()->undo_count());
 
   // Undo as many operations as possible.
   while (undo_service->undo_manager()->undo_count())
     undo_service->undo_manager()->Undo();
 
   EXPECT_EQ(1, parent->child_count());
   const BookmarkNode* node = model->other_node()->GetChild(0);
   EXPECT_EQ(node->GetTitle(), ASCIIToUTF16("foo"));
   EXPECT_EQ(node->url(), GURL("http://www.foo.com"));
 }
 
 } // namespace