Commit fdea8bed authored by jdoerrie's avatar jdoerrie Committed by Commit Bot

Reland "Reland "[base] Harden base::JSONWriter::BuildJSONString""

This is a reland of 3b214250

Original change's description:
> Reland "[base] Harden base::JSONWriter::BuildJSONString"
>
> This is a reland of bbe71c7f
>
> Original change's description:
> > [base] Harden base::JSONWriter::BuildJSONString
> >
> > This change simplifies and hardens base::JSONWriter::BuildJSONString().
> > It replaces the old base::Value API with the new one, thus implicitly
> > replacing DCHECKs with CHECKs.
> >
> > Bug: 859477
> > Change-Id: I5aa68cbc1e5e241d7b7061da2fe7078ac2904ec8
> > Reviewed-on: https://chromium-review.googlesource.com/c/1304477
> > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#603508}
>
> Bug: 859477, 900041
> Change-Id: I0111b5ac553736cdec5b7a163527bf3bb7cb9233
> Reviewed-on: https://chromium-review.googlesource.com/c/1306356
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#603889}

TBR=rsesek@chromium.org

Bug: 859477, 900041
Change-Id: I1a825493332a6889afe61897ee9007b651fe86e8
Reviewed-on: https://chromium-review.googlesource.com/c/1308196
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604035}
parent c823a23d
...@@ -57,47 +57,37 @@ JSONWriter::JSONWriter(int options, std::string* json) ...@@ -57,47 +57,37 @@ JSONWriter::JSONWriter(int options, std::string* json)
bool JSONWriter::BuildJSONString(const Value& node, size_t depth) { bool JSONWriter::BuildJSONString(const Value& node, size_t depth) {
switch (node.type()) { switch (node.type()) {
case Value::Type::NONE: { case Value::Type::NONE:
json_string_->append("null"); json_string_->append("null");
return true; return true;
}
case Value::Type::BOOLEAN: { case Value::Type::BOOLEAN:
bool value; // Note: We are explicitly invoking the std::string constructor in order
bool result = node.GetAsBoolean(&value); // to avoid ASAN errors on Windows: https://crbug.com/900041
DCHECK(result); json_string_->append(node.GetBool() ? std::string("true")
json_string_->append(value ? "true" : "false"); : std::string("false"));
return result; return true;
}
case Value::Type::INTEGER: { case Value::Type::INTEGER:
int value; json_string_->append(IntToString(node.GetInt()));
bool result = node.GetAsInteger(&value); return true;
DCHECK(result);
json_string_->append(IntToString(value));
return result;
}
case Value::Type::DOUBLE: { case Value::Type::DOUBLE: {
double value; double value = node.GetDouble();
bool result = node.GetAsDouble(&value);
DCHECK(result);
if (omit_double_type_preservation_ && if (omit_double_type_preservation_ &&
value <= std::numeric_limits<int64_t>::max() && value <= std::numeric_limits<int64_t>::max() &&
value >= std::numeric_limits<int64_t>::min() && value >= std::numeric_limits<int64_t>::min() &&
std::floor(value) == value) { std::floor(value) == value) {
json_string_->append(Int64ToString(static_cast<int64_t>(value))); json_string_->append(Int64ToString(static_cast<int64_t>(value)));
return result; return true;
} }
std::string real = NumberToString(value); std::string real = NumberToString(value);
// Ensure that the number has a .0 if there's no decimal or 'e'. This // Ensure that the number has a .0 if there's no decimal or 'e'. This
// makes sure that when we read the JSON back, it's interpreted as a // makes sure that when we read the JSON back, it's interpreted as a
// real rather than an int. // real rather than an int.
if (real.find('.') == std::string::npos && if (real.find_first_of(".eE") == std::string::npos)
real.find('e') == std::string::npos &&
real.find('E') == std::string::npos) {
real.append(".0"); real.append(".0");
}
// The JSON spec requires that non-integer values in the range (-1,1) // The JSON spec requires that non-integer values in the range (-1,1)
// have a zero before the decimal point - ".52" is not valid, "0.52" is. // have a zero before the decimal point - ".52" is not valid, "0.52" is.
if (real[0] == '.') { if (real[0] == '.') {
...@@ -107,27 +97,21 @@ bool JSONWriter::BuildJSONString(const Value& node, size_t depth) { ...@@ -107,27 +97,21 @@ bool JSONWriter::BuildJSONString(const Value& node, size_t depth) {
real.insert(static_cast<size_t>(1), static_cast<size_t>(1), '0'); real.insert(static_cast<size_t>(1), static_cast<size_t>(1), '0');
} }
json_string_->append(real); json_string_->append(real);
return result; return true;
} }
case Value::Type::STRING: { case Value::Type::STRING:
std::string value; EscapeJSONString(node.GetString(), true, json_string_);
bool result = node.GetAsString(&value); return true;
DCHECK(result);
EscapeJSONString(value, true, json_string_);
return result;
}
case Value::Type::LIST: { case Value::Type::LIST: {
json_string_->push_back('['); json_string_->push_back('[');
if (pretty_print_) if (pretty_print_)
json_string_->push_back(' '); json_string_->push_back(' ');
const ListValue* list = nullptr;
bool first_value_has_been_output = false; bool first_value_has_been_output = false;
bool result = node.GetAsList(&list); bool result = true;
DCHECK(result); for (const auto& value : node.GetList()) {
for (const auto& value : *list) {
if (omit_binary_values_ && value.type() == Value::Type::BINARY) if (omit_binary_values_ && value.type() == Value::Type::BINARY)
continue; continue;
...@@ -154,15 +138,13 @@ bool JSONWriter::BuildJSONString(const Value& node, size_t depth) { ...@@ -154,15 +138,13 @@ bool JSONWriter::BuildJSONString(const Value& node, size_t depth) {
if (pretty_print_) if (pretty_print_)
json_string_->append(kPrettyPrintLineEnding); json_string_->append(kPrettyPrintLineEnding);
const DictionaryValue* dict = nullptr;
bool first_value_has_been_output = false; bool first_value_has_been_output = false;
bool result = node.GetAsDictionary(&dict); bool result = true;
DCHECK(result); for (const auto& pair : node.DictItems()) {
for (DictionaryValue::Iterator itr(*dict); !itr.IsAtEnd(); const auto& key = pair.first;
itr.Advance()) { const auto& value = pair.second;
if (omit_binary_values_ && itr.value().type() == Value::Type::BINARY) { if (omit_binary_values_ && value.type() == Value::Type::BINARY)
continue; continue;
}
if (first_value_has_been_output) { if (first_value_has_been_output) {
json_string_->push_back(','); json_string_->push_back(',');
...@@ -173,12 +155,12 @@ bool JSONWriter::BuildJSONString(const Value& node, size_t depth) { ...@@ -173,12 +155,12 @@ bool JSONWriter::BuildJSONString(const Value& node, size_t depth) {
if (pretty_print_) if (pretty_print_)
IndentLine(depth + 1U); IndentLine(depth + 1U);
EscapeJSONString(itr.key(), true, json_string_); EscapeJSONString(key, true, json_string_);
json_string_->push_back(':'); json_string_->push_back(':');
if (pretty_print_) if (pretty_print_)
json_string_->push_back(' '); json_string_->push_back(' ');
if (!BuildJSONString(itr.value(), depth + 1U)) if (!BuildJSONString(value, depth + 1U))
result = false; result = false;
first_value_has_been_output = true; first_value_has_been_output = true;
...@@ -198,7 +180,9 @@ bool JSONWriter::BuildJSONString(const Value& node, size_t depth) { ...@@ -198,7 +180,9 @@ bool JSONWriter::BuildJSONString(const Value& node, size_t depth) {
DLOG_IF(ERROR, !omit_binary_values_) << "Cannot serialize binary value."; DLOG_IF(ERROR, !omit_binary_values_) << "Cannot serialize binary value.";
return omit_binary_values_; return omit_binary_values_;
} }
NOTREACHED();
// TODO(crbug.com/859477): Revert to NOTREACHED() after root cause is found.
CHECK(false);
return false; return false;
} }
......
...@@ -126,7 +126,7 @@ class TestPersonalDataManager : public PersonalDataManager { ...@@ -126,7 +126,7 @@ class TestPersonalDataManager : public PersonalDataManager {
base::Optional<bool> autofill_profile_enabled_; base::Optional<bool> autofill_profile_enabled_;
base::Optional<bool> autofill_credit_card_enabled_; base::Optional<bool> autofill_credit_card_enabled_;
base::Optional<bool> autofill_wallet_import_enabled_; base::Optional<bool> autofill_wallet_import_enabled_;
bool sync_feature_enabled_; bool sync_feature_enabled_ = false;
AccountInfo account_info_; AccountInfo account_info_;
DISALLOW_COPY_AND_ASSIGN(TestPersonalDataManager); DISALLOW_COPY_AND_ASSIGN(TestPersonalDataManager);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment