Use platform-appropriate newlines in JSON output.

BUG=15462
TEST=base_unittests, unit_tests, check newlines in bookmarks file
Review URL: http://codereview.chromium.org/147220

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

16 files changed, 188 insertions, 49 deletions

diff --git a/base/data/file_util_unittest/blank_line.txt b/base/data/file_util_unittest/blank_line.txt
new file mode 100644
index 0000000..8892069
--- /dev/null
+++ b/base/data/file_util_unittest/blank_line.txt
@@ -0,0 +1,3 @@
+The next line is blank.
+
+But this one isn't.
diff --git a/base/data/file_util_unittest/blank_line_crlf.txt b/base/data/file_util_unittest/blank_line_crlf.txt
new file mode 100644
index 0000000..3aefe52
--- /dev/null
+++ b/base/data/file_util_unittest/blank_line_crlf.txt
@@ -0,0 +1,3 @@
+The next line is blank.

+

+But this one isn't.

diff --git a/base/data/file_util_unittest/crlf.txt b/base/data/file_util_unittest/crlf.txt
new file mode 100644
index 0000000..0e62728
--- /dev/null
+++ b/base/data/file_util_unittest/crlf.txt
@@ -0,0 +1 @@
+This file is the same.

diff --git a/base/data/file_util_unittest/first1.txt b/base/data/file_util_unittest/first1.txt
new file mode 100644
index 0000000..2c6e300
--- /dev/null
+++ b/base/data/file_util_unittest/first1.txt
@@ -0,0 +1,2 @@
+The first line is the same.
+The second line is different.
diff --git a/base/data/file_util_unittest/first2.txt b/base/data/file_util_unittest/first2.txt
new file mode 100644
index 0000000..e39b5ec
--- /dev/null
+++ b/base/data/file_util_unittest/first2.txt
@@ -0,0 +1,2 @@
+The first line is the same.
+The second line is not.
diff --git a/base/file_util.cc b/base/file_util.cc
index c366a76..505dfb4 100644
--- a/base/file_util.cc
+++ b/base/file_util.cc
@@ -129,21 +129,60 @@ bool ContentsEqual(const FilePath& filename1, const FilePath& filename2) {
     file1.read(buffer1, BUFFER_SIZE);
     file2.read(buffer2, BUFFER_SIZE);
 
-    if ((file1.eof() && !file2.eof()) ||
-        (!file1.eof() && file2.eof()) ||
+    if ((file1.eof() != file2.eof()) ||
         (file1.gcount() != file2.gcount()) ||
         (memcmp(buffer1, buffer2, file1.gcount()))) {
       file1.close();
       file2.close();
       return false;
     }
-  } while (!file1.eof() && !file2.eof());
+  } while (!file1.eof() || !file2.eof());
 
   file1.close();
   file2.close();
   return true;
 }
 
+bool TextContentsEqual(const FilePath& filename1, const FilePath& filename2) {
+  std::ifstream file1(filename1.value().c_str(), std::ios::in);
+  std::ifstream file2(filename2.value().c_str(), std::ios::in);
+
+  // Even if both files aren't openable (and thus, in some sense, "equal"),
+  // any unusable file yields a result of "false".
+  if (!file1.is_open() || !file2.is_open())
+    return false;
+
+  do {
+    std::string line1, line2;
+    getline(file1, line1);
+    getline(file2, line2);
+
+    // Check for mismatched EOF states, or any error state.
+    if ((file1.eof() != file2.eof()) ||
+        file1.bad() || file2.bad()) {
+      return false;
+    }
+
+    // Trim all '\r' and '\n' characters from the end of the line.
+    std::string::size_type end1 = line1.find_last_not_of("\r\n");
+    if (end1 == std::string::npos)
+      line1.clear();
+    else if (end1 + 1 < line1.length())
+      line1.erase(end1 + 1);
+
+    std::string::size_type end2 = line2.find_last_not_of("\r\n");
+    if (end2 == std::string::npos)
+      line2.clear();
+    else if (end2 + 1 < line2.length())
+      line2.erase(end2 + 1);
+
+    if (line1 != line2)
+      return false;
+  } while (!file1.eof() || !file2.eof());
+
+  return true;
+}
+
 bool ReadFileToString(const FilePath& path, std::string* contents) {
   FILE* file = OpenFile(path, "rb");
   if (!file) {
diff --git a/base/file_util.h b/base/file_util.h
index 63bbf1f..35b9dd5 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -212,6 +212,10 @@ bool ContentsEqual(const FilePath& filename1,
 bool ContentsEqual(const std::wstring& filename1,
                    const std::wstring& filename2);
 
+// Returns true if the contents of the two text files given are equal, false
+// otherwise.  This routine treats "\r\n" and "\n" as equivalent.
+bool TextContentsEqual(const FilePath& filename1, const FilePath& filename2);
+
 // Read the file at |path| into |contents|, returning true on success.
 // Useful for unit tests.
 bool ReadFileToString(const FilePath& path, std::string* contents);
diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index e510594..7dff868 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -629,6 +629,56 @@ TEST_F(ReadOnlyFileUtilTest, ContentsEqual) {
   EXPECT_FALSE(file_util::ContentsEqual(binary_file, binary_file_diff));
 }
 
+TEST_F(ReadOnlyFileUtilTest, TextContentsEqual) {
+  FilePath data_dir;
+  ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &data_dir));
+  data_dir = data_dir.Append(FILE_PATH_LITERAL("base"))
+                     .Append(FILE_PATH_LITERAL("data"))
+                     .Append(FILE_PATH_LITERAL("file_util_unittest"));
+  ASSERT_TRUE(file_util::PathExists(data_dir));
+
+  FilePath original_file =
+      data_dir.Append(FILE_PATH_LITERAL("original.txt"));
+  FilePath same_file =
+      data_dir.Append(FILE_PATH_LITERAL("same.txt"));
+  FilePath crlf_file =
+      data_dir.Append(FILE_PATH_LITERAL("crlf.txt"));
+  FilePath shortened_file =
+      data_dir.Append(FILE_PATH_LITERAL("shortened.txt"));
+  FilePath different_file =
+      data_dir.Append(FILE_PATH_LITERAL("different.txt"));
+  FilePath different_first_file =
+      data_dir.Append(FILE_PATH_LITERAL("different_first.txt"));
+  FilePath different_last_file =
+      data_dir.Append(FILE_PATH_LITERAL("different_last.txt"));
+  FilePath first1_file =
+      data_dir.Append(FILE_PATH_LITERAL("first1.txt"));
+  FilePath first2_file =
+      data_dir.Append(FILE_PATH_LITERAL("first2.txt"));
+  FilePath empty1_file =
+      data_dir.Append(FILE_PATH_LITERAL("empty1.txt"));
+  FilePath empty2_file =
+      data_dir.Append(FILE_PATH_LITERAL("empty2.txt"));
+  FilePath blank_line_file =
+      data_dir.Append(FILE_PATH_LITERAL("blank_line.txt"));
+  FilePath blank_line_crlf_file =
+      data_dir.Append(FILE_PATH_LITERAL("blank_line_crlf.txt"));
+
+  EXPECT_TRUE(file_util::TextContentsEqual(original_file, same_file));
+  EXPECT_TRUE(file_util::TextContentsEqual(original_file, crlf_file));
+  EXPECT_FALSE(file_util::TextContentsEqual(original_file, shortened_file));
+  EXPECT_FALSE(file_util::TextContentsEqual(original_file, different_file));
+  EXPECT_FALSE(file_util::TextContentsEqual(original_file,
+                                            different_first_file));
+  EXPECT_FALSE(file_util::TextContentsEqual(original_file,
+                                            different_last_file));
+  EXPECT_FALSE(file_util::TextContentsEqual(first1_file, first2_file));
+  EXPECT_TRUE(file_util::TextContentsEqual(empty1_file, empty2_file));
+  EXPECT_FALSE(file_util::TextContentsEqual(original_file, empty1_file));
+  EXPECT_TRUE(file_util::TextContentsEqual(blank_line_file,
+                                           blank_line_crlf_file));
+}
+
 // We don't need equivalent functionality outside of Windows.
 #if defined(OS_WIN)
 TEST_F(FileUtilTest, ResolveShortcutTest) {
diff --git a/base/json_reader_unittest.cc b/base/json_reader_unittest.cc
index 9153289..0513eb5 100644
--- a/base/json_reader_unittest.cc
+++ b/base/json_reader_unittest.cc
@@ -308,6 +308,26 @@ TEST(JSONReaderTest, Reading) {
 
   root2.reset(JSONReader::Read(
     "{\"number\":9.87654321, \"null\":null , \"\\x53\" : \"str\", }", true));
+  ASSERT_TRUE(root2.get());
+  EXPECT_TRUE(root->Equals(root2.get()));
+
+  // Test newline equivalence.
+  root2.reset(JSONReader::Read(
+    "{\n"
+    "  \"number\":9.87654321,\n"
+    "  \"null\":null,\n"
+    "  \"\\x53\":\"str\",\n"
+    "}\n", true));
+  ASSERT_TRUE(root2.get());
+  EXPECT_TRUE(root->Equals(root2.get()));
+
+  root2.reset(JSONReader::Read(
+    "{\r\n"
+    "  \"number\":9.87654321,\r\n"
+    "  \"null\":null,\r\n"
+    "  \"\\x53\":\"str\",\r\n"
+    "}\r\n", true));
+  ASSERT_TRUE(root2.get());
   EXPECT_TRUE(root->Equals(root2.get()));
 
   // Test nesting
diff --git a/base/json_writer.cc b/base/json_writer.cc
index a95798e..1a9f1b6 100644
--- a/base/json_writer.cc
+++ b/base/json_writer.cc
@@ -9,7 +9,11 @@
 #include "base/values.h"
 #include "base/string_escape.h"
 
-const char kPrettyPrintLineEnding[] = "\r\n";
+#if defined(OS_WIN)
+static const char kPrettyPrintLineEnding[] = "\r\n";
+#else
+static const char kPrettyPrintLineEnding[] = "\n";
+#endif
 
 /* static */
 void JSONWriter::Write(const Value* const node,
diff --git a/base/json_writer_unittest.cc b/base/json_writer_unittest.cc
index 9f24fe1..2792982 100644
--- a/base/json_writer_unittest.cc
+++ b/base/json_writer_unittest.cc
@@ -43,7 +43,7 @@ TEST(JSONWriterTest, Writing) {
   JSONWriter::Write(root, false, &output_js);
   ASSERT_EQ("-0.8", output_js);
   delete root;
-  
+
   // Writer unittests like empty list/dict nesting,
   // list list nesting, etc.
   DictionaryValue root_dict;
@@ -56,13 +56,22 @@ TEST(JSONWriterTest, Writing) {
   list->Append(inner_list);
   list->Append(Value::CreateBooleanValue(true));
 
+  // Test the pretty-printer.
   JSONWriter::Write(&root_dict, false, &output_js);
   ASSERT_EQ("{\"list\":[{\"inner int\":10},[],true]}", output_js);
   JSONWriter::Write(&root_dict, true, &output_js);
-  ASSERT_EQ("{\r\n"
-            "   \"list\": [ {\r\n"
-            "      \"inner int\": 10\r\n"
-            "   }, [  ], true ]\r\n"
-            "}\r\n",
+  // The pretty-printer uses a different newline style on Windows than on
+  // other platforms.
+#if defined(OS_WIN)
+#define JSON_NEWLINE "\r\n"
+#else
+#define JSON_NEWLINE "\n"
+#endif
+  ASSERT_EQ("{" JSON_NEWLINE
+            "   \"list\": [ {" JSON_NEWLINE
+            "      \"inner int\": 10" JSON_NEWLINE
+            "   }, [  ], true ]" JSON_NEWLINE
+            "}" JSON_NEWLINE,
             output_js);
+#undef JSON_NEWLINE
 }
diff --git a/chrome/common/json_value_serializer_unittest.cc b/chrome/common/json_value_serializer_unittest.cc
index 5c50596..877ac6e 100644
--- a/chrome/common/json_value_serializer_unittest.cc
+++ b/chrome/common/json_value_serializer_unittest.cc
@@ -293,7 +293,8 @@ TEST_F(JSONFileValueSerializerTest, Roundtrip) {
   ASSERT_TRUE(file_util::PathExists(written_file_path));
 
   // Now compare file contents.
-  EXPECT_TRUE(file_util::ContentsEqual(original_file_path, written_file_path));
+  EXPECT_TRUE(file_util::TextContentsEqual(original_file_path,
+                                           written_file_path));
   EXPECT_TRUE(file_util::Delete(written_file_path, false));
 }
 
@@ -321,7 +322,8 @@ TEST_F(JSONFileValueSerializerTest, RoundtripNested) {
   ASSERT_TRUE(file_util::PathExists(written_file_path));
 
   // Now compare file contents.
-  EXPECT_TRUE(file_util::ContentsEqual(original_file_path, written_file_path));
+  EXPECT_TRUE(file_util::TextContentsEqual(original_file_path,
+                                           written_file_path));
   EXPECT_TRUE(file_util::Delete(written_file_path, false));
 }
 
diff --git a/chrome/common/pref_service_unittest.cc b/chrome/common/pref_service_unittest.cc
index 1c017a2..29b4b09 100644
--- a/chrome/common/pref_service_unittest.cc
+++ b/chrome/common/pref_service_unittest.cc
@@ -145,7 +145,7 @@ TEST_F(PrefServiceTest, Basic) {
   FilePath golden_output_file = data_dir_.AppendASCII("write.golden.json");
   ASSERT_TRUE(file_util::PathExists(golden_output_file));
   ASSERT_TRUE(prefs.SavePersistentPrefs());
-  EXPECT_TRUE(file_util::ContentsEqual(golden_output_file, output_file));
+  EXPECT_TRUE(file_util::TextContentsEqual(golden_output_file, output_file));
   ASSERT_TRUE(file_util::Delete(output_file, false));
 }
 
diff --git a/chrome/test/data/pref_service/write.golden.json b/chrome/test/data/pref_service/write.golden.json
index 71f8cfa..9a5523c 100644
--- a/chrome/test/data/pref_service/write.golden.json
+++ b/chrome/test/data/pref_service/write.golden.json
@@ -1,11 +1,11 @@
-{

-   "homepage": "http://www.cnn.com",

-   "long_int": {

-      "pref": "214748364842"

-   },

-   "some_directory": "/usr/sbin/",

-   "tabs": {

-      "max_tabs": 10,

-      "new_windows_in_tabs": false

-   }

-}

+{
+   "homepage": "http://www.cnn.com",
+   "long_int": {
+      "pref": "214748364842"
+   },
+   "some_directory": "/usr/sbin/",
+   "tabs": {
+      "max_tabs": 10,
+      "new_windows_in_tabs": false
+   }
+}
diff --git a/chrome/test/data/serializer_nested_test.js b/chrome/test/data/serializer_nested_test.js
index 9b47c3d..cfea8e8 100644
--- a/chrome/test/data/serializer_nested_test.js
+++ b/chrome/test/data/serializer_nested_test.js
@@ -1,17 +1,17 @@
-{

-   "bool": true,

-   "dict": {

-      "bool": true,

-      "dict": {

-         "bees": "knees",

-         "cats": "meow"

-      },

-      "foos": "bar",

-      "list": [ 3.4, "second", null ]

-   },

-   "int": 42,

-   "list": [ 1, 2 ],

-   "null": null,

-   "real": 3.14,

-   "string": "hello"

-}

+{
+   "bool": true,
+   "dict": {
+      "bool": true,
+      "dict": {
+         "bees": "knees",
+         "cats": "meow"
+      },
+      "foos": "bar",
+      "list": [ 3.4, "second", null ]
+   },
+   "int": 42,
+   "list": [ 1, 2 ],
+   "null": null,
+   "real": 3.14,
+   "string": "hello"
+}
diff --git a/chrome/test/data/serializer_test.js b/chrome/test/data/serializer_test.js
index e2472d6..446925e 100644
--- a/chrome/test/data/serializer_test.js
+++ b/chrome/test/data/serializer_test.js
@@ -1,8 +1,8 @@
-{

-   "bool": true,

-   "int": 42,

-   "list": [ 1, 2 ],

-   "null": null,

-   "real": 3.14,

-   "string": "hello"

-}

+{
+   "bool": true,
+   "int": 42,
+   "list": [ 1, 2 ],
+   "null": null,
+   "real": 3.14,
+   "string": "hello"
+}