Correct stats for RTCPeerConnectionStats.dataChannels[Opened/Closed].

webrtc/api/rtcstatscollector.h

 /*
  *  Copyright 2016 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #ifndef WEBRTC_API_RTCSTATSCOLLECTOR_H_
 #define WEBRTC_API_RTCSTATSCOLLECTOR_H_
 
 #include <map>
 #include <memory>
+#include <set>
 #include <vector>
 
+#include "webrtc/api/datachannel.h"
 #include "webrtc/api/datachannelinterface.h"
 #include "webrtc/api/stats/rtcstats_objects.h"
 #include "webrtc/api/stats/rtcstatsreport.h"
 #include "webrtc/base/asyncinvoker.h"
 #include "webrtc/base/refcount.h"
 #include "webrtc/base/scoped_ref_ptr.h"
+#include "webrtc/base/sigslot.h"
 #include "webrtc/base/sslidentity.h"
 #include "webrtc/base/timeutils.h"
 
 namespace cricket {
 class Candidate;
 }  // namespace cricket
 
 namespace rtc {
 class SSLCertificate;
 }  // namespace rtc
 
 namespace webrtc {
 
 class PeerConnection;
 struct SessionStats;
 
 class RTCStatsCollectorCallback : public virtual rtc::RefCountInterface {
  public:
   virtual ~RTCStatsCollectorCallback() {}
 
   virtual void OnStatsDelivered(
       const rtc::scoped_refptr<const RTCStatsReport>& report) = 0;
 };
 
 // All public methods of the collector are to be called on the signaling thread.
 // Stats are gathered on the signaling, worker and network threads
 // asynchronously. The callback is invoked on the signaling thread. Resulting
 // reports are cached for |cache_lifetime_| ms.
-class RTCStatsCollector : public virtual rtc::RefCountInterface {
+class RTCStatsCollector : public virtual rtc::RefCountInterface,
+                          public sigslot::has_slots<> {
  public:
   static rtc::scoped_refptr<RTCStatsCollector> Create(
       PeerConnection* pc,
       int64_t cache_lifetime_us = 50 * rtc::kNumMicrosecsPerMillisec);
 
   // Gets a recent stats report. If there is a report cached that is still fresh
   // it is returned, otherwise new stats are gathered and returned. A report is
   // considered fresh for |cache_lifetime_| ms. const RTCStatsReports are safe
   // to use across multiple threads and may be destructed on any thread.
   void GetStatsReport(rtc::scoped_refptr<RTCStatsCollectorCallback> callback);
   // Clears the cache's reference to the most recent stats report. Subsequently
   // calling |GetStatsReport| guarantees fresh stats.
   void ClearCachedStatsReport();
 
+  // Exposed for testing only.
+  void OnDataChannelOpenedForTesting(DataChannel* channel);
+  void OnDataChannelClosedForTesting(DataChannel* channel);
+
  protected:
   RTCStatsCollector(PeerConnection* pc, int64_t cache_lifetime_us);
 
   // Stats gathering on a particular thread. Calls |AddPartialResults| before
   // returning. Virtual for the sake of testing.
   virtual void ProducePartialResultsOnSignalingThread(int64_t timestamp_us);
   virtual void ProducePartialResultsOnWorkerThread(int64_t timestamp_us);
   virtual void ProducePartialResultsOnNetworkThread(int64_t timestamp_us);
 
   // Can be called on any thread.
   void AddPartialResults(
       const rtc::scoped_refptr<RTCStatsReport>& partial_report);
 
  private:
+  // To allow |pc_| to wire up RTCStatsCollector::OnBlahBlah signal slots.
+  friend class PeerConnection;
+
   struct CertificateStatsPair {
     std::unique_ptr<rtc::SSLCertificateStats> local;
     std::unique_ptr<rtc::SSLCertificateStats> remote;
   };
 
   void AddPartialResults_s(rtc::scoped_refptr<RTCStatsReport> partial_report);
   void DeliverCachedReport();
 
   // Produces |RTCCertificateStats|.
   void ProduceCertificateStats_s(
@@ skipping 17 matching lines @@
   // Produces |RTCTransportStats|.
   void ProduceTransportStats_s(
       int64_t timestamp_us, const SessionStats& session_stats,
       const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
       RTCStatsReport* report) const;
 
   // Helper function to stats-producing functions.
   std::map<std::string, CertificateStatsPair>
   PrepareTransportCertificateStats_s(const SessionStats& session_stats) const;
 
+  // Slots for signals (sigslot).
+  void OnDataChannelOpened(DataChannel* channel);
+  void OnDataChannelClosed(DataChannel* channel);
+
   PeerConnection* const pc_;
   rtc::Thread* const signaling_thread_;
   rtc::Thread* const worker_thread_;
   rtc::Thread* const network_thread_;
   rtc::AsyncInvoker invoker_;
 
   int num_pending_partial_reports_;
   int64_t partial_report_timestamp_us_;
   rtc::scoped_refptr<RTCStatsReport> partial_report_;
   std::vector<rtc::scoped_refptr<RTCStatsCollectorCallback>> callbacks_;
 
   // A timestamp, in microseconds, that is based on a timer that is
   // monotonically increasing. That is, even if the system clock is modified the
   // difference between the timer and this timestamp is how fresh the cached
   // report is.
   int64_t cache_timestamp_us_;
   int64_t cache_lifetime_us_;
   rtc::scoped_refptr<const RTCStatsReport> cached_report_;
+
+  // The opened count goes up when a channel is fully opened and the closed
+  // count goes up if a previously opened channel has fully closed. The opened
+  // count does not go down when a channel closes, meaning (opened - closed) is
+  // the number of channels currently opened. A channel that is closed before
+  // reaching the open state does not affect these counters.
+  uint32_t data_channels_opened_;
+  uint32_t data_channels_closed_;

[hta-webrtc, 2016/11/03 13:49:58]

This is actually the beginning of a new path in stats collecting, where you
allow the StatsCollector object to hold references to data between calls and
depend on them being updated correctly from the start of time to the end of
time.

You might want to flag that with a comment above; somewhere in the future, we
might consider factoring out all that kind of stuff into a (private?) class that
is separate from the statscollector itself.

But for these 2 counters, just making a note should be OK.

[hbos, 2016/11/03 15:02:02]

Created struct InternalRecord and added a comment.

+  // Channels that have been opened, they remain in the set until fully closed.
+  std::set<DataChannel*> opened_data_channels_;

[hta-webrtc, 2016/11/03 13:49:58]

Do we actually need this? I'd suggest not retaining pointers if we don't have a
clear use case for them - they're a temptation to sin.
If they're used as identifiers (and I think they are), make a clear comment that
that's what they are - they must NOT be followed.

It's tempting to cast them to int in order to limit the temptation.

[hbos, 2016/11/03 15:02:02]

Changed to std::set<uintptr_t>

 };
 
 const char* CandidateTypeToRTCIceCandidateTypeForTesting(
     const std::string& type);
 const char* DataStateToRTCDataChannelStateForTesting(
     DataChannelInterface::DataState state);
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_API_RTCSTATSCOLLECTOR_H_