Separate MediaKeys interface from Decryptor interface.

webkit/media/crypto/proxy_decryptor.cc

Index: webkit/media/crypto/proxy_decryptor.cc
diff --git a/webkit/media/crypto/proxy_decryptor.cc b/webkit/media/crypto/proxy_decryptor.cc
index 3e445c418161a272e97b12ea570596ce4531e789..c61975ec440cadf8376e44b0254e52ca76ee7db3 100644
--- a/webkit/media/crypto/proxy_decryptor.cc
+++ b/webkit/media/crypto/proxy_decryptor.cc
@@ -68,6 +68,7 @@ ProxyDecryptor::ProxyDecryptor(
       key_error_cb_(key_error_cb),
       key_message_cb_(key_message_cb),
       need_key_cb_(need_key_cb),
+      decryptor_(NULL),
       weak_ptr_factory_(this) {
 }
 
@@ -75,7 +76,8 @@ ProxyDecryptor::~ProxyDecryptor() {
   // Destroy the decryptor explicitly before destroying the plugin.
   {
     base::AutoLock auto_lock(lock_);
-    decryptor_.reset();
+    media_keys_.reset();
+    decryptor_ = NULL;
   }
 
 #if defined(ENABLE_PEPPER_CDMS)
@@ -105,7 +107,7 @@ void ProxyDecryptor::SetDecryptorReadyCB(
   // Normal decryptor request.
   DCHECK(decryptor_ready_cb_.is_null());
   if (decryptor_) {
-    decryptor_ready_cb.Run(decryptor_.get());
+    decryptor_ready_cb.Run(decryptor_);
     return;
   }
   decryptor_ready_cb_ = decryptor_ready_cb;
@@ -121,23 +123,25 @@ bool ProxyDecryptor::GenerateKeyRequest(const std::string& key_system,
 
   base::AutoLock auto_lock(lock_);
 

[ddorwin, 2013/05/24 20:39:12]

We should probably
  DCHECK((media_keys_ == NULL) == (decryptor_ == NULL));
And maybe similar checks at 128 and 134.

[xhwang, 2013/05/24 23:15:56]

This is not necessary now.

-  if (!decryptor_) {
-    decryptor_ = CreateDecryptor(key_system);
-    if (!decryptor_) {
+  if (!media_keys_) {
+    if (!CreateDecryptor(key_system)) {
       key_error_cb_.Run(
-          key_system, std::string(), media::Decryptor::kClientError, 0);
+          key_system, std::string(), media::MediaKeys::kClientError, 0);
       return false;
     }
   }
 
-  if (!decryptor_->GenerateKeyRequest(key_system, type,
-                                      init_data, init_data_length)) {
-    decryptor_.reset();
+  DCHECK(decryptor_);
+
+  if (!media_keys_->GenerateKeyRequest(key_system, type,

[ddorwin, 2013/05/24 20:39:12]

We only use media_keys_ in three places. This class would probably be cleaner if
we had decryptor_ continue to own the object and called decryptor_->media_keys()
in those three locations.

[xhwang, 2013/05/24 23:15:56]

Done.

+                                       init_data, init_data_length)) {
+    media_keys_.reset();
+    decryptor_ = NULL;
     return false;
   }
 
   if (!decryptor_ready_cb_.is_null())
-    base::ResetAndReturn(&decryptor_ready_cb_).Run(decryptor_.get());
+    base::ResetAndReturn(&decryptor_ready_cb_).Run(decryptor_);
 
   return true;
 }
@@ -151,8 +155,8 @@ void ProxyDecryptor::AddKey(const std::string& key_system,
   DVLOG(1) << "AddKey()";
 
   // WebMediaPlayerImpl ensures GenerateKeyRequest() has been called.
-  decryptor_->AddKey(key_system, key, key_length, init_data, init_data_length,
-                     session_id);
+  media_keys_->AddKey(
+      key_system, key, key_length, init_data, init_data_length, session_id);
 }
 
 void ProxyDecryptor::CancelKeyRequest(const std::string& key_system,
@@ -160,11 +164,11 @@ void ProxyDecryptor::CancelKeyRequest(const std::string& key_system,
   DVLOG(1) << "CancelKeyRequest()";
 
   // WebMediaPlayerImpl ensures GenerateKeyRequest() has been called.
-  decryptor_->CancelKeyRequest(key_system, session_id);
+  media_keys_->CancelKeyRequest(key_system, session_id);
 }
 
 #if defined(ENABLE_PEPPER_CDMS)
-scoped_ptr<media::Decryptor> ProxyDecryptor::CreatePpapiDecryptor(
+scoped_ptr<PpapiDecryptor> ProxyDecryptor::CreatePpapiDecryptor(
     const std::string& key_system) {
   DCHECK(web_media_player_client_);
   DCHECK(web_frame_);
@@ -176,10 +180,10 @@ scoped_ptr<media::Decryptor> ProxyDecryptor::CreatePpapiDecryptor(
   did_create_helper_plugin_ = plugin_instance != NULL;
   if (!did_create_helper_plugin_) {
     DVLOG(1) << "ProxyDecryptor: plugin instance creation failed.";
-    return scoped_ptr<media::Decryptor>();
+    return scoped_ptr<PpapiDecryptor>();
   }
 
-  return scoped_ptr<media::Decryptor>(new PpapiDecryptor(
+  return scoped_ptr<PpapiDecryptor>(new PpapiDecryptor(
       plugin_instance,
       base::Bind(&ProxyDecryptor::KeyAdded, weak_ptr_factory_.GetWeakPtr()),
       base::Bind(&ProxyDecryptor::KeyError, weak_ptr_factory_.GetWeakPtr()),
@@ -188,23 +192,31 @@ scoped_ptr<media::Decryptor> ProxyDecryptor::CreatePpapiDecryptor(
 }
 #endif  // defined(ENABLE_PEPPER_CDMS)
 
-scoped_ptr<media::Decryptor> ProxyDecryptor::CreateDecryptor(
-    const std::string& key_system) {
-  if (CanUseAesDecryptor(key_system))
-    return scoped_ptr<media::Decryptor>(new media::AesDecryptor(
+bool ProxyDecryptor::CreateDecryptor(const std::string& key_system) {
+  if (CanUseAesDecryptor(key_system)) {
+    scoped_ptr<media::AesDecryptor> aes_decryptor(new media::AesDecryptor(

[ddorwin, 2013/05/24 20:39:12]

media_keys_ =

[xhwang, 2013/05/24 23:15:56]

This change is reverted.

         base::Bind(&ProxyDecryptor::KeyAdded, weak_ptr_factory_.GetWeakPtr()),
         base::Bind(&ProxyDecryptor::KeyError, weak_ptr_factory_.GetWeakPtr()),
         base::Bind(&ProxyDecryptor::KeyMessage, weak_ptr_factory_.GetWeakPtr()),
         base::Bind(&ProxyDecryptor::NeedKey, weak_ptr_factory_.GetWeakPtr())));
+    decryptor_ = aes_decryptor.get();
+    media_keys_ = aes_decryptor.Pass();
+    return true;
+  }
 
 #if defined(ENABLE_PEPPER_CDMS)
   // We only support AesDecryptor and PpapiDecryptor. So if we cannot
   // use the AesDecryptor, then we'll try to create a PpapiDecryptor for given
   // |key_system|.
-  return CreatePpapiDecryptor(key_system);
-#else
-  return scoped_ptr<media::Decryptor>();
+  scoped_ptr<PpapiDecryptor> ppapi_decryptor = CreatePpapiDecryptor(key_system);

[ddorwin, 2013/05/24 20:39:12]

same

[xhwang, 2013/05/24 23:15:56]

This change is reverted.

+  if (ppapi_decryptor) {
+    decryptor_ = ppapi_decryptor.get();
+    media_keys_ = ppapi_decryptor.Pass();
+    return true;
+  }
 #endif  // defined(ENABLE_PEPPER_CDMS)
+
+  return false;
 }
 
 void ProxyDecryptor::KeyAdded(const std::string& key_system,
@@ -214,7 +226,7 @@ void ProxyDecryptor::KeyAdded(const std::string& key_system,
 
 void ProxyDecryptor::KeyError(const std::string& key_system,
                               const std::string& session_id,
-                              media::Decryptor::KeyError error_code,
+                              media::MediaKeys::KeyError error_code,
                               int system_code) {
   key_error_cb_.Run(key_system, session_id, error_code, system_code);
 }