Fixes for memory and threading issues in cast

This change fixes one issue in media/cast code.

TickClock is not owned by CastEnvironment
TickClock assigned to CastEnvironment is owned by the creator of
CastSender/CastReceiver. However CastEnvironment is ref-counted.
This means it is possible that TickClock is deleted but deferenced
by some objects that still uses CastEnvironment.

Fixing this requires a sweeping change in unit tests. And it also
affects End2End test. The result is that CastEnvironment is shared
between CastSender and CastReceiver. This adds requirements to
log filtering because now a CastEnvironment used by CastSender can
see receiver events.

Tested this patch with valgrind and cast_unittests reports no error.
BUG=336887

Review URL: https://codereview.chromium.org/145443005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@246863 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 18 insertions, 14 deletions

diff --git a/media/cast/audio_sender/audio_encoder_unittest.cc b/media/cast/audio_sender/audio_encoder_unittest.cc
index 95feab4..877f2f2 100644
--- a/media/cast/audio_sender/audio_encoder_unittest.cc
+++ b/media/cast/audio_sender/audio_encoder_unittest.cc
@@ -90,13 +90,15 @@ class AudioEncoderTest : public ::testing::TestWithParam<TestScenario> {
  public:
   AudioEncoderTest() {
     InitializeMediaLibraryForTesting();
-    testing_clock_.Advance(
+    testing_clock_ = new base::SimpleTestTickClock();
+    testing_clock_->Advance(
         base::TimeDelta::FromMilliseconds(kStartMillisecond));
   }
 
   virtual void SetUp() {
-    task_runner_ = new test::FakeTaskRunner(&testing_clock_);
-    cast_environment_ = new CastEnvironment(&testing_clock_, task_runner_,
+    task_runner_ = new test::FakeTaskRunner(testing_clock_);
+    cast_environment_ = new CastEnvironment(
+        scoped_ptr<base::TickClock>(testing_clock_).Pass(), task_runner_,
         task_runner_, task_runner_, task_runner_, task_runner_, task_runner_,
         GetDefaultCastSenderLoggingConfig());
   }
@@ -110,26 +112,26 @@ class AudioEncoderTest : public ::testing::TestWithParam<TestScenario> {
 
     CreateObjectsForCodec(codec);
 
-    receiver_->SetRecordedTimeLowerBound(testing_clock_.NowTicks());
+    receiver_->SetRecordedTimeLowerBound(testing_clock_->NowTicks());
     for (size_t i = 0; i < scenario.num_durations; ++i) {
       const base::TimeDelta duration =
           base::TimeDelta::FromMilliseconds(scenario.durations_in_ms[i]);
       receiver_->SetRecordedTimeUpperBound(
-          testing_clock_.NowTicks() + duration);
+          testing_clock_->NowTicks() + duration);
 
       const scoped_ptr<AudioBus> bus(
           audio_bus_factory_->NextAudioBus(duration));
 
       const int last_count = release_callback_count_;
       audio_encoder_->InsertAudio(
-          bus.get(), testing_clock_.NowTicks(),
+          bus.get(), testing_clock_->NowTicks(),
           base::Bind(&AudioEncoderTest::IncrementReleaseCallbackCounter,
                      base::Unretained(this)));
       task_runner_->RunTasks();
       EXPECT_EQ(1, release_callback_count_ - last_count)
           << "Release callback was not invoked once.";
 
-      testing_clock_.Advance(duration);
+      testing_clock_->Advance(duration);
     }
 
     DVLOG(1) << "Received " << receiver_->frames_received()
@@ -163,7 +165,7 @@ class AudioEncoderTest : public ::testing::TestWithParam<TestScenario> {
     ++release_callback_count_;
   }
 
-  base::SimpleTestTickClock testing_clock_;
+  base::SimpleTestTickClock* testing_clock_; // Owned by CastEnvironment.
   scoped_refptr<test::FakeTaskRunner> task_runner_;
   scoped_ptr<TestAudioBusFactory> audio_bus_factory_;
   scoped_ptr<TestEncodedAudioFrameReceiver> receiver_;
diff --git a/media/cast/audio_sender/audio_sender_unittest.cc b/media/cast/audio_sender/audio_sender_unittest.cc
index ecbd910..4fdff91 100644
--- a/media/cast/audio_sender/audio_sender_unittest.cc
+++ b/media/cast/audio_sender/audio_sender_unittest.cc
@@ -65,11 +65,13 @@ class AudioSenderTest : public ::testing::Test {
  protected:
   AudioSenderTest() {
     InitializeMediaLibraryForTesting();
-    testing_clock_.Advance(
+    testing_clock_ = new base::SimpleTestTickClock();
+    testing_clock_->Advance(
         base::TimeDelta::FromMilliseconds(kStartMillisecond));
-    task_runner_ = new test::FakeTaskRunner(&testing_clock_);
+    task_runner_ = new test::FakeTaskRunner(testing_clock_);
     cast_environment_ = new CastEnvironment(
-        &testing_clock_, task_runner_, task_runner_, task_runner_, task_runner_,
+        scoped_ptr<base::TickClock>(testing_clock_).Pass(),
+        task_runner_, task_runner_, task_runner_, task_runner_,
         task_runner_, task_runner_, GetDefaultCastSenderLoggingConfig());
     audio_config_.codec = transport::kOpus;
     audio_config_.use_external_encoder = false;
@@ -82,7 +84,7 @@ class AudioSenderTest : public ::testing::Test {
     transport_config.audio_rtp_payload_type = 127;
     transport_config.audio_channels = 2;
     transport_sender_.reset(new transport::CastTransportSenderImpl(
-        &testing_clock_,
+        testing_clock_,
         transport_config,
         base::Bind(&UpdateCastTransportStatus), task_runner_));
     transport_sender_->InsertFakeTransportForTesting(&transport_);
@@ -97,7 +99,7 @@ class AudioSenderTest : public ::testing::Test {
     EXPECT_EQ(status, transport::TRANSPORT_INITIALIZED);
   }
 
-  base::SimpleTestTickClock testing_clock_;
+  base::SimpleTestTickClock* testing_clock_;  // Owned by CastEnvironment.
   TestPacketSender transport_;
   scoped_ptr<transport::CastTransportSenderImpl> transport_sender_;
   scoped_refptr<test::FakeTaskRunner> task_runner_;
@@ -145,7 +147,7 @@ TEST_F(AudioSenderTest, RtcpTimer) {
   // Make sure that we send at least one RTCP packet.
   base::TimeDelta max_rtcp_timeout =
       base::TimeDelta::FromMilliseconds(1 + kDefaultRtcpIntervalMs * 3 / 2);
-  testing_clock_.Advance(max_rtcp_timeout);
+  testing_clock_->Advance(max_rtcp_timeout);
   task_runner_->RunTasks();
   EXPECT_GE(transport_.number_of_rtp_packets(), 1);
   EXPECT_EQ(transport_.number_of_rtcp_packets(), 1);