summaryrefslogtreecommitdiffstats
path: root/media/base/pipeline_unittest.cc
diff options
context:
space:
mode:
authorxhwang@chromium.org <xhwang@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-06 05:46:06 +0000
committerxhwang@chromium.org <xhwang@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-06 05:46:06 +0000
commit61949f628a7da3d98c6a4913eac026abcf6037ca (patch)
tree0f9197869e41d2da4732af0b253b58d0bf7eddde /media/base/pipeline_unittest.cc
parent92d9392f1f567a27734093cf04b2993005b1c31b (diff)
downloadchromium_src-61949f628a7da3d98c6a4913eac026abcf6037ca.zip
chromium_src-61949f628a7da3d98c6a4913eac026abcf6037ca.tar.gz
chromium_src-61949f628a7da3d98c6a4913eac026abcf6037ca.tar.bz2
Pipeline: Use WeakPtr for DemuxerHost Calls and Tasks.
There is a chance that the Demuxer calls host_->OnDemuxerError() right before Stop() is called. The Pipeline always posts a ErrorChangedTask() for OnDemuxerError() with base::Retained(this). After the Demuxer fires the stop callback, the Pipeline could be destroyed immediately. So If the media thread hasn't been destroyed we could end up with running ErrorChangedTask() on null pipeline which causes a crash. This CL uses a weak pointer for DemuxerHost calls so that no task will run after the pipeline is destroyed. BUG=397656, 399417, 365141 TEST=Updated unit tests to cover this case. R=scherkus@chromium.org Review URL: https://codereview.chromium.org/423073012 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287687 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'media/base/pipeline_unittest.cc')
-rw-r--r--media/base/pipeline_unittest.cc53
1 files changed, 31 insertions, 22 deletions
diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc
index 70c35bb..412fd88 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -122,7 +122,7 @@ class PipelineTest : public ::testing::Test {
if (!pipeline_ || !pipeline_->IsRunning())
return;
- ExpectStop();
+ ExpectDemuxerStop();
// The mock demuxer doesn't stop the fake text track stream,
// so just stop it manually.
@@ -132,7 +132,7 @@ class PipelineTest : public ::testing::Test {
}
// Expect a stop callback if we were started.
- EXPECT_CALL(callbacks_, OnStop());
+ ExpectPipelineStopAndDestroyPipeline();
pipeline_->Stop(base::Bind(&CallbackHelper::OnStop,
base::Unretained(&callbacks_)));
message_loop_.RunUntilIdle();
@@ -314,11 +314,22 @@ class PipelineTest : public ::testing::Test {
EXPECT_EQ(seek_time, pipeline_->GetMediaTime());
}
- void ExpectStop() {
+ void DestroyPipeline() {
+ pipeline_.reset();
+ }
+
+ void ExpectDemuxerStop() {
if (demuxer_)
EXPECT_CALL(*demuxer_, Stop(_)).WillOnce(RunClosure<0>());
}
+ void ExpectPipelineStopAndDestroyPipeline() {
+ // After the Pipeline is stopped, it could be destroyed any time. Always
+ // destroy the pipeline immediately after OnStop() to test this.
+ EXPECT_CALL(callbacks_, OnStop())
+ .WillOnce(Invoke(this, &PipelineTest::DestroyPipeline));
+ }
+
MOCK_METHOD2(OnAddTextTrack, void(const TextTrackConfig&,
const AddTextTrackDoneCB&));
@@ -409,7 +420,7 @@ TEST_F(PipelineTest, NeverInitializes) {
}
TEST_F(PipelineTest, StopWithoutStart) {
- EXPECT_CALL(callbacks_, OnStop());
+ ExpectPipelineStopAndDestroyPipeline();
pipeline_->Stop(
base::Bind(&CallbackHelper::OnStop, base::Unretained(&callbacks_)));
message_loop_.RunUntilIdle();
@@ -435,7 +446,7 @@ TEST_F(PipelineTest, StartThenStopImmediately) {
base::Unretained(&callbacks_)));
// Expect a stop callback if we were started.
- EXPECT_CALL(callbacks_, OnStop());
+ ExpectPipelineStopAndDestroyPipeline();
pipeline_->Stop(
base::Bind(&CallbackHelper::OnStop, base::Unretained(&callbacks_)));
message_loop_.RunUntilIdle();
@@ -454,7 +465,7 @@ TEST_F(PipelineTest, DemuxerErrorDuringStop) {
EXPECT_CALL(*demuxer_, Stop(_))
.WillOnce(DoAll(InvokeWithoutArgs(this, &PipelineTest::OnDemuxerError),
RunClosure<0>()));
- EXPECT_CALL(callbacks_, OnStop());
+ ExpectPipelineStopAndDestroyPipeline();
pipeline_->Stop(
base::Bind(&CallbackHelper::OnStop, base::Unretained(&callbacks_)));
@@ -891,11 +902,7 @@ TEST_F(PipelineTest, AudioTimeUpdateDuringSeek) {
EXPECT_EQ(pipeline_->GetMediaTime(), new_time);
}
-static void DeletePipeline(scoped_ptr<Pipeline> pipeline) {
- // |pipeline| will go out of scope.
-}
-
-TEST_F(PipelineTest, DeleteAfterStop) {
+TEST_F(PipelineTest, DestroyAfterStop) {
CreateAudioStream();
MockDemuxerStreamVector streams;
streams.push_back(audio_stream());
@@ -903,10 +910,11 @@ TEST_F(PipelineTest, DeleteAfterStop) {
SetAudioRendererExpectations(audio_stream());
StartPipeline(PIPELINE_OK);
- ExpectStop();
+ ExpectDemuxerStop();
- Pipeline* pipeline = pipeline_.get();
- pipeline->Stop(base::Bind(&DeletePipeline, base::Passed(&pipeline_)));
+ ExpectPipelineStopAndDestroyPipeline();
+ pipeline_->Stop(
+ base::Bind(&CallbackHelper::OnStop, base::Unretained(&callbacks_)));
message_loop_.RunUntilIdle();
}
@@ -959,8 +967,9 @@ TEST_F(PipelineTest, TimeUpdateAfterStop) {
EXPECT_CALL(*demuxer_, Stop(_)).WillOnce(RunClosure<0>());
- Pipeline* pipeline = pipeline_.get();
- pipeline->Stop(base::Bind(&DeletePipeline, base::Passed(&pipeline_)));
+ ExpectPipelineStopAndDestroyPipeline();
+ pipeline_->Stop(
+ base::Bind(&CallbackHelper::OnStop, base::Unretained(&callbacks_)));
message_loop_.RunUntilIdle();
}
@@ -1038,7 +1047,7 @@ class PipelineTeardownTest : public PipelineTest {
EXPECT_CALL(*demuxer_, Initialize(_, _, _))
.WillOnce(DoAll(Stop(pipeline_.get(), stop_cb),
RunCallback<1>(PIPELINE_OK)));
- EXPECT_CALL(callbacks_, OnStop());
+ ExpectPipelineStopAndDestroyPipeline();
} else {
status = DEMUXER_ERROR_COULD_NOT_OPEN;
EXPECT_CALL(*demuxer_, Initialize(_, _, _))
@@ -1061,7 +1070,7 @@ class PipelineTeardownTest : public PipelineTest {
EXPECT_CALL(*audio_renderer_, Initialize(_, _, _, _, _, _, _))
.WillOnce(DoAll(Stop(pipeline_.get(), stop_cb),
RunCallback<1>(PIPELINE_OK)));
- EXPECT_CALL(callbacks_, OnStop());
+ ExpectPipelineStopAndDestroyPipeline();
} else {
status = PIPELINE_ERROR_INITIALIZATION_FAILED;
EXPECT_CALL(*audio_renderer_, Initialize(_, _, _, _, _, _, _))
@@ -1081,7 +1090,7 @@ class PipelineTeardownTest : public PipelineTest {
EXPECT_CALL(*video_renderer_, Initialize(_, _, _, _, _, _, _, _, _, _))
.WillOnce(DoAll(Stop(pipeline_.get(), stop_cb),
RunCallback<2>(PIPELINE_OK)));
- EXPECT_CALL(callbacks_, OnStop());
+ ExpectPipelineStopAndDestroyPipeline();
} else {
status = PIPELINE_ERROR_INITIALIZATION_FAILED;
EXPECT_CALL(*video_renderer_, Initialize(_, _, _, _, _, _, _, _, _, _))
@@ -1126,7 +1135,7 @@ class PipelineTeardownTest : public PipelineTest {
EXPECT_CALL(callbacks_, OnSeek(status));
if (status == PIPELINE_OK) {
- EXPECT_CALL(callbacks_, OnStop());
+ ExpectPipelineStopAndDestroyPipeline();
}
pipeline_->Seek(base::TimeDelta::FromSeconds(10), base::Bind(
@@ -1195,7 +1204,7 @@ class PipelineTeardownTest : public PipelineTest {
switch (stop_or_error) {
case kStop:
- EXPECT_CALL(callbacks_, OnStop());
+ ExpectPipelineStopAndDestroyPipeline();
pipeline_->Stop(base::Bind(
&CallbackHelper::OnStop, base::Unretained(&callbacks_)));
break;
@@ -1207,7 +1216,7 @@ class PipelineTeardownTest : public PipelineTest {
case kErrorAndStop:
EXPECT_CALL(callbacks_, OnError(PIPELINE_ERROR_READ));
- EXPECT_CALL(callbacks_, OnStop());
+ ExpectPipelineStopAndDestroyPipeline();
pipeline_->SetErrorForTesting(PIPELINE_ERROR_READ);
pipeline_->Stop(base::Bind(
&CallbackHelper::OnStop, base::Unretained(&callbacks_)));
generated by cgit v1.1 at 2025-03-15 22:18:58 +0000