Encrypted Media: Replace DecryptorClient with key event callbacks.

media/filters/pipeline_integration_test.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/pipeline_integration_test_base.h"
 
 #include "base/bind.h"
 #include "base/string_util.h"
 #include "media/base/decoder_buffer.h"
-#include "media/base/decryptor_client.h"
 #include "media/base/test_data_util.h"
 #include "media/crypto/aes_decryptor.h"
 
 using testing::AtMost;
 
 namespace media {
 
 static const char kSourceId[] = "SourceId";
 static const char kClearKeySystem[] = "org.w3.clearkey";
 static const uint8 kInitData[] = { 0x69, 0x6e, 0x69, 0x74 };
 
 static const char kWebM[] = "video/webm; codecs=\"vp8,vorbis\"";
 static const char kAudioOnlyWebM[] = "video/webm; codecs=\"vorbis\"";
 static const char kVideoOnlyWebM[] = "video/webm; codecs=\"vp8\"";
 static const char kMP4[] = "video/mp4; codecs=\"avc1.4D4041,mp4a.40.2\"";
 
 // Key used to encrypt video track in test file "bear-320x240-encrypted.webm".
 static const uint8 kSecretKey[] = {
   0xeb, 0xdd, 0x62, 0xf1, 0x68, 0x14, 0xd2, 0x7b,
   0x68, 0xef, 0x12, 0x2a, 0xfc, 0xe4, 0xae, 0x3c
 };
 
 static const int kAppendWholeFile = -1;
 
+class FakeEncryptedMedia {

[ddorwin, 2012/12/21 04:19:06]

FakeEncryptedMediaSink or some similar noun?

[xhwang, 2012/12/21 05:50:07]

The current name is consistent with MockMediaSource (feel we should rename it to
FakeMediaSource though). 

+ public:
+  FakeEncryptedMedia()
+      : decryptor_(base::Bind(&FakeEncryptedMedia::KeyAdded,
+                              base::Unretained(this)),
+                   base::Bind(&FakeEncryptedMedia::KeyError,
+                              base::Unretained(this)),
+                   base::Bind(&FakeEncryptedMedia::KeyMessage,
+                              base::Unretained(this)),
+                   base::Bind(&FakeEncryptedMedia::NeedKey,
+                              base::Unretained(this))) {
+  }
+
+  AesDecryptor* decryptor() {
+    return &decryptor_;
+  }
+
+  // Callbacks for firing key events.
+  void KeyAdded(const std::string& key_system, const std::string& session_id) {
+    EXPECT_EQ(kClearKeySystem, key_system);
+    EXPECT_FALSE(session_id.empty());
+  }
+
+  void KeyError(const std::string& key_system,
+                const std::string& session_id,
+                AesDecryptor::KeyError error_code,
+                int system_code) {
+    NOTIMPLEMENTED();

[ddorwin, 2012/12/21 04:19:06]

Do we not expect this? Should it be:
FAIL() << "Unexpected Key Error";

[xhwang, 2012/12/21 05:50:07]

Done.

+  }
+
+  void KeyMessage(const std::string& key_system,
+                  const std::string& session_id,
+                  const std::string& message,
+                  const std::string& default_url) {
+    EXPECT_EQ(kClearKeySystem, key_system);
+    EXPECT_FALSE(session_id.empty());
+    EXPECT_FALSE(message.empty());
+
+    current_key_system_ = key_system;

[ddorwin, 2012/12/21 04:19:06]

Do we really need this member var given line 68?

[xhwang, 2012/12/21 05:50:07]

This could be triggered by line 88, where the current_key_system_ is empty().

[ddorwin, 2012/12/21 20:05:30]

I guess we are using current_key_system_ as a bool for whether we have requested
a request. Maybe that's the problem.

+    current_session_id_ = session_id;
+  }
+
+  void NeedKey(const std::string& key_system,
+               const std::string& session_id,
+               const std::string& type,
+               scoped_array<uint8> init_data, int init_data_length) {
+    current_key_system_ = key_system;

[ddorwin, 2012/12/21 04:19:06]

Is this ever not clear key?

[xhwang, 2012/12/21 05:50:07]

The demuxer doesn't know about key system, so |key_system| could be empty.

+    current_session_id_ = session_id;

[ddorwin, 2012/12/21 04:19:06]

Should we wipe out a session id if KeyMessage was already called for some
reason? I guess we're really using this as a local variable for limited
scenarios.

[xhwang, 2012/12/21 05:50:07]

Similar to key_system, session_id may not be set in NeedKey call. We need
GenerateKeyRequest call (see line 88) to trigger a KeyMessage where we can set
the session id (see line 73).

[ddorwin, 2012/12/21 20:05:30]

My point was that this line _might_ replace the session_id set by the key
message, possibly with an empty string. This isn't really a problem since we
probably only deal with one session, but that's not called out and the code is
incorrect for the general case.

[xhwang, 2012/12/22 00:46:34]

Hmm, I'll think about this more and change it to a more acceptable way. But that
will be in a separate CL.

+
+    // When NeedKey is called from the demuxer, the |key_system| will be empty.
+    // In this case, we need to call GenerateKeyRequest() to initialize a
+    // session (which will call KeyMessage).
+    if (current_key_system_.empty()) {
+      DCHECK(current_session_id_.empty());

[ddorwin, 2012/12/21 04:19:06]

Lots of DCHECKs here and below. Should these be EXPECT so they run in opt
builds?

[xhwang, 2012/12/21 05:50:07]

Replace this DCHECK with EXPECT_TRUE. If we need to replace other DCHECKs, we
may do that in another CL.

+      EXPECT_TRUE(decryptor_.GenerateKeyRequest(
+          kClearKeySystem, type, kInitData, arraysize(kInitData)));
+    }
+
+    EXPECT_FALSE(current_key_system_.empty());
+    EXPECT_FALSE(current_session_id_.empty());
+    decryptor_.AddKey(current_key_system_, kSecretKey, arraysize(kSecretKey),
+                      init_data.get(), init_data_length, current_session_id_);
+  }
+
+ private:
+  AesDecryptor decryptor_;
+  std::string current_key_system_;
+  std::string current_session_id_;
+};
+
 // Helper class that emulates calls made on the ChunkDemuxer by the
 // Media Source API.
 class MockMediaSource {
  public:
   MockMediaSource(const std::string& filename, const std::string& mimetype,
                   int initial_append_size)
       : file_path_(GetTestDataFilePath(filename)),
         current_position_(0),
         initial_append_size_(initial_append_size),
         mimetype_(mimetype) {
     chunk_demuxer_ = new ChunkDemuxer(
         base::Bind(&MockMediaSource::DemuxerOpened, base::Unretained(this)),
         base::Bind(&MockMediaSource::DemuxerNeedKey, base::Unretained(this)),
         LogCB());
 
     file_data_ = ReadTestDataFile(filename);
 
     if (initial_append_size_ == kAppendWholeFile)
       initial_append_size_ = file_data_->GetDataSize();
 
     DCHECK_GT(initial_append_size_, 0);
     DCHECK_LE(initial_append_size_, file_data_->GetDataSize());
   }
 
   virtual ~MockMediaSource() {}
 
   const scoped_refptr<ChunkDemuxer>& demuxer() const { return chunk_demuxer_; }
-  void set_decryptor_client(DecryptorClient* decryptor_client) {
-    decryptor_client_ = decryptor_client;
+
+  void set_need_key_cb(const NeedKeyCB& need_key_cb) {
+    need_key_cb_ = need_key_cb;
   }
 
   void Seek(int new_position, int seek_append_size) {
     chunk_demuxer_->StartWaitingForSeek();
 
     chunk_demuxer_->Abort(kSourceId);
 
     DCHECK_GE(new_position, 0);
     DCHECK_LT(new_position, file_data_->GetDataSize());
     current_position_ = new_position;
@@ skipping 44 matching lines @@
     Tokenize(codecStr, ",", &codecs);
 
     CHECK_EQ(chunk_demuxer_->AddId(kSourceId, type, codecs), ChunkDemuxer::kOk);
     AppendData(initial_append_size_);
   }
 
   void DemuxerNeedKey(const std::string& type,
                       scoped_array<uint8> init_data, int init_data_size) {
     DCHECK(init_data.get());
     DCHECK_GT(init_data_size, 0);
-    DCHECK(decryptor_client_);
-    decryptor_client_->NeedKey("", "", type, init_data.Pass(), init_data_size);
+    need_key_cb_.Run("", "", type, init_data.Pass(), init_data_size);
   }
 
  private:
   FilePath file_path_;
   scoped_refptr<DecoderBuffer> file_data_;
   int current_position_;
   int initial_append_size_;
   std::string mimetype_;
   scoped_refptr<ChunkDemuxer> chunk_demuxer_;
-  DecryptorClient* decryptor_client_;
-};
-
-class FakeDecryptorClient : public DecryptorClient {
- public:
-  FakeDecryptorClient() : decryptor_(this) {}
-
-  AesDecryptor* decryptor() {
-    return &decryptor_;
-  }
-
-  // DecryptorClient implementation.
-  virtual void KeyAdded(const std::string& key_system,
-                        const std::string& session_id) {
-    EXPECT_EQ(kClearKeySystem, key_system);
-    EXPECT_FALSE(session_id.empty());
-  }
-
-  virtual void KeyError(const std::string& key_system,
-                        const std::string& session_id,
-                        AesDecryptor::KeyError error_code,
-                        int system_code) {
-    NOTIMPLEMENTED();
-  }
-
-  virtual void KeyMessage(const std::string& key_system,
-                          const std::string& session_id,
-                          const std::string& message,
-                          const std::string& default_url) {
-    EXPECT_EQ(kClearKeySystem, key_system);
-    EXPECT_FALSE(session_id.empty());
-    EXPECT_FALSE(message.empty());
-
-    current_key_system_ = key_system;
-    current_session_id_ = session_id;
-  }
-
-  virtual void NeedKey(const std::string& key_system,
-                       const std::string& session_id,
-                       const std::string& type,
-                       scoped_array<uint8> init_data,
-                       int init_data_length) {
-    current_key_system_ = key_system;
-    current_session_id_ = session_id;
-
-    // When NeedKey is called from the demuxer, the |key_system| will be empty.
-    // In this case, we need to call GenerateKeyRequest() to initialize a
-    // session (which will call KeyMessage).
-    if (current_key_system_.empty()) {
-      DCHECK(current_session_id_.empty());
-      EXPECT_TRUE(decryptor_.GenerateKeyRequest(
-          kClearKeySystem, type, kInitData, arraysize(kInitData)));
-    }
-
-    EXPECT_FALSE(current_key_system_.empty());
-    EXPECT_FALSE(current_session_id_.empty());
-    decryptor_.AddKey(current_key_system_, kSecretKey, arraysize(kSecretKey),
-                      init_data.get(), init_data_length, current_session_id_);
-  }
-
- private:
-  AesDecryptor decryptor_;
-  std::string current_key_system_;
-  std::string current_session_id_;
+  NeedKeyCB need_key_cb_;
 };
 
 class PipelineIntegrationTest
     : public testing::Test,
       public PipelineIntegrationTestBase {
  public:
   void StartPipelineWithMediaSource(MockMediaSource* source) {
     EXPECT_CALL(*this, OnBufferingState(Pipeline::kHaveMetadata))
         .Times(AtMost(1));
     EXPECT_CALL(*this, OnBufferingState(Pipeline::kPrerollCompleted))
         .Times(AtMost(1));
     pipeline_->Start(
         CreateFilterCollection(source->demuxer(), NULL),
         base::Bind(&PipelineIntegrationTest::OnEnded, base::Unretained(this)),
         base::Bind(&PipelineIntegrationTest::OnError, base::Unretained(this)),
         QuitOnStatusCB(PIPELINE_OK),
         base::Bind(&PipelineIntegrationTest::OnBufferingState,
                    base::Unretained(this)));
 
     message_loop_.Run();
   }
 
   void StartPipelineWithEncryptedMedia(
       MockMediaSource* source,
-      FakeDecryptorClient* encrypted_media) {
+      FakeEncryptedMedia* encrypted_media) {
     EXPECT_CALL(*this, OnBufferingState(Pipeline::kHaveMetadata))
         .Times(AtMost(1));
     EXPECT_CALL(*this, OnBufferingState(Pipeline::kPrerollCompleted))
         .Times(AtMost(1));
     pipeline_->Start(
         CreateFilterCollection(source->demuxer(), encrypted_media->decryptor()),
         base::Bind(&PipelineIntegrationTest::OnEnded, base::Unretained(this)),
         base::Bind(&PipelineIntegrationTest::OnError, base::Unretained(this)),
         QuitOnStatusCB(PIPELINE_OK),
         base::Bind(&PipelineIntegrationTest::OnBufferingState,
                    base::Unretained(this)));
 
-    source->set_decryptor_client(encrypted_media);
+    source->set_need_key_cb(base::Bind(&FakeEncryptedMedia::NeedKey,
+                                       base::Unretained(encrypted_media)));
 
     message_loop_.Run();
   }
 
   // Verifies that seeking works properly for ChunkDemuxer when the
   // seek happens while there is a pending read on the ChunkDemuxer
   // and no data is available.
   bool TestSeekDuringRead(const std::string& filename,
                           const std::string& mimetype,
                           int initial_append_size,
@@ skipping 16 matching lines @@
       return false;
 
     source.EndOfStream();
 
     source.Abort();
     Stop();
     return true;
   }
 };
 
-
 TEST_F(PipelineIntegrationTest, BasicPlayback) {
   ASSERT_TRUE(Start(GetTestDataFilePath("bear-320x240.webm"), PIPELINE_OK));
 
   Play();
 
   ASSERT_TRUE(WaitUntilOnEnded());
 }
 
 TEST_F(PipelineIntegrationTest, BasicPlaybackHashed) {
   ASSERT_TRUE(Start(GetTestDataFilePath("bear-320x240.webm"),
@@ skipping 77 matching lines @@
 
 TEST_F(PipelineIntegrationTest, BasicPlayback_16x9AspectRatio) {
   ASSERT_TRUE(Start(GetTestDataFilePath("bear-320x240-16x9-aspect.webm"),
                     PIPELINE_OK));
   Play();
   ASSERT_TRUE(WaitUntilOnEnded());
 }
 
 TEST_F(PipelineIntegrationTest, EncryptedPlayback) {
   MockMediaSource source("bear-320x240-encrypted.webm", kWebM, 219816);
-  FakeDecryptorClient encrypted_media;
+  FakeEncryptedMedia encrypted_media;
   StartPipelineWithEncryptedMedia(&source, &encrypted_media);
 
   source.EndOfStream();
   ASSERT_EQ(PIPELINE_OK, pipeline_status_);
 
   Play();
 
   ASSERT_TRUE(WaitUntilOnEnded());
   source.Abort();
   Stop();
@@ skipping 55 matching lines @@
 // Verify video decoder & renderer can handle aborted demuxer reads.
 TEST_F(PipelineIntegrationTest, ChunkDemuxerAbortRead_VideoOnly) {
   ASSERT_TRUE(TestSeekDuringRead("bear-320x240-video-only.webm", kVideoOnlyWebM,
                                  32768,
                                  base::TimeDelta::FromMilliseconds(200),
                                  base::TimeDelta::FromMilliseconds(1668),
                                  0x1C896, 65536));
 }
 
 }  // namespace media