[code-health] Remove DeprecatedDictionaryValue in MediaLogRecord

Bug: 1187061
Change-Id: I19b8d90c3d69b9a6152d246565b9ac4a110f9480
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3896116
Reviewed-by: Will Cassella <cassew@chromium.org>
Commit-Queue: Fabrice de Gans <fdegans@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1051412}

content/browser/media/media_internals.cc

@@ -384,7 +384,7 @@ static bool ConvertEventToUpdate(int render_process_id,
   const double ticks_millis = ticks / base::Time::kMicrosecondsPerMillisecond;
   dict.Set("ticksMillis", ticks_millis);
 
-  base::Value::Dict cloned_params = event.params.GetDict().Clone();
+  base::Value::Dict cloned_params = event.params.Clone();
   switch (event.type) {
     case media::MediaLogRecord::Type::kMessage:
       dict.Set("type", "MEDIA_LOG_ENTRY");

content/renderer/media/batching_media_log.cc

@@ -31,7 +31,7 @@ void Log(const media::MediaLogRecord& event) {
   if (event.type == media::MediaLogRecord::Type::kMediaStatus) {
     DVLOG(1) << "MediaEvent: " << ToJSON(event);
   } else if (event.type == media::MediaLogRecord::Type::kMessage &&
-             event.params.FindKey("error")) {
+             event.params.Find("error")) {
     DVLOG(1) << "MediaEvent: " << ToJSON(event);
   } else if (event.type != media::MediaLogRecord::Type::kMediaPropertyChange) {
     DVLOG(1) << "MediaEvent: " << ToJSON(event);
@@ -105,9 +105,8 @@ void BatchingMediaLog::AddLogRecordLocked(
         break;
 
       case media::MediaLogRecord::Type::kMediaEventTriggered: {
-        DCHECK(event->params.FindKey(MediaLog::kEventKey));
-        const auto* event_key =
-            event->params.FindStringKey(MediaLog::kEventKey);
+        const base::Value* event_key = event->params.Find(MediaLog::kEventKey);
+        DCHECK(event_key);
         if (*event_key == kDurationChangedMessage) {
           // This may fire many times for badly muxed media; only keep the last.
           last_duration_changed_event_ = *event;
@@ -121,7 +120,7 @@ void BatchingMediaLog::AddLogRecordLocked(
       }
 
       case media::MediaLogRecord::Type::kMessage:
-        if (event->params.FindKey(media::MediaLogMessageLevelToString(
+        if (event->params.Find(media::MediaLogMessageLevelToString(
                 media::MediaLogMessageLevel::kERROR)) &&
             !cached_media_error_for_message_) {
           cached_media_error_for_message_ = *event;
@@ -183,9 +182,9 @@ std::string BatchingMediaLog::MediaEventToMessageString(
   switch (event.type) {
     case media::MediaLogRecord::Type::kMediaStatus: {
       const std::string* group =
-          event.params.FindStringKey(media::StatusConstants::kGroupKey);
+          event.params.FindString(media::StatusConstants::kGroupKey);
       auto code =
-          event.params.FindIntKey(media::StatusConstants::kCodeKey).value_or(0);
+          event.params.FindInt(media::StatusConstants::kCodeKey).value_or(0);
       DCHECK_NE(code, 0);
       if (group && *group == media::PipelineStatus::Traits::Group()) {
         return PipelineStatusToString(
@@ -196,13 +195,14 @@ std::string BatchingMediaLog::MediaEventToMessageString(
       return formatted.str();
     }
     case media::MediaLogRecord::Type::kMessage: {
-      std::string result;
-      if (event.params.GetString(
-              MediaLogMessageLevelToString(media::MediaLogMessageLevel::kERROR),
-              &result)) {
-        base::ReplaceChars(result, "\n", " ", &result);
+      const std::string* result = event.params.FindString(
+          MediaLogMessageLevelToString(media::MediaLogMessageLevel::kERROR));
+      if (result) {
+        std::string result_copy;
+        base::ReplaceChars(*result, "\n", " ", &result_copy);
+        return result_copy;
       }
-      return result;
+      return "";
     }
     default:
       NOTREACHED();
@@ -267,7 +267,7 @@ void BatchingMediaLog::MaybeQueueEvent_Locked(
   queued_media_events_.back().id = event->id;
   queued_media_events_.back().type = media::MediaLogRecord::Type::kMessage;
   queued_media_events_.back().time = base::TimeTicks::Now();
-  queued_media_events_.back().params.SetStringPath(
+  queued_media_events_.back().params.Set(
       media::MediaLogMessageLevelToString(
           media::MediaLogMessageLevel::kWARNING),
       message);

content/renderer/media/inspector_media_event_handler.cc

@@ -78,6 +78,12 @@ blink::WebString ToString(const base::Value& value) {
   return blink::WebString::FromUTF8(output_str);
 }
 
+blink::WebString ToString(const base::Value::Dict& value) {
+  std::string output_str;
+  base::JSONWriter::Write(value, &output_str);
+  return blink::WebString::FromUTF8(output_str);
+}
+
 // TODO(tmathmeyer) stop using a string here eventually. This means rewriting
 // the MediaLogRecord mojom interface.
 blink::InspectorPlayerMessage::Level LevelFromString(const std::string& level) {
@@ -117,7 +123,7 @@ void InspectorMediaEventHandler::SendQueuedMediaEvents(
   for (media::MediaLogRecord event : events_to_send) {
     switch (event.type) {
       case media::MediaLogRecord::Type::kMessage: {
-        for (auto&& itr : event.params.DictItems()) {
+        for (auto&& itr : event.params) {
           blink::InspectorPlayerMessage msg = {
               LevelFromString(itr.first),
               blink::WebString::FromUTF8(itr.second.GetString())};
@@ -126,7 +132,7 @@ void InspectorMediaEventHandler::SendQueuedMediaEvents(
         break;
       }
       case media::MediaLogRecord::Type::kMediaPropertyChange: {
-        for (auto&& itr : event.params.DictItems()) {
+        for (auto&& itr : event.params) {
           blink::InspectorPlayerProperty prop = {
               blink::WebString::FromUTF8(itr.first), ToString(itr.second)};
           properties.emplace_back(std::move(prop));
@@ -140,7 +146,7 @@ void InspectorMediaEventHandler::SendQueuedMediaEvents(
       }
       case media::MediaLogRecord::Type::kMediaStatus: {
         absl::optional<blink::InspectorPlayerError> error =
-            ErrorFromParams(event.params.GetDict());
+            ErrorFromParams(event.params);
         if (error.has_value())
           errors.emplace_back(std::move(*error));
       }

content/renderer/media/inspector_media_event_handler_unittest.cc

@@ -80,8 +80,7 @@ class InspectorMediaEventHandlerTest : public testing::Test {
     event.id = 0;
     event.type = media::MediaLogRecord::Type::kMediaEventTriggered;
     event.time = base::TimeTicks();
-    event.params.SetString("event",
-                           media::MediaLogEventTypeSupport<T>::TypeName());
+    event.params.Set("event", media::MediaLogEventTypeSupport<T>::TypeName());
     return event;
   }
 
@@ -92,7 +91,7 @@ class InspectorMediaEventHandlerTest : public testing::Test {
     event.type = media::MediaLogRecord::Type::kMediaPropertyChange;
     event.time = base::TimeTicks();
     for (auto p : props) {
-      event.params.SetString(std::get<0>(p), std::get<1>(p));
+      event.params.Set(std::get<0>(p), std::get<1>(p));
     }
     return event;
   }
@@ -102,7 +101,7 @@ class InspectorMediaEventHandlerTest : public testing::Test {
     event.id = 0;
     event.type = media::MediaLogRecord::Type::kMessage;
     event.time = base::TimeTicks();
-    event.params.SetString("warning", msg);
+    event.params.Set("warning", msg);
     return event;
   }
 
@@ -111,9 +110,9 @@ class InspectorMediaEventHandlerTest : public testing::Test {
     error.id = 0;
     error.type = media::MediaLogRecord::Type::kMediaStatus;
     error.time = base::TimeTicks();
-    error.params.SetIntPath(media::StatusConstants::kCodeKey, errorcode);
-    error.params.SetStringPath(media::StatusConstants::kGroupKey,
-                               media::PipelineStatus::Traits::Group());
+    error.params.Set(media::StatusConstants::kCodeKey, errorcode);
+    error.params.Set(media::StatusConstants::kGroupKey,
+                     media::PipelineStatus::Traits::Group());
     return error;
   }
 };

media/base/media_log.cc

@@ -52,8 +52,7 @@ void MediaLog::AddMessage(MediaLogMessageLevel level, std::string message) {
       CreateRecord(MediaLogRecord::Type::kMessage));
   if (!base::IsStringUTF8AllowingNoncharacters(message))
     message = "WARNING: system message could not be rendered!";
-  record->params.SetStringPath(MediaLogMessageLevelToString(level),
-                               std::move(message));
+  record->params.Set(MediaLogMessageLevelToString(level), std::move(message));
   AddLogRecord(std::move(record));
 }
 

media/base/media_log.h

@@ -97,8 +97,9 @@ class MEDIA_EXPORT MediaLog {
     DCHECK(!status.is_ok());
     std::unique_ptr<MediaLogRecord> record =
         CreateRecord(MediaLogRecord::Type::kMediaStatus);
-    auto serialized = MediaSerialize(status);
-    record->params.MergeDictionary(&serialized);
+    base::Value serialized = MediaSerialize(status);
+    DCHECK(serialized.is_dict());
+    record->params.Merge(std::move(serialized.GetDict()));
     AddLogRecord(std::move(record));
   }
 
@@ -147,16 +148,16 @@ class MEDIA_EXPORT MediaLog {
   template <MediaLogProperty P, typename T>
   std::unique_ptr<MediaLogRecord> CreatePropertyRecord(const T& value) {
     auto record = CreateRecord(MediaLogRecord::Type::kMediaPropertyChange);
-    record->params.SetKey(MediaLogPropertyKeyToString(P),
-                          MediaLogPropertyTypeSupport<P, T>::Convert(value));
+    record->params.Set(MediaLogPropertyKeyToString(P),
+                       MediaLogPropertyTypeSupport<P, T>::Convert(value));
     return record;
   }
   template <MediaLogEvent E, typename... Opt>
   std::unique_ptr<MediaLogRecord> CreateEventRecord() {
     std::unique_ptr<MediaLogRecord> record(
         CreateRecord(MediaLogRecord::Type::kMediaEventTriggered));
-    record->params.SetString(MediaLog::kEventKey,
-                             MediaLogEventTypeSupport<E, Opt...>::TypeName());
+    record->params.Set(MediaLog::kEventKey,
+                       MediaLogEventTypeSupport<E, Opt...>::TypeName());
     return record;
   }
 

media/base/media_log_record.h

@@ -6,7 +6,6 @@
 #define MEDIA_BASE_MEDIA_LOG_RECORD_H_
 
 #include <stdint.h>
-#include <memory>
 
 #include "base/time/time.h"
 #include "base/values.h"
@@ -23,9 +22,7 @@ struct MediaLogRecord {
   MediaLogRecord& operator=(const MediaLogRecord& event) {
     id = event.id;
     type = event.type;
-    params.Swap(base::DictionaryValue::From(
-                    base::Value::ToUniquePtrValue(event.params.Clone()))
-                    .get());
+    params = event.params.Clone();
     time = event.time;
     return *this;
   }
@@ -56,7 +53,7 @@ struct MediaLogRecord {
 
   int32_t id;
   Type type;
-  base::DictionaryValue params;
+  base::Value::Dict params;
   base::TimeTicks time;
 };
 

media/base/media_log_type_enforcement.h

@@ -36,32 +36,32 @@ struct MediaLogEventTypeSupport {};
     }                                                                    \
   }
 
-#define MEDIA_LOG_EVENT_NAMED_DATA(EVENT, TYPE, DISPLAY)           \
-  template <>                                                      \
-  struct MediaLogEventTypeSupport<MediaLogEvent::EVENT, TYPE> {    \
-    static void AddExtraData(base::Value* params, const TYPE& t) { \
-      DCHECK(params);                                              \
-      params->SetKey(DISPLAY, MediaSerialize<TYPE>(t));            \
-    }                                                              \
-    static std::string TypeName() { return #EVENT; }               \
+#define MEDIA_LOG_EVENT_NAMED_DATA(EVENT, TYPE, DISPLAY)                 \
+  template <>                                                            \
+  struct MediaLogEventTypeSupport<MediaLogEvent::EVENT, TYPE> {          \
+    static void AddExtraData(base::Value::Dict* params, const TYPE& t) { \
+      DCHECK(params);                                                    \
+      params->Set(DISPLAY, MediaSerialize<TYPE>(t));                     \
+    }                                                                    \
+    static std::string TypeName() { return #EVENT; }                     \
   }
 
-#define MEDIA_LOG_EVENT_NAMED_DATA_OP(EVENT, TYPE, DISPLAY, OP)    \
-  template <>                                                      \
-  struct MediaLogEventTypeSupport<MediaLogEvent::EVENT, TYPE> {    \
-    static void AddExtraData(base::Value* params, const TYPE& t) { \
-      DCHECK(params);                                              \
-      params->SetKey(DISPLAY, MediaSerialize<TYPE>(OP(t)));        \
-    }                                                              \
-    static std::string TypeName() { return #EVENT; }               \
+#define MEDIA_LOG_EVENT_NAMED_DATA_OP(EVENT, TYPE, DISPLAY, OP)          \
+  template <>                                                            \
+  struct MediaLogEventTypeSupport<MediaLogEvent::EVENT, TYPE> {          \
+    static void AddExtraData(base::Value::Dict* params, const TYPE& t) { \
+      DCHECK(params);                                                    \
+      params->Set(DISPLAY, MediaSerialize<TYPE>(OP(t)));                 \
+    }                                                                    \
+    static std::string TypeName() { return #EVENT; }                     \
   }
 
 // Specifically do not create the Convert or DisplayName methods
-#define MEDIA_LOG_EVENT_TYPELESS(EVENT)                   \
-  template <>                                             \
-  struct MediaLogEventTypeSupport<MediaLogEvent::EVENT> { \
-    static std::string TypeName() { return #EVENT; }      \
-    static void AddExtraData(base::Value* params) {}      \
+#define MEDIA_LOG_EVENT_TYPELESS(EVENT)                    \
+  template <>                                              \
+  struct MediaLogEventTypeSupport<MediaLogEvent::EVENT> {  \
+    static std::string TypeName() { return #EVENT; }       \
+    static void AddExtraData(base::Value::Dict* params) {} \
   }
 
 }  // namespace media

media/base/media_log_unittest.cc

@@ -58,13 +58,13 @@ TEST_F(MediaLogTest, DontTruncateShortUrlString) {
   root_log_->AddEvent<MediaLogEvent::kLoad>(short_url);
   auto event = root_log_->take_most_recent_event();
   EXPECT_NE(event, nullptr);
-  EXPECT_EQ(*event->params.FindStringPath("url"), "chromium.org");
+  EXPECT_EQ(*event->params.FindString("url"), "chromium.org");
 
   // Verify that CreatedEvent does not truncate the short URL.
   root_log_->AddEvent<MediaLogEvent::kWebMediaPlayerCreated>(short_url);
   event = root_log_->take_most_recent_event();
   EXPECT_NE(event, nullptr);
-  EXPECT_EQ(*event->params.FindStringPath("origin_url"), "chromium.org");
+  EXPECT_EQ(*event->params.FindString("origin_url"), "chromium.org");
 }
 
 TEST_F(MediaLogTest, TruncateLongUrlStrings) {
@@ -90,13 +90,13 @@ TEST_F(MediaLogTest, TruncateLongUrlStrings) {
   root_log_->AddEvent<MediaLogEvent::kLoad>(long_url);
   auto event = root_log_->take_most_recent_event();
   EXPECT_NE(event, nullptr);
-  EXPECT_EQ(*event->params.FindStringPath("url"), expected_url);
+  EXPECT_EQ(*event->params.FindString("url"), expected_url);
 
   // Verify that CreatedEvent does not truncate the short URL.
   root_log_->AddEvent<MediaLogEvent::kWebMediaPlayerCreated>(long_url);
   event = root_log_->take_most_recent_event();
   EXPECT_NE(event, nullptr);
-  EXPECT_EQ(*event->params.FindStringPath("origin_url"), expected_url);
+  EXPECT_EQ(*event->params.FindString("origin_url"), expected_url);
 }
 
 }  // namespace media

media/base/mock_media_log.cc

@@ -23,9 +23,9 @@ std::string MockMediaLog::MediaEventToLogString(const MediaLogRecord& event) {
   // event for figuring out media pipeline failures, and just reporting
   // pipeline status as numeric code is not very helpful/user-friendly.
   if (event.type == MediaLogRecord::Type::kMediaStatus) {
-    const std::string* group = event.params.FindStringKey("group");
+    const std::string* group = event.params.FindString("group");
     if (group && *group == "PipelineStatus") {
-      auto code = event.params.FindIntKey("code").value_or(0);
+      auto code = event.params.FindInt("code").value_or(0);
       return PipelineStatusToString(static_cast<PipelineStatusCodes>(code));
     }
   }

media/mojo/mojom/stable/stable_video_decoder_types.mojom

@@ -525,7 +525,7 @@ struct MediaLogRecord {
 
   int32 id@0;
   Type type@1;
-  mojo_base.mojom.DeprecatedDictionaryValue params@2;
+  mojo_base.mojom.DictionaryValue params@2;
   mojo_base.mojom.TimeTicks time@3;
 };
 

media/mojo/mojom/stable/stable_video_decoder_types_mojom_traits.cc

@@ -686,11 +686,11 @@ StructTraits<media::stable::mojom::MediaLogRecordDataView,
 }
 
 // static
-const base::Value& StructTraits<
+const base::Value::Dict& StructTraits<
     media::stable::mojom::MediaLogRecordDataView,
     media::MediaLogRecord>::params(const media::MediaLogRecord& input) {
   static_assert(std::is_same<decltype(::media::MediaLogRecord::params),
-                             base::DictionaryValue>::value,
+                             base::Value::Dict>::value,
                 "Unexpected type for media::MediaLogRecord::params. If you "
                 "need to change this assertion, please contact "
                 "chromeos-gfx-video@google.com.");
@@ -722,9 +722,7 @@ bool StructTraits<media::stable::mojom::MediaLogRecordDataView,
   if (!data.ReadType(&output->type))
     return false;
 
-  if (!data.ReadParams(static_cast<base::Value*>(&output->params)))
-    return false;
-  if (!output->params.is_dict())
+  if (!data.ReadParams(&output->params))
     return false;
 
   if (!data.ReadTime(&output->time))

media/mojo/mojom/stable/stable_video_decoder_types_mojom_traits.h

@@ -681,7 +681,7 @@ struct StructTraits<media::stable::mojom::MediaLogRecordDataView,
 
   static media::MediaLogRecord::Type type(const media::MediaLogRecord& input);
 
-  static const base::Value& params(const media::MediaLogRecord& input);
+  static const base::Value::Dict& params(const media::MediaLogRecord& input);
 
   static base::TimeTicks time(const media::MediaLogRecord& input);
 

media/mojo/services/stable_video_decoder_service_unittest.cc

@@ -767,7 +767,7 @@ TEST_F(StableVideoDecoderServiceTest,
   MediaLogRecord media_log_record_to_send;
   media_log_record_to_send.id = 2;
   media_log_record_to_send.type = MediaLogRecord::Type::kMediaStatus;
-  media_log_record_to_send.params.SetStringKey("Test", "Value");
+  media_log_record_to_send.params.Set("Test", "Value");
   media_log_record_to_send.time = base::TimeTicks::Now();
 
   EXPECT_CALL(*auxiliary_endpoints->mock_stable_media_log,