Add DecryptingDemuxerStream::Stop().

media/filters/decrypting_demuxer_stream_unittest.cc

Index: media/filters/decrypting_demuxer_stream_unittest.cc
diff --git a/media/filters/decrypting_demuxer_stream_unittest.cc b/media/filters/decrypting_demuxer_stream_unittest.cc
index 17a33425d31c371f8e8dec8713362c7a881ce48a..01752130e142b1fe8c45bae8874330ae95343946 100644
--- a/media/filters/decrypting_demuxer_stream_unittest.cc
+++ b/media/filters/decrypting_demuxer_stream_unittest.cc
@@ -108,7 +108,7 @@ class DecryptingDemuxerStreamTest : public testing::Test {
     EXPECT_CALL(*this, RequestDecryptorNotification(_))
         .WillOnce(RunCallbackIfNotNull(decryptor_.get()));
     EXPECT_CALL(*decryptor_, RegisterNewKeyCB(Decryptor::kAudio, _))
-        .WillOnce(SaveArg<1>(&key_added_cb_));
+        .WillRepeatedly(SaveArg<1>(&key_added_cb_));

[ddorwin, 2014/02/14 01:55:13]

Why the change?

[xhwang, 2014/02/14 03:00:53]

Because now we may call RegisterNewKeyCB() twice (register and cancel).

 
     AudioDecoderConfig input_config(
         kCodecVorbis, kSampleFormatPlanarF32, CHANNEL_LAYOUT_STEREO, 44100,
@@ -229,6 +229,16 @@ class DecryptingDemuxerStreamTest : public testing::Test {
     message_loop_.RunUntilIdle();
   }
 
+  // During the teardown process, media pipeline will be waiting on the render
+  // main thread. If a Decryptor depends on the render main thread
+  // (e.g. PpapiDecryptor), the pending DecryptCB will not be satisfied.
+  // This function simulates this case.

[ddorwin, 2014/02/14 01:55:13]

nit: Does it really simulate this case or just ensure that CancelDecrypt is
called, which means we're doing something other than waiting for the decryptor
to complete?

Does this comment and expected call really stand alone for all Stop() calls or
does it only apply to the actual test case with a pending decrypt? Maybe just
combine this into the test below?

[xhwang, 2014/02/14 03:00:53]

AbortPendingDecryptCB() is not in this case, so the pending_decrypt_cb will
never be satisfied.
Updated the comment.

+  void Stop() {
+    EXPECT_CALL(*decryptor_, CancelDecrypt(Decryptor::kAudio));
+    demuxer_stream_->Stop(NewExpectedClosure());
+    message_loop_.RunUntilIdle();
+  }
+
   MOCK_METHOD1(RequestDecryptorNotification, void(const DecryptorReadyCB&));
 
   MOCK_METHOD2(BufferReady, void(DemuxerStream::Status,
@@ -467,7 +477,7 @@ TEST_F(DecryptingDemuxerStreamTest, DemuxerRead_ConfigChanged) {
   ReadAndExpectBufferReadyWith(DemuxerStream::kConfigChanged, NULL);
 }
 
-// Test resetting when DecryptingDemuxerStream is waiting for an config changed
+// Test resetting when DecryptingDemuxerStream is waiting for a config changed
 // read.
 TEST_F(DecryptingDemuxerStreamTest, Reset_DuringConfigChangedDemuxerRead) {
   Initialize();
@@ -481,4 +491,13 @@ TEST_F(DecryptingDemuxerStreamTest, Reset_DuringConfigChangedDemuxerRead) {
   message_loop_.RunUntilIdle();
 }
 
+// Test resetting when the DecryptingDemuxerStream is in kPendingDecrypt state.
+TEST_F(DecryptingDemuxerStreamTest, Stop_DuringPendingDecrypt) {
+  Initialize();
+  EnterPendingDecryptState();
+
+  EXPECT_CALL(*this, BufferReady(DemuxerStream::kAborted, IsNull()));
+  Stop();
+}

[xhwang, 2014/02/14 01:33:19]

I'll add more tests shortly.

[ddorwin, 2014/02/14 01:55:13]

Okay - at least need a test for calling Stop() when doing nothing.

[xhwang, 2014/02/14 03:00:53]

Done.

+
 }  // namespace media