Make ID of datachannel stats not depend on dc.id

The ID of stats was based on the datachannel's "id"
attribute, but that could change - it was -1 before ID
allocation, and a number afterwards.

This CL changes the stats ID to depend on a monotonically
increasing counter for allocated datachannels.

Bug: webrtc:10842
Change-Id: I3e0c5dc07df8a7a502396de06bbedc9f676994a0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147642
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28720}

pc/data_channel.cc

@@ -28,6 +28,16 @@ namespace webrtc {
 static size_t kMaxQueuedReceivedDataBytes = 16 * 1024 * 1024;
 static size_t kMaxQueuedSendDataBytes = 16 * 1024 * 1024;
 
+namespace {
+
+static std::atomic<int> g_unique_id{0};
+
+int GenerateUniqueId() {
+  return ++g_unique_id;
+}
+
+}  // namespace
+
 InternalDataChannelInit::InternalDataChannelInit(const DataChannelInit& base)
     : DataChannelInit(base), open_handshake_role(kOpener) {
   // If the channel is externally negotiated, do not send the OPEN message.
@@ -144,7 +154,8 @@ bool DataChannel::IsSctpLike(cricket::DataChannelType type) {
 DataChannel::DataChannel(DataChannelProviderInterface* provider,
                          cricket::DataChannelType dct,
                          const std::string& label)
-    : label_(label),
+    : internal_id_(GenerateUniqueId()),
+      label_(label),
       observer_(nullptr),
       state_(kConnecting),
       messages_sent_(0),
@@ -705,4 +716,9 @@ bool DataChannel::SendControlMessage(const rtc::CopyOnWriteBuffer& buffer) {
   return retval;
 }
 
+// static
+void DataChannel::ResetInternalIdAllocatorForTesting(int new_value) {
+  g_unique_id = new_value;
+}
+
 }  // namespace webrtc

pc/data_channel.h

@@ -142,6 +142,7 @@ class DataChannel : public DataChannelInterface, public sigslot::has_slots<> {
   virtual std::string protocol() const { return config_.protocol; }
   virtual bool negotiated() const { return config_.negotiated; }
   virtual int id() const { return config_.id; }
+  virtual int internal_id() const { return internal_id_; }
   virtual uint64_t buffered_amount() const;
   virtual void Close();
   virtual DataState state() const { return state_; }
@@ -214,6 +215,10 @@ class DataChannel : public DataChannelInterface, public sigslot::has_slots<> {
   // channel's sid is free.
   sigslot::signal1<DataChannel*> SignalClosed;
 
+  // Reset the allocator for internal ID values for testing, so that
+  // the internal IDs generated are predictable. Test only.
+  static void ResetInternalIdAllocatorForTesting(int new_value);
+
  protected:
   DataChannel(DataChannelProviderInterface* client,
               cricket::DataChannelType dct,
@@ -267,6 +272,7 @@ class DataChannel : public DataChannelInterface, public sigslot::has_slots<> {
   void QueueControlMessage(const rtc::CopyOnWriteBuffer& buffer);
   bool SendControlMessage(const rtc::CopyOnWriteBuffer& buffer);
 
+  const int internal_id_;
   std::string label_;
   InternalDataChannelInit config_;
   DataChannelObserver* observer_;

pc/rtc_stats_collector.cc

@@ -1224,7 +1224,7 @@ void RTCStatsCollector::ProduceDataChannelStats_s(
        pc_->sctp_data_channels()) {
     std::unique_ptr<RTCDataChannelStats> data_channel_stats(
         new RTCDataChannelStats(
-            "RTCDataChannel_" + rtc::ToString(data_channel->id()),
+            "RTCDataChannel_" + rtc::ToString(data_channel->internal_id()),
             timestamp_us));
     data_channel_stats->label = data_channel->label();
     data_channel_stats->protocol = data_channel->protocol();

pc/rtc_stats_collector_unittest.cc

@@ -942,7 +942,20 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsChain) {
   ExpectReportContainsCertificateInfo(report, *remote_certinfo);
 }
 
+TEST_F(RTCStatsCollectorTest, CollectTwoRTCDataChannelStatsWithPendingId) {
+  pc_->AddSctpDataChannel(
+      new MockDataChannel(/*id=*/-1, DataChannelInterface::kConnecting));
+  pc_->AddSctpDataChannel(
+      new MockDataChannel(/*id=*/-1, DataChannelInterface::kConnecting));
+
+  rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
+}
+
 TEST_F(RTCStatsCollectorTest, CollectRTCDataChannelStats) {
+  // Note: The test assumes data channel IDs are predictable.
+  // This is not a safe assumption, but in order to make it work for
+  // the test, we reset the ID allocator at test start.
+  DataChannel::ResetInternalIdAllocatorForTesting(-1);
   pc_->AddSctpDataChannel(new MockDataChannel(0, "MockDataChannel0",
                                               DataChannelInterface::kConnecting,
                                               "udp", 1, 2, 3, 4));