Code clean-up in AesDecryptor and test.

media/crypto/aes_decryptor.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/crypto/aes_decryptor.h"
 
+#include <vector>
+
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/string_number_conversions.h"
 #include "crypto/encryptor.h"
 #include "crypto/hmac.h"
 #include "crypto/symmetric_key.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/decrypt_config.h"
 #include "media/base/decryptor_client.h"
 
+using crypto::SymmetricKey;
+

[Ryan Sleevi, 2012/07/30 06:52:50]

FWIW, please see
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/qSrl_UAqAkQ/SGDZG...
for the general sentiment regarding 'using' declarations

[xhwang, 2012/07/30 16:22:30]

Thanks for the info. Personally I think either way is okay. But since a lot of
people prefer to not using "using", I'll just remove them. Done.

 namespace media {
 
 // The size is from the WebM encrypted specification. Current encrypted WebM
 // request for comments specification is here
 // http://wiki.webmproject.org/encryption/webm-encryption-rfc
 static const int kWebmSha1DigestSize = 20;
 static const char kWebmHmacSeed[] = "hmac-key";
 static const char kWebmEncryptionSeed[] = "encryption-key";
 
 uint32 AesDecryptor::next_session_id_ = 1;
@@ skipping 50 matching lines @@
 
   base::StringPiece data_to_check(
       reinterpret_cast<const char*>(input.GetData()), input.GetDataSize());
 
   return hmac.VerifyTruncated(data_to_check,
                               input.GetDecryptConfig()->checksum());
 }
 
 enum ClearBytesBufferSel {
   kSrcContainsClearBytes,
-  kDstContainsClearBytes,
+  kDstContainsClearBytes
 };
 
 static void CopySubsamples(const std::vector<SubsampleEntry>& subsamples,
                            const ClearBytesBufferSel sel,
                            const uint8* src,
                            uint8* dst) {
   for (size_t i = 0; i < subsamples.size(); i++) {
     const SubsampleEntry& subsample = subsamples[i];
     if (sel == kSrcContainsClearBytes) {
       src += subsample.clear_bytes;
     } else {
       dst += subsample.clear_bytes;
     }
     memcpy(dst, src, subsample.cypher_bytes);
     src += subsample.cypher_bytes;
     dst += subsample.cypher_bytes;
   }
 }
 
 // Decrypts |input| using |key|.  Returns a DecoderBuffer with the decrypted
 // data if decryption succeeded or NULL if decryption failed.
 static scoped_refptr<DecoderBuffer> DecryptData(const DecoderBuffer& input,
-                                                crypto::SymmetricKey* key) {
+                                                SymmetricKey* key) {
   CHECK(input.GetDataSize());
   CHECK(input.GetDecryptConfig());
   CHECK(key);
 
   crypto::Encryptor encryptor;
   if (!encryptor.Init(key, crypto::Encryptor::CTR, "")) {
     DVLOG(1) << "Could not initialize decryptor.";
     return NULL;
   }
 
@@ skipping 122 matching lines @@
     client_->KeyError(key_system, session_id, Decryptor::kUnknownError, 0);
     return;
   }
 
   if (!decryption_key->Init()) {
     DVLOG(1) << "Could not initialize decryption key.";
     client_->KeyError(key_system, session_id, Decryptor::kUnknownError, 0);
     return;
   }
 
-  {
-    base::AutoLock auto_lock(key_map_lock_);
-    KeyMap::iterator found = key_map_.find(key_id_string);
-    if (found != key_map_.end()) {
-      delete found->second;
-      key_map_.erase(found);
-    }
-    key_map_[key_id_string] = decryption_key.release();
-  }
-
+  SetKey(key_id_string, decryption_key.Pass());
   client_->KeyAdded(key_system, session_id);
 }
 
 void AesDecryptor::CancelKeyRequest(const std::string& key_system,
                                     const std::string& session_id) {
 }
 
 void AesDecryptor::Decrypt(const scoped_refptr<DecoderBuffer>& encrypted,
                            const DecryptCB& decrypt_cb) {
   CHECK(encrypted->GetDecryptConfig());
   const std::string& key_id = encrypted->GetDecryptConfig()->key_id();
 
-  DecryptionKey* key = NULL;
-  {
-    base::AutoLock auto_lock(key_map_lock_);
-    KeyMap::const_iterator found = key_map_.find(key_id);
-    if (found != key_map_.end())
-      key = found->second;
-  }
-
+  DecryptionKey* key = GetKey(key_id);
   if (!key) {
-    // TODO(fgalligan): Fire a need_key event here and add a test.
-    DVLOG(1) << "Could not find a matching key for given key ID.";
+    // TODO(xhwang): Fire a need_key event here and add a test.
+    DVLOG(1) << "Could not find a matching key for the given key ID.";
     decrypt_cb.Run(kError, NULL);
     return;
   }
 
   int checksum_size = encrypted->GetDecryptConfig()->checksum().size();
   // According to the WebM encrypted specification, it is an open question
   // what should happen when a frame fails the integrity check.
   // http://wiki.webmproject.org/encryption/webm-encryption-rfc
   if (checksum_size > 0 &&
       !key->hmac_key().empty() &&
       !CheckData(*encrypted, key->hmac_key())) {
     DVLOG(1) << "Integrity check failed.";
     decrypt_cb.Run(kError, NULL);
     return;
   }
 
   // TODO(strobe): Currently, presence of checksum is used to indicate the use
   // of normal or WebM decryption keys. Consider a more explicit signaling
   // mechanism and the removal of the webm_decryption_key member.
-  crypto::SymmetricKey* decryption_key = (checksum_size > 0) ?
+  SymmetricKey* decryption_key = (checksum_size > 0) ?
       key->webm_decryption_key() : key->decryption_key();
   scoped_refptr<DecoderBuffer> decrypted =
       DecryptData(*encrypted, decryption_key);
   if (!decrypted) {
     DVLOG(1) << "Decryption failed.";
     decrypt_cb.Run(kError, NULL);
     return;
   }
 
   decrypted->SetTimestamp(encrypted->GetTimestamp());
   decrypted->SetDuration(encrypted->GetDuration());
   decrypt_cb.Run(kSuccess, decrypted);
 }
 
-AesDecryptor::DecryptionKey::DecryptionKey(
-    const std::string& secret)
+void AesDecryptor::SetKey(const std::string& key_id,
+                          scoped_ptr<DecryptionKey> decryption_key) {
+  base::AutoLock auto_lock(key_map_lock_);
+  KeyMap::iterator found = key_map_.find(key_id);
+  if (found != key_map_.end()) {
+    delete found->second;
+    key_map_.erase(found);
+  }
+  key_map_[key_id] = decryption_key.release();
+}
+
+AesDecryptor::DecryptionKey* AesDecryptor::GetKey(
+    const std::string& key_id) const {
+  base::AutoLock auto_lock(key_map_lock_);
+  KeyMap::const_iterator found = key_map_.find(key_id);
+  if (found == key_map_.end())
+    return NULL;
+
+  return found->second;
+}
+
+AesDecryptor::DecryptionKey::DecryptionKey(const std::string& secret)
     : secret_(secret) {
 }
 
 AesDecryptor::DecryptionKey::~DecryptionKey() {}
 
 bool AesDecryptor::DecryptionKey::Init() {
   CHECK(!secret_.empty());
-  decryption_key_.reset(
-      crypto::SymmetricKey::Import(crypto::SymmetricKey::AES, secret_));
-  if (!decryption_key_.get()) {
+  decryption_key_.reset(SymmetricKey::Import(SymmetricKey::AES, secret_));
+  if (!decryption_key_.get())
     return false;
-  }
 
   std::string raw_key = DeriveKey(secret_,
                                   kWebmEncryptionSeed,
                                   secret_.length());
-  if (raw_key.empty()) {
+  if (raw_key.empty())
     return false;
-  }
-  webm_decryption_key_.reset(
-      crypto::SymmetricKey::Import(crypto::SymmetricKey::AES, raw_key));
-  if (!webm_decryption_key_.get()) {
+
+  webm_decryption_key_.reset(SymmetricKey::Import(SymmetricKey::AES, raw_key));
+  if (!webm_decryption_key_.get())
     return false;
-  }
 
   hmac_key_ = DeriveKey(secret_, kWebmHmacSeed, kWebmSha1DigestSize);
-  if (hmac_key_.empty()) {
+  if (hmac_key_.empty())
     return false;
-  }
 
   return true;
 }
 
 }  // namespace media