Introduce Rvalue `base::Dict::SetByDottedPath` methods
This allows people to use these methods while creating a dictionary
builder style.
Bug: 1417179
Change-Id: Ie0d1e6cc5daca1fc0993ee533bacf16288f70d6e
Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4370138
Auto-Submit: Jeroen Dhollander <jeroendh@google.com>
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1177865}
diff --git a/base/json/json_writer_unittest.cc b/base/json/json_writer_unittest.cc
index 479feb4..4eecb94 100644
--- a/base/json/json_writer_unittest.cc
+++ b/base/json/json_writer_unittest.cc
@@ -94,7 +94,7 @@
R"({"a.b":3,"c":2,"d.e.f":{"g.h.i.j":1}})");
EXPECT_EQ(WriteJson(Value::Dict() //
- .Set("a", Value::Dict().Set("b", 2))
+ .SetByDottedPath("a.b", 2)
.Set("a.b", 1)),
R"({"a":{"b":2},"a.b":1})");
}
diff --git a/base/values.cc b/base/values.cc
index 28c6f40..af08d13 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -695,7 +695,7 @@
return v ? v->GetIfList() : nullptr;
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, Value&& value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, Value&& value) & {
DCHECK(!path.empty());
DCHECK(IsStringUTF8AllowingNoncharacters(path));
@@ -723,47 +723,47 @@
}
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, bool value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, bool value) & {
return SetByDottedPath(path, Value(value));
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, int value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, int value) & {
return SetByDottedPath(path, Value(value));
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, double value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, double value) & {
return SetByDottedPath(path, Value(value));
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, StringPiece value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, StringPiece value) & {
return SetByDottedPath(path, Value(value));
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, StringPiece16 value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, StringPiece16 value) & {
return SetByDottedPath(path, Value(value));
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, const char* value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, const char* value) & {
return SetByDottedPath(path, Value(value));
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, const char16_t* value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, const char16_t* value) & {
return SetByDottedPath(path, Value(value));
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, std::string&& value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, std::string&& value) & {
return SetByDottedPath(path, Value(std::move(value)));
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, BlobStorage&& value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, BlobStorage&& value) & {
return SetByDottedPath(path, Value(std::move(value)));
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, Dict&& value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, Dict&& value) & {
return SetByDottedPath(path, Value(std::move(value)));
}
-Value* Value::Dict::SetByDottedPath(StringPiece path, List&& value) {
+Value* Value::Dict::SetByDottedPath(StringPiece path, List&& value) & {
return SetByDottedPath(path, Value(std::move(value)));
}
@@ -771,6 +771,72 @@
return ExtractByDottedPath(path).has_value();
}
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path, Value&& value) && {
+ SetByDottedPath(path, std::move(value));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path, bool value) && {
+ SetByDottedPath(path, Value(value));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path, int value) && {
+ SetByDottedPath(path, Value(value));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path, double value) && {
+ SetByDottedPath(path, Value(value));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path,
+ StringPiece value) && {
+ SetByDottedPath(path, Value(value));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path,
+ StringPiece16 value) && {
+ SetByDottedPath(path, Value(value));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path,
+ const char* value) && {
+ SetByDottedPath(path, Value(value));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path,
+ const char16_t* value) && {
+ SetByDottedPath(path, Value(value));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path,
+ std::string&& value) && {
+ SetByDottedPath(path, Value(std::move(value)));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path,
+ BlobStorage&& value) && {
+ SetByDottedPath(path, Value(std::move(value)));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path, Dict&& value) && {
+ SetByDottedPath(path, Value(std::move(value)));
+ return std::move(*this);
+}
+
+Value::Dict&& Value::Dict::SetByDottedPath(StringPiece path, List&& value) && {
+ SetByDottedPath(path, Value(std::move(value)));
+ return std::move(*this);
+}
+
absl::optional<Value> Value::Dict::ExtractByDottedPath(StringPiece path) {
DCHECK(!path.empty());
DCHECK(IsStringUTF8AllowingNoncharacters(path));
diff --git a/base/values.h b/base/values.h
index 42e4bf9..465a7f4 100644
--- a/base/values.h
+++ b/base/values.h
@@ -502,20 +502,76 @@
// missing an entry while performing the path traversal. Will fail if any
// non-last component of the path refers to an already-existing entry that
// is not a dictionary. Returns `nullptr` on failure.
- Value* SetByDottedPath(StringPiece path, Value&& value);
- Value* SetByDottedPath(StringPiece path, bool value);
+ //
+ // Warning: repeatedly using this API to enter entries in the same nested
+ // dictionary is inefficient, so please do not write the following:
+ //
+ // bad_example.SetByDottedPath("a.nested.dictionary.field_1", 1);
+ // bad_example.SetByDottedPath("a.nested.dictionary.field_2", "value");
+ // bad_example.SetByDottedPath("a.nested.dictionary.field_3", 1);
+ //
+ Value* SetByDottedPath(StringPiece path, Value&& value) &;
+ Value* SetByDottedPath(StringPiece path, bool value) &;
template <typename T>
- Value* SetByDottedPath(StringPiece, const T*) = delete;
- Value* SetByDottedPath(StringPiece path, int value);
- Value* SetByDottedPath(StringPiece path, double value);
- Value* SetByDottedPath(StringPiece path, StringPiece value);
- Value* SetByDottedPath(StringPiece path, StringPiece16 value);
- Value* SetByDottedPath(StringPiece path, const char* value);
- Value* SetByDottedPath(StringPiece path, const char16_t* value);
- Value* SetByDottedPath(StringPiece path, std::string&& value);
- Value* SetByDottedPath(StringPiece path, BlobStorage&& value);
- Value* SetByDottedPath(StringPiece path, Dict&& value);
- Value* SetByDottedPath(StringPiece path, List&& value);
+ Value* SetByDottedPath(StringPiece, const T*) & = delete;
+ Value* SetByDottedPath(StringPiece path, int value) &;
+ Value* SetByDottedPath(StringPiece path, double value) &;
+ Value* SetByDottedPath(StringPiece path, StringPiece value) &;
+ Value* SetByDottedPath(StringPiece path, StringPiece16 value) &;
+ Value* SetByDottedPath(StringPiece path, const char* value) &;
+ Value* SetByDottedPath(StringPiece path, const char16_t* value) &;
+ Value* SetByDottedPath(StringPiece path, std::string&& value) &;
+ Value* SetByDottedPath(StringPiece path, BlobStorage&& value) &;
+ Value* SetByDottedPath(StringPiece path, Dict&& value) &;
+ Value* SetByDottedPath(StringPiece path, List&& value) &;
+
+ // Rvalue overrides of the `SetByDottedPath` methods, which allow you to
+ // construct a `Value::Dict` builder-style:
+ //
+ // Value::Dict result =
+ // Value::Dict()
+ // .SetByDottedPath("a.nested.dictionary.with.key-1", "first value")
+ // .Set("local-key-1", 2));
+ //
+ // Each method returns a rvalue reference to `this`, so this is as efficient
+ // as (and less mistake-prone than) stand-alone calls to `Set`.
+ //
+ // Warning: repeatedly using this API to enter entries in the same nested
+ // dictionary is inefficient, so do not write this:
+ //
+ // Value::Dict bad_example =
+ // Value::Dict()
+ // .SetByDottedPath("nested.dictionary.key-1", "first value")
+ // .SetByDottedPath("nested.dictionary.key-2", "second value")
+ // .SetByDottedPath("nested.dictionary.key-3", "third value");
+ //
+ // Instead, simply write this
+ //
+ // Value::Dict good_example =
+ // Value::Dict()
+ // .Set("nested",
+ // base::Value::Dict()
+ // .Set("dictionary",
+ // base::Value::Dict()
+ // .Set(key-1", "first value")
+ // .Set(key-2", "second value")
+ // .Set(key-3", "third value")));
+ //
+ //
+ Dict&& SetByDottedPath(StringPiece path, Value&& value) &&;
+ Dict&& SetByDottedPath(StringPiece path, bool value) &&;
+ template <typename T>
+ Dict&& SetByDottedPath(StringPiece, const T*) && = delete;
+ Dict&& SetByDottedPath(StringPiece path, int value) &&;
+ Dict&& SetByDottedPath(StringPiece path, double value) &&;
+ Dict&& SetByDottedPath(StringPiece path, StringPiece value) &&;
+ Dict&& SetByDottedPath(StringPiece path, StringPiece16 value) &&;
+ Dict&& SetByDottedPath(StringPiece path, const char* value) &&;
+ Dict&& SetByDottedPath(StringPiece path, const char16_t* value) &&;
+ Dict&& SetByDottedPath(StringPiece path, std::string&& value) &&;
+ Dict&& SetByDottedPath(StringPiece path, BlobStorage&& value) &&;
+ Dict&& SetByDottedPath(StringPiece path, Dict&& value) &&;
+ Dict&& SetByDottedPath(StringPiece path, List&& value) &&;
bool RemoveByDottedPath(StringPiece path);
diff --git a/base/values_unittest.cc b/base/values_unittest.cc
index c237009..5d2aa329 100644
--- a/base/values_unittest.cc
+++ b/base/values_unittest.cc
@@ -675,6 +675,43 @@
EXPECT_EQ(c, b->Find("c"));
}
+TEST(ValuesTest, RvalueDictSetByDottedPath) {
+ Value::Dict dict =
+ Value::Dict()
+ .SetByDottedPath("nested.dictionary.null", Value())
+ .SetByDottedPath("nested.dictionary.bool", false)
+ .SetByDottedPath("nested.dictionary.int", 42)
+ .SetByDottedPath("nested.dictionary.double", 1.2)
+ .SetByDottedPath("nested.dictionary.string", "value")
+ .SetByDottedPath("nested.dictionary.u16-string", u"u16-value")
+ .SetByDottedPath("nested.dictionary.std-string",
+ std::string("std-value"))
+ .SetByDottedPath("nested.dictionary.blob", Value::BlobStorage({1, 2}))
+ .SetByDottedPath("nested.dictionary.list",
+ Value::List().Append("value in list"))
+ .SetByDottedPath("nested.dictionary.dict",
+ Value::Dict().Set("key", "value"));
+
+ Value::Dict expected =
+ Value::Dict() //
+ .Set("nested",
+ base::Value::Dict() //
+ .Set("dictionary",
+ base::Value::Dict()
+ .Set("null", Value())
+ .Set("bool", false)
+ .Set("int", 42)
+ .Set("double", 1.2)
+ .Set("string", "value")
+ .Set("u16-string", u"u16-value")
+ .Set("std-string", std::string("std-value"))
+ .Set("blob", Value::BlobStorage({1, 2}))
+ .Set("list", Value::List().Append("value in list"))
+ .Set("dict", Value::Dict().Set("key", "value"))));
+
+ EXPECT_EQ(dict, expected);
+}
+
TEST(ValuesTest, DictSetWithDottedKey) {
Value::Dict dict;
diff --git a/chrome/browser/media/router/providers/cast/cast_media_controller.cc b/chrome/browser/media/router/providers/cast/cast_media_controller.cc
index c216914d..52ebd6c 100644
--- a/chrome/browser/media/router/providers/cast/cast_media_controller.cc
+++ b/chrome/browser/media/router/providers/cast/cast_media_controller.cc
@@ -93,32 +93,32 @@
void CastMediaController::SetMute(bool mute) {
if (session_id_.empty())
return;
- base::Value::Dict request = CreateVolumeRequest();
- request.SetByDottedPath("message.volume.muted", mute);
- request.Set("type", "v2_message");
- request.Set("clientId", sender_id_);
- auto message = CastInternalMessage::From(std::move(request));
+ auto message = CastInternalMessage::From(
+ CreateVolumeRequest()
+ .SetByDottedPath("message.volume.muted", mute)
+ .Set("type", "v2_message")
+ .Set("clientId", sender_id_));
activity_->SendSetVolumeRequestToReceiver(*message, base::DoNothing());
}
void CastMediaController::SetVolume(float volume) {
if (session_id_.empty())
return;
- base::Value::Dict request = CreateVolumeRequest();
- request.SetByDottedPath("message.volume.level", volume);
- request.Set("type", "v2_message");
- request.Set("clientId", sender_id_);
activity_->SendSetVolumeRequestToReceiver(
- *CastInternalMessage::From(std::move(request)), base::DoNothing());
+ *CastInternalMessage::From(
+ CreateVolumeRequest()
+ .SetByDottedPath("message.volume.level", volume)
+ .Set("type", "v2_message")
+ .Set("clientId", sender_id_)),
+ base::DoNothing());
}
void CastMediaController::Seek(base::TimeDelta time) {
if (session_id_.empty())
return;
- base::Value::Dict request = CreateMediaRequest(V2MessageType::kSeek);
- request.SetByDottedPath("message.currentTime", time.InSecondsF());
- activity_->SendMediaRequestToReceiver(
- *CastInternalMessage::From(std::move(request)));
+ activity_->SendMediaRequestToReceiver(*CastInternalMessage::From(
+ CreateMediaRequest(V2MessageType::kSeek)
+ .SetByDottedPath("message.currentTime", time.InSecondsF())));
}
void CastMediaController::NextTrack() {
@@ -126,10 +126,9 @@
return;
// We do not use |kQueueNext| because not all receiver apps support it.
// See crbug.com/1078601.
- base::Value::Dict request = CreateMediaRequest(V2MessageType::kQueueUpdate);
- request.SetByDottedPath("message.jump", kQueueNextJumpValue);
- activity_->SendMediaRequestToReceiver(
- *CastInternalMessage::From(std::move(request)));
+ activity_->SendMediaRequestToReceiver(*CastInternalMessage::From(
+ CreateMediaRequest(V2MessageType::kQueueUpdate)
+ .SetByDottedPath("message.jump", kQueueNextJumpValue)));
}
void CastMediaController::PreviousTrack() {
@@ -137,10 +136,9 @@
return;
// We do not use |kQueuePrev| because not all receiver apps support it.
// See crbug.com/1078601.
- base::Value::Dict request = CreateMediaRequest(V2MessageType::kQueueUpdate);
- request.SetByDottedPath("message.jump", kQueuePrevJumpValue);
- activity_->SendMediaRequestToReceiver(
- *CastInternalMessage::From(std::move(request)));
+ activity_->SendMediaRequestToReceiver(*CastInternalMessage::From(
+ CreateMediaRequest(V2MessageType::kQueueUpdate)
+ .SetByDottedPath("message.jump", kQueuePrevJumpValue)));
}
void CastMediaController::AddMediaController(
@@ -177,28 +175,26 @@
}
base::Value::Dict CastMediaController::CreateMediaRequest(V2MessageType type) {
- base::Value::Dict message;
- message.Set("mediaSessionId", media_session_id_);
- message.Set("sessionId", session_id_);
- message.Set("type", cast_util::EnumToString(type).value().data());
- base::Value::Dict request;
- request.Set("message", std::move(message));
- request.Set("type", "v2_message");
- request.Set("clientId", sender_id_);
- return request;
+ return base::Value::Dict()
+ .Set("message",
+ base::Value::Dict()
+ .Set("mediaSessionId", media_session_id_)
+ .Set("sessionId", session_id_)
+ .Set("type", cast_util::EnumToString(type).value().data()))
+ .Set("type", "v2_message")
+ .Set("clientId", sender_id_);
}
base::Value::Dict CastMediaController::CreateVolumeRequest() {
- base::Value::Dict message;
- message.Set("sessionId", session_id_);
- // Muting also uses the |kSetVolume| message type.
- message.Set(
- "type",
- cast_util::EnumToString(V2MessageType::kSetVolume).value().data());
- message.Set("volume", base::Value::Dict());
- base::Value::Dict request;
- request.Set("message", std::move(message));
- return request;
+ return base::Value::Dict().Set(
+ "message",
+ base::Value::Dict()
+ .Set("sessionId", session_id_)
+ // Muting also uses the |kSetVolume| message type.
+ .Set(
+ "type",
+ cast_util::EnumToString(V2MessageType::kSetVolume).value().data())
+ .Set("volume", base::Value::Dict()));
}
void CastMediaController::UpdateMediaStatus(