EME: Address TODOs related to the removal of prefixed EME

media/cdm/aes_decryptor_unittest.cc

 // Copyright 2013 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/cdm/aes_decryptor.h"
 
 #include <stdint.h>
 #include <memory>
 #include <string>
 #include <vector>
@@ skipping 321 matching lines @@
     // for AesDecryptor.
     return session_id_;
   }
 
   // Closes the session specified by |session_id|.
   void CloseSession(const std::string& session_id) {
     EXPECT_CALL(*this, OnSessionClosed(session_id));
     cdm_->CloseSession(session_id, CreatePromise(RESOLVED));
   }
 
-  // Removes the session specified by |session_id|. This should simply do a
-  // CloseSession().
-  // TODO(jrummell): Clean this up when the prefixed API is removed.
-  // http://crbug.com/249976.
+  // Only persistent sessions can be removed.
   void RemoveSession(const std::string& session_id) {
-    if (GetParam() == "AesDecryptor") {
-      EXPECT_CALL(*this, OnSessionClosed(session_id));
-      cdm_->RemoveSession(session_id, CreatePromise(RESOLVED));
-    } else {
-      // CdmAdapter fails as only persistent sessions can be removed.
-      cdm_->RemoveSession(session_id, CreatePromise(REJECTED));
-    }
+    cdm_->RemoveSession(session_id, CreatePromise(REJECTED));
   }
 
   MOCK_METHOD2(OnSessionKeysChangeCalled,
                void(const std::string& session_id,
                     bool has_additional_usable_key));
 
   void OnSessionKeysChange(const std::string& session_id,
                            bool has_additional_usable_key,
                            CdmKeysInfo keys_info) {
     keys_info_.swap(keys_info);
@@ skipping 126 matching lines @@
   // Constants for testing.
   const std::vector<uint8_t> original_data_;
   const std::vector<uint8_t> encrypted_data_;
   const std::vector<uint8_t> subsample_encrypted_data_;
   const std::vector<uint8_t> key_id_;
   const std::vector<uint8_t> iv_;
   const std::vector<SubsampleEntry> normal_subsample_entries_;
   const std::vector<SubsampleEntry> no_subsample_entries_;
 };
 
-TEST_P(AesDecryptorTest, CreateSessionWithNullInitData) {
-  EXPECT_CALL(*this,
-              OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL()));
+TEST_P(AesDecryptorTest, CreateSessionWithEmptyInitData) {
   cdm_->CreateSessionAndGenerateRequest(
       MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
-      std::vector<uint8_t>(), CreateSessionPromise(RESOLVED));
+      std::vector<uint8_t>(), CreateSessionPromise(REJECTED));
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::CENC,
+      std::vector<uint8_t>(), CreateSessionPromise(REJECTED));
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::KEYIDS,
+      std::vector<uint8_t>(), CreateSessionPromise(REJECTED));
+}
+
+TEST_P(AesDecryptorTest, CreateSessionWithVariousLengthInitData_WebM) {
+  std::vector<uint8_t> init_data;
+  init_data.resize(1);
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
+      std::vector<uint8_t>(init_data), CreateSessionPromise(RESOLVED));
+
+  init_data.resize(16);  // The expected size.
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
+      std::vector<uint8_t>(init_data), CreateSessionPromise(RESOLVED));
+
+  init_data.resize(512);

[jrummell, 2016/06/25 00:39:38]

Should limits::kMaxKeyIdLength be used here (and +1 below)?

[ddorwin, 2016/08/11 20:28:32]

Tests should avoid using the same constants as the code they are testing. That
way, you'll know when the behavior changes. If someone changed the constant to 1
MB, we'd want the test to catch that. This may be unlikely in this case, but
consider a constant that was based on other constants or a change that might
make sense in one case and the author might not realize it affects the code
under this specific test.

[jrummell, 2016/08/12 19:24:25]

Acknowledged.

+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
+      std::vector<uint8_t>(init_data), CreateSessionPromise(RESOLVED));
+
+  init_data.resize(513);
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
+      std::vector<uint8_t>(init_data), CreateSessionPromise(REJECTED));
 }
 
 TEST_P(AesDecryptorTest, MultipleCreateSession) {
-  EXPECT_CALL(*this,
-              OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL()));
+  EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsNotEmpty(),
+                                      GURL::EmptyGURL()));
   cdm_->CreateSessionAndGenerateRequest(
       MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
-      std::vector<uint8_t>(), CreateSessionPromise(RESOLVED));
+      std::vector<uint8_t>(1), CreateSessionPromise(RESOLVED));
 
-  EXPECT_CALL(*this,
-              OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL()));
+  EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsNotEmpty(),
+                                      GURL::EmptyGURL()));
   cdm_->CreateSessionAndGenerateRequest(
       MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
-      std::vector<uint8_t>(), CreateSessionPromise(RESOLVED));
+      std::vector<uint8_t>(1), CreateSessionPromise(RESOLVED));
 
-  EXPECT_CALL(*this,
-              OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL()));
+  EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsNotEmpty(),
+                                      GURL::EmptyGURL()));
   cdm_->CreateSessionAndGenerateRequest(
       MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
-      std::vector<uint8_t>(), CreateSessionPromise(RESOLVED));
+      std::vector<uint8_t>(1), CreateSessionPromise(RESOLVED));
 }
 
 TEST_P(AesDecryptorTest, CreateSessionWithCencInitData) {
   const uint8_t init_data[] = {
       0x00, 0x00, 0x00, 0x44,                          // size = 68
       0x70, 0x73, 0x73, 0x68,                          // 'pssh'
       0x01,                                            // version
       0x00, 0x00, 0x00,                                // flags
       0x10, 0x77, 0xEF, 0xEC, 0xC0, 0xB2, 0x4D, 0x02,  // SystemID
       0xAC, 0xE3, 0x3C, 0x1E, 0x52, 0xE2, 0xFB, 0x4B,
@@ skipping 222 matching lines @@
   scoped_refptr<DecoderBuffer> encrypted_buffer = CreateEncryptedBuffer(
       encrypted_data_, key_id_, iv_, no_subsample_entries_);
 
   UpdateSessionAndExpect(session_id, kKeyAsJWK, RESOLVED, true);
   ASSERT_NO_FATAL_FAILURE(
       DecryptAndExpect(encrypted_buffer, original_data_, SUCCESS));
 
   CloseSession(session_id);
 }
 
-TEST_P(AesDecryptorTest, RemoveSession) {
-  // TODO(jrummell): Clean this up when the prefixed API is removed.
-  // http://crbug.com/249976.
+// Re-enable if https://crbug.com/416194 is fixed.
+TEST_P(AesDecryptorTest, RemoveSession_DISABLED) {
   std::string session_id = CreateSession(key_id_);
   scoped_refptr<DecoderBuffer> encrypted_buffer = CreateEncryptedBuffer(
       encrypted_data_, key_id_, iv_, no_subsample_entries_);
 
   UpdateSessionAndExpect(session_id, kKeyAsJWK, RESOLVED, true);
   ASSERT_NO_FATAL_FAILURE(
       DecryptAndExpect(encrypted_buffer, original_data_, SUCCESS));
 
   RemoveSession(session_id);
 }
@@ skipping 239 matching lines @@
 INSTANTIATE_TEST_CASE_P(CdmAdapter,
                         AesDecryptorTest,
                         testing::Values("CdmAdapter"));
 #endif
 
 // TODO(jrummell): Once MojoCdm/MojoCdmService/MojoDecryptor/
 // MojoDecryptorService are implemented, add a third version that tests the
 // CDM via mojo.
 
 }  // namespace media