diff options
author | xhwang@chromium.org <xhwang@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-08-08 00:51:25 +0000 |
---|---|---|
committer | xhwang@chromium.org <xhwang@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-08-08 00:51:25 +0000 |
commit | 06e9ed577ae93247583bde146707bb4dca2f8f75 (patch) | |
tree | 63bf95f098ee29bbb2db2c9800d3f77462eddbdf | |
parent | 36ef2cb971376d4c7b12d3774f9148ab3bb7d2a1 (diff) | |
download | chromium_src-06e9ed577ae93247583bde146707bb4dca2f8f75.zip chromium_src-06e9ed577ae93247583bde146707bb4dca2f8f75.tar.gz chromium_src-06e9ed577ae93247583bde146707bb4dca2f8f75.tar.bz2 |
Pipeline: Invalidate weak pointers before returning stop callback.
This is a follow up CL of r287687, which missed the corner case that an error
happened before Stop().
This CL also updated the PipelineTest so that we explicitly check that all weak
pointers must have been invalidated before the stop callback returns. With this
check PipelineTeardownTest.Error_* tests will cover this corner case.
BUG=399417
TEST=Updated unit test.
Review URL: https://codereview.chromium.org/444333007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288181 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | media/base/pipeline.cc | 12 | ||||
-rw-r--r-- | media/base/pipeline.h | 1 | ||||
-rw-r--r-- | media/base/pipeline_unittest.cc | 3 |
3 files changed, 16 insertions, 0 deletions
diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc index c144682..bc55981 100644 --- a/media/base/pipeline.cc +++ b/media/base/pipeline.cc @@ -200,6 +200,11 @@ void Pipeline::SetErrorForTesting(PipelineStatus status) { OnError(status); } +bool Pipeline::HasWeakPtrsForTesting() const { + DCHECK(task_runner_->BelongsToCurrentThread()); + return weak_factory_.HasWeakPtrs(); +} + void Pipeline::SetState(State next_state) { DVLOG(1) << GetStateString(state_) << " -> " << GetStateString(next_state); @@ -602,7 +607,14 @@ void Pipeline::StopTask(const base::Closure& stop_cb) { DCHECK(stop_cb_.is_null()); if (state_ == kStopped) { + // Invalid all weak pointers so it's safe to destroy |this| on the render + // main thread. + weak_factory_.InvalidateWeakPtrs(); + + // NOTE: pipeline may be deleted at this point in time as a result of + // executing |stop_cb|. stop_cb.Run(); + return; } diff --git a/media/base/pipeline.h b/media/base/pipeline.h index 7c79b3f..fe5962a 100644 --- a/media/base/pipeline.h +++ b/media/base/pipeline.h @@ -183,6 +183,7 @@ class MEDIA_EXPORT Pipeline : public DemuxerHost { } void SetTimeDeltaInterpolatorForTesting(TimeDeltaInterpolator* interpolator); void SetErrorForTesting(PipelineStatus status); + bool HasWeakPtrsForTesting() const; private: FRIEND_TEST_ALL_PREFIXES(PipelineTest, GetBufferedTimeRanges); diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc index 412fd88..4eb84f6 100644 --- a/media/base/pipeline_unittest.cc +++ b/media/base/pipeline_unittest.cc @@ -315,6 +315,9 @@ class PipelineTest : public ::testing::Test { } void DestroyPipeline() { + // In real code Pipeline could be destroyed on a different thread. All weak + // pointers must have been invalidated before the stop callback returns. + DCHECK(!pipeline_->HasWeakPtrsForTesting()); pipeline_.reset(); } |