Introduce mutable Value::FindStringKey
This CL introduces a Value::FindStringKey implementation that returns a mutable
string which can be modified inline. This reduces the number of temporary
copies needed for the LogBuffer.
Bug: 928595
Change-Id: Ic09b0bd029805b983a888b691420a9d3b32f4ee3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1724516
Commit-Queue: Dominic Battré <battre@chromium.org>
Auto-Submit: Dominic Battré <battre@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682580}
diff --git a/base/values.cc b/base/values.cc
index 7d042cd..02e92b7 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -320,6 +320,11 @@
return string_value_;
}
+std::string& Value::GetString() {
+ CHECK(is_string());
+ return string_value_;
+}
+
const Value::BlobStorage& Value::GetBlob() const {
CHECK(is_blob());
return binary_value_;
@@ -386,6 +391,11 @@
return result ? &result->string_value_ : nullptr;
}
+std::string* Value::FindStringKey(StringPiece key) {
+ Value* result = FindKeyOfType(key, Type::STRING);
+ return result ? &result->string_value_ : nullptr;
+}
+
const Value::BlobStorage* Value::FindBlobKey(StringPiece key) const {
const Value* value = FindKeyOfType(key, Type::BINARY);
return value ? &value->binary_value_ : nullptr;
@@ -526,6 +536,11 @@
return &cur->string_value_;
}
+std::string* Value::FindStringPath(StringPiece path) {
+ return const_cast<std::string*>(
+ static_cast<const Value*>(this)->FindStringPath(path));
+}
+
const Value::BlobStorage* Value::FindBlobPath(StringPiece path) const {
const Value* cur = FindPath(path);
if (!cur || !cur->is_blob())
diff --git a/base/values.h b/base/values.h
index ef24b19..38e4f37 100644
--- a/base/values.h
+++ b/base/values.h
@@ -172,6 +172,7 @@
int GetInt() const;
double GetDouble() const; // Implicitly converts from int if necessary.
const std::string& GetString() const;
+ std::string& GetString();
const BlobStorage& GetBlob() const;
ListStorage& GetList();
@@ -211,6 +212,7 @@
// |FindStringKey| returns |nullptr| if value is not found or not a string.
const std::string* FindStringKey(StringPiece key) const;
+ std::string* FindStringKey(StringPiece key);
// Returns nullptr is value is not found or not a binary.
const BlobStorage* FindBlobKey(StringPiece key) const;
@@ -318,6 +320,7 @@
base::Optional<int> FindIntPath(StringPiece path) const;
base::Optional<double> FindDoublePath(StringPiece path) const;
const std::string* FindStringPath(StringPiece path) const;
+ std::string* FindStringPath(StringPiece path);
const BlobStorage* FindBlobPath(StringPiece path) const;
Value* FindDictPath(StringPiece path);
const Value* FindDictPath(StringPiece path) const;
diff --git a/base/values_unittest.cc b/base/values_unittest.cc
index 884309b..c8be7e5f 100644
--- a/base/values_unittest.cc
+++ b/base/values_unittest.cc
@@ -748,6 +748,20 @@
EXPECT_EQ(nullptr, dict.FindStringKey("dict"));
}
+TEST(ValuesTest, MutableFindStringKey) {
+ Value::DictStorage storage;
+ storage.emplace("string", std::make_unique<Value>("foo"));
+ Value dict(std::move(storage));
+
+ *(dict.FindStringKey("string")) = "bar";
+
+ Value::DictStorage expected_storage;
+ expected_storage.emplace("string", std::make_unique<Value>("bar"));
+ Value expected_dict(std::move(expected_storage));
+
+ EXPECT_EQ(expected_dict, dict);
+}
+
TEST(ValuesTest, FindDictKey) {
Value::DictStorage storage;
storage.emplace("null", std::make_unique<Value>(Value::Type::NONE));
@@ -2374,4 +2388,22 @@
EXPECT_EQ(dict_copy, *val);
}
+TEST(ValuesTest, MutableFindStringPath) {
+ Value dict(Value::Type::DICTIONARY);
+ dict.SetStringPath("foo.bar", "value");
+
+ *(dict.FindStringPath("foo.bar")) = "new_value";
+
+ Value expected_dict(Value::Type::DICTIONARY);
+ expected_dict.SetStringPath("foo.bar", "new_value");
+
+ EXPECT_EQ(expected_dict, dict);
+}
+
+TEST(ValuesTest, MutableGetString) {
+ Value value("value");
+ value.GetString() = "new_value";
+ EXPECT_EQ("new_value", value.GetString());
+}
+
} // namespace base
diff --git a/components/autofill/core/browser/logging/log_buffer.cc b/components/autofill/core/browser/logging/log_buffer.cc
index a9fe4db..c678c92 100644
--- a/components/autofill/core/browser/logging/log_buffer.cc
+++ b/components/autofill/core/browser/logging/log_buffer.cc
@@ -55,9 +55,6 @@
// and the lengths of strings should be relatively small and we reduce the
// memory consumption of the DOM, which may grow rather large.
//
-// TODO(crbug.com/928595) Provide a FindStringKey that returns a mutable string
-// and append to that string.
-//
// If the last child of the element in buffer is a text node, append |text| to
// it and return true (successful coalescing). Otherwise return false.
bool TryCoalesceString(std::vector<base::Value>* buffer,
@@ -72,8 +69,8 @@
auto& last_child = children->GetList().back();
if (!IsTextNode(last_child))
return false;
- const std::string* old_text = last_child.FindStringKey("value");
- last_child.SetStringKey("value", base::StrCat({*old_text, text}));
+ std::string* old_text = last_child.FindStringKey("value");
+ old_text->append(text.data(), text.size());
return true;
}