Use platform-appropriate newlines in JSON output

Use platform-appropriate newlines in JSON output.

BUG=15462
TEST=base_unittests, unit_tests, check newlines in bookmarks file

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=19418

[Erik does not do reviews, 2009-06-26 18:27:31]

LGTM - just minor tweaks

http://codereview.chromium.org/147220/diff/1/2
File base/json_writer.cc (right):

http://codereview.chromium.org/147220/diff/1/2#newcode13
Line 13: static const char kPrettyPrintLineEnding[] = "\r\n";
since this is used by the unit test, could you make it a static member of the
class?

http://codereview.chromium.org/147220/diff/1/3
File base/json_writer_unittest.cc (right):

http://codereview.chromium.org/147220/diff/1/3#newcode61
Line 61: JSONWriter::Write(&root_dict, true, &output_js);
add comment to indicate pretty print testing

http://codereview.chromium.org/147220/diff/1/3#newcode63
Line 63: #define JSON_NEWLINE "\r\n"
use static from JSONWriter when you add it

[Mark Mentovai, 2009-06-26 19:45:12]

New version up, avoids breaking tests by providing a line-by-line file
comparison utility that's not sensitive to the difference between LF and CRLF.

[Erik does not do reviews, 2009-06-26 20:00:32]

http://codereview.chromium.org/147220/diff/1007/1012
File base/file_util.cc (right):

http://codereview.chromium.org/147220/diff/1007/1012#newcode153
Line 153: if (!file1.is_open() || !file2.is_open()) {
remove {}

http://codereview.chromium.org/147220/diff/1007/1012#newcode163
Line 163: if ((file1.eof() && !file2.eof()) ||
can this be replaced with if (file1.eof() != file2.eof())?

http://codereview.chromium.org/147220/diff/1007/1012#newcode171
Line 171: if (end1 != std::string::npos && end1 + 1 < line1.length()) {
remove {}

http://codereview.chromium.org/147220/diff/1007/1012#newcode171
Line 171: if (end1 != std::string::npos && end1 + 1 < line1.length()) {
if end1 == npos, then you need to clear line1 or blank lines won't compare equal
(same goes for the next test).  You should add blank lines to the tests.

http://codereview.chromium.org/147220/diff/1007/1012#newcode176
Line 176: if (end2 != std::string::npos && end2 + 1 < line2.length()) {
remove {}

http://codereview.chromium.org/147220/diff/1007/1012#newcode180
Line 180: if (line1 != line2) {
remove {}

http://codereview.chromium.org/147220/diff/1007/1020
File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/147220/diff/1007/1020#newcode636
Line 636: //    DLOG(WARNING) << "Canceling a request that wasn't found";
I assume this was an accident

[Mark Mentovai, 2009-06-26 20:13:30]

New version up.

http://codereview.chromium.org/147220/diff/1007/1012
File base/file_util.cc (right):

http://codereview.chromium.org/147220/diff/1007/1012#newcode171
Line 171: if (end1 != std::string::npos && end1 + 1 < line1.length()) {
Erik Kay wrote:
> if end1 == npos, then you need to clear line1 or blank lines won't compare
equal
> (same goes for the next test).  You should add blank lines to the tests.

Good catch.

http://codereview.chromium.org/147220/diff/1007/1020
File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/147220/diff/1007/1020#newcode636
Line 636: //    DLOG(WARNING) << "Canceling a request that wasn't found";
Erik Kay wrote:
> I assume this was an accident

Yeah, it was.  That warning makes it difficult to find other logging output.  I
hate it, but I never meant to check this in.

[Erik does not do reviews, 2009-06-26 21:16:39]

LGTM