Allow setContentDecryptionModule() to get called when setting is done.

content/renderer/media/webmediaplayer_impl.cc

Index: content/renderer/media/webmediaplayer_impl.cc
diff --git a/content/renderer/media/webmediaplayer_impl.cc b/content/renderer/media/webmediaplayer_impl.cc
index 1a61233b71ad0fb0002ba1257674d03742893df7..1302604e6c5c6e5f302cde186c853b60c026ccd9 100644
--- a/content/renderer/media/webmediaplayer_impl.cc
+++ b/content/renderer/media/webmediaplayer_impl.cc
@@ -61,6 +61,7 @@
 #include "media/filters/video_renderer_impl.h"
 #include "media/filters/vpx_video_decoder.h"
 #include "third_party/WebKit/public/platform/WebContentDecryptionModule.h"
+#include "third_party/WebKit/public/platform/WebContentDecryptionModuleResult.h"
 #include "third_party/WebKit/public/platform/WebMediaSource.h"
 #include "third_party/WebKit/public/platform/WebRect.h"
 #include "third_party/WebKit/public/platform/WebSize.h"
@@ -783,7 +784,7 @@ WebMediaPlayerImpl::GenerateKeyRequestInternal(const std::string& key_system,
 
     if (proxy_decryptor_ && !decryptor_ready_cb_.is_null()) {
       base::ResetAndReturn(&decryptor_ready_cb_)
-          .Run(proxy_decryptor_->GetDecryptor());
+          .Run(proxy_decryptor_->GetDecryptor(), base::Closure());

[xhwang, 2014/07/29 20:58:07]

Here and below, use base::Bind(&base::DoNothing) instead of base::Closure() so
that we can assume |done_cb| is never null. Then we can drop the done_cb
checking in Decrypting* classes.

[jrummell, 2014/07/30 00:15:13]

Done.

     }
 
     current_key_system_ = key_system;
@@ -888,6 +889,8 @@ WebMediaPlayer::MediaKeyException WebMediaPlayerImpl::CancelKeyRequestInternal(
 void WebMediaPlayerImpl::setContentDecryptionModule(
     blink::WebContentDecryptionModule* cdm) {
   DCHECK(main_loop_->BelongsToCurrentThread());
+  // TODO(jrummell): Remove this method once blink starts calling with 2
+  // parameters.
 
   // TODO(xhwang): Support setMediaKeys(0) if necessary: http://crbug.com/330324
   if (!cdm)
@@ -896,7 +899,40 @@ void WebMediaPlayerImpl::setContentDecryptionModule(
   web_cdm_ = ToWebContentDecryptionModuleImpl(cdm);
 
   if (web_cdm_ && !decryptor_ready_cb_.is_null())
-    base::ResetAndReturn(&decryptor_ready_cb_).Run(web_cdm_->GetDecryptor());
+    base::ResetAndReturn(&decryptor_ready_cb_)
+        .Run(web_cdm_->GetDecryptor(), base::Closure());
+}
+
+void WebMediaPlayerImpl::setContentDecryptionModule(
+    blink::WebContentDecryptionModule* cdm,
+    blink::WebContentDecryptionModuleResult result) {
+  DCHECK(main_loop_->BelongsToCurrentThread());
+
+  // TODO(xhwang): Support setMediaKeys(0) if necessary: http://crbug.com/330324
+  if (!cdm) {
+    result.completeWithError(
+        blink::WebContentDecryptionModuleExceptionNotSupportedError,
+        0,
+        "The existing MediaKeys object cannot be removed.");

[xhwang, 2014/07/29 20:58:07]

We may not have a CDM set yet. So more accurately this should be something like:
"Setting a null MediaKeys object not supported."

[jrummell, 2014/07/30 00:15:13]

Done.

+    return;
+  }
+
+  web_cdm_ = ToWebContentDecryptionModuleImpl(cdm);
+
+  if (web_cdm_ && !decryptor_ready_cb_.is_null()) {
+    base::ResetAndReturn(&decryptor_ready_cb_)
+        .Run(web_cdm_->GetDecryptor(),
+             BIND_TO_RENDER_LOOP1(
+                 &WebMediaPlayerImpl::ContentDecryptionModuleSet, result));
+  } else {
+    // No player, so setting CDM is complete.

[xhwang, 2014/07/29 20:58:07]

This is more of a spec question, so +ddorwin for comments.

This comment here let me feel a bit of inconsistency. If there's no player,
setting a CDM finishes immediately. If there is a player, setting a CDM will be
delayed until the playback resumes/starts. So setMeidaKeys + load is quite
different from load + setMediaKeys.

This will be more complicated on Chrome for Android because we send an IPC to
the browser to really "set" the CDM.

How about we remove "Attempt to Resume Playback If Necessary" as a condition to
resolve the promise? We still attempt to resume playback if necessary, but
that's not a necessary condition to resolve the promise.

If we do so, then we can simply return the promise after l.920. I might be
missing some important cases from JS's perspective though....

[ddorwin, 2014/07/30 22:41:34]

As I noted in the next PS, I'm not sure this comment is correct. Or at least
it's not obvious that the logic means "no player".

We don't need to successfully (re)start playback to resolve the promise, but the
pipeline and CDM must be successfully connected. In the no player case (handled
in Blink?), we can resolve immediately since there isn't a pipeline. load() (and
the subsequent pipeline initialization would use the CDM (make sure this is what
happens!) and fail if there are problems.

+    ContentDecryptionModuleSet(result);
+  }
+}
+
+void WebMediaPlayerImpl::ContentDecryptionModuleSet(
+    blink::WebContentDecryptionModuleResult result) {
+  result.complete();
 }
 
 void WebMediaPlayerImpl::InvalidateOnMainThread() {
@@ -1315,7 +1351,7 @@ void WebMediaPlayerImpl::SetDecryptorReadyCB(
   // Cancels the previous decryptor request.
   if (decryptor_ready_cb.is_null()) {
     if (!decryptor_ready_cb_.is_null())
-      base::ResetAndReturn(&decryptor_ready_cb_).Run(NULL);
+      base::ResetAndReturn(&decryptor_ready_cb_).Run(NULL, base::Closure());
     return;
   }
 
@@ -1330,12 +1366,12 @@ void WebMediaPlayerImpl::SetDecryptorReadyCB(
   DCHECK(!proxy_decryptor_ || !web_cdm_);
 
   if (proxy_decryptor_) {
-    decryptor_ready_cb.Run(proxy_decryptor_->GetDecryptor());
+    decryptor_ready_cb.Run(proxy_decryptor_->GetDecryptor(), base::Closure());
     return;
   }
 
   if (web_cdm_) {
-    decryptor_ready_cb.Run(web_cdm_->GetDecryptor());
+    decryptor_ready_cb.Run(web_cdm_->GetDecryptor(), base::Closure());
     return;
   }