summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorxhwang@chromium.org <xhwang@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-08 00:51:25 +0000
committerxhwang@chromium.org <xhwang@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-08 00:51:25 +0000
commit06e9ed577ae93247583bde146707bb4dca2f8f75 (patch)
tree63bf95f098ee29bbb2db2c9800d3f77462eddbdf
parent36ef2cb971376d4c7b12d3774f9148ab3bb7d2a1 (diff)
downloadchromium_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.cc12
-rw-r--r--media/base/pipeline.h1
-rw-r--r--media/base/pipeline_unittest.cc3
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();
}