Skip to content

Commit

Permalink
Removing warning suppression flags from pc/.
Browse files Browse the repository at this point in the history 
Bug: webrtc:9251
Change-Id: Ic12126fc03309448fe71a17e6b65343949496f4f
Reviewed-on: https://webrtc-review.googlesource.com/86820
Commit-Queue: Mirko Bonadei <[email protected]>
Reviewed-by: Karl Wiberg <[email protected]>
Cr-Commit-Position: refs/heads/master@{#23838}
  • Loading branch information
@MirkoBonadei
MirkoBonadei authored and Commit Bot committed Jul 4, 2018
1 parent 36c69d5 commit e12c1fe
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 96 deletions.
46 changes: 1 addition & 45 deletions pc/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,6 @@ rtc_source_set("rtc_pc") {
]
}

config("libjingle_peerconnection_warnings_config") {
# GN orders flags on a target before flags from configs. The default config
# adds these flags so to cancel them out they need to come from a config and
# cannot be on the target directly.
if (!is_win && !is_clang) {
cflags = [ "-Wno-maybe-uninitialized" ] # Only exists for GCC.
}
}

rtc_static_library("peerconnection") {
visibility = [ "*" ]
cflags = []
Expand Down Expand Up @@ -182,8 +173,6 @@ rtc_static_library("peerconnection") {
"webrtcsessiondescriptionfactory.h",
]

configs += [ ":libjingle_peerconnection_warnings_config" ]

if (!build_with_chromium && is_clang) {
# Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163).
suppressed_configs += [ "//build/config/clang:find_bad_constructs" ]
Expand Down Expand Up @@ -249,8 +238,6 @@ rtc_static_library("create_pc_factory") {
"../rtc_base:rtc_base_approved",
]

configs += [ ":libjingle_peerconnection_warnings_config" ]

if (!build_with_chromium && is_clang) {
# Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163).
suppressed_configs += [ "//build/config/clang:find_bad_constructs" ]
Expand All @@ -271,15 +258,6 @@ rtc_source_set("libjingle_peerconnection") {
}

if (rtc_include_tests) {
config("rtc_pc_unittests_config") {
# GN orders flags on a target before flags from configs. The default config
# adds -Wall, and this flag have to be after -Wall -- so they need to
# come from a config and can't be on the target directly.
if (!is_win && !is_clang) {
cflags = [ "-Wno-maybe-uninitialized" ] # Only exists for GCC.
}
}

rtc_test("rtc_pc_unittests") {
testonly = true

Expand All @@ -301,8 +279,6 @@ if (rtc_include_tests) {

include_dirs = [ "//third_party/libsrtp/srtp" ]

configs += [ ":rtc_pc_unittests_config" ]

if (!build_with_chromium && is_clang) {
# Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163).
suppressed_configs += [ "//build/config/clang:find_bad_constructs" ]
Expand Down Expand Up @@ -440,25 +416,6 @@ if (rtc_include_tests) {
}
}

config("peerconnection_unittests_config") {
# The warnings below are enabled by default. Since GN orders compiler flags
# for a target before flags from configs, the only way to disable such
# warnings is by having them in a separate config, loaded from the target.
# TODO(kjellander): Make the code compile without disabling these flags.
# See https://bugs.webrtc.org/3307.
if (is_clang && is_win) {
cflags = [
# See https://bugs.chromium.org/p/webrtc/issues/detail?id=6267
# for -Wno-sign-compare
"-Wno-sign-compare",
]
}

if (!is_win) {
cflags = [ "-Wno-sign-compare" ]
}
}

rtc_test("peerconnection_unittests") {
testonly = true
sources = [
Expand Down Expand Up @@ -505,8 +462,6 @@ if (rtc_include_tests) {
defines = [ "HAVE_SCTP" ]
}

configs += [ ":peerconnection_unittests_config" ]

if (!build_with_chromium && is_clang) {
# Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163).
suppressed_configs += [ "//build/config/clang:find_bad_constructs" ]
Expand Down Expand Up @@ -562,6 +517,7 @@ if (rtc_include_tests) {
"../rtc_base:rtc_base_tests_main",
"../rtc_base:rtc_base_tests_utils",
"../rtc_base:rtc_task_queue",
"../rtc_base:safe_conversions",
"../system_wrappers:metrics_default",
"../system_wrappers:runtime_enabled_features_default",
"../test:audio_codec_mocks",
Expand Down
4 changes: 3 additions & 1 deletion pc/datachannel_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "pc/sctputils.h"
#include "pc/test/fakedatachannelprovider.h"
#include "rtc_base/gunit.h"
#include "rtc_base/numerics/safe_conversions.h"

using webrtc::DataChannel;
using webrtc::SctpSidAllocator;
Expand Down Expand Up @@ -172,7 +173,8 @@ TEST_F(SctpDataChannelTest, BufferedAmountWhenBlocked) {
}
EXPECT_EQ(buffer.data.size() * number_of_packets,
webrtc_data_channel_->buffered_amount());
EXPECT_EQ(number_of_packets, observer_->on_buffered_amount_change_count());
EXPECT_EQ(rtc::checked_cast<size_t>(number_of_packets),
observer_->on_buffered_amount_change_count());
}

// Tests that the queued data are sent when the channel transitions from blocked
Expand Down
31 changes: 16 additions & 15 deletions pc/peerconnection_integrationtest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
#include "rtc_base/fakenetwork.h"
#include "rtc_base/firewallsocketserver.h"
#include "rtc_base/gunit.h"
#include "rtc_base/numerics/safe_conversions.h"
#include "rtc_base/testcertificateverifier.h"
#include "rtc_base/virtualsocketserver.h"
#include "test/gmock.h"
Expand Down Expand Up @@ -1595,8 +1596,8 @@ TEST_P(PeerConnectionIntegrationTest,
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
// Should be one receiver each for audio/video.
EXPECT_EQ(2, caller()->rtp_receiver_observers().size());
EXPECT_EQ(2, callee()->rtp_receiver_observers().size());
EXPECT_EQ(2U, caller()->rtp_receiver_observers().size());
EXPECT_EQ(2U, callee()->rtp_receiver_observers().size());
// Wait for all "first packet received" callbacks to be fired.
EXPECT_TRUE_WAIT(
std::all_of(caller()->rtp_receiver_observers().begin(),
Expand All @@ -1616,8 +1617,8 @@ TEST_P(PeerConnectionIntegrationTest,
// callback should still be invoked.
caller()->ResetRtpReceiverObservers();
callee()->ResetRtpReceiverObservers();
EXPECT_EQ(2, caller()->rtp_receiver_observers().size());
EXPECT_EQ(2, callee()->rtp_receiver_observers().size());
EXPECT_EQ(2U, caller()->rtp_receiver_observers().size());
EXPECT_EQ(2U, callee()->rtp_receiver_observers().size());
EXPECT_TRUE(
std::all_of(caller()->rtp_receiver_observers().begin(),
caller()->rtp_receiver_observers().end(),
Expand Down Expand Up @@ -1903,7 +1904,7 @@ TEST_P(PeerConnectionIntegrationTest, AudioToVideoUpgrade) {
// The caller creates a new transceiver to receive video on when receiving
// the offer, but by default it is send only.
auto transceivers = caller()->pc()->GetTransceivers();
ASSERT_EQ(3, transceivers.size());
ASSERT_EQ(3U, transceivers.size());
ASSERT_EQ(cricket::MEDIA_TYPE_VIDEO,
transceivers[2]->receiver()->media_type());
transceivers[2]->sender()->SetTrack(caller()->CreateLocalVideoTrack());
Expand Down Expand Up @@ -2590,7 +2591,7 @@ TEST_P(PeerConnectionIntegrationTest,
//
// Also, we use "EXPECT_TRUE_WAIT" because the stats collector may decide to
// return cached stats if not enough time has passed since the last update.
EXPECT_TRUE_WAIT(callee()->OldGetStats()->BytesReceived() > 0U,
EXPECT_TRUE_WAIT(callee()->OldGetStats()->BytesReceived() > 0,
kDefaultTimeout);
}

Expand Down Expand Up @@ -3209,10 +3210,10 @@ TEST_P(PeerConnectionIntegrationTest, StressTestUnorderedSctpDataChannel) {
}

// Wait for all messages to be received.
EXPECT_EQ_WAIT(kNumMessages,
EXPECT_EQ_WAIT(rtc::checked_cast<size_t>(kNumMessages),
caller()->data_observer()->received_message_count(),
kDefaultTimeout);
EXPECT_EQ_WAIT(kNumMessages,
EXPECT_EQ_WAIT(rtc::checked_cast<size_t>(kNumMessages),
callee()->data_observer()->received_message_count(),
kDefaultTimeout);

Expand Down Expand Up @@ -3517,17 +3518,17 @@ TEST_P(PeerConnectionIntegrationIceStatesTest, VerifyBestConnection) {
if (TestIPv6()) {
// When IPv6 is enabled, we should prefer an IPv6 connection over an IPv4
// connection.
EXPECT_EQ(0u, num_best_ipv4);
EXPECT_EQ(1u, num_best_ipv6);
EXPECT_EQ(0, num_best_ipv4);
EXPECT_EQ(1, num_best_ipv6);
} else {
EXPECT_EQ(1u, num_best_ipv4);
EXPECT_EQ(0u, num_best_ipv6);
EXPECT_EQ(1, num_best_ipv4);
EXPECT_EQ(0, num_best_ipv6);
}

EXPECT_EQ(0u, metrics_observer->GetEnumCounter(
EXPECT_EQ(0, metrics_observer->GetEnumCounter(
webrtc::kEnumCounterIceCandidatePairTypeUdp,
webrtc::kIceCandidatePairHostHost));
EXPECT_EQ(1u, metrics_observer->GetEnumCounter(
EXPECT_EQ(1, metrics_observer->GetEnumCounter(
webrtc::kEnumCounterIceCandidatePairTypeUdp,
webrtc::kIceCandidatePairHostPublicHostPublic));
}
Expand Down Expand Up @@ -3829,7 +3830,7 @@ TEST_F(PeerConnectionIntegrationTestPlanB, CanSendRemoteVideoTrack) {
caller()->AddVideoTrack();
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kMaxWaitForActivationMs);
ASSERT_EQ(1, callee()->remote_streams()->count());
ASSERT_EQ(1U, callee()->remote_streams()->count());

// Echo the stream back, and do a new offer/anwer (initiated by callee this
// time).
Expand Down
12 changes: 6 additions & 6 deletions pc/peerconnectionendtoend_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,8 @@ TEST_P(PeerConnectionEndToEndTest, CallWithCustomCodec) {
// the same ID because they were passed to the same PeerConnectionFactory,
// and the second pair got the same ID---but these two IDs are not equal,
// because each PeerConnectionFactory has its own ID.
EXPECT_EQ(1, encoder_id1.size());
EXPECT_EQ(1, encoder_id2.size());
EXPECT_EQ(1U, encoder_id1.size());
EXPECT_EQ(1U, encoder_id2.size());
EXPECT_EQ(encoder_id1, decoder_id1);
EXPECT_EQ(encoder_id2, decoder_id2);
EXPECT_NE(encoder_id1, encoder_id2);
Expand Down Expand Up @@ -540,16 +540,16 @@ TEST_P(PeerConnectionEndToEndTest, DataChannelIdAssignment) {
Negotiate();
WaitForConnection();

EXPECT_EQ(1U, caller_dc_1->id() % 2);
EXPECT_EQ(0U, callee_dc_1->id() % 2);
EXPECT_EQ(1, caller_dc_1->id() % 2);
EXPECT_EQ(0, callee_dc_1->id() % 2);

rtc::scoped_refptr<DataChannelInterface> caller_dc_2(
caller_->CreateDataChannel("data", init));
rtc::scoped_refptr<DataChannelInterface> callee_dc_2(
callee_->CreateDataChannel("data", init));

EXPECT_EQ(1U, caller_dc_2->id() % 2);
EXPECT_EQ(0U, callee_dc_2->id() % 2);
EXPECT_EQ(1, caller_dc_2->id() % 2);
EXPECT_EQ(0, callee_dc_2->id() % 2);
}

// Verifies that the message is received by the right remote DataChannel when
Expand Down
4 changes: 2 additions & 2 deletions pc/peerconnectioninterface_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3408,13 +3408,13 @@ TEST_P(PeerConnectionInterfaceTest, SetConfigurationCausingPartialIceRestart) {

// Grab the ufrags.
std::vector<std::string> initial_ufrags = GetUfrags(pc_->local_description());
ASSERT_EQ(2, initial_ufrags.size());
ASSERT_EQ(2U, initial_ufrags.size());

// Create offer and grab the new ufrags.
CreateOfferAsLocalDescription();
std::vector<std::string> subsequent_ufrags =
GetUfrags(pc_->local_description());
ASSERT_EQ(2, subsequent_ufrags.size());
ASSERT_EQ(2U, subsequent_ufrags.size());

// Ensure that only the ufrag for the second m= section changed.
EXPECT_EQ(initial_ufrags[0], subsequent_ufrags[0]);
Expand Down
2 changes: 1 addition & 1 deletion pc/rtcstats_integrationtest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ class RTCStatsReportVerifier {
bool verify_successful = true;
std::vector<const RTCTransportStats*> transport_stats =
report_->GetStatsOfType<RTCTransportStats>();
EXPECT_EQ(transport_stats.size(), 1);
EXPECT_EQ(transport_stats.size(), 1U);
std::string selected_candidate_pair_id =
*transport_stats[0]->selected_candidate_pair_id;
for (const RTCStats& stats : *report_) {
Expand Down
22 changes: 11 additions & 11 deletions pc/rtcstatscollector_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ template <typename T>
std::string IdForType(const RTCStatsReport* report) {
auto stats_of_my_type = report->RTCStatsReport::GetStatsOfType<T>();
// We cannot use ASSERT here, since we're within a function.
EXPECT_EQ(1, stats_of_my_type.size())
EXPECT_EQ(1U, stats_of_my_type.size())
<< "Unexpected number of stats of this type";
if (stats_of_my_type.size() == 1) {
return stats_of_my_type[0]->id();
Expand Down Expand Up @@ -1470,9 +1470,9 @@ TEST_F(RTCStatsCollectorTest,
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();

auto stats_of_my_type = report->GetStatsOfType<RTCMediaStreamStats>();
ASSERT_EQ(1, stats_of_my_type.size()) << "No stream in " << report->ToJson();
ASSERT_EQ(1U, stats_of_my_type.size()) << "No stream in " << report->ToJson();
auto stats_of_track_type = report->GetStatsOfType<RTCMediaStreamTrackStats>();
ASSERT_EQ(1, stats_of_track_type.size())
ASSERT_EQ(1U, stats_of_track_type.size())
<< "Wrong number of tracks in " << report->ToJson();

RTCMediaStreamStats expected_local_stream(stats_of_my_type[0]->id(),
Expand Down Expand Up @@ -1533,9 +1533,9 @@ TEST_F(RTCStatsCollectorTest,
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();

auto stats_of_my_type = report->GetStatsOfType<RTCMediaStreamStats>();
ASSERT_EQ(1, stats_of_my_type.size()) << "No stream in " << report->ToJson();
ASSERT_EQ(1U, stats_of_my_type.size()) << "No stream in " << report->ToJson();
auto stats_of_track_type = report->GetStatsOfType<RTCMediaStreamTrackStats>();
ASSERT_EQ(1, stats_of_track_type.size())
ASSERT_EQ(1U, stats_of_track_type.size())
<< "Wrong number of tracks in " << report->ToJson();
ASSERT_TRUE(*(stats_of_track_type[0]->remote_source));

Expand Down Expand Up @@ -1598,7 +1598,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Audio) {
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();

auto stats_of_track_type = report->GetStatsOfType<RTCMediaStreamTrackStats>();
ASSERT_EQ(1, stats_of_track_type.size());
ASSERT_EQ(1U, stats_of_track_type.size());

RTCInboundRTPStreamStats expected_audio("RTCInboundRTPAudioStream_1",
report->timestamp_us());
Expand Down Expand Up @@ -1775,9 +1775,9 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) {
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();

auto stats_of_my_type = report->GetStatsOfType<RTCOutboundRTPStreamStats>();
ASSERT_EQ(1, stats_of_my_type.size());
ASSERT_EQ(1U, stats_of_my_type.size());
auto stats_of_track_type = report->GetStatsOfType<RTCMediaStreamTrackStats>();
ASSERT_EQ(1, stats_of_track_type.size());
ASSERT_EQ(1U, stats_of_track_type.size());

RTCOutboundRTPStreamStats expected_video(stats_of_my_type[0]->id(),
report->timestamp_us());
Expand Down Expand Up @@ -2086,11 +2086,11 @@ TEST_F(RTCStatsCollectorTest, StatsReportedOnZeroSsrc) {

std::vector<const RTCMediaStreamTrackStats*> track_stats =
report->GetStatsOfType<RTCMediaStreamTrackStats>();
EXPECT_EQ(1, track_stats.size());
EXPECT_EQ(1U, track_stats.size());

std::vector<const RTCRTPStreamStats*> rtp_stream_stats =
report->GetStatsOfType<RTCRTPStreamStats>();
EXPECT_EQ(0, rtp_stream_stats.size());
EXPECT_EQ(0U, rtp_stream_stats.size());
}

TEST_F(RTCStatsCollectorTest, DoNotCrashOnSsrcChange) {
Expand All @@ -2106,7 +2106,7 @@ TEST_F(RTCStatsCollectorTest, DoNotCrashOnSsrcChange) {

std::vector<const RTCMediaStreamTrackStats*> track_stats =
report->GetStatsOfType<RTCMediaStreamTrackStats>();
EXPECT_EQ(1, track_stats.size());
EXPECT_EQ(1U, track_stats.size());
}

// Used for test below, to test calling GetStatsReport during a callback.
Expand Down
Loading

0 comments on commit e12c1fe

Please sign in to comment.