media: Refactor MediaCrypto creation and notification.

media/base/android/media_codec_player.cc

Index: media/base/android/media_codec_player.cc
diff --git a/media/base/android/media_codec_player.cc b/media/base/android/media_codec_player.cc
index a58ac27b6e52bce5fbf0fc082b537d008a55bee2..35ea50e88344fb9bb0a60d7237c33e6890c76d8c 100644
--- a/media/base/android/media_codec_player.cc
+++ b/media/base/android/media_codec_player.cc
@@ -937,7 +937,12 @@ 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.

[Tima Vaisburd, 2015/12/11 02:15:21]

Wouldn't this has to be taken care of in this CL or a prerequisite CL? Min,
WDYT? The same consideration applies to MediaSourcePlayer.

[xhwang, 2015/12/11 05:50:19]

I don't think so. This CL doesn't regress anything. 

Previously if MediaCrypto creation failed MediaCodecPlayer won't get notified,
and we'll just be waiting forever. This CL doesn't change this.

Sometimes it takes multiple steps to fix one problem. I don't think we should
just have one big CL to fix everything.

[Tima Vaisburd, 2015/12/11 19:31:24]

Good point, and good explanation! I think it's worth putting into the CL
description (before it just never get notified, now it is notified as error).
You are right and I take my objection back.

However, the only thing I would do to "fallback nicely" is to go to the error
state and report an error to the upper level. Both things are done with one line
  GetMediaTaskRunner()->PostTask(FROM_HERE, internal_error_cb_);

Would you be able to add it if media_crypto is null? Then there would be no
TODO.

[xhwang, 2015/12/11 21:45:16]

Thanks. As discussed offline, I'd like this TODO be addressed in a different CL.

+    LOG(ERROR) << "MediaCrypto creation failed.";
+    return;
+  }
 
   media_crypto_ = media_crypto.Pass();