Refactoring: move ownership of RtcEventLog from Call to PeerConnection

webrtc/video/end_to_end_tests.cc

Index: webrtc/video/end_to_end_tests.cc
diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc
index 0e087f1c420e51659ba131c2c647484ea165ac44..9c9cf4d75c994ecd7a3dc9220aa93310eb74fbaa 100644
--- a/webrtc/video/end_to_end_tests.cc
+++ b/webrtc/video/end_to_end_tests.cc
@@ -23,6 +23,7 @@
 #include "webrtc/call.h"
 #include "webrtc/call/transport_adapter.h"
 #include "webrtc/common_video/include/frame_callback.h"
+#include "webrtc/logging/rtc_event_log/rtc_event_log.h"
 #include "webrtc/media/base/fakevideorenderer.h"
 #include "webrtc/modules/include/module_common_types.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h"
@@ -131,7 +132,9 @@ class EndToEndTest : public test::CallTest {
 };
 
 TEST_F(EndToEndTest, ReceiverCanBeStartedTwice) {
-  CreateCalls(Call::Config(), Call::Config());
+  Call::Config config;
+  config.event_log = &event_log_;
+  CreateCalls(config, config);

[stefan-webrtc, 2016/10/05 07:35:22]

What do you think of having a constructor of Call::Config which takes an event
log so that we can do:

CreateCalls(Call::Config(&event_log_), Call::Config(&event_log_));

?

[skvlad, 2016/10/06 01:31:38]

I think this would be adding test-only methods to production code, which is not
desirable. But then again, I also think that doing these extensive changes just
to get rid of ~5 "if" statements is not worth the time and effort.
I've added the constructor and started using it. Since we're already at it, I
made it the only available constructor and added a DCHECK to make sure it's not
null.

[stefan-webrtc, 2016/10/06 08:55:53]

If you prefer, you could have a helper method which does this instead:

Call::Config config = CreateConfigWithEventLog(event_log_);
CreateCalls(config, config);

It has the downside of not being able to DCHECK though.

[stefan-webrtc, 2016/10/06 09:18:06]

terelius pointed out that DCHECK is still possible, although we'd put it in the
Call ctor instead of the Call::Config ctor. Maybe that's a better option then?

I'm fine with both solutions.

[skvlad, 2016/10/07 01:35:04]

This was the approach I took in patch set #10. The helper method was defined in
fakewebrtccall.h.

 
   test::NullTransport transport;
   CreateSendConfig(1, 0, &transport);
@@ -146,7 +149,9 @@ TEST_F(EndToEndTest, ReceiverCanBeStartedTwice) {
 }
 
 TEST_F(EndToEndTest, ReceiverCanBeStoppedTwice) {
-  CreateCalls(Call::Config(), Call::Config());
+  Call::Config config;
+  config.event_log = &event_log_;
+  CreateCalls(config, config);
 
   test::NullTransport transport;
   CreateSendConfig(1, 0, &transport);
@@ -194,7 +199,9 @@ TEST_F(EndToEndTest, RendersSingleDelayedFrame) {
     rtc::Event event_;
   };
 
-  CreateCalls(Call::Config(), Call::Config());
+  Call::Config config;
+  config.event_log = &event_log_;
+  CreateCalls(config, config);
 
   test::DirectTransport sender_transport(sender_call_.get());
   test::DirectTransport receiver_transport(receiver_call_.get());
@@ -244,7 +251,9 @@ TEST_F(EndToEndTest, TransmitsFirstFrame) {
     rtc::Event event_;
   } renderer;
 
-  CreateCalls(Call::Config(), Call::Config());
+  Call::Config config;
+  config.event_log = &event_log_;
+  CreateCalls(config, config);
 
   test::DirectTransport sender_transport(sender_call_.get());
   test::DirectTransport receiver_transport(receiver_call_.get());
@@ -772,6 +781,7 @@ TEST_F(EndToEndTest, ReceivedFecPacketsNotNacked) {
     // is 10 kbps.
     Call::Config GetSenderCallConfig() override {
       Call::Config config;
+      config.event_log = &event_log_;
       const int kMinBitrateBps = 30000;
       config.bitrate_config.min_bitrate_bps = kMinBitrateBps;
       return config;
@@ -1102,7 +1112,9 @@ TEST_F(EndToEndTest, UnknownRtpPacketGivesUnknownSsrcReturnCode) {
     rtc::Event delivered_packet_;
   };
 
-  CreateCalls(Call::Config(), Call::Config());
+  Call::Config config;
+  config.event_log = &event_log_;
+  CreateCalls(config, config);
 
   test::DirectTransport send_transport(sender_call_.get());
   test::DirectTransport receive_transport(receiver_call_.get());
@@ -1243,8 +1255,11 @@ class MultiStreamTest {
   virtual ~MultiStreamTest() {}
 
   void RunTest() {
-    std::unique_ptr<Call> sender_call(Call::Create(Call::Config()));
-    std::unique_ptr<Call> receiver_call(Call::Create(Call::Config()));
+    webrtc::RtcEventLogNullImpl event_log;
+    Call::Config config;
+    config.event_log = &event_log;
+    std::unique_ptr<Call> sender_call(Call::Create(config));
+    std::unique_ptr<Call> receiver_call(Call::Create(config));
     std::unique_ptr<test::DirectTransport> sender_transport(
         CreateSendTransport(sender_call.get()));
     std::unique_ptr<test::DirectTransport> receiver_transport(
@@ -1739,7 +1754,9 @@ TEST_F(EndToEndTest, ObserversEncodedFrames) {
   EncodedFrameTestObserver post_encode_observer;
   EncodedFrameTestObserver pre_decode_observer;
 
-  CreateCalls(Call::Config(), Call::Config());
+  Call::Config config;
+  config.event_log = &event_log_;
+  CreateCalls(config, config);
 
   test::DirectTransport sender_transport(sender_call_.get());
   test::DirectTransport receiver_transport(receiver_call_.get());
@@ -1891,6 +1908,7 @@ TEST_F(EndToEndTest, RembWithSendSideBwe) {
 
     Call::Config GetSenderCallConfig() override {
       Call::Config config;
+      config.event_log = &event_log_;
       // Set a high start bitrate to reduce the test completion time.
       config.bitrate_config.start_bitrate_bps = remb_bitrate_bps_;
       return config;
@@ -3268,7 +3286,9 @@ void EndToEndTest::TestRtpStatePreservation(bool use_rtx,
     std::map<uint32_t, bool> ssrc_observed_ GUARDED_BY(crit_);
   } observer(use_rtx);
 
-  CreateCalls(Call::Config(), Call::Config());
+  Call::Config config;
+  config.event_log = &event_log_;
+  CreateCalls(config, config);
 
   test::PacketTransport send_transport(sender_call_.get(), &observer,
                                        test::PacketTransport::kSender,
@@ -3563,8 +3583,9 @@ TEST_F(EndToEndTest, RespectsNetworkState) {
 TEST_F(EndToEndTest, CallReportsRttForSender) {
   static const int kSendDelayMs = 30;
   static const int kReceiveDelayMs = 70;
-
-  CreateCalls(Call::Config(), Call::Config());
+  Call::Config call_config;
+  call_config.event_log = &event_log_;
+  CreateCalls(call_config, call_config);
 
   FakeNetworkPipe::Config config;
   config.queue_delay_ms = kSendDelayMs;
@@ -3606,7 +3627,9 @@ void EndToEndTest::VerifyNewVideoSendStreamsRespectNetworkState(
     MediaType network_to_bring_down,
     VideoEncoder* encoder,
     Transport* transport) {
-  CreateSenderCall(Call::Config());
+  Call::Config config;
+  config.event_log = &event_log_;
+  CreateSenderCall(config);
   sender_call_->SignalChannelNetworkState(network_to_bring_down, kNetworkDown);
 
   CreateSendConfig(1, 0, transport);
@@ -3625,7 +3648,9 @@ void EndToEndTest::VerifyNewVideoSendStreamsRespectNetworkState(
 void EndToEndTest::VerifyNewVideoReceiveStreamsRespectNetworkState(
     MediaType network_to_bring_down,
     Transport* transport) {
-  CreateCalls(Call::Config(), Call::Config());
+  Call::Config config;
+  config.event_log = &event_log_;
+  CreateCalls(config, config);
   receiver_call_->SignalChannelNetworkState(network_to_bring_down,
                                             kNetworkDown);