Refactor MediaStatus message to base::Value::Dict

LOW_COVERAGE_REASON=Minor code edits which were already not covered well.

Bug: 1187032, 1291670, 1187023
Change-Id: I218956bfc1482dd2b453d5e2218fcdb71200665c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3927943
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: Ahmed Moussa <ahmedmoussa@google.com>
Cr-Commit-Position: refs/heads/main@{#1054063}

chrome/browser/media/router/providers/cast/app_activity.cc

@@ -94,8 +94,9 @@ void AppActivity::SendSetVolumeRequestToReceiver(
       cast_message.client_id(), std::move(callback));
 }
 
-void AppActivity::SendMediaStatusToClients(const base::Value& media_status,
-                                           absl::optional<int> request_id) {
+void AppActivity::SendMediaStatusToClients(
+    const base::Value::Dict& media_status,
+    absl::optional<int> request_id) {
   CastActivity::SendMediaStatusToClients(media_status, request_id);
   if (media_controller_)
     media_controller_->SetMediaStatus(media_status);

chrome/browser/media/router/providers/cast/app_activity.h

@@ -38,7 +38,7 @@ class AppActivity : public CastActivity {
               CastSessionTracker* session_tracker);
   ~AppActivity() override;
 
-  void SendMediaStatusToClients(const base::Value& media_status,
+  void SendMediaStatusToClients(const base::Value::Dict& media_status,
                                 absl::optional<int> request_id) override;
   void OnAppMessage(const cast::channel::CastMessage& message) override;
   void OnInternalMessage(const cast_channel::InternalMessage& message) override;

chrome/browser/media/router/providers/cast/app_activity_unittest.cc

@@ -314,8 +314,8 @@ TEST_F(AppActivityTest, OnAppMessage) {
 
   auto* client = AddMockClient("theClientId");
   auto message = cast_channel::CreateCastMessage(
-      "urn:x-cast:com.google.foo", base::Value(base::Value::Type::DICTIONARY),
-      "sourceId", "theClientId");
+      "urn:x-cast:com.google.foo", base::Value(base::Value::Dict()), "sourceId",
+      "theClientId");
   EXPECT_CALL(*client,
               SendMessageToClient(IsPresentationConnectionMessage(
                   CreateAppMessage("theSessionId", "theClientId", message)
@@ -329,8 +329,8 @@ TEST_F(AppActivityTest, OnAppMessageAllClients) {
   auto* client1 = AddMockClient("theClientId1");
   auto* client2 = AddMockClient("theClientId2");
   auto message = cast_channel::CreateCastMessage(
-      "urn:x-cast:com.google.foo", base::Value(base::Value::Type::DICTIONARY),
-      "sourceId", "*");
+      "urn:x-cast:com.google.foo", base::Value(base::Value::Dict()), "sourceId",
+      "*");
   EXPECT_CALL(*client1,
               SendMessageToClient(IsPresentationConnectionMessage(
                   CreateAppMessage("theSessionId", "theClientId1", message)

chrome/browser/media/router/providers/cast/cast_activity.cc

@@ -106,8 +106,9 @@ void CastActivity::SendMessageToClient(
   it->second->SendMessageToClient(std::move(message));
 }
 
-void CastActivity::SendMediaStatusToClients(const base::Value& media_status,
-                                            absl::optional<int> request_id) {
+void CastActivity::SendMediaStatusToClients(
+    const base::Value::Dict& media_status,
+    absl::optional<int> request_id) {
   for (auto& client : connected_clients_)
     client.second->SendMediaStatusToClient(media_status, request_id);
 }

chrome/browser/media/router/providers/cast/cast_activity.h

@@ -87,7 +87,7 @@ class CastActivity {
       const std::string& client_id,
       blink::mojom::PresentationConnectionMessagePtr message);
 
-  virtual void SendMediaStatusToClients(const base::Value& media_status,
+  virtual void SendMediaStatusToClients(const base::Value::Dict& media_status,
                                         absl::optional<int> request_id);
 
   // Handles a message forwarded by CastActivityManager.

chrome/browser/media/router/providers/cast/cast_activity_manager.cc

@@ -623,9 +623,10 @@ void CastActivityManager::OnSessionRemoved(const MediaSinkInternal& sink) {
   }
 }
 
-void CastActivityManager::OnMediaStatusUpdated(const MediaSinkInternal& sink,
-                                               const base::Value& media_status,
-                                               absl::optional<int> request_id) {
+void CastActivityManager::OnMediaStatusUpdated(
+    const MediaSinkInternal& sink,
+    const base::Value::Dict& media_status,
+    absl::optional<int> request_id) {
   auto it = FindActivityBySink(sink);
   if (it != activities_.end()) {
     it->second->SendMediaStatusToClients(media_status, request_id);

chrome/browser/media/router/providers/cast/cast_activity_manager.h

@@ -117,7 +117,7 @@ class CastActivityManager : public CastActivityManagerBase,
                                const CastSession& session) override;
   void OnSessionRemoved(const MediaSinkInternal& sink) override;
   void OnMediaStatusUpdated(const MediaSinkInternal& sink,
-                            const base::Value& media_status,
+                            const base::Value::Dict& media_status,
                             absl::optional<int> request_id) override;
 
   static void SetActitityFactoryForTest(CastActivityFactoryForTest* factory) {

chrome/browser/media/router/providers/cast/cast_activity_manager_unittest.cc

@@ -44,7 +44,6 @@
 #include "third_party/openscreen/src/cast/common/public/cast_streaming_app_ids.h"
 
 using base::test::IsJson;
-using base::test::ParseJson;
 using base::test::ParseJsonDict;
 using testing::_;
 using testing::AnyNumber;
@@ -846,8 +845,8 @@ TEST_F(CastActivityManagerTest, AppMessageFromReceiver) {
 
   // Destination ID matches client ID.
   cast::channel::CastMessage message = cast_channel::CreateCastMessage(
-      "urn:x-cast:com.google.foo", base::Value(base::Value::Type::DICTIONARY),
-      "sourceId", "theClientId");
+      "urn:x-cast:com.google.foo", base::Value(base::Value::Dict()), "sourceId",
+      "theClientId");
 
   EXPECT_CALL(*app_activity_, OnAppMessage(IsCastChannelMessage(message)));
   manager_->OnAppMessage(kChannelId, message);
@@ -861,7 +860,7 @@ TEST_F(CastActivityManagerTest, OnMediaStatusUpdated) {
 
   EXPECT_CALL(*app_activity_,
               SendMediaStatusToClients(IsJson(status), request_id));
-  manager_->OnMediaStatusUpdated(sink_, ParseJson(status), request_id);
+  manager_->OnMediaStatusUpdated(sink_, ParseJsonDict(status), request_id);
 }
 
 TEST_F(CastActivityManagerTest, SecondPendingRequestCancelsTheFirst) {

chrome/browser/media/router/providers/cast/cast_activity_test_base.h

@@ -38,7 +38,7 @@ class MockCastSessionClient : public CastSessionClient {
   MOCK_METHOD1(SendMessageToClient,
                void(blink::mojom::PresentationConnectionMessagePtr message));
   MOCK_METHOD2(SendMediaStatusToClient,
-               void(const base::Value& media_status,
+               void(const base::Value::Dict& media_status,
                     absl::optional<int> request_id));
   MOCK_METHOD1(
       CloseConnection,
@@ -50,7 +50,8 @@ class MockCastSessionClient : public CastSessionClient {
                void(int sequence_number,
                     CastInternalMessage::ErrorCode error_code,
                     absl::optional<std::string> description));
-  MOCK_METHOD2(SendErrorToClient, void(int sequence_number, base::Value error));
+  MOCK_METHOD2(SendErrorToClient,
+               void(int sequence_number, base::Value::Dict error));
   MOCK_METHOD1(OnMessage,
                void(blink::mojom::PresentationConnectionMessagePtr message));
   MOCK_METHOD1(DidChangeState,

chrome/browser/media/router/providers/cast/cast_internal_message_util.cc

@@ -163,17 +163,25 @@ base::Value::Dict CreateReceiver(const MediaSinkInternal& sink,
 
 blink::mojom::PresentationConnectionMessagePtr CreateMessageCommon(
     CastInternalMessage::Type type,
-    base::Value payload,
+    base::Value::Dict payload,
     const std::string& client_id,
     absl::optional<int> sequence_number = absl::nullopt) {
-  base::Value message(base::Value::Type::DICTIONARY);
-  message.SetKey("type", base::Value(CastInternalMessageTypeToString(type)));
-  message.SetKey("message", std::move(payload));
+  base::Value::Dict message;
+
+  message.Set("type", base::Value(CastInternalMessageTypeToString(type)));
+
+  // When `payload` is empty, we want to set `message` to null instead of {} in
+  // the JSON that is generated.
+  if (payload.empty())
+    message.Set("message", base::Value());
+  else
+    message.Set("message", std::move(payload));
+
   if (sequence_number) {
-    message.SetKey("sequenceNumber", base::Value(*sequence_number));
+    message.Set("sequenceNumber", base::Value(*sequence_number));
   }
-  message.SetKey("timeoutMillis", base::Value(0));
-  message.SetKey("clientId", base::Value(client_id));
+  message.Set("timeoutMillis", base::Value(0));
+  message.Set("clientId", base::Value(client_id));
 
   std::string str;
   CHECK(base::JSONWriter::Write(message, &str));
@@ -190,28 +198,28 @@ blink::mojom::PresentationConnectionMessagePtr CreateReceiverActionMessage(
   message.Set("action", action_type);
 
   return CreateMessageCommon(CastInternalMessage::Type::kReceiverAction,
-                             base::Value(std::move(message)), client_id);
+                             std::move(message), client_id);
 }
 
-base::Value CreateAppMessageBody(
+base::Value::Dict CreateAppMessageBody(
     const std::string& session_id,
     const cast::channel::CastMessage& cast_message) {
   // TODO(https://crbug.com/862532): Investigate whether it is possible to move
   // instead of copying the contents of |cast_message|. Right now copying is
   // done because the message is passed as a const ref at the
   // CastSocket::Observer level.
-  base::Value message(base::Value::Type::DICTIONARY);
-  message.SetKey("sessionId", base::Value(session_id));
-  message.SetKey("namespaceName", base::Value(cast_message.namespace_()));
+  base::Value::Dict message;
+  message.Set("sessionId", base::Value(session_id));
+  message.Set("namespaceName", base::Value(cast_message.namespace_()));
   switch (cast_message.payload_type()) {
     case cast::channel::CastMessage_PayloadType_STRING:
-      message.SetKey("message", base::Value(cast_message.payload_utf8()));
+      message.Set("message", base::Value(cast_message.payload_utf8()));
       break;
     case cast::channel::CastMessage_PayloadType_BINARY: {
       const auto& payload = cast_message.payload_binary();
-      message.SetKey("message",
-                     base::Value(base::Value::BlobStorage(
-                         payload.front(), payload.front() + payload.size())));
+      message.Set("message",
+                  base::Value(base::Value::BlobStorage(
+                      payload.front(), payload.front() + payload.size())));
       break;
     }
     default:
@@ -235,8 +243,8 @@ blink::mojom::PresentationConnectionMessagePtr CreateSessionMessage(
   DCHECK(!session_with_receiver_label.FindByDottedPath("receiver.label"));
   session_with_receiver_label.SetByDottedPath(
       "receiver.label", base::Value(GetReceiverLabel(sink, hash_token)));
-  return CreateMessageCommon(
-      type, base::Value(std::move(session_with_receiver_label)), client_id);
+  return CreateMessageCommon(type, std::move(session_with_receiver_label),
+                             client_id);
 }
 
 }  // namespace
@@ -481,7 +489,7 @@ blink::mojom::PresentationConnectionMessagePtr CreateAppMessageAck(
     const std::string& client_id,
     int sequence_number) {
   return CreateMessageCommon(CastInternalMessage::Type::kAppMessage,
-                             base::Value(), client_id, sequence_number);
+                             base::Value::Dict(), client_id, sequence_number);
 }
 
 blink::mojom::PresentationConnectionMessagePtr CreateAppMessage(
@@ -495,7 +503,7 @@ blink::mojom::PresentationConnectionMessagePtr CreateAppMessage(
 
 blink::mojom::PresentationConnectionMessagePtr CreateV2Message(
     const std::string& client_id,
-    const base::Value& payload,
+    const base::Value::Dict& payload,
     absl::optional<int> sequence_number) {
   return CreateMessageCommon(CastInternalMessage::Type::kV2Message,
                              payload.Clone(), client_id, sequence_number);
@@ -505,12 +513,12 @@ blink::mojom::PresentationConnectionMessagePtr CreateLeaveSessionAckMessage(
     const std::string& client_id,
     absl::optional<int> sequence_number) {
   return CreateMessageCommon(CastInternalMessage::Type::kLeaveSession,
-                             base::Value(), client_id, sequence_number);
+                             base::Value::Dict(), client_id, sequence_number);
 }
 
 blink::mojom::PresentationConnectionMessagePtr CreateErrorMessage(
     const std::string& client_id,
-    base::Value error,
+    base::Value::Dict error,
     absl::optional<int> sequence_number) {
   return CreateMessageCommon(CastInternalMessage::Type::kError,
                              std::move(error), client_id, sequence_number);

chrome/browser/media/router/providers/cast/cast_internal_message_util.h

@@ -230,11 +230,11 @@ blink::mojom::PresentationConnectionMessagePtr CreateAppMessage(
     const CastMessage& cast_message);
 blink::mojom::PresentationConnectionMessagePtr CreateV2Message(
     const std::string& client_id,
-    const base::Value& payload,
+    const base::Value::Dict& payload,
     absl::optional<int> sequence_number);
 blink::mojom::PresentationConnectionMessagePtr CreateErrorMessage(
     const std::string& client_id,
-    base::Value error,
+    base::Value::Dict error,
     absl::optional<int> sequence_number);
 blink::mojom::PresentationConnectionMessagePtr CreateLeaveSessionAckMessage(
     const std::string& client_id,

chrome/browser/media/router/providers/cast/cast_internal_message_util_unittest.cc

@@ -518,8 +518,8 @@ TEST(CastInternalMessageUtilTest, CreateAppMessage) {
 }
 
 TEST(CastInternalMessageUtilTest, CreateV2Message) {
-  base::Value message_body(base::Value::Type::DICTIONARY);
-  message_body.SetKey("foo", base::Value("bar"));
+  base::Value::Dict message_body;
+  message_body.Set("foo", base::Value("bar"));
 
   auto message = CreateV2Message("client_id", message_body, 12345);
   EXPECT_THAT(message, IsPresentationConnectionMessage(R"({

chrome/browser/media/router/providers/cast/cast_media_controller.cc

@@ -166,7 +166,8 @@ void CastMediaController::SetSession(const CastSession& session) {
   observer_->OnMediaStatusUpdated(media_status_.Clone());
 }
 
-void CastMediaController::SetMediaStatus(const base::Value& status_value) {
+void CastMediaController::SetMediaStatus(
+    const base::Value::Dict& status_value) {
   UpdateMediaStatus(status_value);
   observer_->OnMediaStatusUpdated(media_status_.Clone());
 }
@@ -196,66 +197,66 @@ base::Value::Dict CastMediaController::CreateVolumeRequest() {
   return request;
 }
 
-void CastMediaController::UpdateMediaStatus(const base::Value& message_value) {
-  const base::Value* status_list_value = message_value.FindKey("status");
-  if (!status_list_value || !status_list_value->is_list())
+void CastMediaController::UpdateMediaStatus(
+    const base::Value::Dict& message_value) {
+  const base::Value::List* status_list = message_value.FindList("status");
+  if (!status_list)
     return;
-  base::Value::ConstListView status_list =
-      status_list_value->GetListDeprecated();
-  if (status_list.empty())
+  if (status_list->empty())
     return;
-  const base::Value& status_value = status_list[0];
+  const base::Value& status_value = (*status_list)[0];
   if (!status_value.is_dict())
     return;
-  SetIfNonNegative(&media_session_id_, status_value.FindKey("mediaSessionId"));
+  SetIfNonNegative(&media_session_id_,
+                   status_value.GetDict().Find("mediaSessionId"));
   SetIfValid(&media_status_.title,
-             status_value.FindPath("media.metadata.title"));
-  SetIfValid(&media_status_.secondary_title,
-             status_value.FindPath("media.metadata.subtitle"));
+             status_value.GetDict().FindByDottedPath("media.metadata.title"));
+  SetIfValid(
+      &media_status_.secondary_title,
+      status_value.GetDict().FindByDottedPath("media.metadata.subtitle"));
   SetIfNonNegative(&media_status_.current_time,
-                   status_value.FindKey("currentTime"));
+                   status_value.GetDict().Find("currentTime"));
   SetIfNonNegative(&media_status_.duration,
-                   status_value.FindPath("media.duration"));
+                   status_value.GetDict().FindByDottedPath("media.duration"));
 
-  const base::Value* images = status_value.FindPath("media.metadata.images");
-  if (images && images->is_list()) {
+  const base::Value::List* images =
+      status_value.GetDict().FindListByDottedPath("media.metadata.images");
+  if (images) {
     media_status_.images.clear();
-    for (const base::Value& image_value : images->GetListDeprecated()) {
+    for (const base::Value& image_value : *images) {
       if (!image_value.is_dict())
         continue;
-      const std::string* url_string = image_value.FindStringKey("url");
+      const std::string* url_string = image_value.GetDict().FindString("url");
       if (!url_string)
         continue;
       media_status_.images.emplace_back(absl::in_place, GURL(*url_string),
                                         GetValidSize(&image_value));
     }
   }
 
-  const base::Value* commands_value =
-      status_value.FindListKey("supportedMediaCommands");
-  if (commands_value) {
-    const base::ListValue& commands_list =
-        base::Value::AsListValue(*commands_value);
+  const base::Value::List* commands_list =
+      status_value.GetDict().FindList("supportedMediaCommands");
+  if (commands_list) {
     // |can_set_volume| and |can_mute| are not used, because the receiver volume
     // info obtained in SetSession() is used instead.
-    media_status_.can_play_pause = base::Contains(
-        commands_list.GetListDeprecated(), base::Value(kMediaCommandPause));
-    media_status_.can_seek = base::Contains(commands_list.GetListDeprecated(),
-                                            base::Value(kMediaCommandSeek));
-    media_status_.can_skip_to_next_track = base::Contains(
-        commands_list.GetListDeprecated(), base::Value(kMediaCommandQueueNext));
-    media_status_.can_skip_to_previous_track = base::Contains(
-        commands_list.GetListDeprecated(), base::Value(kMediaCommandQueuePrev));
+    media_status_.can_play_pause =
+        base::Contains(*commands_list, base::Value(kMediaCommandPause));
+    media_status_.can_seek =
+        base::Contains(*commands_list, base::Value(kMediaCommandSeek));
+    media_status_.can_skip_to_next_track =
+        base::Contains(*commands_list, base::Value(kMediaCommandQueueNext));
+    media_status_.can_skip_to_previous_track =
+        base::Contains(*commands_list, base::Value(kMediaCommandQueuePrev));
   }
 
-  const base::Value* player_state = status_value.FindKey("playerState");
-  if (player_state && player_state->is_string()) {
-    const std::string& state = player_state->GetString();
-    if (state == "PLAYING") {
+  const std::string* player_state =
+      status_value.GetDict().FindString("playerState");
+  if (player_state) {
+    if (*player_state == "PLAYING") {
       media_status_.play_state = mojom::MediaStatus::PlayState::PLAYING;
-    } else if (state == "PAUSED") {
+    } else if (*player_state == "PAUSED") {
       media_status_.play_state = mojom::MediaStatus::PlayState::PAUSED;
-    } else if (state == "BUFFERING") {
+    } else if (*player_state == "BUFFERING") {
       media_status_.play_state = mojom::MediaStatus::PlayState::BUFFERING;
     }
   }