media: Refactor MediaCrypto creation and notification.

In Java MediaDrmBridge, when MediaCrypto creation failed, always release
everything and notify native code with a null MediaCrypto.

Remove Java GetMediaCrypto() function. MediaCrypto will always be notified to
native code immediately after creation succeeded or failed.

BUG=564246
TEST=Tested MediaCrypto creation success and failure cases.

Review URL: https://codereview.chromium.org/1512173003

Cr-Commit-Position: refs/heads/master@{#364825}

9 files changed, 103 insertions, 76 deletions

diff --git a/media/base/android/audio_decoder_job.cc b/media/base/android/audio_decoder_job.cc
index 06f728a..3cf2c5f 100644
--- a/media/base/android/audio_decoder_job.cc
+++ b/media/base/android/audio_decoder_job.cc
@@ -166,7 +166,7 @@ MediaDecoderJob::MediaDecoderJobStatus
                                num_channels_, &audio_extra_data_[0],
                                audio_extra_data_.size(), audio_codec_delay_ns_,
                                audio_seek_preroll_ns_, true,
-                               GetMediaCrypto().obj())) {
+                               GetMediaCrypto())) {
     media_codec_bridge_.reset();
     return STATUS_FAILURE;
   }
diff --git a/media/base/android/java/src/org/chromium/media/MediaDrmBridge.java b/media/base/android/java/src/org/chromium/media/MediaDrmBridge.java
index bb7e7d6..b0ce6f0 100644
--- a/media/base/android/java/src/org/chromium/media/MediaDrmBridge.java
+++ b/media/base/android/java/src/org/chromium/media/MediaDrmBridge.java
@@ -227,17 +227,16 @@ public class MediaDrmBridge {
      * @return whether a MediaCrypto object is successfully created.
      */
     private boolean createMediaCrypto() throws android.media.NotProvisionedException {
-        if (mMediaDrm == null) {
-            return false;
-        }
+        assert mMediaDrm != null;
         assert !mProvisioningPending;
         assert mMediaCryptoSession == null;
         assert mMediaCrypto == null;
 
-        // Open media crypto session.
+        // Open media crypto session. This could throw NotProvisionedException.
         mMediaCryptoSession = openSession();
         if (mMediaCryptoSession == null) {
             Log.e(TAG, "Cannot create MediaCrypto Session.");
+            release();
             return false;
         }
         Log.d(TAG, "MediaCrypto Session created: %s", bytesToHexString(mMediaCryptoSession));
@@ -248,8 +247,6 @@ public class MediaDrmBridge {
             if (MediaCrypto.isCryptoSchemeSupported(mSchemeUUID)) {
                 mMediaCrypto = new MediaCrypto(mSchemeUUID, mMediaCryptoSession);
                 Log.d(TAG, "MediaCrypto successfully created!");
-                // Notify the native code that MediaCrypto is ready.
-                onMediaCryptoReady();
                 return true;
             } else {
                 Log.e(TAG, "Cannot create MediaCrypto for unsupported scheme.");
@@ -393,14 +390,6 @@ public class MediaDrmBridge {
     }
 
     /**
-     * Return the MediaCrypto object if available.
-     */
-    @CalledByNative
-    private MediaCrypto getMediaCrypto() {
-        return mMediaCrypto;
-    }
-
-    /**
      * Reset the device DRM credentials.
      */
     @CalledByNative
@@ -582,10 +571,16 @@ public class MediaDrmBridge {
         byte[] sessionId = null;
         try {
             // Create MediaCrypto if necessary.
-            if (mMediaCrypto == null && !createMediaCrypto()) {
-                onPromiseRejected(promiseId, "MediaCrypto creation failed.");
-                return;
+            if (mMediaCrypto == null) {
+                boolean success = createMediaCrypto();
+                // |mMediaCrypto| could be null upon failure. Notify the native code in all cases.
+                onMediaCryptoReady();
+                if (!success) {
+                    onPromiseRejected(promiseId, "MediaCrypto creation failed.");
+                    return;
+                }
             }
+
             assert mMediaCryptoSession != null;
             assert mMediaCrypto != null;
 
@@ -806,7 +801,7 @@ public class MediaDrmBridge {
 
     private void onMediaCryptoReady() {
         if (isNativeMediaDrmBridgeValid()) {
-            nativeOnMediaCryptoReady(mNativeMediaDrmBridge);
+            nativeOnMediaCryptoReady(mNativeMediaDrmBridge, mMediaCrypto);
         }
     }
 
@@ -972,7 +967,8 @@ public class MediaDrmBridge {
 
     // Native functions. At the native side, must post the task immediately to
     // avoid reentrancy issues.
-    private native void nativeOnMediaCryptoReady(long nativeMediaDrmBridge);
+    private native void nativeOnMediaCryptoReady(
+            long nativeMediaDrmBridge, MediaCrypto mediaCrypto);
 
     private native void nativeOnStartProvisioning(
             long nativeMediaDrmBridge, String defaultUrl, byte[] requestData);
diff --git a/media/base/android/media_codec_player.cc b/media/base/android/media_codec_player.cc
index a58ac27b..1fb821d 100644
--- a/media/base/android/media_codec_player.cc
+++ b/media/base/android/media_codec_player.cc
@@ -937,7 +937,15 @@ void MediaCodecPlayer::OnMediaCryptoReady(
   // and the surface requirement does not change until new SetCdm() is called.
 
   DCHECK(media_crypto);
-  DCHECK(!media_crypto->is_null());
+
+  if (media_crypto->is_null()) {
+    // TODO(timav): Fail playback nicely here if needed. Note that we could get
+    // here even though the stream to play is unencrypted and therefore
+    // MediaCrypto is not needed. In that case, we may ignore this error and
+    // continue playback, or fail the playback.
+    LOG(ERROR) << "MediaCrypto creation failed.";
+    return;
+  }
 
   media_crypto_ = media_crypto.Pass();
 
diff --git a/media/base/android/media_decoder_job.cc b/media/base/android/media_decoder_job.cc
index ed367d7..9e0a216 100644
--- a/media/base/android/media_decoder_job.cc
+++ b/media/base/android/media_decoder_job.cc
@@ -199,11 +199,8 @@ void MediaDecoderJob::ReleaseDecoderResources() {
   release_resources_pending_ = true;
 }
 
-base::android::ScopedJavaLocalRef<jobject> MediaDecoderJob::GetMediaCrypto() {
-  base::android::ScopedJavaLocalRef<jobject> media_crypto;
-  if (drm_bridge_)
-    media_crypto = drm_bridge_->GetMediaCrypto();
-  return media_crypto;
+jobject MediaDecoderJob::GetMediaCrypto() {
+  return drm_bridge_ ? drm_bridge_->GetMediaCrypto() : nullptr;
 }
 
 bool MediaDecoderJob::SetCurrentFrameToPreviouslyCachedKeyFrame() {
@@ -662,8 +659,7 @@ MediaDecoderJob::MediaDecoderJobStatus
   if (media_codec_bridge_ && !need_to_reconfig_decoder_job_)
     return STATUS_SUCCESS;
 
-  base::android::ScopedJavaLocalRef<jobject> media_crypto = GetMediaCrypto();
-  if (is_content_encrypted_ && media_crypto.is_null())
+  if (is_content_encrypted_ && !GetMediaCrypto())
     return STATUS_FAILURE;
 
   ReleaseMediaCodecBridge();
diff --git a/media/base/android/media_decoder_job.h b/media/base/android/media_decoder_job.h
index 752ac88..d82f89a 100644
--- a/media/base/android/media_decoder_job.h
+++ b/media/base/android/media_decoder_job.h
@@ -140,7 +140,7 @@ class MediaDecoderJob {
   virtual bool ComputeTimeToRender() const = 0;
 
   // Gets MediaCrypto object from |drm_bridge_|.
-  base::android::ScopedJavaLocalRef<jobject> GetMediaCrypto();
+  jobject GetMediaCrypto();
 
   // Releases the |media_codec_bridge_|.
   void ReleaseMediaCodecBridge();
diff --git a/media/base/android/media_drm_bridge.cc b/media/base/android/media_drm_bridge.cc
index 7e63815..cb95f43 100644
--- a/media/base/android/media_drm_bridge.cc
+++ b/media/base/android/media_drm_bridge.cc
@@ -492,11 +492,9 @@ void MediaDrmBridge::RejectPromise(uint32_t promise_id,
                                      error_message);
 }
 
-ScopedJavaLocalRef<jobject> MediaDrmBridge::GetMediaCrypto() {
+jobject MediaDrmBridge::GetMediaCrypto() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-
-  JNIEnv* env = AttachCurrentThread();
-  return Java_MediaDrmBridge_getMediaCrypto(env, j_media_drm_.obj());
+  return j_media_crypto_->obj();
 }
 
 void MediaDrmBridge::SetMediaCryptoReadyCB(
@@ -517,15 +515,14 @@ void MediaDrmBridge::SetMediaCryptoReadyCB(
   }
 
   DCHECK(media_crypto_ready_cb_.is_null());
+  media_crypto_ready_cb_ = media_crypto_ready_cb;
 
-  // |media_crypto_ready_cb| is already bound to the correct thread
-  // (either UI or Media).
-  if (!GetMediaCrypto().is_null()) {
-    NotifyMediaCryptoReady(media_crypto_ready_cb);
+  if (!j_media_crypto_)
     return;
-  }
 
-  media_crypto_ready_cb_ = media_crypto_ready_cb;
+  base::ResetAndReturn(&media_crypto_ready_cb_)
+      .Run(CreateJavaObjectPtr(j_media_crypto_->obj()),
+           IsProtectedSurfaceRequired());
 }
 
 //------------------------------------------------------------------------------
@@ -534,17 +531,16 @@ void MediaDrmBridge::SetMediaCryptoReadyCB(
 
 void MediaDrmBridge::OnMediaCryptoReady(
     JNIEnv* env,
-    const JavaParamRef<jobject>& j_media_drm) {
+    const JavaParamRef<jobject>& j_media_drm,
+    const JavaParamRef<jobject>& j_media_crypto) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__;
 
-  if (media_crypto_ready_cb_.is_null())
-    return;
-
   task_runner_->PostTask(
-      FROM_HERE, base::Bind(&MediaDrmBridge::NotifyMediaCryptoReady,
-                            weak_factory_.GetWeakPtr(),
-                            base::ResetAndReturn(&media_crypto_ready_cb_)));
+      FROM_HERE,
+      base::Bind(&MediaDrmBridge::NotifyMediaCryptoReady,
+                 weak_factory_.GetWeakPtr(),
+                 base::Passed(CreateJavaObjectPtr(j_media_crypto.obj()))));
 }
 
 void MediaDrmBridge::OnStartProvisioning(
@@ -759,6 +755,11 @@ MediaDrmBridge::~MediaDrmBridge() {
 
   player_tracker_.NotifyCdmUnset();
 
+  if (!media_crypto_ready_cb_.is_null()) {
+    base::ResetAndReturn(&media_crypto_ready_cb_)
+        .Run(CreateJavaObjectPtr(nullptr), IsProtectedSurfaceRequired());
+  }
+
   // Rejects all pending promises.
   cdm_promise_adapter_.Clear();
 }
@@ -779,18 +780,29 @@ MediaDrmBridge::SecurityLevel MediaDrmBridge::GetSecurityLevel() {
   return GetSecurityLevelFromString(security_level_str);
 }
 
-void MediaDrmBridge::NotifyMediaCryptoReady(const MediaCryptoReadyCB& cb) {
+// We have to use scoped_ptr to pass ScopedJavaGlobalRef with a callback.
+// TODO(timav): Check whether we can simply pass j_media_crypto_->obj() in the
+// callback.
+MediaDrmBridge::JavaObjectPtr MediaDrmBridge::CreateJavaObjectPtr(
+    jobject object) {
+  JavaObjectPtr j_object_ptr(new ScopedJavaGlobalRef<jobject>());
+  j_object_ptr->Reset(AttachCurrentThread(), object);
+  return j_object_ptr;
+}
+
+void MediaDrmBridge::NotifyMediaCryptoReady(JavaObjectPtr j_media_crypto) {
   DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(j_media_crypto);
+  DCHECK(!j_media_crypto_);
 
-  DCHECK(!cb.is_null());
-  DCHECK(!GetMediaCrypto().is_null());
+  j_media_crypto_ = std::move(j_media_crypto);
 
-  // We can use scoped_ptr to pass ScopedJavaGlobalRef with a callback.
-  scoped_ptr<ScopedJavaGlobalRef<jobject>> j_object_ptr(
-      new ScopedJavaGlobalRef<jobject>());
-  j_object_ptr->Reset(AttachCurrentThread(), GetMediaCrypto().obj());
+  if (media_crypto_ready_cb_.is_null())
+    return;
 
-  cb.Run(j_object_ptr.Pass(), IsProtectedSurfaceRequired());
+  base::ResetAndReturn(&media_crypto_ready_cb_)
+      .Run(CreateJavaObjectPtr(j_media_crypto_->obj()),
+           IsProtectedSurfaceRequired());
 }
 
 void MediaDrmBridge::SendProvisioningRequest(const std::string& default_url,
diff --git a/media/base/android/media_drm_bridge.h b/media/base/android/media_drm_bridge.h
index 80f3c47..0cf315a 100644
--- a/media/base/android/media_drm_bridge.h
+++ b/media/base/android/media_drm_bridge.h
@@ -145,9 +145,11 @@ class MEDIA_EXPORT MediaDrmBridge : public MediaKeys, public PlayerTracker {
                                  const std::string& session_id);
   void RejectPromise(uint32_t promise_id, const std::string& error_message);
 
-  // Returns a MediaCrypto object if it's already created. Returns a null object
-  // otherwise.
-  base::android::ScopedJavaLocalRef<jobject> GetMediaCrypto();
+  // Returns a MediaCrypto object. Can only be called after |j_media_crypto_|
+  // is set.
+  // TODO(xhwang): This is only used by MediaSourcePlayer et al. Remove this
+  // method when MediaSourcePlayer is deprecated.
+  jobject GetMediaCrypto();
 
   // Registers a callback which will be called when MediaCrypto is ready.
   // Can be called on any thread. Only one callback should be registered.
@@ -162,7 +164,8 @@ class MEDIA_EXPORT MediaDrmBridge : public MediaKeys, public PlayerTracker {
   // Called by Java after a MediaCrypto object is created.
   void OnMediaCryptoReady(
       JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& j_media_drm);
+      const base::android::JavaParamRef<jobject>& j_media_drm,
+      const base::android::JavaParamRef<jobject>& j_media_crypto);
 
   // Called by Java when we need to send a provisioning request,
   void OnStartProvisioning(
@@ -260,9 +263,11 @@ class MEDIA_EXPORT MediaDrmBridge : public MediaKeys, public PlayerTracker {
   // Get the security level of the media.
   SecurityLevel GetSecurityLevel();
 
-  // A helper method that calculates the |media_crypto_ready_cb_| arguments and
-  // run this callback.
-  void NotifyMediaCryptoReady(const MediaCryptoReadyCB& cb);
+  // A helper method to create a JavaObjectPtr.
+  JavaObjectPtr CreateJavaObjectPtr(jobject object);
+
+  // A helper method that is called when MediaCrypto is ready.
+  void NotifyMediaCryptoReady(JavaObjectPtr j_media_crypto);
 
   // Sends HTTP provisioning request to a provisioning server.
   void SendProvisioningRequest(const std::string& default_url,
@@ -277,6 +282,15 @@ class MEDIA_EXPORT MediaDrmBridge : public MediaKeys, public PlayerTracker {
   // Java MediaDrm instance.
   base::android::ScopedJavaGlobalRef<jobject> j_media_drm_;
 
+  // Java MediaCrypto instance. Possible values are:
+  // !j_media_crypto_:
+  //   MediaCrypto creation has not been notified via NotifyMediaCryptoReady().
+  // !j_media_crypto_->is_null():
+  //   MediaCrypto creation succeeded and it has been notified.
+  // j_media_crypto_->is_null():
+  //   MediaCrypto creation failed and it has been notified.
+  JavaObjectPtr j_media_crypto_;
+
   // The callback to create a ProvisionFetcher.
   CreateFetcherCB create_fetcher_cb_;
 
diff --git a/media/base/android/media_source_player.cc b/media/base/android/media_source_player.cc
index 8aec0ab..a00b028 100644
--- a/media/base/android/media_source_player.cc
+++ b/media/base/android/media_source_player.cc
@@ -266,11 +266,19 @@ void MediaSourcePlayer::OnDemuxerDurationChanged(base::TimeDelta duration) {
 }
 
 void MediaSourcePlayer::OnMediaCryptoReady(
-    MediaDrmBridge::JavaObjectPtr /* media_crypto */,
+    MediaDrmBridge::JavaObjectPtr media_crypto,
     bool /* needs_protected_surface */) {
-  // Callback parameters are ignored in this player. They are intended for
-  // MediaCodecPlayer which uses a different threading scheme.
-  DCHECK(!static_cast<MediaDrmBridge*>(cdm_.get())->GetMediaCrypto().is_null());
+  DCHECK(media_crypto);
+  DCHECK(static_cast<MediaDrmBridge*>(cdm_.get())->GetMediaCrypto());
+
+  if (media_crypto->is_null()) {
+    // TODO(xhwang): Fail playback nicely here if needed. Note that we could get
+    // here even though the stream to play is unencrypted and therefore
+    // MediaCrypto is not needed. In that case, we may ignore this error and
+    // continue playback, or fail the playback.
+    LOG(ERROR) << "MediaCrypto creation failed!";
+    return;
+  }
 
   // Retry decoder creation if the decoders are waiting for MediaCrypto.
   RetryDecoderCreation(true, true);
@@ -306,16 +314,9 @@ void MediaSourcePlayer::SetCdm(const scoped_refptr<MediaKeys>& cdm) {
   audio_decoder_job_->SetDrmBridge(drm_bridge);
   video_decoder_job_->SetDrmBridge(drm_bridge);
 
-  if (drm_bridge->GetMediaCrypto().is_null()) {
-    // Use BindToCurrentLoop to avoid reentrancy.
-    MediaDrmBridge::MediaCryptoReadyCB cb = BindToCurrentLoop(
-        base::Bind(&MediaSourcePlayer::OnMediaCryptoReady, weak_this_));
-    drm_bridge->SetMediaCryptoReadyCB(cb);
-    return;
-  }
-
-  // If the player is previously waiting for CDM, retry decoder creation.
-  RetryDecoderCreation(true, true);
+  // Use BindToCurrentLoop to avoid reentrancy.
+  drm_bridge->SetMediaCryptoReadyCB(BindToCurrentLoop(
+      base::Bind(&MediaSourcePlayer::OnMediaCryptoReady, weak_this_)));
 }
 
 void MediaSourcePlayer::OnDemuxerSeekDone(
diff --git a/media/base/android/video_decoder_job.cc b/media/base/android/video_decoder_job.cc
index d74ba4a..3dfe3b8 100644
--- a/media/base/android/video_decoder_job.cc
+++ b/media/base/android/video_decoder_job.cc
@@ -136,7 +136,7 @@ MediaDecoderJob::MediaDecoderJobStatus
 
   media_codec_bridge_.reset(VideoCodecBridge::CreateDecoder(
       video_codec_, is_secure, gfx::Size(config_width_, config_height_),
-      surface_.j_surface().obj(), GetMediaCrypto().obj()));
+      surface_.j_surface().obj(), GetMediaCrypto()));
 
   if (!media_codec_bridge_)
     return STATUS_FAILURE;