Pass key_information on SessionKeysChange message

media/cdm/aes_decryptor_unittest.cc

Index: media/cdm/aes_decryptor_unittest.cc
diff --git a/media/cdm/aes_decryptor_unittest.cc b/media/cdm/aes_decryptor_unittest.cc
index 6af82d719ffb44a7c991ac56fbc18c37a23089b1..53e504c8978623b5dabaf2d9d9cbda498ddd2bca 100644
--- a/media/cdm/aes_decryptor_unittest.cc
+++ b/media/cdm/aes_decryptor_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/json/json_reader.h"
 #include "base/values.h"
 #include "media/base/cdm_callback_promise.h"
+#include "media/base/cdm_key_information.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/decrypt_config.h"
 #include "media/base/mock_filters.h"
@@ -23,6 +24,7 @@ using ::testing::IsNull;
 using ::testing::NotNull;
 using ::testing::SaveArg;
 using ::testing::StrNe;
+using ::testing::Unused;
 
 MATCHER(IsEmpty, "") { return arg.empty(); }
 MATCHER(IsNotEmpty, "") { return !arg.empty(); }
@@ -308,6 +310,18 @@ class AesDecryptorTest : public testing::Test {
     decryptor_.RemoveSession(session_id, CreatePromise(RESOLVED));
   }
 
+  // Helper method to make a local copy of |usable_key_ids|.
+  void SaveUsableKeys(Unused, Unused, const CdmKeysInfo& usable_key_ids) {

[xhwang, 2015/01/05 22:53:15]

If you pass CdmKeysInfo by value, you can probably just use Pass() and transfer
the ownership over. Will that be simpler?

[jrummell, 2015/01/06 02:36:25]

MOCK functions don't like ScopedVector, so changed to have a proper method for
OnSessionKeysChange() and a mock'd method OnSessionKeysChangeCalled().

+    usable_key_ids_.clear();
+    for (const auto& key_id : usable_key_ids) {
+      scoped_ptr<CdmKeyInformation> new_key_id(new CdmKeyInformation);
+      new_key_id->key_id = key_id->key_id;
+      new_key_id->status = key_id->status;
+      new_key_id->system_code = key_id->system_code;
+      usable_key_ids_.push_back(new_key_id.release());

[xhwang, 2015/01/05 22:53:15]

It seems here we save ALL key IDs regardless of it's status, i.e. even for
non-usable key IDs. So "Usable" in the function name doesn't seem accurate.

How about we rename SaveUsableKeys to something like SaveKeysInfo?

[jrummell, 2015/01/06 02:36:25]

Removed.

+    }
+  }
+
   // Updates the session specified by |session_id| with |key|. |result|
   // tests that the update succeeds or generates an error.
   void UpdateSessionAndExpect(std::string session_id,
@@ -316,9 +330,10 @@ class AesDecryptorTest : public testing::Test {
     DCHECK(!key.empty());
 
     if (expected_result == RESOLVED) {
-      EXPECT_CALL(*this, OnSessionKeysChange(session_id, true));
+      EXPECT_CALL(*this, OnSessionKeysChange(session_id, true, _))
+          .WillOnce(Invoke(this, &AesDecryptorTest::SaveUsableKeys));
     } else {
-      EXPECT_CALL(*this, OnSessionKeysChange(_, _)).Times(0);
+      EXPECT_CALL(*this, OnSessionKeysChange(_, _, _)).Times(0);
     }
 
     decryptor_.UpdateSession(session_id,
@@ -327,6 +342,14 @@ class AesDecryptorTest : public testing::Test {
                              CreatePromise(expected_result));
   }
 
+  bool UsableKeyIdsContains(std::vector<uint8> expected) {

[xhwang, 2015/01/05 22:53:15]

Ditto about "Usable".

[jrummell, 2015/01/06 02:36:25]

Done.

+    for (const auto& key_id : usable_key_ids_) {
+      if (key_id->key_id == expected)
+        return true;
+    }
+    return false;
+  }
+
   MOCK_METHOD2(BufferDecrypted, void(Decryptor::Status,
                                      const scoped_refptr<DecoderBuffer>&));
 
@@ -390,14 +413,16 @@ class AesDecryptorTest : public testing::Test {
                void(const std::string& web_session_id,
                     const std::vector<uint8>& message,
                     const GURL& destination_url));
-  MOCK_METHOD2(OnSessionKeysChange,
+  MOCK_METHOD3(OnSessionKeysChange,
                void(const std::string& web_session_id,
-                    bool has_additional_usable_key));
+                    bool has_additional_usable_key,
+                    const CdmKeysInfo& keys_info));
   MOCK_METHOD1(OnSessionClosed, void(const std::string& web_session_id));
 
   AesDecryptor decryptor_;
   AesDecryptor::DecryptCB decrypt_cb_;
   std::string web_session_id_;
+  CdmKeysInfo usable_key_ids_;

[xhwang, 2015/01/05 22:53:15]

ditto about "usable".

[jrummell, 2015/01/06 02:36:25]

Done.

 
   // Constants for testing.
   const std::vector<uint8> original_data_;
@@ -849,4 +874,23 @@ TEST_F(AesDecryptorTest, JWKKey) {
   CloseSession(session_id);
 }
 
+TEST_F(AesDecryptorTest, GetKeyIds) {
+  std::vector<uint8> key_id1(kKeyId, kKeyId + arraysize(kKeyId));
+  std::vector<uint8> key_id2(kKeyId2, kKeyId2 + arraysize(kKeyId2));
+
+  std::string session_id = CreateSession(key_id_);
+  EXPECT_FALSE(UsableKeyIdsContains(key_id1));
+  EXPECT_FALSE(UsableKeyIdsContains(key_id2));
+
+  // Add 1 key, verify it is returned.
+  UpdateSessionAndExpect(session_id, kKeyAsJWK, RESOLVED);
+  EXPECT_TRUE(UsableKeyIdsContains(key_id1));
+  EXPECT_FALSE(UsableKeyIdsContains(key_id2));
+
+  // Add second key, verify both IDs returned.
+  UpdateSessionAndExpect(session_id, kKey2AsJWK, RESOLVED);
+  EXPECT_TRUE(UsableKeyIdsContains(key_id1));
+  EXPECT_TRUE(UsableKeyIdsContains(key_id2));
+}
+
 }  // namespace media