Do not allow multiline input when naming bookmarks/folders on Mac

chrome/browser/bookmarks/bookmark_model_unittest.cc

 // Copyright (c) 2011 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 <set>
 #include <string>
 
 #include "base/base_paths.h"
 #include "base/basictypes.h"
 #include "base/command_line.h"
@@ skipping 244 matching lines @@
               new_node->id() != model_.other_node()->id() &&
               new_node->id() != model_.synced_node()->id());
 
   // Add another folder, just to make sure folder_ids are incremented correctly.
   ClearCounts();
   model_.AddFolder(root, 0, title);
   AssertObserverCount(1, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 }
 
+TEST_F(BookmarkModelTest, AddFolderWithNewLine) {

[Ilya Sherman, 2011/11/22 01:18:16]

There are a bunch more things that could be tested:

(1) Adding URLs (regular bookmarks) rather than folders.
(2) Renaming the bookmark/folder after it has been added.
(3) Other whitespace: leading whitespace, trailing whitespace, multiple
consecutive spaces, tab characters, etc.

Rather than create lots of test functions for these, I would recommend creating
a single "TitleWithWhitespace" (or something like that) test.  One common
pattern in tests with lots of niggly sub-cases like this is to create an array
mapping inputs to expectations, e.g.

struct {
  string16 input_title;
  string16 expected_title;
} test_cases[] = {
  {"foobar", "foobar"},
  {"foo\nbar", "foo bar"},
  {"foo\n\nbar", "foo bar"},
  /* Add more test cases here */
};

and then iterate over that array (if you do this, the ARRAYSIZE_UNSAFE macro
will probably be helpful).

+  const BookmarkNode* root = model_.bookmark_bar_node();
+  const string16 title(ASCIIToUTF16("foo\nbar"));
+
+  const BookmarkNode* new_node = model_.AddFolder(root, 0, title);
+  AssertObserverCount(1, 0, 0, 0, 0);
+  observer_details_.ExpectEquals(root, NULL, 0, -1);
+
+  ASSERT_EQ(1, root->child_count());
+  ASSERT_EQ(ASCIIToUTF16("foo bar"), new_node->GetTitle());
+  ASSERT_EQ(BookmarkNode::FOLDER, new_node->type());

[Ilya Sherman, 2011/11/22 01:18:16]

nit: I know the style is wrong in other tests in this file, but these should all
be EXPECT_EQ rather than ASSERT_EQ.  ASSERT* should only be used when EXPECT*
won't suffice, e.g. if the test would crash were the assertion to fail.

+
+  EXPECT_TRUE(new_node->id() != root->id() &&
+              new_node->id() != model_.other_node()->id() &&
+              new_node->id() != model_.synced_node()->id());

[Ilya Sherman, 2011/11/22 01:18:16]

nit: No need for this EXPECT_TRUE stmt in all of the added tests -- it's already
tested for generic bookmark additions, and it's not so relevant to the newline
behavior.

+}
+
+TEST_F(BookmarkModelTest, AddFolderWithMultipleNewLines) {
+  const BookmarkNode* root = model_.bookmark_bar_node();
+  const string16 title(ASCIIToUTF16("foo\n\n\nbar"));
+
+  const BookmarkNode* new_node = model_.AddFolder(root, 0, title);
+  AssertObserverCount(1, 0, 0, 0, 0);
+  observer_details_.ExpectEquals(root, NULL, 0, -1);
+
+  ASSERT_EQ(1, root->child_count());
+  ASSERT_EQ(ASCIIToUTF16("foo bar"), new_node->GetTitle());
+  ASSERT_EQ(BookmarkNode::FOLDER, new_node->type());
+
+  EXPECT_TRUE(new_node->id() != root->id() &&
+              new_node->id() != model_.other_node()->id() &&
+              new_node->id() != model_.synced_node()->id());
+}
+
+TEST_F(BookmarkModelTest, AddFolderWithCarriageReturn) {
+  const BookmarkNode* root = model_.bookmark_bar_node();
+
+  // Newline is represented by CR+LF.
+  const string16 title(ASCIIToUTF16("foo\r\nbar"));
+
+  const BookmarkNode* new_node = model_.AddFolder(root, 0, title);
+  AssertObserverCount(1, 0, 0, 0, 0);
+  observer_details_.ExpectEquals(root, NULL, 0, -1);
+
+  ASSERT_EQ(1, root->child_count());
+  ASSERT_EQ(ASCIIToUTF16("foo bar"), new_node->GetTitle());
+  ASSERT_EQ(BookmarkNode::FOLDER, new_node->type());
+
+  EXPECT_TRUE(new_node->id() != root->id() &&
+              new_node->id() != model_.other_node()->id() &&
+              new_node->id() != model_.synced_node()->id());
+}
+
 TEST_F(BookmarkModelTest, RemoveURL) {
   const BookmarkNode* root = model_.bookmark_bar_node();
   const string16 title(ASCIIToUTF16("foo"));
   const GURL url("http://foo.com");
   model_.AddURL(root, 0, title, url);
   ClearCounts();
 
   model_.Remove(root, 0);
   ASSERT_EQ(0, root->child_count());
   AssertObserverCount(0, 0, 1, 0, 0);
@@ skipping 762 matching lines @@
 TEST_F(BookmarkModelTest, SyncNodeVisibileWithChildren) {
   const BookmarkNode* root = model_.synced_node();
   const string16 title(ASCIIToUTF16("foo"));
   const GURL url("http://foo.com");
 
   model_.AddURL(root, 0, title, url);
   EXPECT_TRUE(model_.synced_node()->IsVisible());
 }
 
 }  // namespace