Generalize AesDecryptor to make it more spec compliant.

media/crypto/aes_decryptor.h

Index: media/crypto/aes_decryptor.h
diff --git a/media/crypto/aes_decryptor.h b/media/crypto/aes_decryptor.h
index d62528f0fa73beeedf61e39596b2856e9548196c..1cbd2814c3403eb8b48ebe60e8d9c44ba6c9230e 100644
--- a/media/crypto/aes_decryptor.h
+++ b/media/crypto/aes_decryptor.h
@@ -20,23 +20,52 @@ class SymmetricKey;
 namespace media {
 
 class DecoderBuffer;
+class DecryptorClient;
 
 // Decrypts AES encrypted buffer into unencrypted buffer.
 class MEDIA_EXPORT AesDecryptor {
  public:
+  enum KeyError {
+    UnknownError = 1,

[scherkus (not reviewing), 2012/06/12 03:15:58]

these all get k prefix

[xhwang, 2012/06/12 19:01:15]

Done.

+    ClientError,
+    ServiceError,
+    OutputError,
+    HardwareChangeError,
+    DomainError
+  };
+
   AesDecryptor();
   ~AesDecryptor();
 
-  // Add a |key_id| and |key| pair to the key system. The key is not limited to
-  // a decryption key. It can be any data that the key system accepts, such as
-  // a license. If multiple calls of this function set different keys for the
-  // same |key_id|, the older key will be replaced by the newer key.
-  void AddKey(const uint8* key_id, int key_id_size,
-              const uint8* key, int key_size);
+  // The AesDecryptor does not take ownership of the |client|. The |client|
+  // must be valid throughout the lifeime of the AesDecryptor.

[ddorwin, 2012/06/11 21:02:40]

lifetime

[xhwang, 2012/06/12 19:01:15]

Done.

+  void Init(DecryptorClient* client);

[scherkus (not reviewing), 2012/06/12 03:15:58]

why not pass into ctor?

I know you eventually want to make this an interface at some point but I have a
feeling we'll still be able to pass the client pointer into the ctor even after
the interface is generalized or at least make that change when needed

[xhwang, 2012/06/12 19:01:15]

Done. It actually make the code much cleaner. Thanks!

+
+  // Generates a key request. The result of this call will be reported via the
+  // client's KeyMessage or KeyError methods.

[scherkus (not reviewing), 2012/06/12 03:15:58]

suffix fn names with () in docs here + below

[xhwang, 2012/06/12 19:01:15]

Done.

+  void GenerateKeyRequest(const std::string& key_system,
+                          const uint8* init_data,
+                          int init_data_length);
+
+  // Adds a |key| to the key system. The key is not limited to a decryption key.
+  // It can be any data that the key system accepts, such as a license.
+  // If multiple calls of this function set different keys for the same
+  // |key_id|, the older key will be replaced by the newer key.
+  // The result of this call will be reported via the client's KeyAdded,
+  // KeyMessage or KeyError methods.
+  void AddKey(const std::string& key_system,
+              const uint8* key,
+              int key_length,
+              const uint8* init_data,
+              int init_data_length,
+              const std::string& session_id);
 
-  // Decrypt |input| buffer. The |input| should not be NULL.
-  // Return a DecoderBuffer with the decrypted data if decryption succeeded.
-  // Return NULL if decryption failed.
+  void CancelKeyRequest(const std::string& key_system,

[ddorwin, 2012/06/11 21:02:40]

// Cancels the key request specified by |session_id|.

[xhwang, 2012/06/12 19:01:15]

Done.

+                        const std::string& session_id);
+
+  // Decrypts the |input| buffer, which should not be NULL.
+  // Returns a DecoderBuffer with the decrypted data if decryption succeeded.
+  // Returns NULL if decryption failed.
   scoped_refptr<DecoderBuffer> Decrypt(
       const scoped_refptr<DecoderBuffer>& input);
 
@@ -44,8 +73,12 @@ class MEDIA_EXPORT AesDecryptor {
   // KeyMap owns the crypto::SymmetricKey* and must delete them when they are
   // not needed any more.
   typedef base::hash_map<std::string, crypto::SymmetricKey*> KeyMap;
-  KeyMap key_map_;
-  base::Lock lock_;
+  KeyMap key_map_;  // Protected by the |lock_|.
+  base::Lock lock_;  // Protects the |key_map_|.

[ddorwin, 2012/06/11 21:02:40]

if it is specific to the key_map_, you might name it key_map_lock_.

[xhwang, 2012/06/12 19:01:15]

Done.

+
+  DecryptorClient* client_;
+
+  uint32_t session_id_;

[ddorwin, 2012/06/11 21:02:40]

last_session_id_ or session_id_incrementer_ 

[scherkus (not reviewing), 2012/06/12 03:15:58]

I like "next_session_id_" then inside the .cc convert to a string prior to
incrementing

[xhwang, 2012/06/12 19:01:15]

Done.

[xhwang, 2012/06/12 19:01:15]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(AesDecryptor);
 };