Skip to content

Commit

Permalink
Convert JsonWriter::Write to taking a const ref for the in-param
Browse files Browse the repository at this point in the history
Clearer API; flushes out a lot of unnecessary heap allocations.

depends on https://codereview.chromium.org/1129083003/

BUG=none

Review URL: https://codereview.chromium.org/1131113004

Cr-Commit-Position: refs/heads/master@{#330255}
  • Loading branch information
estade authored and Commit bot committed May 16, 2015
1 parent f39e5af commit 8d04646
Show file tree
Hide file tree
Showing 213 changed files with 594 additions and 629 deletions.
2 changes: 1 addition & 1 deletion android_webview/browser/aw_dev_tools_discovery_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ std::string GetViewDescription(WebContents* web_contents) {
description.SetInteger("height", screen_rect.height());
}
std::string json;
base::JSONWriter::Write(&description, &json);
base::JSONWriter::Write(description, &json);
return json;
}

Expand Down
2 changes: 1 addition & 1 deletion base/json/json_string_value_serializer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ bool JSONStringValueSerializer::SerializeInternal(const Value& root,
if (pretty_print_)
options |= base::JSONWriter::OPTIONS_PRETTY_PRINT;

return base::JSONWriter::WriteWithOptions(&root, options, json_string_);
return base::JSONWriter::WriteWithOptions(root, options, json_string_);
}

JSONStringValueDeserializer::JSONStringValueDeserializer(
Expand Down
2 changes: 1 addition & 1 deletion base/json/json_value_serializer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ TEST(JSONValueSerializerTest, StringEscape) {
std::string output_js;
DictionaryValue valueRoot;
valueRoot.SetString("all_chars", all_chars);
JSONWriter::Write(&valueRoot, &output_js);
JSONWriter::Write(valueRoot, &output_js);
ASSERT_EQ(expected_output, output_js);

// Test JSONValueSerializer interface (uses JSONWriter).
Expand Down
25 changes: 13 additions & 12 deletions base/json/json_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ const char kPrettyPrintLineEnding[] = "\n";
#endif

// static
bool JSONWriter::Write(const Value* const node, std::string* json) {
bool JSONWriter::Write(const Value& node, std::string* json) {
return WriteWithOptions(node, 0, json);
}

// static
bool JSONWriter::WriteWithOptions(const Value* const node, int options,
bool JSONWriter::WriteWithOptions(const Value& node,
int options,
std::string* json) {
json->clear();
// Is there a better way to estimate the size of the output?
Expand All @@ -50,32 +51,32 @@ JSONWriter::JSONWriter(int options, std::string* json)
DCHECK(json);
}

bool JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
switch (node->GetType()) {
bool JSONWriter::BuildJSONString(const Value& node, size_t depth) {
switch (node.GetType()) {
case Value::TYPE_NULL: {
json_string_->append("null");
return true;
}

case Value::TYPE_BOOLEAN: {
bool value;
bool result = node->GetAsBoolean(&value);
bool result = node.GetAsBoolean(&value);
DCHECK(result);
json_string_->append(value ? "true" : "false");
return result;
}

case Value::TYPE_INTEGER: {
int value;
bool result = node->GetAsInteger(&value);
bool result = node.GetAsInteger(&value);
DCHECK(result);
json_string_->append(IntToString(value));
return result;
}

case Value::TYPE_DOUBLE: {
double value;
bool result = node->GetAsDouble(&value);
bool result = node.GetAsDouble(&value);
DCHECK(result);
if (omit_double_type_preservation_ &&
value <= kint64max &&
Expand Down Expand Up @@ -107,7 +108,7 @@ bool JSONWriter::BuildJSONString(const Value* const node, size_t depth) {

case Value::TYPE_STRING: {
std::string value;
bool result = node->GetAsString(&value);
bool result = node.GetAsString(&value);
DCHECK(result);
EscapeJSONString(value, true, json_string_);
return result;
Expand All @@ -120,7 +121,7 @@ bool JSONWriter::BuildJSONString(const Value* const node, size_t depth) {

const ListValue* list = NULL;
bool first_value_has_been_output = false;
bool result = node->GetAsList(&list);
bool result = node.GetAsList(&list);
DCHECK(result);
for (ListValue::const_iterator it = list->begin(); it != list->end();
++it) {
Expand All @@ -134,7 +135,7 @@ bool JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
json_string_->push_back(' ');
}

if (!BuildJSONString(value, depth))
if (!BuildJSONString(*value, depth))
result = false;

first_value_has_been_output = true;
Expand All @@ -153,7 +154,7 @@ bool JSONWriter::BuildJSONString(const Value* const node, size_t depth) {

const DictionaryValue* dict = NULL;
bool first_value_has_been_output = false;
bool result = node->GetAsDictionary(&dict);
bool result = node.GetAsDictionary(&dict);
DCHECK(result);
for (DictionaryValue::Iterator itr(*dict); !itr.IsAtEnd();
itr.Advance()) {
Expand All @@ -176,7 +177,7 @@ bool JSONWriter::BuildJSONString(const Value* const node, size_t depth) {
if (pretty_print_)
json_string_->push_back(' ');

if (!BuildJSONString(&itr.value(), depth + 1U))
if (!BuildJSONString(itr.value(), depth + 1U))
result = false;

first_value_has_been_output = true;
Expand Down
7 changes: 4 additions & 3 deletions base/json/json_writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,20 @@ class BASE_EXPORT JSONWriter {
// 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)? Return true on success and false on failure.
static bool Write(const Value* const node, std::string* json);
static bool Write(const Value& node, std::string* json);

// Same as above but with |options| which is a bunch of JSONWriter::Options
// bitwise ORed together. Return true on success and false on failure.
static bool WriteWithOptions(const Value* const node, int options,
static bool WriteWithOptions(const Value& node,
int options,
std::string* json);

private:
JSONWriter(int options, std::string* json);

// Called recursively to build the JSON string. When completed,
// |json_string_| will contain the JSON.
bool BuildJSONString(const Value* const node, size_t depth);
bool BuildJSONString(const Value& node, size_t depth);

// Adds space to json_string_ for the indent level.
void IndentLine(size_t depth);
Expand Down
52 changes: 21 additions & 31 deletions base/json/json_writer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,47 +12,39 @@ TEST(JSONWriterTest, BasicTypes) {
std::string output_js;

// Test null.
EXPECT_TRUE(JSONWriter::Write(Value::CreateNullValue().get(), &output_js));
EXPECT_TRUE(JSONWriter::Write(*Value::CreateNullValue(), &output_js));
EXPECT_EQ("null", output_js);

// Test empty dict.
DictionaryValue dict;
EXPECT_TRUE(JSONWriter::Write(&dict, &output_js));
EXPECT_TRUE(JSONWriter::Write(DictionaryValue(), &output_js));
EXPECT_EQ("{}", output_js);

// Test empty list.
ListValue list;
EXPECT_TRUE(JSONWriter::Write(&list, &output_js));
EXPECT_TRUE(JSONWriter::Write(ListValue(), &output_js));
EXPECT_EQ("[]", output_js);

// Test integer values.
FundamentalValue int_value(42);
EXPECT_TRUE(JSONWriter::Write(&int_value, &output_js));
EXPECT_TRUE(JSONWriter::Write(FundamentalValue(42), &output_js));
EXPECT_EQ("42", output_js);

// Test boolean values.
FundamentalValue bool_value(true);
EXPECT_TRUE(JSONWriter::Write(&bool_value, &output_js));
EXPECT_TRUE(JSONWriter::Write(FundamentalValue(true), &output_js));
EXPECT_EQ("true", output_js);

// Test Real values should always have a decimal or an 'e'.
FundamentalValue double_value(1.0);
EXPECT_TRUE(JSONWriter::Write(&double_value, &output_js));
EXPECT_TRUE(JSONWriter::Write(FundamentalValue(1.0), &output_js));
EXPECT_EQ("1.0", output_js);

// Test Real values in the the range (-1, 1) must have leading zeros
FundamentalValue double_value2(0.2);
EXPECT_TRUE(JSONWriter::Write(&double_value2, &output_js));
EXPECT_TRUE(JSONWriter::Write(FundamentalValue(0.2), &output_js));
EXPECT_EQ("0.2", output_js);

// Test Real values in the the range (-1, 1) must have leading zeros
FundamentalValue double_value3(-0.8);
EXPECT_TRUE(JSONWriter::Write(&double_value3, &output_js));
EXPECT_TRUE(JSONWriter::Write(FundamentalValue(-0.8), &output_js));
EXPECT_EQ("-0.8", output_js);

// Test String values.
StringValue string_value("foo");
EXPECT_TRUE(JSONWriter::Write(&string_value, &output_js));
EXPECT_TRUE(JSONWriter::Write(StringValue("foo"), &output_js));
EXPECT_EQ("\"foo\"", output_js);
}

Expand All @@ -71,11 +63,10 @@ TEST(JSONWriterTest, NestedTypes) {
root_dict.Set("list", list.Pass());

// Test the pretty-printer.
EXPECT_TRUE(JSONWriter::Write(&root_dict, &output_js));
EXPECT_TRUE(JSONWriter::Write(root_dict, &output_js));
EXPECT_EQ("{\"list\":[{\"inner int\":10},[],true]}", output_js);
EXPECT_TRUE(JSONWriter::WriteWithOptions(&root_dict,
JSONWriter::OPTIONS_PRETTY_PRINT,
&output_js));
EXPECT_TRUE(JSONWriter::WriteWithOptions(
root_dict, JSONWriter::OPTIONS_PRETTY_PRINT, &output_js));

// The pretty-printer uses a different newline style on Windows than on
// other platforms.
Expand All @@ -102,13 +93,13 @@ TEST(JSONWriterTest, KeysWithPeriods) {
scoped_ptr<DictionaryValue> period_dict2(new DictionaryValue());
period_dict2->SetIntegerWithoutPathExpansion("g.h.i.j", 1);
period_dict.SetWithoutPathExpansion("d.e.f", period_dict2.Pass());
EXPECT_TRUE(JSONWriter::Write(&period_dict, &output_js));
EXPECT_TRUE(JSONWriter::Write(period_dict, &output_js));
EXPECT_EQ("{\"a.b\":3,\"c\":2,\"d.e.f\":{\"g.h.i.j\":1}}", output_js);

DictionaryValue period_dict3;
period_dict3.SetInteger("a.b", 2);
period_dict3.SetIntegerWithoutPathExpansion("a.b", 1);
EXPECT_TRUE(JSONWriter::Write(&period_dict3, &output_js));
EXPECT_TRUE(JSONWriter::Write(period_dict3, &output_js));
EXPECT_EQ("{\"a\":{\"b\":2},\"a.b\":1}", output_js);
}

Expand All @@ -118,9 +109,9 @@ TEST(JSONWriterTest, BinaryValues) {
// Binary values should return errors unless suppressed via the
// OPTIONS_OMIT_BINARY_VALUES flag.
scoped_ptr<Value> root(BinaryValue::CreateWithCopiedBuffer("asdf", 4));
EXPECT_FALSE(JSONWriter::Write(root.get(), &output_js));
EXPECT_FALSE(JSONWriter::Write(*root, &output_js));
EXPECT_TRUE(JSONWriter::WriteWithOptions(
root.get(), JSONWriter::OPTIONS_OMIT_BINARY_VALUES, &output_js));
*root, JSONWriter::OPTIONS_OMIT_BINARY_VALUES, &output_js));
EXPECT_TRUE(output_js.empty());

ListValue binary_list;
Expand All @@ -129,9 +120,9 @@ TEST(JSONWriterTest, BinaryValues) {
binary_list.Append(BinaryValue::CreateWithCopiedBuffer("asdf", 4));
binary_list.Append(make_scoped_ptr(new FundamentalValue(2)));
binary_list.Append(BinaryValue::CreateWithCopiedBuffer("asdf", 4));
EXPECT_FALSE(JSONWriter::Write(&binary_list, &output_js));
EXPECT_FALSE(JSONWriter::Write(binary_list, &output_js));
EXPECT_TRUE(JSONWriter::WriteWithOptions(
&binary_list, JSONWriter::OPTIONS_OMIT_BINARY_VALUES, &output_js));
binary_list, JSONWriter::OPTIONS_OMIT_BINARY_VALUES, &output_js));
EXPECT_EQ("[5,2]", output_js);

DictionaryValue binary_dict;
Expand All @@ -143,9 +134,9 @@ TEST(JSONWriterTest, BinaryValues) {
binary_dict.SetInteger("d", 2);
binary_dict.Set(
"e", make_scoped_ptr(BinaryValue::CreateWithCopiedBuffer("asdf", 4)));
EXPECT_FALSE(JSONWriter::Write(&binary_dict, &output_js));
EXPECT_FALSE(JSONWriter::Write(binary_dict, &output_js));
EXPECT_TRUE(JSONWriter::WriteWithOptions(
&binary_dict, JSONWriter::OPTIONS_OMIT_BINARY_VALUES, &output_js));
binary_dict, JSONWriter::OPTIONS_OMIT_BINARY_VALUES, &output_js));
EXPECT_EQ("{\"b\":5,\"d\":2}", output_js);
}

Expand All @@ -155,8 +146,7 @@ TEST(JSONWriterTest, DoublesAsInts) {
// Test allowing a double with no fractional part to be written as an integer.
FundamentalValue double_value(1e10);
EXPECT_TRUE(JSONWriter::WriteWithOptions(
&double_value,
JSONWriter::OPTIONS_OMIT_DOUBLE_TYPE_PRESERVATION,
double_value, JSONWriter::OPTIONS_OMIT_DOUBLE_TYPE_PRESERVATION,
&output_js));
EXPECT_EQ("10000000000", output_js);
}
Expand Down
2 changes: 1 addition & 1 deletion base/trace_event/trace_event_argument.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ ListValue* TracedValue::GetCurrentArray() {

void TracedValue::AppendAsTraceFormat(std::string* out) const {
std::string tmp;
JSONWriter::Write(stack_.front(), &tmp);
JSONWriter::Write(*stack_.front(), &tmp);
*out += tmp;
DCHECK_EQ(1u, stack_.size()) << tmp;
}
Expand Down
2 changes: 1 addition & 1 deletion base/trace_event/trace_event_system_stats_monitor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ bool TraceEventSystemStatsMonitor::IsTimerRunningForTest() const {
void AppendSystemProfileAsTraceFormat(const SystemMetrics& system_metrics,
std::string* output) {
std::string tmp;
base::JSONWriter::Write(system_metrics.ToValue().get(), &tmp);
base::JSONWriter::Write(*system_metrics.ToValue(), &tmp);
*output += tmp;
}

Expand Down
4 changes: 1 addition & 3 deletions base/values.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1168,9 +1168,7 @@ ValueDeserializer::~ValueDeserializer() {

std::ostream& operator<<(std::ostream& out, const Value& value) {
std::string json;
JSONWriter::WriteWithOptions(&value,
JSONWriter::OPTIONS_PRETTY_PRINT,
&json);
JSONWriter::WriteWithOptions(value, JSONWriter::OPTIONS_PRETTY_PRINT, &json);
return out << json;
}

Expand Down
10 changes: 4 additions & 6 deletions cc/debug/traced_picture.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,16 @@ void TracedPicture::AppendPictureAlias(std::string* out) const {
scoped_ptr<base::DictionaryValue> alias(new base::DictionaryValue());
alias->SetString("id_ref", base::StringPrintf("%p", picture_.get()));

scoped_ptr<base::DictionaryValue> res(new base::DictionaryValue());
res->Set("alias", alias.release());

base::DictionaryValue res;
res.Set("alias", alias.release());
std::string tmp;
base::JSONWriter::Write(res.get(), &tmp);
base::JSONWriter::Write(res, &tmp);
out->append(tmp);
}

void TracedPicture::AppendPicture(std::string* out) const {
scoped_ptr<base::Value> value = picture_->AsValue();
std::string tmp;
base::JSONWriter::Write(value.get(), &tmp);
base::JSONWriter::Write(*picture_->AsValue(), &tmp);
out->append(tmp);
}

Expand Down
2 changes: 1 addition & 1 deletion cc/trees/layer_tree_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3124,7 +3124,7 @@ std::string LayerTreeHostImpl::LayerTreeAsJson() const {
if (active_tree_->root_layer()) {
scoped_ptr<base::Value> json(active_tree_->root_layer()->LayerTreeAsJson());
base::JSONWriter::WriteWithOptions(
json.get(), base::JSONWriter::OPTIONS_PRETTY_PRINT, &str);
*json, base::JSONWriter::OPTIONS_PRETTY_PRINT, &str);
}
return str;
}
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/caps/generate_state_json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class TaskManagerDataDumper :

std::string json;
auto options = base::JSONWriter::OPTIONS_PRETTY_PRINT;
if (!base::JSONWriter::WriteWithOptions(&dict, options, &json))
if (!base::JSONWriter::WriteWithOptions(dict, options, &json))
return;

file_.WriteAtCurrentPos(json.c_str(), json.size());
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/chromeos/app_mode/kiosk_app_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ bool IsValidKioskAppManifest(const extensions::Manifest& manifest) {
return false;
}

std::string ValueToString(const base::Value* value) {
std::string ValueToString(const base::Value& value) {
std::string json;
base::JSONWriter::Write(value, &json);
return json;
Expand Down Expand Up @@ -602,7 +602,7 @@ void KioskAppData::OnWebstoreResponseParseSuccess(
icon_url_string);
if (!icon_url.is_valid()) {
LOG(ERROR) << "Webstore response error (icon url): "
<< ValueToString(webstore_data.get());
<< ValueToString(*webstore_data);
OnWebstoreResponseParseFailure(kInvalidWebstoreResponseError);
return;
}
Expand All @@ -626,7 +626,7 @@ bool KioskAppData::CheckResponseKeyValue(const base::DictionaryValue* response,
std::string* value) {
if (!response->GetString(key, value)) {
LOG(ERROR) << "Webstore response error (" << key
<< "): " << ValueToString(response);
<< "): " << ValueToString(*response);
OnWebstoreResponseParseFailure(kInvalidWebstoreResponseError);
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void BluetoothPairingDialog::GetDialogSize(gfx::Size* size) const {

std::string BluetoothPairingDialog::GetDialogArgs() const {
std::string data;
base::JSONWriter::Write(&device_data_, &data);
base::JSONWriter::Write(device_data_, &data);
return data;
}

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chromeos/boot_times_recorder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ std::string BootTimesRecorder::Stats::SerializeToString() const {
dictionary.SetString(kDisk, disk_);

std::string result;
if (!base::JSONWriter::Write(&dictionary, &result)) {
if (!base::JSONWriter::Write(dictionary, &result)) {
LOG(WARNING) << "BootTimesRecorder::Stats::SerializeToString(): failed.";
return std::string();
}
Expand Down
Loading

0 comments on commit 8d04646

Please sign in to comment.