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}

pc/peerconnection.cc

@@ -1040,9 +1040,9 @@ bool PeerConnection::Initialize(
         RtpTransceiverProxyWithInternal<RtpTransceiver>::Create(
             signaling_thread(), new RtpTransceiver(cricket::MEDIA_TYPE_VIDEO)));
   }
-  signaling_thread()->PostDelayed(
+  int delay_ms =
-      RTC_FROM_HERE,
+      return_histogram_very_quickly_ ? 0 : REPORT_USAGE_PATTERN_DELAY_MS;
-      return_histogram_very_quickly_ ? 0 : REPORT_USAGE_PATTERN_DELAY_MS, this,
+  signaling_thread()->PostDelayed(RTC_FROM_HERE, delay_ms, this,
                                   MSG_REPORT_USAGE_PATTERN, nullptr);
   return true;
 }
@@ -4547,6 +4547,7 @@ void PeerConnection::OnDataChannelOpenMessage(
   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>>

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 @@ class PeerConnectionFactoryForUsageHistogramTest
   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 @@ class PeerConnectionWrapperForUsageHistogramTest
             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 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
 
   PeerConnectionUsageHistogramTest()
       : vss_(new rtc::VirtualSocketServer()), main_(vss_.get()) {
-#ifdef WEBRTC_ANDROID
-    InitializeAndroidObjects();
-#endif
   }
 
   WrapperPtr CreatePeerConnection() {
@@ -113,7 +150,7 @@ class PeerConnectionUsageHistogramTest : public ::testing::Test {
     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 @@ TEST_F(PeerConnectionUsageHistogramTest, UsageFingerprintHistogramFromTimeout) {
                    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

pc/peerconnection_integrationtest.cc

@@ -178,14 +178,6 @@ int FindFirstMediaStatsIndexByKind(
   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 @@ TEST_P(PeerConnectionIntegrationInteropTest,
   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,