Correct data histogram entry for incoming DC
Both incoming and outgoing datachannels should cause
the DATA_ADDED flag to be set.
This CL also moves all tests into their own file, and
improves scaffolding.
Bug: chromium:718508
Change-Id: I5c4c257ccb6f26799f7593bce8b27ebf59015b1e
Reviewed-on: https://webrtc-review.googlesource.com/85348
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23766}diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index d953833..15966ba 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -1040,10 +1040,10 @@
RtpTransceiverProxyWithInternal<RtpTransceiver>::Create(
signaling_thread(), new RtpTransceiver(cricket::MEDIA_TYPE_VIDEO)));
}
- signaling_thread()->PostDelayed(
- RTC_FROM_HERE,
- return_histogram_very_quickly_ ? 0 : REPORT_USAGE_PATTERN_DELAY_MS, this,
- MSG_REPORT_USAGE_PATTERN, nullptr);
+ int delay_ms =
+ return_histogram_very_quickly_ ? 0 : REPORT_USAGE_PATTERN_DELAY_MS;
+ signaling_thread()->PostDelayed(RTC_FROM_HERE, delay_ms, this,
+ MSG_REPORT_USAGE_PATTERN, nullptr);
return true;
}
@@ -4547,6 +4547,7 @@
rtc::scoped_refptr<DataChannelInterface> proxy_channel =
DataChannelProxy::Create(signaling_thread(), channel);
observer_->OnDataChannel(std::move(proxy_channel));
+ NoteUsageEvent(UsageEvent::DATA_ADDED);
}
rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
diff --git a/pc/peerconnection_histogram_unittest.cc b/pc/peerconnection_histogram_unittest.cc
index b554e86..e6255d5 100644
--- a/pc/peerconnection_histogram_unittest.cc
+++ b/pc/peerconnection_histogram_unittest.cc
@@ -18,9 +18,6 @@
#include "pc/peerconnectionfactory.h"
#include "pc/peerconnectionwrapper.h"
#include "pc/sdputils.h"
-#ifdef WEBRTC_ANDROID
-#include "pc/test/androidtestinitializer.h"
-#endif
#include "pc/test/fakesctptransport.h"
#include "rtc_base/gunit.h"
#include "rtc_base/ptr_util.h"
@@ -64,7 +61,23 @@
void ReturnHistogramVeryQuickly() { return_histogram_very_quickly_ = true; }
private:
- bool return_histogram_very_quickly_;
+ bool return_histogram_very_quickly_ = false;
+};
+
+class PeerConnectionWrapperForUsageHistogramTest;
+typedef PeerConnectionWrapperForUsageHistogramTest* RawWrapperPtr;
+
+class ObserverForUsageHistogramTest : public MockPeerConnectionObserver {
+ public:
+ void OnIceCandidate(const webrtc::IceCandidateInterface* candidate) override;
+ void PrepareToExchangeCandidates(RawWrapperPtr other) {
+ candidate_target_ = other;
+ }
+
+ bool HaveDataChannel() { return last_datachannel_; }
+
+ private:
+ RawWrapperPtr candidate_target_; // Note: Not thread-safe against deletions.
};
class PeerConnectionWrapperForUsageHistogramTest
@@ -78,8 +91,35 @@
pc());
return static_cast<PeerConnection*>(pci->internal());
}
+
+ void PrepareToExchangeCandidates(
+ PeerConnectionWrapperForUsageHistogramTest* other) {
+ static_cast<ObserverForUsageHistogramTest*>(observer())
+ ->PrepareToExchangeCandidates(other);
+ static_cast<ObserverForUsageHistogramTest*>(other->observer())
+ ->PrepareToExchangeCandidates(this);
+ }
+
+ bool IsConnected() {
+ return pc()->ice_connection_state() ==
+ PeerConnectionInterface::kIceConnectionConnected ||
+ pc()->ice_connection_state() ==
+ PeerConnectionInterface::kIceConnectionCompleted;
+ }
+
+ bool HaveDataChannel() {
+ return static_cast<ObserverForUsageHistogramTest*>(observer())
+ ->HaveDataChannel();
+ }
};
+void ObserverForUsageHistogramTest::OnIceCandidate(
+ const webrtc::IceCandidateInterface* candidate) {
+ if (candidate_target_) {
+ candidate_target_->pc()->AddIceCandidate(candidate);
+ }
+}
+
class PeerConnectionUsageHistogramTest : public ::testing::Test {
protected:
typedef std::unique_ptr<PeerConnectionWrapperForUsageHistogramTest>
@@ -87,9 +127,6 @@
PeerConnectionUsageHistogramTest()
: vss_(new rtc::VirtualSocketServer()), main_(vss_.get()) {
-#ifdef WEBRTC_ANDROID
- InitializeAndroidObjects();
-#endif
}
WrapperPtr CreatePeerConnection() {
@@ -113,7 +150,7 @@
if (immediate_report) {
pc_factory->ReturnHistogramVeryQuickly();
}
- auto observer = rtc::MakeUnique<MockPeerConnectionObserver>();
+ auto observer = rtc::MakeUnique<ObserverForUsageHistogramTest>();
auto pc = pc_factory->CreatePeerConnection(config, nullptr, nullptr,
observer.get());
if (!pc) {
@@ -142,4 +179,67 @@
kDefaultTimeout);
}
+#ifndef WEBRTC_ANDROID
+// These tests do not work on Android. Why is unclear.
+// https://bugs.webrtc.org/9461
+
+// Test getting the usage fingerprint for an audio/video connection.
+TEST_F(PeerConnectionUsageHistogramTest, FingerprintAudioVideo) {
+ auto caller = CreatePeerConnection();
+ auto callee = CreatePeerConnection();
+ // Register UMA observer before signaling begins.
+ auto caller_observer = caller->RegisterFakeMetricsObserver();
+ auto callee_observer = callee->RegisterFakeMetricsObserver();
+ caller->AddAudioTrack("audio");
+ caller->AddVideoTrack("video");
+ caller->PrepareToExchangeCandidates(callee.get());
+ ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+ ASSERT_TRUE_WAIT(caller->IsConnected(), kDefaultTimeout);
+ ASSERT_TRUE_WAIT(callee->IsConnected(), kDefaultTimeout);
+ caller->pc()->Close();
+ callee->pc()->Close();
+ int expected_fingerprint = MakeUsageFingerprint(
+ {PeerConnection::UsageEvent::AUDIO_ADDED,
+ PeerConnection::UsageEvent::VIDEO_ADDED,
+ PeerConnection::UsageEvent::SET_LOCAL_DESCRIPTION_CALLED,
+ PeerConnection::UsageEvent::SET_REMOTE_DESCRIPTION_CALLED,
+ PeerConnection::UsageEvent::CANDIDATE_COLLECTED,
+ PeerConnection::UsageEvent::REMOTE_CANDIDATE_ADDED,
+ PeerConnection::UsageEvent::ICE_STATE_CONNECTED,
+ PeerConnection::UsageEvent::CLOSE_CALLED});
+ EXPECT_TRUE(caller_observer->ExpectOnlySingleEnumCount(
+ webrtc::kEnumCounterUsagePattern, expected_fingerprint));
+ EXPECT_TRUE(callee_observer->ExpectOnlySingleEnumCount(
+ webrtc::kEnumCounterUsagePattern, expected_fingerprint));
+}
+
+#ifdef HAVE_SCTP
+TEST_F(PeerConnectionUsageHistogramTest, FingerprintDataOnly) {
+ auto caller = CreatePeerConnection();
+ auto callee = CreatePeerConnection();
+ // Register UMA observer before signaling begins.
+ auto caller_observer = caller->RegisterFakeMetricsObserver();
+ auto callee_observer = callee->RegisterFakeMetricsObserver();
+ caller->CreateDataChannel("foodata");
+ caller->PrepareToExchangeCandidates(callee.get());
+ ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+ ASSERT_TRUE_WAIT(callee->HaveDataChannel(), kDefaultTimeout);
+ caller->pc()->Close();
+ callee->pc()->Close();
+ int expected_fingerprint = MakeUsageFingerprint(
+ {PeerConnection::UsageEvent::DATA_ADDED,
+ PeerConnection::UsageEvent::SET_LOCAL_DESCRIPTION_CALLED,
+ PeerConnection::UsageEvent::SET_REMOTE_DESCRIPTION_CALLED,
+ PeerConnection::UsageEvent::CANDIDATE_COLLECTED,
+ PeerConnection::UsageEvent::REMOTE_CANDIDATE_ADDED,
+ PeerConnection::UsageEvent::ICE_STATE_CONNECTED,
+ PeerConnection::UsageEvent::CLOSE_CALLED});
+ EXPECT_TRUE(caller_observer->ExpectOnlySingleEnumCount(
+ webrtc::kEnumCounterUsagePattern, expected_fingerprint));
+ EXPECT_TRUE(callee_observer->ExpectOnlySingleEnumCount(
+ webrtc::kEnumCounterUsagePattern, expected_fingerprint));
+}
+#endif // HAVE_SCTP
+#endif // WEBRTC_ANDROID
+
} // namespace webrtc
diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc
index c30ecef..dc0f06f 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -178,14 +178,6 @@
return -1;
}
-int MakeUsageFingerprint(std::set<PeerConnection::UsageEvent> events) {
- int signature = 0;
- for (const auto it : events) {
- signature |= static_cast<int>(it);
- }
- return signature;
-}
-
class SignalingMessageReceiver {
public:
virtual void ReceiveSdpMessage(SdpType type, const std::string& msg) = 0;
@@ -4633,38 +4625,6 @@
ASSERT_TRUE(ExpectNewFrames(media_expectations));
}
-// Test getting the usage fingerprint for a simple test case.
-TEST_P(PeerConnectionIntegrationTest, UsageFingerprintHistogram) {
- ASSERT_TRUE(CreatePeerConnectionWrappers());
- ConnectFakeSignaling();
- // Register UMA observer before signaling begins.
- rtc::scoped_refptr<webrtc::FakeMetricsObserver> caller_observer =
- new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
- caller()->pc()->RegisterUMAObserver(caller_observer);
- rtc::scoped_refptr<webrtc::FakeMetricsObserver> callee_observer =
- new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
- callee()->pc()->RegisterUMAObserver(callee_observer);
- caller()->AddAudioTrack();
- caller()->AddVideoTrack();
- caller()->CreateAndSetAndSignalOffer();
- ASSERT_TRUE_WAIT(DtlsConnected(), kDefaultTimeout);
- caller()->pc()->Close();
- callee()->pc()->Close();
- int expected_fingerprint = MakeUsageFingerprint(
- {PeerConnection::UsageEvent::AUDIO_ADDED,
- PeerConnection::UsageEvent::VIDEO_ADDED,
- PeerConnection::UsageEvent::SET_LOCAL_DESCRIPTION_CALLED,
- PeerConnection::UsageEvent::SET_REMOTE_DESCRIPTION_CALLED,
- PeerConnection::UsageEvent::CANDIDATE_COLLECTED,
- PeerConnection::UsageEvent::REMOTE_CANDIDATE_ADDED,
- PeerConnection::UsageEvent::ICE_STATE_CONNECTED,
- PeerConnection::UsageEvent::CLOSE_CALLED});
- EXPECT_TRUE(caller_observer->ExpectOnlySingleEnumCount(
- webrtc::kEnumCounterUsagePattern, expected_fingerprint));
- EXPECT_TRUE(callee_observer->ExpectOnlySingleEnumCount(
- webrtc::kEnumCounterUsagePattern, expected_fingerprint));
-}
-
INSTANTIATE_TEST_CASE_P(
PeerConnectionIntegrationTest,
PeerConnectionIntegrationInteropTest,