Invalidate all weak pointers during Decrypting{Audio|Video}Decoder::Stop().

This is a clean up CL following r251277.

During media pipeline shutdown, WebMediaPlayerImpl::Destroy() is blocking the
main render thread to wait for the media filters to be stopped correctly.
As a result, Decrypting{Audio|Video}Decoder::Stop() cannot wait for the
Decryptor to finish all pending operations because some Decryptors rely on the
main render thread to work. Instead, Decrypting{Audio|Video}Decoder::Stop()
clears its internal data, fires all pending callbacks, sets it's state to
kStopped and fires the stop callback immediately.

However, after this process, there may still be callbacks outstanding on
the Decrypting{Audio|Video}Decoder object. Previously we check the condition
"state_ == kStopped" in all callback functions to turn those callbacks into a
no-op. In this CL, we invalidate all weak pointers during the Stop() process so
that no pending callbacks will ever be fired back, which is cleaner and safer.

BUG=343748
TEST=Existing tests still pass.

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

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

2 files changed, 13 insertions, 26 deletions

diff --git a/media/filters/decrypting_audio_decoder.cc b/media/filters/decrypting_audio_decoder.cc
index 6a2dd3b..b311446 100644
--- a/media/filters/decrypting_audio_decoder.cc
+++ b/media/filters/decrypting_audio_decoder.cc
@@ -149,6 +149,10 @@ void DecryptingAudioDecoder::Stop(const base::Closure& closure) {
   DVLOG(2) << "Stop() - state: " << state_;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
+  // Invalidate all weak pointers so that pending callbacks won't be fired into
+  // this object.
+  weak_factory_.InvalidateWeakPtrs();
+
   if (decryptor_) {
     decryptor_->RegisterNewKeyCB(Decryptor::kAudio, Decryptor::NewKeyCB());
     decryptor_->DeinitializeDecoder(Decryptor::kAudio);
@@ -163,6 +167,7 @@ void DecryptingAudioDecoder::Stop(const base::Closure& closure) {
     base::ResetAndReturn(&read_cb_).Run(kAborted, NULL);
   if (!reset_cb_.is_null())
     base::ResetAndReturn(&reset_cb_).Run();
+
   state_ = kStopped;
   task_runner_->PostTask(FROM_HERE, closure);
 }
@@ -189,10 +194,6 @@ DecryptingAudioDecoder::~DecryptingAudioDecoder() {
 void DecryptingAudioDecoder::SetDecryptor(Decryptor* decryptor) {
   DVLOG(2) << "SetDecryptor()";
   DCHECK(task_runner_->BelongsToCurrentThread());
-
-  if (state_ == kStopped)
-    return;
-
   DCHECK_EQ(state_, kDecryptorRequested) << state_;
   DCHECK(!init_cb_.is_null());
   DCHECK(!set_decryptor_ready_cb_.is_null());
@@ -232,10 +233,6 @@ void DecryptingAudioDecoder::SetDecryptor(Decryptor* decryptor) {
 void DecryptingAudioDecoder::FinishInitialization(bool success) {
   DVLOG(2) << "FinishInitialization()";
   DCHECK(task_runner_->BelongsToCurrentThread());
-
-  if (state_ == kStopped)
-    return;
-
   DCHECK_EQ(state_, kPendingDecoderInit) << state_;
   DCHECK(!init_cb_.is_null());
   DCHECK(reset_cb_.is_null());  // No Reset() before initialization finished.
@@ -375,10 +372,6 @@ void DecryptingAudioDecoder::DeliverFrame(
     const Decryptor::AudioBuffers& frames) {
   DVLOG(3) << "DeliverFrame() - status: " << status;
   DCHECK(task_runner_->BelongsToCurrentThread());
-
-  if (state_ == kStopped)
-    return;
-
   DCHECK_EQ(state_, kPendingDecode) << state_;
   DCHECK(!read_cb_.is_null());
   DCHECK(pending_buffer_to_decode_.get());
diff --git a/media/filters/decrypting_video_decoder.cc b/media/filters/decrypting_video_decoder.cc
index bae5ab1..35fb709 100644
--- a/media/filters/decrypting_video_decoder.cc
+++ b/media/filters/decrypting_video_decoder.cc
@@ -127,6 +127,10 @@ void DecryptingVideoDecoder::Stop(const base::Closure& closure) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DVLOG(2) << "Stop() - state: " << state_;
 
+  // Invalidate all weak pointers so that pending callbacks won't be fired into
+  // this object.
+  weak_factory_.InvalidateWeakPtrs();
+
   // At this point the render thread is likely paused (in WebMediaPlayerImpl's
   // Destroy()), so running |closure| can't wait for anything that requires the
   // render thread to be processing messages to complete (such as PPAPI
@@ -145,6 +149,7 @@ void DecryptingVideoDecoder::Stop(const base::Closure& closure) {
     base::ResetAndReturn(&decode_cb_).Run(kAborted, NULL);
   if (!reset_cb_.is_null())
     base::ResetAndReturn(&reset_cb_).Run();
+
   state_ = kStopped;
   BindToCurrentLoop(closure).Run();
 }
@@ -156,10 +161,6 @@ DecryptingVideoDecoder::~DecryptingVideoDecoder() {
 void DecryptingVideoDecoder::SetDecryptor(Decryptor* decryptor) {
   DVLOG(2) << "SetDecryptor()";
   DCHECK(task_runner_->BelongsToCurrentThread());
-
-  if (state_ == kStopped)
-    return;
-
   DCHECK_EQ(state_, kDecryptorRequested) << state_;
   DCHECK(!init_cb_.is_null());
   DCHECK(!set_decryptor_ready_cb_.is_null());
@@ -183,10 +184,6 @@ void DecryptingVideoDecoder::SetDecryptor(Decryptor* decryptor) {
 void DecryptingVideoDecoder::FinishInitialization(bool success) {
   DVLOG(2) << "FinishInitialization()";
   DCHECK(task_runner_->BelongsToCurrentThread());
-
-  if (state_ == kStopped)
-    return;
-
   DCHECK_EQ(state_, kPendingDecoderInit) << state_;
   DCHECK(!init_cb_.is_null());
   DCHECK(reset_cb_.is_null());  // No Reset() before initialization finished.
@@ -229,16 +226,13 @@ void DecryptingVideoDecoder::DeliverFrame(
     const scoped_refptr<VideoFrame>& frame) {
   DVLOG(3) << "DeliverFrame() - status: " << status;
   DCHECK(task_runner_->BelongsToCurrentThread());
-  TRACE_EVENT_ASYNC_END0(
-      "media", "DecryptingVideoDecoder::DecodePendingBuffer", trace_id_);
-
-  if (state_ == kStopped)
-    return;
-
   DCHECK_EQ(state_, kPendingDecode) << state_;
   DCHECK(!decode_cb_.is_null());
   DCHECK(pending_buffer_to_decode_.get());
 
+  TRACE_EVENT_ASYNC_END0(
+      "media", "DecryptingVideoDecoder::DecodePendingBuffer", trace_id_);
+
   bool need_to_try_again_if_nokey_is_returned = key_added_while_decode_pending_;
   key_added_while_decode_pending_ = false;