Stop DecryptingDemuxerStream in AudioRendererImpl::Stop().

BUG=343669, 343617
TEST=none
R=ddorwin@chromium.org

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

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

5 files changed, 27 insertions, 3 deletions

diff --git a/media/filters/audio_renderer_impl.cc b/media/filters/audio_renderer_impl.cc
index 5225842..a102b6e 100644
--- a/media/filters/audio_renderer_impl.cc
+++ b/media/filters/audio_renderer_impl.cc
@@ -216,6 +216,19 @@ void AudioRendererImpl::Stop(const base::Closure& callback) {
     sink_ = NULL;
   }
 
+  if (decrypting_demuxer_stream_) {
+    decrypting_demuxer_stream_->Stop(
+        base::Bind(&AudioRendererImpl::StopDecoder, weak_this_));
+    return;
+  }
+
+  StopDecoder();
+}
+
+void AudioRendererImpl::StopDecoder() {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(!stop_cb_.is_null());
+
   if (decoder_) {
     decoder_->Stop(base::ResetAndReturn(&stop_cb_));
     return;
diff --git a/media/filters/audio_renderer_impl.h b/media/filters/audio_renderer_impl.h
index 9d7167c..2617bdd 100644
--- a/media/filters/audio_renderer_impl.h
+++ b/media/filters/audio_renderer_impl.h
@@ -184,6 +184,9 @@ class MEDIA_EXPORT AudioRendererImpl
   // Called when the |decoder_|.Reset() has completed.
   void ResetDecoderDone();
 
+  // Stops the |decoder_| if present. Ensures |stop_cb_| is called.
+  void StopDecoder();
+
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   base::WeakPtrFactory<AudioRendererImpl> weak_factory_;
   base::WeakPtr<AudioRendererImpl> weak_this_;
diff --git a/media/filters/decoder_selector.cc b/media/filters/decoder_selector.cc
index d9c7ea6..caf1440 100644
--- a/media/filters/decoder_selector.cc
+++ b/media/filters/decoder_selector.cc
@@ -123,7 +123,7 @@ void DecoderSelector<StreamType>::Abort() {
 
   if (decoder_) {
     // |decrypted_stream_| is either NULL or already initialized. We don't
-    // need to Reset() |decrypted_stream_| in either case.
+    // need to Stop() |decrypted_stream_| in either case.
     decoder_->Stop(base::Bind(&DecoderSelector<StreamType>::ReturnNullDecoder,
                               weak_ptr_factory_.GetWeakPtr()));
     return;
diff --git a/media/filters/decrypting_demuxer_stream.cc b/media/filters/decrypting_demuxer_stream.cc
index 988d530..1598be6 100644
--- a/media/filters/decrypting_demuxer_stream.cc
+++ b/media/filters/decrypting_demuxer_stream.cc
@@ -116,7 +116,8 @@ void DecryptingDemuxerStream::Stop(const base::Closure& closure) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(state_ != kUninitialized) << state_;
 
-  // Invalidate all weak pointers so that pending callbacks won't fire.
+  // 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
@@ -124,6 +125,9 @@ void DecryptingDemuxerStream::Stop(const base::Closure& closure) {
   // render thread to process messages to complete (such as PPAPI methods).
   if (decryptor_) {
     // Clear the callback.
+    // TODO(xhwang): Since we invalidate all weak pointers during Stop(),
+    // canceling NewKeyCB seems unnecessary. Clean this up in all Decrypting*
+    // classes.
     decryptor_->RegisterNewKeyCB(GetDecryptorStreamType(),
                                  Decryptor::NewKeyCB());
     decryptor_->CancelDecrypt(GetDecryptorStreamType());
diff --git a/media/filters/decrypting_demuxer_stream_unittest.cc b/media/filters/decrypting_demuxer_stream_unittest.cc
index 1cc6ace..bfb2219 100644
--- a/media/filters/decrypting_demuxer_stream_unittest.cc
+++ b/media/filters/decrypting_demuxer_stream_unittest.cc
@@ -50,6 +50,9 @@ ACTION_P(ReturnBuffer, buffer) {
   arg0.Run(buffer.get() ? DemuxerStream::kOk : DemuxerStream::kAborted, buffer);
 }
 
+// Sets the |decryptor| if the DecryptorReadyCB (arg0) is not null. Sets
+// |is_decryptor_set| to true if a non-NULL |decryptor| has been set through the
+// callback.
 ACTION_P2(SetDecryptorIfNotNull, decryptor, is_decryptor_set) {
   if (!arg0.is_null())
     arg0.Run(decryptor);
@@ -110,6 +113,7 @@ class DecryptingDemuxerStreamTest : public testing::Test {
   void Initialize() {
     EXPECT_CALL(*this, RequestDecryptorNotification(_))
         .WillOnce(SetDecryptorIfNotNull(decryptor_.get(), &is_decryptor_set_));
+    // TODO(xhwang): Use WillOnce after we remove the cancellation of NewKeyCB.
     EXPECT_CALL(*decryptor_, RegisterNewKeyCB(Decryptor::kAudio, _))
         .WillRepeatedly(SaveArg<1>(&key_added_cb_));
 
@@ -251,7 +255,7 @@ class DecryptingDemuxerStreamTest : public testing::Test {
   base::MessageLoop message_loop_;
   scoped_ptr<DecryptingDemuxerStream> demuxer_stream_;
   scoped_ptr<StrictMock<MockDecryptor> > decryptor_;
-  // Whether a valid Decryptor is set to the |demuxer_stream_|.
+  // Whether a valid Decryptor has been set in the |demuxer_stream_|.
   bool is_decryptor_set_;
   scoped_ptr<StrictMock<MockDemuxerStream> > input_audio_stream_;
   scoped_ptr<StrictMock<MockDemuxerStream> > input_video_stream_;