[CodeHealth] Modernising CastAudioJsonProvider json use

This CL removes the use of JSONReader::ReadDeprecated from
CastAudioJsonProvider and adjacent code, while also narrowing the
base::Value type being passed around.

Bug: 1339314, 1187001
Change-Id: I4142c3232bddea95fa5772c006fefee2faa04264
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4262681
Commit-Queue: Claudio DeSouza <cdesouza@igalia.com>
Reviewed-by: Kenneth MacKay <kmackay@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1106948}

chromecast/media/cma/backend/BUILD.gn

@@ -195,6 +195,7 @@ test("cast_audio_backend_unittests") {
     ":volume_map",
     "//base",
     "//base/test:run_all_unittests",
+    "//base/test:test_support",
     "//chromecast/media/cma/backend/mixer:unittests",
     "//media",
     "//testing/gmock",

chromecast/media/cma/backend/cast_audio_json.cc

@@ -29,10 +29,9 @@ void ReadFileRunCallback(CastAudioJsonProvider::TuningChangedCallback callback,
 
   std::string contents;
   base::ReadFileToString(path, &contents);
-  std::unique_ptr<base::Value> value =
-      base::JSONReader::ReadDeprecated(contents);
-  if (value) {
-    callback.Run(std::move(value));
+  absl::optional<base::Value> value = base::JSONReader::Read(contents);
+  if (value && value->is_dict()) {
+    callback.Run(std::move(*value).TakeDict());
     return;
   }
   LOG(ERROR) << "Unable to parse JSON in " << path;
@@ -77,10 +76,16 @@ CastAudioJsonProviderImpl::CastAudioJsonProviderImpl() {
 
 CastAudioJsonProviderImpl::~CastAudioJsonProviderImpl() = default;
 
-std::unique_ptr<base::Value> CastAudioJsonProviderImpl::GetCastAudioConfig() {
+absl::optional<base::Value::Dict>
+CastAudioJsonProviderImpl::GetCastAudioConfig() {
   std::string contents;
   base::ReadFileToString(CastAudioJson::GetFilePath(), &contents);
-  return base::JSONReader::ReadDeprecated(contents);
+  absl::optional<base::Value> value = base::JSONReader::Read(contents);
+  if (!value || value->is_dict()) {
+    return absl::nullopt;
+  }
+
+  return std::move(*value).TakeDict();
 }
 
 void CastAudioJsonProviderImpl::SetTuningChangedCallback(

chromecast/media/cma/backend/cast_audio_json.h

@@ -5,14 +5,13 @@
 #ifndef CHROMECAST_MEDIA_CMA_BACKEND_CAST_AUDIO_JSON_H_
 #define CHROMECAST_MEDIA_CMA_BACKEND_CAST_AUDIO_JSON_H_
 
-#include <memory>
-
 #include "base/files/file_path.h"
 #include "base/files/file_path_watcher.h"
 #include "base/functional/callback.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/threading/sequence_bound.h"
 #include "base/values.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace chromecast {
 namespace media {
@@ -31,7 +30,7 @@ class CastAudioJson {
 class CastAudioJsonProvider {
  public:
   using TuningChangedCallback =
-      base::RepeatingCallback<void(std::unique_ptr<base::Value> contents)>;
+      base::RepeatingCallback<void(absl::optional<base::Value::Dict> contents)>;
 
   virtual ~CastAudioJsonProvider() = default;
 
@@ -41,7 +40,7 @@ class CastAudioJsonProvider {
   // at CastAudioJson::GetReadOnlyFilePath() will be returned.
   // This function will run on the thread on which it is called, and may
   // perform blocking I/O.
-  virtual std::unique_ptr<base::Value> GetCastAudioConfig() = 0;
+  virtual absl::optional<base::Value::Dict> GetCastAudioConfig() = 0;
 
   // |callback| will be called when a new cast_audio config is available.
   // |callback| will always be called from the same thread, but not necessarily
@@ -72,7 +71,7 @@ class CastAudioJsonProviderImpl : public CastAudioJsonProvider {
   };
 
   // CastAudioJsonProvider implementation:
-  std::unique_ptr<base::Value> GetCastAudioConfig() override;
+  absl::optional<base::Value::Dict> GetCastAudioConfig() override;
   void SetTuningChangedCallback(TuningChangedCallback callback) override;
 
   base::SequenceBound<FileWatcher> cast_audio_watcher_;

chromecast/media/cma/backend/cplay/cplay.cc

@@ -340,7 +340,12 @@ int CplayMain(int argc, char* argv[]) {
   // Set volume.
   std::string contents;
   base::ReadFileToString(params.cast_audio_json_path, &contents);
-  GetVolumeMap().LoadVolumeMap(base::JSONReader::ReadDeprecated(contents));
+  absl::optional<base::Value> parsed_json = base::JSONReader::Read(contents);
+  if (parsed_json && parsed_json->is_dict()) {
+    GetVolumeMap().LoadVolumeMap(std::move(*parsed_json).TakeDict());
+  } else {
+    GetVolumeMap().LoadVolumeMap(absl::nullopt);
+  }
   float volume_dbfs = GetVolumeMap().VolumeToDbFS(params.cast_volume);
   float volume_multiplier = std::pow(10.0, volume_dbfs / 20.0);
   mixer_input.SetVolumeMultiplier(1.0);

chromecast/media/cma/backend/volume_map.cc

@@ -48,15 +48,16 @@ void VolumeMap::LoadFromFile() {
   LoadVolumeMap(config_provider_->GetCastAudioConfig());
 }
 
-void VolumeMap::LoadVolumeMap(std::unique_ptr<base::Value> cast_audio_config) {
-  if (!cast_audio_config || !cast_audio_config->is_dict()) {
+void VolumeMap::LoadVolumeMap(
+    absl::optional<base::Value::Dict> cast_audio_config) {
+  if (!cast_audio_config) {
     LOG(WARNING) << "No cast audio config found; using default volume map.";
     UseDefaultVolumeMap();
     return;
   }
 
   const base::Value::List* volume_map_list =
-      cast_audio_config->GetDict().FindList(kKeyVolumeMap);
+      cast_audio_config->FindList(kKeyVolumeMap);
   if (!volume_map_list) {
     LOG(WARNING) << "No volume map found; using default volume map.";
     UseDefaultVolumeMap();

chromecast/media/cma/backend/volume_map.h

@@ -36,7 +36,7 @@ class VolumeMap {
 
   float DbFSToVolume(float db);
 
-  void LoadVolumeMap(std::unique_ptr<base::Value> cast_audio_config);
+  void LoadVolumeMap(absl::optional<base::Value::Dict> cast_audio_config);
 
  private:
   struct LevelToDb {

chromecast/media/cma/backend/volume_map_unittest.cc

@@ -7,7 +7,7 @@
 #include <string>
 
 #include "base/check.h"
-#include "base/json/json_reader.h"
+#include "base/test/values_test_util.h"
 #include "chromecast/media/cma/backend/cast_audio_json.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -35,12 +35,12 @@ class TestFileProvider : public CastAudioJsonProvider {
 
   void CallTuningChangedCallback(const std::string& new_config) {
     DCHECK(callback_);
-    callback_.Run(base::JSONReader::ReadDeprecated(new_config));
+    callback_.Run(base::test::ParseJsonDict(new_config));
   }
 
  private:
-  std::unique_ptr<base::Value> GetCastAudioConfig() override {
-    return base::JSONReader::ReadDeprecated(file_contents_);
+  absl::optional<base::Value::Dict> GetCastAudioConfig() override {
+    return base::test::ParseJsonDict(file_contents_);
   }
 
   void SetTuningChangedCallback(TuningChangedCallback callback) override {
@@ -52,7 +52,7 @@ class TestFileProvider : public CastAudioJsonProvider {
 };
 
 TEST(VolumeMapTest, UsesDefaultMapIfConfigEmpty) {
-  VolumeMap volume_map(std::make_unique<TestFileProvider>(""));
+  VolumeMap volume_map(std::make_unique<TestFileProvider>("{}"));
   EXPECT_NEAR(-58.0f, volume_map.VolumeToDbFS(0.01f), kEpsilon);
   EXPECT_NEAR(-48.0f, volume_map.VolumeToDbFS(1.0 / 11.0), kEpsilon);
   EXPECT_NEAR(-8.0f, volume_map.VolumeToDbFS(9.0 / 11.0), kEpsilon);
@@ -82,7 +82,7 @@ TEST(VolumeMapTest, DbFSToVolumeInterpolates) {
 }
 
 TEST(VolumeMapTest, LoadsNewMapWhenFileChanges) {
-  auto provider = std::make_unique<TestFileProvider>("");
+  auto provider = std::make_unique<TestFileProvider>("{}");
   TestFileProvider* provider_ptr = provider.get();
   VolumeMap volume_map(std::move(provider));