Allow setContentDecryptionModule() to get called when setting is done.

media/filters/decrypting_video_decoder_unittest.cc

Index: media/filters/decrypting_video_decoder_unittest.cc
diff --git a/media/filters/decrypting_video_decoder_unittest.cc b/media/filters/decrypting_video_decoder_unittest.cc
index ed956ea1d51e0a172fb9e09cb7edc2f854dafddc..647a936678e0344aefda1b93a7d22e7f56a51e3e 100644
--- a/media/filters/decrypting_video_decoder_unittest.cc
+++ b/media/filters/decrypting_video_decoder_unittest.cc
@@ -18,6 +18,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 
 using ::testing::_;
+using ::testing::Eq;
 using ::testing::Invoke;
 using ::testing::SaveArg;
 using ::testing::StrictMock;
@@ -44,13 +45,8 @@ static scoped_refptr<DecoderBuffer> CreateFakeEncryptedBuffer() {
 // times across multiple test files. Sadly we can't use static for them.
 namespace {
 
-ACTION_P(RunCallbackIfNotNull, param) {
-  if (!arg0.is_null())
-    arg0.Run(param);
-}
-
-ACTION_P2(ResetAndRunCallback, callback, param) {
-  base::ResetAndReturn(callback).Run(param);
+ACTION_P3(ResetAndRunCallback, callback, p1, p2) {
+  base::ResetAndReturn(callback).Run(p1, p2);
 }
 
 }  // namespace
@@ -70,14 +66,20 @@ class DecryptingVideoDecoderTest : public testing::Test {
         decoded_video_frame_(VideoFrame::CreateBlackFrame(
             TestVideoConfig::NormalCodedSize())),
         null_video_frame_(scoped_refptr<VideoFrame>()) {
-    EXPECT_CALL(*this, RequestDecryptorNotification(_))
-        .WillRepeatedly(RunCallbackIfNotNull(decryptor_.get()));
   }
 
   virtual ~DecryptingVideoDecoderTest() {
     Destroy();
   }
 
+  void ExpectDecryptorNotification() {

[ddorwin, 2014/08/04 18:59:08]

Why is this extracted into a function only in this file?

[jrummell, 2014/08/07 01:54:25]

It was needed in 2 places. However, I've parameterized it so it can be used in
more places. Also updated the other tests to match.

+    EXPECT_CALL(*this, RequestDecryptorNotification(_)).WillOnce(
+        RunCallback<0>(decryptor_.get(),
+                       base::Bind(&DecryptingVideoDecoderTest::DecryptorSet,
+                                  base::Unretained(this))));
+    EXPECT_CALL(*this, DecryptorSet(Eq(true)));
+  }
+
   // Initializes the |decoder_| and expects |status|. Note the initialization
   // can succeed or fail.
   void InitializeAndExpectStatus(const VideoDecoderConfig& config,
@@ -90,6 +92,7 @@ class DecryptingVideoDecoderTest : public testing::Test {
 
   // Initialize the |decoder_| and expects it to succeed.
   void Initialize() {
+    ExpectDecryptorNotification();
     EXPECT_CALL(*decryptor_, InitializeVideoDecoder(_, _))
         .WillOnce(RunCallback<1>(true));
     EXPECT_CALL(*decryptor_, RegisterNewKeyCB(Decryptor::kVideo, _))
@@ -223,6 +226,8 @@ class DecryptingVideoDecoderTest : public testing::Test {
   MOCK_METHOD1(FrameReady, void(const scoped_refptr<VideoFrame>&));
   MOCK_METHOD1(DecodeDone, void(VideoDecoder::Status));
 
+  MOCK_METHOD1(DecryptorSet, void(bool));
+
   base::MessageLoop message_loop_;
   scoped_ptr<DecryptingVideoDecoder> decoder_;
   scoped_ptr<StrictMock<MockDecryptor> > decryptor_;
@@ -249,8 +254,11 @@ TEST_F(DecryptingVideoDecoderTest, Initialize_Normal) {
 }
 
 TEST_F(DecryptingVideoDecoderTest, Initialize_NullDecryptor) {
-  EXPECT_CALL(*this, RequestDecryptorNotification(_))
-      .WillRepeatedly(RunCallbackIfNotNull(static_cast<Decryptor*>(NULL)));
+  EXPECT_CALL(*this, RequestDecryptorNotification(_)).WillOnce(
+      RunCallback<0>(static_cast<Decryptor*>(NULL),
+                     base::Bind(&DecryptingVideoDecoderTest::DecryptorSet,
+                                base::Unretained(this))));
+  EXPECT_CALL(*this, DecryptorSet(Eq(false)));

[ddorwin, 2014/08/04 18:59:08]

_IF_ you're going to use the function above, this should be replaced too. That
means a bool for expected_notification_response or something like that.

[jrummell, 2014/08/07 01:54:25]

Done.

   InitializeAndExpectStatus(TestVideoConfig::NormalEncrypted(),
                             DECODER_ERROR_NOT_SUPPORTED);
 }
@@ -415,14 +423,17 @@ TEST_F(DecryptingVideoDecoderTest, Destroy_DuringDecryptorRequested) {
 
   // During destruction, RequestDecryptorNotification() should be called with a
   // NULL callback to cancel the |decryptor_ready_cb|.
-  EXPECT_CALL(*this, RequestDecryptorNotification(IsNullCallback()))
-      .WillOnce(ResetAndRunCallback(&decryptor_ready_cb,
-                                    reinterpret_cast<Decryptor*>(NULL)));
+  EXPECT_CALL(*this, RequestDecryptorNotification(IsNullCallback())).WillOnce(
+      ResetAndRunCallback(&decryptor_ready_cb,
+                          reinterpret_cast<Decryptor*>(NULL),
+                          base::Bind(&DecryptingVideoDecoderTest::DecryptorSet,
+                                     base::Unretained(this))));
   Destroy();

[ddorwin, 2014/08/04 18:59:08]

No EXPECT_CALL(DecryptorSet) ?
If that is intentional, the callback should be some "never called"
function/macro.

[jrummell, 2014/08/07 01:54:25]

Done. I tried tracing this to figure out why the callback never happens. I think
what happens is that the SetDecryptor call is scheduled with a WeakPtr, and
since this call happens during destruction, the original object is gone by the
time decryptor_ready_cb is actually called. Thus SetDecryptor() never really
executes.

 }
 
 // Test destruction when the decoder is in kPendingDecoderInit state.
 TEST_F(DecryptingVideoDecoderTest, Destroy_DuringPendingDecoderInit) {
+  ExpectDecryptorNotification();

[ddorwin, 2014/08/04 18:59:08]

This is a successful set?

[jrummell, 2014/08/07 01:54:25]

Yup (at least the test passes this way).

   EXPECT_CALL(*decryptor_, InitializeVideoDecoder(_, _))
       .WillOnce(SaveArg<1>(&pending_init_cb_));