Remove base::ListValue usage in some WebRTC code.

Update WebRTCInternals::OnAddStandardStats(),
WebRTCInternals::OnAddLegacyStats(), their callers, and some related
code to new base::Value APIs. Fix nits and lint errors along the way.

Bug: 646113
Change-Id: Ib7fff9a87dbcb9203c182269ff739b1d70270640
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894111Reviewed-by: Marina Ciocea <marinaciocea@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711751}

content/browser/renderer_host/media/peer_connection_tracker_host.cc

@@ -4,6 +4,8 @@
 
 #include "content/browser/renderer_host/media/peer_connection_tracker_host.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/power_monitor/power_monitor.h"
 #include "base/task/post_task.h"
@@ -20,7 +22,7 @@ PeerConnectionTrackerHost::PeerConnectionTrackerHost(RenderProcessHost* rph)
   base::PowerMonitor::AddObserver(this);
 
   mojo::PendingRemote<blink::mojom::PeerConnectionManager> pending_tracker;
-  content::BindInterface(rph, &pending_tracker);
+  BindInterface(rph, &pending_tracker);
   tracker_.Bind(std::move(pending_tracker));
 }
 
@@ -40,7 +42,7 @@ void PeerConnectionTrackerHost::AddPeerConnection(
         info->rtc_configuration, info->constraints);
   }
 
-  WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
+  WebRtcEventLogger* logger = WebRtcEventLogger::Get();
   if (logger) {
     logger->PeerConnectionAdded(render_process_id_, info->lid,
                                 base::OnceCallback<void(bool)>());
@@ -54,7 +56,7 @@ void PeerConnectionTrackerHost::RemovePeerConnection(int lid) {
   if (webrtc_internals) {
     webrtc_internals->OnRemovePeerConnection(peer_pid_, lid);
   }
-  WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
+  WebRtcEventLogger* logger = WebRtcEventLogger::Get();
   if (logger) {
     logger->PeerConnectionRemoved(render_process_id_, lid,
                                   base::OnceCallback<void(bool)>());
@@ -68,7 +70,7 @@ void PeerConnectionTrackerHost::UpdatePeerConnection(int lid,
 
   // TODO(eladalon): Get rid of magic value. https://crbug.com/810383
   if (type == "stop") {
-    WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
+    WebRtcEventLogger* logger = WebRtcEventLogger::Get();
     if (logger) {
       logger->PeerConnectionStopped(render_process_id_, lid,
                                     base::OnceCallback<void(bool)>());
@@ -86,38 +88,27 @@ void PeerConnectionTrackerHost::OnPeerConnectionSessionIdSet(
     const std::string& session_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
-  if (!logger) {
-    return;
+  WebRtcEventLogger* logger = WebRtcEventLogger::Get();
+  if (logger) {
+    logger->PeerConnectionSessionIdSet(render_process_id_, lid, session_id,
+                                       base::OnceCallback<void(bool)>());
   }
-  logger->PeerConnectionSessionIdSet(render_process_id_, lid, session_id,
-                                     base::OnceCallback<void(bool)>());
 }
 
 void PeerConnectionTrackerHost::AddStandardStats(int lid, base::Value value) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   WebRTCInternals* webrtc_internals = WebRTCInternals::GetInstance();
-  if (webrtc_internals) {
-    // Churn to convert from base::Value to std::unique_ptr<base::ListValue>.
-    auto value_ptr = base::Value::ToUniquePtrValue(std::move(value));
-    auto list_value_ptr = base::ListValue::From(std::move(value_ptr));
-    webrtc_internals->OnAddStandardStats(peer_pid_, lid,
-                                         std::move(*list_value_ptr.get()));
-  }
+  if (webrtc_internals)
+    webrtc_internals->OnAddStandardStats(peer_pid_, lid, std::move(value));
 }
 
 void PeerConnectionTrackerHost::AddLegacyStats(int lid, base::Value value) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   WebRTCInternals* webrtc_internals = WebRTCInternals::GetInstance();
-  if (webrtc_internals) {
-    // Churn to convert from base::Value to std::unique_ptr<base::ListValue>.
-    auto value_ptr = base::Value::ToUniquePtrValue(std::move(value));
-    auto list_value_ptr = base::ListValue::From(std::move(value_ptr));
-    webrtc_internals->OnAddLegacyStats(peer_pid_, lid,
-                                       std::move(*list_value_ptr.get()));
-  }
+  if (webrtc_internals)
+    webrtc_internals->OnAddLegacyStats(peer_pid_, lid, std::move(value));
 }
 
 void PeerConnectionTrackerHost::GetUserMedia(
@@ -140,7 +131,7 @@ void PeerConnectionTrackerHost::WebRtcEventLogWrite(int lid,
                                                     const std::string& output) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
+  WebRtcEventLogger* logger = WebRtcEventLogger::Get();
   if (logger) {
     logger->OnWebRtcEventLogWrite(
         render_process_id_, lid, output,

content/browser/renderer_host/media/peer_connection_tracker_host.h

@@ -5,7 +5,7 @@
 #ifndef CONTENT_BROWSER_RENDERER_HOST_MEDIA_PEER_CONNECTION_TRACKER_HOST_H_
 #define CONTENT_BROWSER_RENDERER_HOST_MEDIA_PEER_CONNECTION_TRACKER_HOST_H_
 
-#include <stdint.h>
+#include <string>
 
 #include "base/macros.h"
 #include "base/power_monitor/power_observer.h"

content/browser/webrtc/webrtc_internals.cc

@@ -253,7 +253,7 @@ void WebRTCInternals::OnUpdatePeerConnection(
 
 void WebRTCInternals::OnAddStandardStats(base::ProcessId pid,
                                          int lid,
-                                         const base::ListValue& value) {
+                                         base::Value value) {
   if (!observers_.might_have_observers())
     return;
 
@@ -261,14 +261,14 @@ void WebRTCInternals::OnAddStandardStats(base::ProcessId pid,
   dict->SetInteger("pid", static_cast<int>(pid));
   dict->SetInteger("lid", lid);
 
-  dict->SetKey("reports", value.Clone());
+  dict->SetKey("reports", std::move(value));
 
   SendUpdate("addStandardStats", std::move(dict));
 }
 
 void WebRTCInternals::OnAddLegacyStats(base::ProcessId pid,
                                        int lid,
-                                       const base::ListValue& value) {
+                                       base::Value value) {
   if (!observers_.might_have_observers())
     return;
 
@@ -276,7 +276,7 @@ void WebRTCInternals::OnAddLegacyStats(base::ProcessId pid,
   dict->SetInteger("pid", static_cast<int>(pid));
   dict->SetInteger("lid", lid);
 
-  dict->SetKey("reports", value.Clone());
+  dict->SetKey("reports", std::move(value));
 
   SendUpdate("addLegacyStats", std::move(dict));
 }

content/browser/webrtc/webrtc_internals.h

@@ -86,12 +86,8 @@ class CONTENT_EXPORT WebRTCInternals : public RenderProcessHostObserver,
   // PeerConnectionInterface::GetStats (legacy or standard API) are available.
   // |pid| is the renderer process id, |lid| is the renderer local id, |value|
   // is the list of stats reports.
-  void OnAddStandardStats(base::ProcessId pid,
-                          int lid,
-                          const base::ListValue& value);
-  void OnAddLegacyStats(base::ProcessId pid,
-                        int lid,
-                        const base::ListValue& value);
+  void OnAddStandardStats(base::ProcessId pid, int lid, base::Value value);
+  void OnAddLegacyStats(base::ProcessId pid, int lid, base::Value value);
 
   // This method is called when getUserMedia is called. |render_process_id| is
   // the id of the render process (not OS pid), which is needed because we might

content/browser/webrtc/webrtc_internals_unittest.cc

@@ -24,13 +24,13 @@ namespace content {
 
 namespace {
 
-static const char kContraints[] = "c";
-static const char kRtcConfiguration[] = "r";
-static const char kUrl[] = "u";
-static const char* kWakeLockConnectingValues[] = {"checking", "connected",
-                                                  "completed"};
-static const char* kWakeLockDisconnectingValues[] = {"disconnected", "closed",
-                                                     "failed", "new"};
+const char kContraints[] = "c";
+const char kRtcConfiguration[] = "r";
+const char kUrl[] = "u";
+const char* const kWakeLockConnectingValues[] = {"checking", "connected",
+                                                 "completed"};
+const char* const kWakeLockDisconnectingValues[] = {"disconnected", "closed",
+                                                    "failed", "new"};
 
 class MockWebRtcInternalsProxy : public WebRTCInternalsUIObserver {
  public:
@@ -136,11 +136,13 @@ class WebRtcInternalsTest : public testing::Test {
     EXPECT_EQ(expected, actual);
   }
 
-  void VerifyList(const base::DictionaryValue* dict,
-                  const std::string& key,
-                  const base::ListValue& expected) {
-    const base::ListValue* actual = nullptr;
-    EXPECT_TRUE(dict->GetList(key, &actual));
+  void VerifyList(const base::Value& dict,
+                  base::StringPiece key,
+                  const base::Value& expected) {
+    ASSERT_TRUE(dict.is_dict());
+    ASSERT_TRUE(expected.is_list());
+    const base::Value* actual = dict.FindListKey(key);
+    ASSERT_TRUE(actual);
     EXPECT_TRUE(expected.Equals(actual));
   }
 
@@ -431,7 +433,9 @@ TEST_F(WebRtcInternalsTest, SendAllUpdatesWithPeerConnectionUpdate) {
 }
 
 TEST_F(WebRtcInternalsTest, OnAddStandardStats) {
-  const int rid = 0, pid = 1, lid = 2;
+  const int rid = 0;
+  const int pid = 1;
+  const int lid = 2;
   base::RunLoop loop;
   MockWebRtcInternalsProxy observer(&loop);
   WebRTCInternalsForTest webrtc_internals;
@@ -439,10 +443,10 @@ TEST_F(WebRtcInternalsTest, OnAddStandardStats) {
   webrtc_internals.OnAddPeerConnection(rid, pid, lid, kUrl, kRtcConfiguration,
                                        kContraints);
 
-  base::ListValue list;
-  list.AppendString("xxx");
-  list.AppendString("yyy");
-  webrtc_internals.OnAddStandardStats(pid, lid, list);
+  base::Value list(base::Value::Type::LIST);
+  list.Append("xxx");
+  list.Append("yyy");
+  webrtc_internals.OnAddStandardStats(pid, lid, list.Clone());
 
   loop.Run();
 
@@ -454,13 +458,15 @@ TEST_F(WebRtcInternalsTest, OnAddStandardStats) {
 
   VerifyInt(dict, "pid", pid);
   VerifyInt(dict, "lid", lid);
-  VerifyList(dict, "reports", list);
+  VerifyList(*dict, "reports", list);
 
   base::RunLoop().RunUntilIdle();
 }
 
 TEST_F(WebRtcInternalsTest, OnAddLegacyStats) {
-  const int rid = 0, pid = 1, lid = 2;
+  const int rid = 0;
+  const int pid = 1;
+  const int lid = 2;
   base::RunLoop loop;
   MockWebRtcInternalsProxy observer(&loop);
   WebRTCInternalsForTest webrtc_internals;
@@ -468,10 +474,10 @@ TEST_F(WebRtcInternalsTest, OnAddLegacyStats) {
   webrtc_internals.OnAddPeerConnection(rid, pid, lid, kUrl, kRtcConfiguration,
                                        kContraints);
 
-  base::ListValue list;
-  list.AppendString("xxx");
-  list.AppendString("yyy");
-  webrtc_internals.OnAddLegacyStats(pid, lid, list);
+  base::Value list(base::Value::Type::LIST);
+  list.Append("xxx");
+  list.Append("yyy");
+  webrtc_internals.OnAddLegacyStats(pid, lid, list.Clone());
 
   loop.Run();
 
@@ -483,7 +489,7 @@ TEST_F(WebRtcInternalsTest, OnAddLegacyStats) {
 
   VerifyInt(dict, "pid", pid);
   VerifyInt(dict, "lid", lid);
-  VerifyList(dict, "reports", list);
+  VerifyList(*dict, "reports", list);
 
   base::RunLoop().RunUntilIdle();
 }