From fc5e0504ea0dd341046779881908d4640617a2f0 Mon Sep 17 00:00:00 2001 From: hbos Date: Thu, 6 Oct 2016 02:06:10 -0700 Subject: [PATCH] rtc_stats: Update code to remove chromium style warnings suppression. The warning previously suppressed made it possible to define tings like constructors in the header, and "complex" objects did not need to have an explicit out-of-line copy constructor, destructor, etc. To be able to not suppress this warning, the RTCStats macro was split into a WEBRTC_RTCSTATS_DECL() and WEBRTC_RTCSTATS_IMPL() for .h and .cc respectively. Some copy constructors are also defined. BUG=chromium:627816 Review-Url: https://codereview.webrtc.org/2373503002 Cr-Commit-Position: refs/heads/master@{#14545} --- webrtc/api/rtcstatscollector_unittest.cc | 8 +-- webrtc/api/stats/rtcstats.h | 65 +++++++++++++++--------- webrtc/api/stats/rtcstats_objects.h | 18 +++---- webrtc/stats/BUILD.gn | 10 ---- webrtc/stats/rtcstats_objects.cc | 32 +++++++++++- webrtc/stats/rtcstats_unittest.cc | 16 +++--- webrtc/stats/rtcstatsreport_unittest.cc | 24 ++++----- webrtc/stats/test/rtcteststats.cc | 37 +++++++++++++- webrtc/stats/test/rtcteststats.h | 20 ++------ 9 files changed, 144 insertions(+), 86 deletions(-) diff --git a/webrtc/api/rtcstatscollector_unittest.cc b/webrtc/api/rtcstatscollector_unittest.cc index 020a759a68..14abbd251a 100644 --- a/webrtc/api/rtcstatscollector_unittest.cc +++ b/webrtc/api/rtcstatscollector_unittest.cc @@ -140,17 +140,17 @@ class RTCStatsCollectorTestHelper : public SetSessionDescriptionObserver { class RTCTestStats : public RTCStats { public: + WEBRTC_RTCSTATS_DECL(); + RTCTestStats(const std::string& id, int64_t timestamp_us) : RTCStats(id, timestamp_us), dummy_stat("dummyStat") {} - WEBRTC_RTCSTATS_IMPL(RTCStats, RTCTestStats, - &dummy_stat); - RTCStatsMember dummy_stat; }; -const char RTCTestStats::kType[] = "test-stats"; +WEBRTC_RTCSTATS_IMPL(RTCTestStats, RTCStats, "test-stats", + &dummy_stat); // Overrides the stats collection to verify thread usage and that the resulting // partial reports are merged. diff --git a/webrtc/api/stats/rtcstats.h b/webrtc/api/stats/rtcstats.h index 988291bd11..6917548302 100644 --- a/webrtc/api/stats/rtcstats.h +++ b/webrtc/api/stats/rtcstats.h @@ -91,14 +91,15 @@ class RTCStats { int64_t timestamp_us_; }; -// All |RTCStats| classes should use this macro in a public section of the class -// definition. +// All |RTCStats| classes should use these macros. +// |WEBRTC_RTCSTATS_DECL| is placed in a public section of the class definition. +// |WEBRTC_RTCSTATS_IMPL| is placed outside the class definition (in a .cc). // -// This macro declares the static |kType| and overrides methods as required by -// subclasses of |RTCStats|: |copy|, |type|, and +// These macros declare (in _DECL) and define (in _IMPL) the static |kType| and +// overrides methods as required by subclasses of |RTCStats|: |copy|, |type| and // |MembersOfThisObjectAndAncestors|. The |...| argument is a list of addresses -// to each member defined in the implementing class (list cannot be empty, must -// have at least one new member). +// to each member defined in the implementing class. The list must have at least +// one member. // // (Since class names need to be known to implement these methods this cannot be // part of the base |RTCStats|. While these methods could be implemented using @@ -112,34 +113,53 @@ class RTCStats { // rtcfoostats.h: // class RTCFooStats : public RTCStats { // public: -// RTCFooStats(const std::string& id, int64_t timestamp_us) -// : RTCStats(id, timestamp_us), -// foo("foo"), -// bar("bar") { -// } +// WEBRTC_RTCSTATS_DECL(); // -// WEBRTC_RTCSTATS_IMPL(RTCStats, RTCFooStats, -// &foo, -// &bar); +// RTCFooStats(const std::string& id, int64_t timestamp_us); // // RTCStatsMember foo; // RTCStatsMember bar; // }; // // rtcfoostats.cc: -// const char RTCFooStats::kType[] = "foo-stats"; +// WEBRTC_RTCSTATS_IMPL(RTCFooStats, RTCStats, "foo-stats" +// &foo, +// &bar); // -#define WEBRTC_RTCSTATS_IMPL(parent_class, this_class, ...) \ +// RTCFooStats::RTCFooStats(const std::string& id, int64_t timestamp_us) +// : RTCStats(id, timestamp_us), +// foo("foo"), +// bar("bar") { +// } +// +#define WEBRTC_RTCSTATS_DECL() \ public: \ static const char kType[]; \ - std::unique_ptr copy() const override { \ - return std::unique_ptr(new this_class(*this)); \ - } \ - const char* type() const override { return this_class::kType; } \ + \ + std::unique_ptr copy() const override; \ + const char* type() const override; \ + \ protected: \ std::vector \ MembersOfThisObjectAndAncestors( \ - size_t local_var_additional_capacity) const override { \ + size_t local_var_additional_capacity) const override; \ + \ + public: + +#define WEBRTC_RTCSTATS_IMPL(this_class, parent_class, type_str, ...) \ + const char this_class::kType[] = type_str; \ + \ + std::unique_ptr this_class::copy() const { \ + return std::unique_ptr(new this_class(*this)); \ + } \ + \ + const char* this_class::type() const { \ + return this_class::kType; \ + } \ + \ + std::vector \ + this_class::MembersOfThisObjectAndAncestors( \ + size_t local_var_additional_capacity) const { \ const webrtc::RTCStatsMemberInterface* local_var_members[] = { \ __VA_ARGS__ \ }; \ @@ -155,8 +175,7 @@ class RTCStats { &local_var_members[0], \ &local_var_members[local_var_members_count]); \ return local_var_members_vec; \ - } \ - public: + } // Interface for |RTCStats| members, which have a name and a value of a type // defined in a subclass. Only the types listed in |Type| are supported, these diff --git a/webrtc/api/stats/rtcstats_objects.h b/webrtc/api/stats/rtcstats_objects.h index 6ff9f9949a..f816d48b55 100644 --- a/webrtc/api/stats/rtcstats_objects.h +++ b/webrtc/api/stats/rtcstats_objects.h @@ -20,14 +20,12 @@ namespace webrtc { // https://w3c.github.io/webrtc-stats/#certificatestats-dict* class RTCCertificateStats : public RTCStats { public: + WEBRTC_RTCSTATS_DECL(); + RTCCertificateStats(const std::string& id, int64_t timestamp_us); RTCCertificateStats(std::string&& id, int64_t timestamp_us); - - WEBRTC_RTCSTATS_IMPL(RTCStats, RTCCertificateStats, - &fingerprint, - &fingerprint_algorithm, - &base64_certificate, - &issuer_certificate_id); + RTCCertificateStats(const RTCCertificateStats& other); + ~RTCCertificateStats() override; RTCStatsMember fingerprint; RTCStatsMember fingerprint_algorithm; @@ -39,12 +37,12 @@ class RTCCertificateStats : public RTCStats { // TODO(hbos): Tracking bug crbug.com/636818 class RTCPeerConnectionStats : public RTCStats { public: + WEBRTC_RTCSTATS_DECL(); + RTCPeerConnectionStats(const std::string& id, int64_t timestamp_us); RTCPeerConnectionStats(std::string&& id, int64_t timestamp_us); - - WEBRTC_RTCSTATS_IMPL(RTCStats, RTCPeerConnectionStats, - &data_channels_opened, - &data_channels_closed); + RTCPeerConnectionStats(const RTCPeerConnectionStats& other); + ~RTCPeerConnectionStats() override; RTCStatsMember data_channels_opened; RTCStatsMember data_channels_closed; diff --git a/webrtc/stats/BUILD.gn b/webrtc/stats/BUILD.gn index 896674405a..914cb45c43 100644 --- a/webrtc/stats/BUILD.gn +++ b/webrtc/stats/BUILD.gn @@ -23,11 +23,6 @@ rtc_static_library("rtc_stats") { "rtcstatsreport.cc", ] - if (is_clang) { - # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). - suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] - } - deps = [ "../api:rtc_stats_api", "../base:rtc_base_approved", @@ -41,11 +36,6 @@ rtc_source_set("rtc_stats_test_utils") { "test/rtcteststats.h", ] - if (is_clang) { - # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). - suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] - } - deps = [ ":rtc_stats", "../api:rtc_stats_api", diff --git a/webrtc/stats/rtcstats_objects.cc b/webrtc/stats/rtcstats_objects.cc index fbd16ed39d..b90941a5c0 100644 --- a/webrtc/stats/rtcstats_objects.cc +++ b/webrtc/stats/rtcstats_objects.cc @@ -12,7 +12,11 @@ namespace webrtc { -const char RTCCertificateStats::kType[] = "certificate"; +WEBRTC_RTCSTATS_IMPL(RTCCertificateStats, RTCStats, "certificate", + &fingerprint, + &fingerprint_algorithm, + &base64_certificate, + &issuer_certificate_id); RTCCertificateStats::RTCCertificateStats( const std::string& id, int64_t timestamp_us) @@ -28,7 +32,21 @@ RTCCertificateStats::RTCCertificateStats( issuer_certificate_id("issuerCertificateId") { } -const char RTCPeerConnectionStats::kType[] = "peer-connection"; +RTCCertificateStats::RTCCertificateStats( + const RTCCertificateStats& other) + : RTCStats(other.id(), other.timestamp_us()), + fingerprint(other.fingerprint), + fingerprint_algorithm(other.fingerprint_algorithm), + base64_certificate(other.base64_certificate), + issuer_certificate_id(other.issuer_certificate_id) { +} + +RTCCertificateStats::~RTCCertificateStats() { +} + +WEBRTC_RTCSTATS_IMPL(RTCPeerConnectionStats, RTCStats, "peer-connection", + &data_channels_opened, + &data_channels_closed); RTCPeerConnectionStats::RTCPeerConnectionStats( const std::string& id, int64_t timestamp_us) @@ -42,4 +60,14 @@ RTCPeerConnectionStats::RTCPeerConnectionStats( data_channels_closed("dataChannelsClosed") { } +RTCPeerConnectionStats::RTCPeerConnectionStats( + const RTCPeerConnectionStats& other) + : RTCStats(other.id(), other.timestamp_us()), + data_channels_opened(other.data_channels_opened), + data_channels_closed(other.data_channels_closed) { +} + +RTCPeerConnectionStats::~RTCPeerConnectionStats() { +} + } // namespace webrtc diff --git a/webrtc/stats/rtcstats_unittest.cc b/webrtc/stats/rtcstats_unittest.cc index 88a2e03472..ad839d811d 100644 --- a/webrtc/stats/rtcstats_unittest.cc +++ b/webrtc/stats/rtcstats_unittest.cc @@ -20,31 +20,31 @@ namespace webrtc { class RTCChildStats : public RTCStats { public: + WEBRTC_RTCSTATS_DECL(); + RTCChildStats(const std::string& id, int64_t timestamp_us) : RTCStats(id, timestamp_us), child_int("childInt") {} - WEBRTC_RTCSTATS_IMPL(RTCStats, RTCChildStats, - &child_int); - RTCStatsMember child_int; }; -const char RTCChildStats::kType[] = "child-stats"; +WEBRTC_RTCSTATS_IMPL(RTCChildStats, RTCStats, "child-stats", + &child_int); class RTCGrandChildStats : public RTCChildStats { public: + WEBRTC_RTCSTATS_DECL(); + RTCGrandChildStats(const std::string& id, int64_t timestamp_us) : RTCChildStats(id, timestamp_us), grandchild_int("grandchildInt") {} - WEBRTC_RTCSTATS_IMPL(RTCChildStats, RTCGrandChildStats, - &grandchild_int); - RTCStatsMember grandchild_int; }; -const char RTCGrandChildStats::kType[] = "grandchild-stats"; +WEBRTC_RTCSTATS_IMPL(RTCGrandChildStats, RTCChildStats, "grandchild-stats", + &grandchild_int); TEST(RTCStatsTest, RTCStatsAndMembers) { RTCTestStats stats("testId", 42); diff --git a/webrtc/stats/rtcstatsreport_unittest.cc b/webrtc/stats/rtcstatsreport_unittest.cc index 9a0bb1a125..3bfbd44cba 100644 --- a/webrtc/stats/rtcstatsreport_unittest.cc +++ b/webrtc/stats/rtcstatsreport_unittest.cc @@ -18,45 +18,45 @@ namespace webrtc { class RTCTestStats1 : public RTCStats { public: + WEBRTC_RTCSTATS_DECL(); + RTCTestStats1(const std::string& id, int64_t timestamp_us) : RTCStats(id, timestamp_us), integer("integer") {} - WEBRTC_RTCSTATS_IMPL(RTCStats, RTCTestStats1, - &integer); - RTCStatsMember integer; }; -const char RTCTestStats1::kType[] = "test-stats-1"; +WEBRTC_RTCSTATS_IMPL(RTCTestStats1, RTCStats, "test-stats-1", + &integer); class RTCTestStats2 : public RTCStats { public: + WEBRTC_RTCSTATS_DECL(); + RTCTestStats2(const std::string& id, int64_t timestamp_us) : RTCStats(id, timestamp_us), number("number") {} - WEBRTC_RTCSTATS_IMPL(RTCStats, RTCTestStats2, - &number); - RTCStatsMember number; }; -const char RTCTestStats2::kType[] = "test-stats-2"; +WEBRTC_RTCSTATS_IMPL(RTCTestStats2, RTCStats, "test-stats-2", + &number); class RTCTestStats3 : public RTCStats { public: + WEBRTC_RTCSTATS_DECL(); + RTCTestStats3(const std::string& id, int64_t timestamp_us) : RTCStats(id, timestamp_us), string("string") {} - WEBRTC_RTCSTATS_IMPL(RTCStats, RTCTestStats3, - &string); - RTCStatsMember string; }; -const char RTCTestStats3::kType[] = "test-stats-3"; +WEBRTC_RTCSTATS_IMPL(RTCTestStats3, RTCStats, "test-stats-3", + &string); TEST(RTCStatsReport, AddAndGetStats) { rtc::scoped_refptr report = RTCStatsReport::Create(); diff --git a/webrtc/stats/test/rtcteststats.cc b/webrtc/stats/test/rtcteststats.cc index a4bff77564..f60bd0f8f0 100644 --- a/webrtc/stats/test/rtcteststats.cc +++ b/webrtc/stats/test/rtcteststats.cc @@ -12,7 +12,21 @@ namespace webrtc { -const char RTCTestStats::kType[] = "test-stats"; +WEBRTC_RTCSTATS_IMPL(RTCTestStats, RTCStats, "test-stats", + &m_bool, + &m_int32, + &m_uint32, + &m_int64, + &m_uint64, + &m_double, + &m_string, + &m_sequence_bool, + &m_sequence_int32, + &m_sequence_uint32, + &m_sequence_int64, + &m_sequence_uint64, + &m_sequence_double, + &m_sequence_string); RTCTestStats::RTCTestStats(const std::string& id, int64_t timestamp_us) : RTCStats(id, timestamp_us), @@ -32,4 +46,25 @@ RTCTestStats::RTCTestStats(const std::string& id, int64_t timestamp_us) m_sequence_string("mSequenceString") { } +RTCTestStats::RTCTestStats(const RTCTestStats& other) + : RTCStats(other.id(), other.timestamp_us()), + m_bool(other.m_bool), + m_int32(other.m_int32), + m_uint32(other.m_uint32), + m_int64(other.m_int64), + m_uint64(other.m_uint64), + m_double(other.m_double), + m_string(other.m_string), + m_sequence_bool(other.m_sequence_bool), + m_sequence_int32(other.m_sequence_int32), + m_sequence_uint32(other.m_sequence_uint32), + m_sequence_int64(other.m_sequence_int64), + m_sequence_uint64(other.m_sequence_uint64), + m_sequence_double(other.m_sequence_double), + m_sequence_string(other.m_sequence_string) { +} + +RTCTestStats::~RTCTestStats() { +} + } // namespace webrtc diff --git a/webrtc/stats/test/rtcteststats.h b/webrtc/stats/test/rtcteststats.h index 8571fe89b1..a0ac8a1726 100644 --- a/webrtc/stats/test/rtcteststats.h +++ b/webrtc/stats/test/rtcteststats.h @@ -20,23 +20,11 @@ namespace webrtc { class RTCTestStats : public RTCStats { public: - RTCTestStats(const std::string& id, int64_t timestamp_us); + WEBRTC_RTCSTATS_DECL(); - WEBRTC_RTCSTATS_IMPL(RTCStats, RTCTestStats, - &m_bool, - &m_int32, - &m_uint32, - &m_int64, - &m_uint64, - &m_double, - &m_string, - &m_sequence_bool, - &m_sequence_int32, - &m_sequence_uint32, - &m_sequence_int64, - &m_sequence_uint64, - &m_sequence_double, - &m_sequence_string); + RTCTestStats(const std::string& id, int64_t timestamp_us); + RTCTestStats(const RTCTestStats& other); + ~RTCTestStats() override; RTCStatsMember m_bool; RTCStatsMember m_int32;