Reland "[Stats] Attribute::ToString(), to replace member ValueToString/ToJson."

This is a reland of commit 54be7084e0861a0179a5fccd0b27edf7d7994bbb

Previously reverted due to an importer issue (b/320646178) and later a
dependency on RTCStatsMember<T>::ValueToString().

In this reland, we add Attribute::ToString() but we don't delete the
RTCStatsMember<T> stringifier methods, allowing downstream to migrate
before they are deleted.

Original change's description:
> [Stats] Attribute::ToString(), to replace member ValueToString/ToJson.
>
> Delete RTCStatsMember<T>::ValueToString() and ValueToJson() in favor of
> Attribute::ToString().
>
> The difference between "ToString" and "ToJson" is that the "ToJson"
> version converts 64-bit integers and doubles to floating points with no
> more than ~15 digits of precision as to not exceed JSON's precision
> limitations. So only in edge cases of really large numbers or numbers
> with a silly number of digits will the two methods produce different
> results. Also JSON puts '\"' around map key names, e.g. "{\"foo\":123}"
> as opposed to "{foo:123}".
>
> Going forward we see no reason to maintain two different string
> converted paths that are this similar, so we only implement one
> Attribute::ToString() method which does what "ToJson" did.
>
> In the next CL we can delete RTCStatsMember<T>.
>
> Bug: webrtc:15164
> Change-Id: Iaa8cf3bf14b40dc44664f75989832469603131c5
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/334640
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Evan Shrubsole <eshr@google.com>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#41544}

Bug: webrtc:15164
Change-Id: I281ccf5b23d8f194b5ce00186a32846c757b46fc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/334860
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41575}

api/stats/attribute.h

@@ -74,8 +74,11 @@ class RTC_EXPORT Attribute {
 
   bool is_sequence() const;
   bool is_string() const;
-  std::string ValueToString() const;
+  // Converts the attribute to a string that is parseable as a JSON object.
-  std::string ValueToJson() const;
+  std::string ToString() const;
+  // TODO(https://crbug.com/15164): Use ToString() instead and delete these.
+  std::string ValueToString() const { return ToString(); }
+  std::string ValueToJson() const { return ToString(); }
 
   bool operator==(const Attribute& other) const;
   bool operator!=(const Attribute& other) const;

api/stats/rtc_stats_member.h

@@ -65,13 +65,6 @@ class RTCStatsMemberInterface {
   bool operator!=(const RTCStatsMemberInterface& other) const {
     return !(*this == other);
   }
-  virtual std::string ValueToString() const = 0;
-  // This is the same as ValueToString except for kInt64 and kUint64 types,
-  // where the value is represented as a double instead of as an integer.
-  // Since JSON stores numbers as floating point numbers, very large integers
-  // cannot be accurately represented, so we prefer to display them as doubles
-  // instead.
-  virtual std::string ValueToJson() const = 0;
 
   virtual const RTCStatsMemberInterface* member_ptr() const { return this; }
   template <typename T>
@@ -97,8 +90,10 @@ class RTCStatsMember : public RTCStatsMemberInterface {
   Type type() const override { return StaticType(); }
   bool is_sequence() const override;
   bool is_string() const override;
-  std::string ValueToString() const override;
+  // TODO(https://crbug.com/webrtc/15164): Delete both in favor of
-  std::string ValueToJson() const override;
+  // Attribute::ToString().
+  std::string ValueToString() const;
+  std::string ValueToJson() const;
 
   template <typename U>
   inline T value_or(U default_value) const {

pc/peer_connection_rampup_tests.cc

@@ -313,7 +313,9 @@ class PeerConnectionRampUpTest : public ::testing::Test {
       return 0;
     }
     std::string selected_ice_id =
-        transport_stats[0]->selected_candidate_pair_id.ValueToString();
+        transport_stats[0]
+            ->GetAttribute(transport_stats[0]->selected_candidate_pair_id)
+            .ToString();
     // Use the selected ICE candidate pair ID to get the appropriate ICE stats.
     const RTCIceCandidatePairStats ice_candidate_pair_stats =
         stats->Get(selected_ice_id)->cast_to<const RTCIceCandidatePairStats>();

pc/rtc_stats_integrationtest.cc

@@ -228,9 +228,10 @@ class RTCStatsVerifier {
 
   template <typename T>
   void TestAttributeIsUndefined(const RTCStatsMember<T>& field) {
+    Attribute attribute = stats_->GetAttribute(field);
     EXPECT_FALSE(field.has_value())
-        << stats_->type() << "." << stats_->GetAttribute(field).name() << "["
+        << stats_->type() << "." << attribute.name() << "[" << stats_->id()
-        << stats_->id() << "] was defined (" << field.ValueToString() << ").";
+        << "] was defined (" << attribute.ToString() << ").";
     MarkAttributeTested(field, !field.has_value());
   }
 
@@ -246,7 +247,7 @@ class RTCStatsVerifier {
     bool is_positive = field.value() > T(0);
     EXPECT_TRUE(is_positive)
         << stats_->type() << "." << attribute.name() << "[" << stats_->id()
-        << "] was not positive (" << attribute.ValueToString() << ").";
+        << "] was not positive (" << attribute.ToString() << ").";
     MarkAttributeTested(field, is_positive);
   }
 
@@ -262,7 +263,7 @@ class RTCStatsVerifier {
     bool is_non_negative = field.value() >= T(0);
     EXPECT_TRUE(is_non_negative)
         << stats_->type() << "." << attribute.name() << "[" << stats_->id()
-        << "] was not non-negative (" << attribute.ValueToString() << ").";
+        << "] was not non-negative (" << attribute.ToString() << ").";
     MarkAttributeTested(field, is_non_negative);
   }
 
@@ -323,8 +324,7 @@ class RTCStatsVerifier {
         << stats_->type() << "." << attribute.name()
         << " is not a reference to an "
            "existing dictionary of type "
-        << expected_type << " (value: "
+        << expected_type << " (value: " << attribute.ToString() << ").";
-        << (field.has_value() ? attribute.ValueToString() : "null") << ").";
     MarkAttributeTested(field, valid_reference);
   }
 

stats/attribute.cc

@@ -10,8 +10,13 @@
 
 #include "api/stats/attribute.h"
 
+#include <string>
+
 #include "absl/types/variant.h"
+#include "rtc_base/arraysize.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/string_encode.h"
+#include "rtc_base/strings/string_builder.h"
 
 namespace webrtc {
 
@@ -30,6 +35,82 @@ struct VisitIsSequence {
   }
 };
 
+// Converts the attribute to string in a JSON-compatible way.
+struct VisitToString {
+  template <typename T,
+            typename std::enable_if_t<
+                std::is_same_v<T, int32_t> || std::is_same_v<T, uint32_t> ||
+                    std::is_same_v<T, bool> || std::is_same_v<T, std::string>,
+                bool> = true>
+  std::string ValueToString(const T& value) {
+    return rtc::ToString(value);
+  }
+  // Convert 64-bit integers to doubles before converting to string because JSON
+  // represents all numbers as floating points with ~15 digits of precision.
+  template <typename T,
+            typename std::enable_if_t<std::is_same_v<T, int64_t> ||
+                                          std::is_same_v<T, uint64_t> ||
+                                          std::is_same_v<T, double>,
+                                      bool> = true>
+  std::string ValueToString(const T& value) {
+    char buf[32];
+    const int len = std::snprintf(&buf[0], arraysize(buf), "%.16g",
+                                  static_cast<double>(value));
+    RTC_DCHECK_LE(len, arraysize(buf));
+    return std::string(&buf[0], len);
+  }
+
+  // Vector attributes.
+  template <typename T>
+  std::string operator()(const RTCStatsMember<std::vector<T>>* attribute) {
+    rtc::StringBuilder sb;
+    sb << "[";
+    const char* separator = "";
+    constexpr bool element_is_string = std::is_same<T, std::string>::value;
+    for (const T& element : attribute->value()) {
+      sb << separator;
+      if (element_is_string) {
+        sb << "\"";
+      }
+      sb << ValueToString(element);
+      if (element_is_string) {
+        sb << "\"";
+      }
+      separator = ",";
+    }
+    sb << "]";
+    return sb.Release();
+  }
+  // Map attributes.
+  template <typename T>
+  std::string operator()(
+      const RTCStatsMember<std::map<std::string, T>>* attribute) {
+    rtc::StringBuilder sb;
+    sb << "{";
+    const char* separator = "";
+    constexpr bool element_is_string = std::is_same<T, std::string>::value;
+    for (const auto& pair : attribute->value()) {
+      sb << separator;
+      sb << "\"" << pair.first << "\":";
+      if (element_is_string) {
+        sb << "\"";
+      }
+      sb << ValueToString(pair.second);
+      if (element_is_string) {
+        sb << "\"";
+      }
+      separator = ",";
+    }
+    sb << "}";
+    return sb.Release();
+  }
+  // Simple attributes.
+  template <typename T>
+  std::string operator()(const RTCStatsMember<T>* attribute) {
+    return ValueToString(attribute->value());
+  }
+};
+
 struct VisitIsEqual {
   template <typename T>
   bool operator()(const RTCStatsMember<T>* attribute) {
@@ -69,14 +150,11 @@ bool Attribute::is_string() const {
       attribute_);
 }
 
-std::string Attribute::ValueToString() const {
+std::string Attribute::ToString() const {
-  return absl::visit([](const auto* attr) { return attr->ValueToString(); },
+  if (!has_value()) {
-                     attribute_);
+    return "null";
   }
-
+  return absl::visit(VisitToString(), attribute_);
-std::string Attribute::ValueToJson() const {
-  return absl::visit([](const auto* attr) { return attr->ValueToJson(); },
-                     attribute_);
 }
 
 bool Attribute::operator==(const Attribute& other) const {

stats/rtc_stats.cc

@@ -54,8 +54,7 @@ std::string RTCStats::ToJson() const {
       if (attribute.holds_alternative<std::string>()) {
         sb << "\"";
       }
-      sb << absl::visit([](const auto* attr) { return attr->ValueToJson(); },
+      sb << attribute.ToString();
-                        attribute.as_variant());
       if (attribute.holds_alternative<std::string>()) {
         sb << "\"";
       }

stats/rtc_stats_member.cc

@@ -17,6 +17,9 @@ namespace webrtc {
 
 namespace {
 
+// TODO(https://crbug.com/webrtc/15164): Delete all stringified functions in
+// favor of Attribute::ToString().
+
 // Produces "[a,b,c]". Works for non-vector `RTCStatsMemberInterface::Type`
 // types.
 template <typename T>

stats/rtc_stats_unittest.cc

@@ -459,45 +459,50 @@ TEST(RTCStatsTest, IsString) {
   EXPECT_FALSE(stats.m_map_string_double.is_string());
 }
 
-TEST(RTCStatsTest, ValueToString) {
+TEST(RTCStatsTest, AttributeToString) {
   RTCTestStats stats("statsId", Timestamp::Micros(42));
   stats.m_bool = true;
-  EXPECT_EQ("true", stats.m_bool.ValueToString());
+  EXPECT_EQ("true", stats.GetAttribute(stats.m_bool).ToString());
 
   stats.m_string = "foo";
-  EXPECT_EQ("foo", stats.m_string.ValueToString());
+  EXPECT_EQ("foo", stats.GetAttribute(stats.m_string).ToString());
   stats.m_int32 = -32;
-  EXPECT_EQ("-32", stats.m_int32.ValueToString());
+  EXPECT_EQ("-32", stats.GetAttribute(stats.m_int32).ToString());
   stats.m_uint32 = 32;
-  EXPECT_EQ("32", stats.m_uint32.ValueToString());
+  EXPECT_EQ("32", stats.GetAttribute(stats.m_uint32).ToString());
   stats.m_int64 = -64;
-  EXPECT_EQ("-64", stats.m_int64.ValueToString());
+  EXPECT_EQ("-64", stats.GetAttribute(stats.m_int64).ToString());
   stats.m_uint64 = 64;
-  EXPECT_EQ("64", stats.m_uint64.ValueToString());
+  EXPECT_EQ("64", stats.GetAttribute(stats.m_uint64).ToString());
   stats.m_double = 0.5;
-  EXPECT_EQ("0.5", stats.m_double.ValueToString());
+  EXPECT_EQ("0.5", stats.GetAttribute(stats.m_double).ToString());
   stats.m_sequence_bool = {true, false};
-  EXPECT_EQ("[true,false]", stats.m_sequence_bool.ValueToString());
+  EXPECT_EQ("[true,false]",
+            stats.GetAttribute(stats.m_sequence_bool).ToString());
   stats.m_sequence_int32 = {-32, 32};
-  EXPECT_EQ("[-32,32]", stats.m_sequence_int32.ValueToString());
+  EXPECT_EQ("[-32,32]", stats.GetAttribute(stats.m_sequence_int32).ToString());
   stats.m_sequence_uint32 = {64, 32};
-  EXPECT_EQ("[64,32]", stats.m_sequence_uint32.ValueToString());
+  EXPECT_EQ("[64,32]", stats.GetAttribute(stats.m_sequence_uint32).ToString());
   stats.m_sequence_int64 = {-64, 32};
-  EXPECT_EQ("[-64,32]", stats.m_sequence_int64.ValueToString());
+  EXPECT_EQ("[-64,32]", stats.GetAttribute(stats.m_sequence_int64).ToString());
   stats.m_sequence_uint64 = {16, 32};
-  EXPECT_EQ("[16,32]", stats.m_sequence_uint64.ValueToString());
+  EXPECT_EQ("[16,32]", stats.GetAttribute(stats.m_sequence_uint64).ToString());
   stats.m_sequence_double = {0.5, 0.25};
-  EXPECT_EQ("[0.5,0.25]", stats.m_sequence_double.ValueToString());
+  EXPECT_EQ("[0.5,0.25]",
+            stats.GetAttribute(stats.m_sequence_double).ToString());
   stats.m_sequence_string = {"foo", "bar"};
-  EXPECT_EQ("[\"foo\",\"bar\"]", stats.m_sequence_string.ValueToString());
+  EXPECT_EQ("[\"foo\",\"bar\"]",
+            stats.GetAttribute(stats.m_sequence_string).ToString());
   stats.m_map_string_uint64 = std::map<std::string, uint64_t>();
   stats.m_map_string_uint64->emplace("foo", 32);
   stats.m_map_string_uint64->emplace("bar", 64);
-  EXPECT_EQ("{bar:64,foo:32}", stats.m_map_string_uint64.ValueToString());
+  EXPECT_EQ("{\"bar\":64,\"foo\":32}",
+            stats.GetAttribute(stats.m_map_string_uint64).ToString());
   stats.m_map_string_double = std::map<std::string, double>();
   stats.m_map_string_double->emplace("foo", 0.5);
   stats.m_map_string_double->emplace("bar", 0.25);
-  EXPECT_EQ("{bar:0.25,foo:0.5}", stats.m_map_string_double.ValueToString());
+  EXPECT_EQ("{\"bar\":0.25,\"foo\":0.5}",
+            stats.GetAttribute(stats.m_map_string_double).ToString());
 }
 
 // Death tests.

test/pc/e2e/analyzer/video/video_quality_metrics_reporter.cc

@@ -63,7 +63,9 @@ void VideoQualityMetricsReporter::OnStatsReports(
   }
   RTC_DCHECK_EQ(transport_stats.size(), 1);
   std::string selected_ice_id =
-      transport_stats[0]->selected_candidate_pair_id.ValueToString();
+      transport_stats[0]
+          ->GetAttribute(transport_stats[0]->selected_candidate_pair_id)
+          .ToString();
   // Use the selected ICE candidate pair ID to get the appropriate ICE stats.
   const RTCIceCandidatePairStats ice_candidate_pair_stats =
       report->Get(selected_ice_id)->cast_to<const RTCIceCandidatePairStats>();