Initialize the CDM asynchronously

content/renderer/media/crypto/render_cdm_factory.cc

Index: content/renderer/media/crypto/render_cdm_factory.cc
diff --git a/content/renderer/media/crypto/render_cdm_factory.cc b/content/renderer/media/crypto/render_cdm_factory.cc
index e551226fb3750d7e09195d7119956b50e5cb6dff..e33aea1d4f9d5ddb7471c4821fc0952fe2d22120 100644
--- a/content/renderer/media/crypto/render_cdm_factory.cc
+++ b/content/renderer/media/crypto/render_cdm_factory.cc
@@ -7,8 +7,11 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "media/base/cdm_promise.h"
 #include "media/base/key_systems.h"
+#include "media/base/media_keys.h"
 #include "media/cdm/aes_decryptor.h"
 #include "url/gurl.h"
 #if defined(ENABLE_PEPPER_CDMS)
@@ -19,6 +22,33 @@
 
 namespace content {
 
+class CdmInitializedPromise : public media::SimpleCdmPromise {
+ public:
+  CdmInitializedPromise(const RenderCdmFactory::CdmCreatedCB& cdm_created_cb,
+                        scoped_ptr<media::MediaKeys> cdm)
+      : cdm_created_cb_(cdm_created_cb), cdm_(cdm.Pass()) {
+  }
+
+  ~CdmInitializedPromise() override {
+  }
+
+  void resolve() override {
+    MarkPromiseSettled();
+    cdm_created_cb_.Run(cdm_.Pass());
+  }
+
+  void reject(media::MediaKeys::Exception exception_code,
+              uint32 system_code,
+              const std::string& error_message) override {
+    MarkPromiseSettled();
+    cdm_created_cb_.Run(nullptr);
+  }
+
+ private:
+  RenderCdmFactory::CdmCreatedCB cdm_created_cb_;
+  scoped_ptr<media::MediaKeys> cdm_;
+};
+
 RenderCdmFactory::RenderCdmFactory(
 #if defined(ENABLE_PEPPER_CDMS)
     const CreatePepperCdmCB& create_pepper_cdm_cb,
@@ -58,35 +88,59 @@ void RenderCdmFactory::Create(
     return;
   }
 
-  scoped_ptr<media::MediaKeys> cdm;
-
   if (media::CanUseAesDecryptor(key_system)) {
     // TODO(sandersd): Currently the prefixed API always allows distinctive
     // identifiers and persistent state. Once that changes we can sanity check
     // here that neither is allowed for AesDecryptor, since it does not support
     // them and should never be configured that way. http://crbug.com/455271
-    cdm.reset(new media::AesDecryptor(security_origin, session_message_cb,
-                                      session_closed_cb,
-                                      session_keys_change_cb));
-  } else {
+    scoped_ptr<media::MediaKeys> cdm(
+        new media::AesDecryptor(security_origin, session_message_cb,
+                                session_closed_cb, session_keys_change_cb));
+    base::MessageLoopProxy::current()->PostTask(
+        FROM_HERE, base::Bind(cdm_created_cb, base::Passed(&cdm)));
+    return;
+  }
+
 #if defined(ENABLE_PEPPER_CDMS)
-    cdm = PpapiDecryptor::Create(
-        key_system, allow_distinctive_identifier, allow_persistent_state,
-        security_origin, create_pepper_cdm_cb_, session_message_cb,
-        session_closed_cb, legacy_session_error_cb, session_keys_change_cb,
-        session_expiration_update_cb);
+  scoped_ptr<PpapiDecryptor> cdm;
+  cdm = PpapiDecryptor::Create(
+      key_system, security_origin, create_pepper_cdm_cb_, session_message_cb,
+      session_closed_cb, legacy_session_error_cb, session_keys_change_cb,
+      session_expiration_update_cb);
+  if (!cdm) {
+    base::MessageLoopProxy::current()->PostTask(
+        FROM_HERE, base::Bind(cdm_created_cb, nullptr));
+    return;
+  }
+  // PpapiDecryptor ownership passed to the promise, but keep a copy in order
+  // to call InitializeCdm().
+  PpapiDecryptor* ppapi_decryptor = cdm.get();
+  scoped_ptr<CdmInitializedPromise> promise(
+      new CdmInitializedPromise(cdm_created_cb, cdm.Pass()));
+  ppapi_decryptor->InitializeCdm(key_system, allow_distinctive_identifier,

[ddorwin, 2015/04/27 23:52:18]

If this is only called by us here, why not hide it in Create?

If we need asynchronicity, why not make Create return a promise and hide all the
initialization there?

[jrummell, 2015/04/28 18:02:45]

Changed to do all the work in ::Create(). Not returning anything as
|cdm_created_cb| should be called when the CDM is initialized.

+                                 allow_persistent_state, promise.Pass());
 #elif defined(ENABLE_BROWSER_CDMS)
-    DCHECK(allow_distinctive_identifier);
-    DCHECK(allow_persistent_state);
-    cdm = ProxyMediaKeys::Create(
-        key_system, security_origin, manager_, session_message_cb,
-        session_closed_cb, legacy_session_error_cb, session_keys_change_cb,
-        session_expiration_update_cb);
-#endif  // defined(ENABLE_PEPPER_CDMS)
+  DCHECK(allow_distinctive_identifier);
+  DCHECK(allow_persistent_state);
+  scoped_ptr<ProxyMediaKeys> cdm;
+  cdm = ProxyMediaKeys::Create(manager_, session_message_cb, session_closed_cb,
+                               legacy_session_error_cb, session_keys_change_cb,
+                               session_expiration_update_cb);
+  if (!cdm) {

[ddorwin, 2015/04/27 23:52:18]

Duplicating this logic seems wrong.

+    base::MessageLoopProxy::current()->PostTask(
+        FROM_HERE, base::Bind(cdm_created_cb, nullptr));
+    return;
   }
-
+  // ProxyMediaKeys ownership passed to the promise, but keep a copy in order
+  // to call InitializeCdm().
+  ProxyMediaKeys* proxy_media_keys = cdm.get();
+  scoped_ptr<CdmInitializedPromise> promise(
+      new CdmInitializedPromise(cdm_created_cb, cdm.Pass()));
+  proxy_media_keys->InitializeCdm(key_system, security_origin, promise.Pass());
+#else
   base::MessageLoopProxy::current()->PostTask(
-      FROM_HERE, base::Bind(cdm_created_cb, base::Passed(&cdm)));
+      FROM_HERE, base::Bind(cdm_created_cb, nullptr));
+#endif  // defined(ENABLE_PEPPER_CDMS)
 }
 
 }  // namespace content