JSONWriter cleanup: integrate pretty print into write options.

BUG=
TEST=base_unittests

TBR=abodenha@chromium.org,ajwong@chromium.org,chocobo@chromium.org,mnissler@chromium.org,akalin@chromium.org,brettw@chromium.org,arv@chromium.org

Review URL: http://codereview.chromium.org/9590002

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

5 files changed, 80 insertions, 77 deletions

diff --git a/base/json/json_string_value_serializer.cc b/base/json/json_string_value_serializer.cc
index 94e660e..3045fa5 100644
--- a/base/json/json_string_value_serializer.cc
+++ b/base/json/json_string_value_serializer.cc
@@ -24,11 +24,13 @@ bool JSONStringValueSerializer::SerializeInternal(const Value& root,
   if (!json_string_ || initialized_with_const_string_)
     return false;
 
-  base::JSONWriter::WriteWithOptions(
-      &root,
-      pretty_print_,
-      omit_binary_values ? base::JSONWriter::OPTIONS_OMIT_BINARY_VALUES : 0,
-      json_string_);
+  int options = 0;
+  if (omit_binary_values)
+    options |= base::JSONWriter::OPTIONS_OMIT_BINARY_VALUES;
+  if (pretty_print_)
+    options |= base::JSONWriter::OPTIONS_PRETTY_PRINT;
+
+  base::JSONWriter::WriteWithOptions(&root, options, json_string_);
   return true;
 }
 
@@ -42,4 +44,3 @@ Value* JSONStringValueSerializer::Deserialize(int* error_code,
                                               error_code,
                                               error_str);
 }
-
diff --git a/base/json/json_writer.cc b/base/json/json_writer.cc
index 27300c0..e814003 100644
--- a/base/json/json_writer.cc
+++ b/base/json/json_writer.cc
@@ -25,42 +25,43 @@ static const char kPrettyPrintLineEnding[] = "\n";
 const char* JSONWriter::kEmptyArray = "[]";
 
 /* static */
-void JSONWriter::Write(const Value* const node,
-                       bool pretty_print,
-                       std::string* json) {
-  WriteWithOptions(node, pretty_print, 0, json);
+void JSONWriter::Write(const Value* const node, std::string* json) {
+  WriteWithOptions(node, 0, json);
 }
 
 /* static */
-void JSONWriter::WriteWithOptions(const Value* const node,
-                                  bool pretty_print,
-                                  int options,
+void JSONWriter::WriteWithOptions(const Value* const node, int options,
                                   std::string* json) {
   json->clear();
   // Is there a better way to estimate the size of the output?
   json->reserve(1024);
-  JSONWriter writer(pretty_print, json);
+
   bool escape = !(options & OPTIONS_DO_NOT_ESCAPE);
   bool omit_binary_values = !!(options & OPTIONS_OMIT_BINARY_VALUES);
   bool omit_double_type_preservation =
       !!(options & OPTIONS_OMIT_DOUBLE_TYPE_PRESERVATION);
-  writer.BuildJSONString(node, 0, escape, omit_binary_values,
-                         omit_double_type_preservation);
+  bool pretty_print = !!(options & OPTIONS_PRETTY_PRINT);
+
+  JSONWriter writer(escape, omit_binary_values, omit_double_type_preservation,
+                    pretty_print, json);
+  writer.BuildJSONString(node, 0);
+
   if (pretty_print)
     json->append(kPrettyPrintLineEnding);
 }
 
-JSONWriter::JSONWriter(bool pretty_print, std::string* json)
-    : json_string_(json),
-      pretty_print_(pretty_print) {
+JSONWriter::JSONWriter(bool escape, bool omit_binary_values,
+                       bool omit_double_type_preservation, bool pretty_print,
+                       std::string* json)
+    : escape_(escape),
+      omit_binary_values_(omit_binary_values),
+      omit_double_type_preservation_(omit_double_type_preservation),
+      pretty_print_(pretty_print),
+      json_string_(json) {
   DCHECK(json);
 }
 
-void JSONWriter::BuildJSONString(const Value* const node,
-                                 int depth,
-                                 bool escape,
-                                 bool omit_binary_values,
-                                 bool omit_double_type_preservation) {
+void JSONWriter::BuildJSONString(const Value* const node, int depth) {
   switch (node->GetType()) {
     case Value::TYPE_NULL:
       json_string_->append("null");
@@ -89,7 +90,7 @@ void JSONWriter::BuildJSONString(const Value* const node,
         double value;
         bool result = node->GetAsDouble(&value);
         DCHECK(result);
-        if (omit_double_type_preservation &&
+        if (omit_double_type_preservation_ &&
             value <= kint64max &&
             value >= kint64min &&
             std::floor(value) == value) {
@@ -122,7 +123,7 @@ void JSONWriter::BuildJSONString(const Value* const node,
         std::string value;
         bool result = node->GetAsString(&value);
         DCHECK(result);
-        if (escape) {
+        if (escape_) {
           JsonDoubleQuote(UTF8ToUTF16(value), true, json_string_);
         } else {
           JsonDoubleQuote(value, true, json_string_);
@@ -142,7 +143,7 @@ void JSONWriter::BuildJSONString(const Value* const node,
           bool result = list->Get(i, &value);
           DCHECK(result);
 
-          if (omit_binary_values && value->GetType() == Value::TYPE_BINARY) {
+          if (omit_binary_values_ && value->GetType() == Value::TYPE_BINARY) {
             continue;
           }
 
@@ -152,8 +153,7 @@ void JSONWriter::BuildJSONString(const Value* const node,
               json_string_->append(" ");
           }
 
-          BuildJSONString(value, depth, escape, omit_binary_values,
-                          omit_double_type_preservation);
+          BuildJSONString(value, depth);
         }
 
         if (pretty_print_)
@@ -177,7 +177,7 @@ void JSONWriter::BuildJSONString(const Value* const node,
           bool result = dict->GetWithoutPathExpansion(*key_itr, &value);
           DCHECK(result);
 
-          if (omit_binary_values && value->GetType() == Value::TYPE_BINARY) {
+          if (omit_binary_values_ && value->GetType() == Value::TYPE_BINARY) {
             continue;
           }
 
@@ -195,8 +195,7 @@ void JSONWriter::BuildJSONString(const Value* const node,
           } else {
             json_string_->append(":");
           }
-          BuildJSONString(value, depth + 1, escape, omit_binary_values,
-                          omit_double_type_preservation);
+          BuildJSONString(value, depth + 1);
         }
 
         if (pretty_print_) {
@@ -211,7 +210,7 @@ void JSONWriter::BuildJSONString(const Value* const node,
 
     case Value::TYPE_BINARY:
       {
-        if (!omit_binary_values) {
+        if (!omit_binary_values_) {
           NOTREACHED() << "Cannot serialize binary value.";
         }
         break;
diff --git a/base/json/json_writer.h b/base/json/json_writer.h
index 43f8e56..d27fd6b 100644
--- a/base/json/json_writer.h
+++ b/base/json/json_writer.h
@@ -31,36 +31,36 @@ class BASE_EXPORT JSONWriter {
     // part as a normal integer (i.e., without using exponential notation
     // or appending a '.0') as long as the value is within the range of a
     // 64-bit int.
-    OPTIONS_OMIT_DOUBLE_TYPE_PRESERVATION = 1 << 2
+    OPTIONS_OMIT_DOUBLE_TYPE_PRESERVATION = 1 << 2,
+
+    // Return a slightly nicer formatted json string (pads with whitespace to
+    // help with readability).
+    OPTIONS_PRETTY_PRINT = 1 << 3
   };
 
   // Given a root node, generates a JSON string and puts it into |json|.
-  // If |pretty_print| is true, return a slightly nicer formated json string
-  // (pads with whitespace to help readability).  If |pretty_print| is false,
-  // we try to generate as compact a string as possible.
   // TODO(tc): Should we generate json if it would be invalid json (e.g.,
   // |node| is not a DictionaryValue/ListValue or if there are inf/-inf float
   // values)?
-  static void Write(const Value* const node, bool pretty_print,
-                    std::string* json);
+  static void Write(const Value* const node, std::string* json);
 
   // Same as above but with |options| which is a bunch of JSONWriter::Options
   // bitwise ORed together.
-  static void WriteWithOptions(const Value* const node, bool pretty_print,
-                               int options, std::string* json);
+  static void WriteWithOptions(const Value* const node, int options,
+                               std::string* json);
 
   // A static, constant JSON string representing an empty array.  Useful
   // for empty JSON argument passing.
   static const char* kEmptyArray;
 
  private:
-  JSONWriter(bool pretty_print, std::string* json);
+  JSONWriter(bool escape, bool omit_binary_values,
+             bool omit_double_type_preservation, bool pretty_print,
+             std::string* json);
 
   // Called recursively to build the JSON string.  Whe completed, value is
   // json_string_ will contain the JSON.
-  void BuildJSONString(const Value* const node, int depth, bool escape,
-                       bool ignore_binary_values,
-                       bool omit_double_type_preservation);
+  void BuildJSONString(const Value* const node, int depth);
 
   // Appends a quoted, escaped, version of (UTF-8) str to json_string_.
   void AppendQuotedString(const std::string& str);
@@ -68,11 +68,14 @@ class BASE_EXPORT JSONWriter {
   // Adds space to json_string_ for the indent level.
   void IndentLine(int depth);
 
+  bool escape_;
+  bool omit_binary_values_;
+  bool omit_double_type_preservation_;
+  bool pretty_print_;
+
   // Where we write JSON data as we generate it.
   std::string* json_string_;
 
-  bool pretty_print_;
-
   DISALLOW_COPY_AND_ASSIGN(JSONWriter);
 };
 
diff --git a/base/json/json_writer_unittest.cc b/base/json/json_writer_unittest.cc
index 6e73f94..c52a1df4 100644
--- a/base/json/json_writer_unittest.cc
+++ b/base/json/json_writer_unittest.cc
@@ -12,37 +12,37 @@ TEST(JSONWriterTest, Writing) {
   // Test null
   Value* root = Value::CreateNullValue();
   std::string output_js;
-  JSONWriter::Write(root, false, &output_js);
+  JSONWriter::Write(root, &output_js);
   ASSERT_EQ("null", output_js);
   delete root;
 
   // Test empty dict
   root = new DictionaryValue;
-  JSONWriter::Write(root, false, &output_js);
+  JSONWriter::Write(root, &output_js);
   ASSERT_EQ("{}", output_js);
   delete root;
 
   // Test empty list
   root = new ListValue;
-  JSONWriter::Write(root, false, &output_js);
+  JSONWriter::Write(root, &output_js);
   ASSERT_EQ("[]", output_js);
   delete root;
 
   // Test Real values should always have a decimal or an 'e'.
   root = Value::CreateDoubleValue(1.0);
-  JSONWriter::Write(root, false, &output_js);
+  JSONWriter::Write(root, &output_js);
   ASSERT_EQ("1.0", output_js);
   delete root;
 
   // Test Real values in the the range (-1, 1) must have leading zeros
   root = Value::CreateDoubleValue(0.2);
-  JSONWriter::Write(root, false, &output_js);
+  JSONWriter::Write(root, &output_js);
   ASSERT_EQ("0.2", output_js);
   delete root;
 
   // Test Real values in the the range (-1, 1) must have leading zeros
   root = Value::CreateDoubleValue(-0.8);
-  JSONWriter::Write(root, false, &output_js);
+  JSONWriter::Write(root, &output_js);
   ASSERT_EQ("-0.8", output_js);
   delete root;
 
@@ -59,9 +59,10 @@ TEST(JSONWriterTest, Writing) {
   list->Append(Value::CreateBooleanValue(true));
 
   // Test the pretty-printer.
-  JSONWriter::Write(&root_dict, false, &output_js);
+  JSONWriter::Write(&root_dict, &output_js);
   ASSERT_EQ("{\"list\":[{\"inner int\":10},[],true]}", output_js);
-  JSONWriter::Write(&root_dict, true, &output_js);
+  JSONWriter::WriteWithOptions(&root_dict, JSONWriter::OPTIONS_PRETTY_PRINT,
+                               &output_js);
   // The pretty-printer uses a different newline style on Windows than on
   // other platforms.
 #if defined(OS_WIN)
@@ -85,19 +86,18 @@ TEST(JSONWriterTest, Writing) {
   period_dict2->SetWithoutPathExpansion("g.h.i.j",
                                         Value::CreateIntegerValue(1));
   period_dict.SetWithoutPathExpansion("d.e.f", period_dict2);
-  JSONWriter::Write(&period_dict, false, &output_js);
+  JSONWriter::Write(&period_dict, &output_js);
   ASSERT_EQ("{\"a.b\":3,\"c\":2,\"d.e.f\":{\"g.h.i.j\":1}}", output_js);
 
   DictionaryValue period_dict3;
   period_dict3.Set("a.b", Value::CreateIntegerValue(2));
   period_dict3.SetWithoutPathExpansion("a.b", Value::CreateIntegerValue(1));
-  JSONWriter::Write(&period_dict3, false, &output_js);
+  JSONWriter::Write(&period_dict3, &output_js);
   ASSERT_EQ("{\"a\":{\"b\":2},\"a.b\":1}", output_js);
 
-  // Test ignoring binary values.
+  // Test omitting binary values.
   root = BinaryValue::CreateWithCopiedBuffer("asdf", 4);
-  JSONWriter::WriteWithOptions(root, false,
-                               JSONWriter::OPTIONS_OMIT_BINARY_VALUES,
+  JSONWriter::WriteWithOptions(root, JSONWriter::OPTIONS_OMIT_BINARY_VALUES,
                                &output_js);
   ASSERT_TRUE(output_js.empty());
   delete root;
@@ -106,7 +106,7 @@ TEST(JSONWriterTest, Writing) {
   binary_list.Append(Value::CreateIntegerValue(5));
   binary_list.Append(BinaryValue::CreateWithCopiedBuffer("asdf", 4));
   binary_list.Append(Value::CreateIntegerValue(2));
-  JSONWriter::WriteWithOptions(&binary_list, false,
+  JSONWriter::WriteWithOptions(&binary_list,
                                JSONWriter::OPTIONS_OMIT_BINARY_VALUES,
                                &output_js);
   ASSERT_EQ("[5,2]", output_js);
@@ -115,7 +115,7 @@ TEST(JSONWriterTest, Writing) {
   binary_dict.Set("a", Value::CreateIntegerValue(5));
   binary_dict.Set("b", BinaryValue::CreateWithCopiedBuffer("asdf", 4));
   binary_dict.Set("c", Value::CreateIntegerValue(2));
-  JSONWriter::WriteWithOptions(&binary_dict, false,
+  JSONWriter::WriteWithOptions(&binary_dict,
                                JSONWriter::OPTIONS_OMIT_BINARY_VALUES,
                                &output_js);
   ASSERT_EQ("{\"a\":5,\"c\":2}", output_js);
@@ -123,7 +123,7 @@ TEST(JSONWriterTest, Writing) {
   // Test allowing a double with no fractional part to be written as an integer.
   FundamentalValue double_value(1e10);
   JSONWriter::WriteWithOptions(
-      &double_value, false,
+      &double_value,
       JSONWriter::OPTIONS_OMIT_DOUBLE_TYPE_PRESERVATION,
       &output_js);
   ASSERT_EQ("10000000000", output_js);
diff --git a/base/tracked_objects_unittest.cc b/base/tracked_objects_unittest.cc
index d57ddee..837a449 100644
--- a/base/tracked_objects_unittest.cc
+++ b/base/tracked_objects_unittest.cc
@@ -172,7 +172,7 @@ TEST_F(TrackedObjectsTest, ParentChildTest) {
 
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
       "\"descendants\":["
         "{"
@@ -260,7 +260,7 @@ TEST_F(TrackedObjectsTest, DeathDataTest) {
 
   scoped_ptr<base::Value> value(data->ToValue());
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
       "\"count\":2,"
       "\"queue_ms\":16,"
@@ -289,7 +289,7 @@ TEST_F(TrackedObjectsTest, DeactivatedBirthOnlyToValueWorkerThread) {
 
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
       "\"descendants\":["
       "],"
@@ -317,7 +317,7 @@ TEST_F(TrackedObjectsTest, DeactivatedBirthOnlyToValueMainThread) {
 
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
       "\"descendants\":["
       "],"
@@ -342,7 +342,7 @@ TEST_F(TrackedObjectsTest, BirthOnlyToValueWorkerThread) {
 
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
       "\"descendants\":["
       "],"
@@ -387,7 +387,7 @@ TEST_F(TrackedObjectsTest, BirthOnlyToValueMainThread) {
 
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string birth_only_result = "{"
       "\"descendants\":["
       "],"
@@ -445,7 +445,7 @@ TEST_F(TrackedObjectsTest, LifeCycleToValueMainThread) {
 
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
       "\"descendants\":["
       "],"
@@ -511,7 +511,7 @@ TEST_F(TrackedObjectsTest, LifeCycleMidDeactivatedToValueMainThread) {
 
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
       "\"descendants\":["
       "],"
@@ -570,7 +570,7 @@ TEST_F(TrackedObjectsTest, LifeCyclePreDeactivatedToValueMainThread) {
 
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
       "\"descendants\":["
       "],"
@@ -606,7 +606,7 @@ TEST_F(TrackedObjectsTest, LifeCycleToValueWorkerThread) {
   // Call for the ToValue, but tell it to not the maxes after scanning.
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
       "\"descendants\":["
       "],"
@@ -637,14 +637,14 @@ TEST_F(TrackedObjectsTest, LifeCycleToValueWorkerThread) {
   // We'll still get the same values, but the data will be reset (which we'll
   // see in a moment).
   value.reset(ThreadData::ToValue(true));
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   // Result should be unchanged.
   EXPECT_EQ(one_line_result, json);
 
   // Call for the ToValue, and now we'll see the result of the last translation,
   // as the max will have been pushed back to zero.
   value.reset(ThreadData::ToValue(false));
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result_with_zeros = "{"
       "\"descendants\":["
       "],"
@@ -710,7 +710,7 @@ TEST_F(TrackedObjectsTest, TwoLives) {
 
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
       "\"descendants\":["
       "],"
@@ -772,7 +772,7 @@ TEST_F(TrackedObjectsTest, DifferentLives) {
 
   scoped_ptr<base::Value> value(ThreadData::ToValue(false));
   std::string json;
-  base::JSONWriter::Write(value.get(), false, &json);
+  base::JSONWriter::Write(value.get(), &json);
   std::string one_line_result = "{"
       "\"descendants\":["
       "],"