Remove VideoDecodeAccelerator::SetCdm() and pass the cdm in Initialize()

media/gpu/android_video_decode_accelerator.cc

Index: media/gpu/android_video_decode_accelerator.cc
diff --git a/media/gpu/android_video_decode_accelerator.cc b/media/gpu/android_video_decode_accelerator.cc
index af112754e1d66cc0646440712884622b78fc998b..ac5ffca4761dc37f20352528d84f2948e61d34e7 100644
--- a/media/gpu/android_video_decode_accelerator.cc
+++ b/media/gpu/android_video_decode_accelerator.cc
@@ -339,7 +339,6 @@ bool AndroidVideoDecodeAccelerator::Initialize(const Config& config,
   DCHECK(!media_codec_);
   DCHECK(thread_checker_.CalledOnValidThread());
   TRACE_EVENT0("media", "AVDA::Initialize");
-
   DVLOG(1) << __FUNCTION__ << ": " << config.AsHumanReadableString();
 
   if (make_context_current_cb_.is_null() || get_gles2_decoder_cb_.is_null()) {
@@ -360,23 +359,18 @@ bool AndroidVideoDecodeAccelerator::Initialize(const Config& config,
       config.initial_expected_coded_size;
   is_encrypted_ = config.is_encrypted;
 
-  bool profile_supported = codec_config_->codec_ == media::kCodecVP8 ||
-                           codec_config_->codec_ == media::kCodecVP9 ||
-                           codec_config_->codec_ == media::kCodecH264;
-
   // We signalled that we support deferred initialization, so see if the client
   // does also.
   deferred_initialization_pending_ = config.is_deferred_initialization_allowed;
 
+  bool profile_supported = codec_config_->codec_ == media::kCodecVP8 ||
+                           codec_config_->codec_ == media::kCodecVP9 ||
+                           codec_config_->codec_ == media::kCodecH264;
   if (!profile_supported) {
     LOG(ERROR) << "Unsupported profile: " << config.profile;
     return false;
   }
 
-  // For encrypted streams we postpone configuration until MediaCrypto is
-  // available.
-  DCHECK(!is_encrypted_ || deferred_initialization_pending_);

[watk, 2016/05/09 23:39:59]

DCHECK is below now

-
   // Only use MediaCodec for VP8/9 if it's likely backed by hardware
   // or if the stream is encrypted.
   if ((codec_config_->codec_ == media::kCodecVP8 ||
@@ -433,13 +427,16 @@ bool AndroidVideoDecodeAccelerator::Initialize(const Config& config,
   // Start the thread for async configuration, even if we don't need it now.
   // ResetCodecState might rebuild the codec later, for example.
   if (!g_avda_timer.Pointer()->StartThread(this)) {
-    LOG(ERROR) << "Failed to start thread for AVDA timer";

[watk, 2016/05/09 23:39:59]

The timer doesn't run on that thread.

+    LOG(ERROR) << "Failed to start AVDA thread";
     return false;
   }
 
   // If we are encrypted, then we aren't able to create the codec yet.
-  if (is_encrypted_)
+  if (is_encrypted_) {
+    DCHECK(deferred_initialization_pending_);

[sandersd (OOO until July 31), 2016/05/10 00:09:54]

We should handle this case gracefully (return false).

+    InitializeCdm(config.cdm_id);
     return true;
+  }
 
   if (deferred_initialization_pending_) {
     ConfigureMediaCodecAsynchronously();
@@ -451,55 +448,6 @@ bool AndroidVideoDecodeAccelerator::Initialize(const Config& config,
   return ConfigureMediaCodecSynchronously();
 }
 
-void AndroidVideoDecodeAccelerator::SetCdm(int cdm_id) {

[watk, 2016/05/09 23:39:59]

I moved this block because it's now in the private: section, but it's almost
unchanged. The difference is that I flipped the order of defined/!defined
because i think it's clearer, and removed a few lines at the start.

-  DVLOG(2) << __FUNCTION__ << ": " << cdm_id;
-
-#if defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
-  DCHECK(client_) << "SetCdm() must be called after Initialize().";
-
-  if (media_drm_bridge_cdm_context_) {
-    NOTREACHED() << "We do not support resetting CDM.";
-    NotifyInitializationComplete(false);
-    return;
-  }
-
-  // Store the CDM to hold a reference to it.
-  cdm_for_reference_holding_only_ = media::MojoCdmService::LegacyGetCdm(cdm_id);
-  DCHECK(cdm_for_reference_holding_only_);
-
-  // On Android platform the CdmContext must be a MediaDrmBridgeCdmContext.
-  media_drm_bridge_cdm_context_ = static_cast<media::MediaDrmBridgeCdmContext*>(
-      cdm_for_reference_holding_only_->GetCdmContext());
-  DCHECK(media_drm_bridge_cdm_context_);
-
-  // Register CDM callbacks. The callbacks registered will be posted back to
-  // this thread via BindToCurrentLoop.
-
-  // Since |this| holds a reference to the |cdm_|, by the time the CDM is
-  // destructed, UnregisterPlayer() must have been called and |this| has been
-  // destructed as well. So the |cdm_unset_cb| will never have a chance to be
-  // called.
-  // TODO(xhwang): Remove |cdm_unset_cb| after it's not used on all platforms.
-  cdm_registration_id_ = media_drm_bridge_cdm_context_->RegisterPlayer(
-      media::BindToCurrentLoop(
-          base::Bind(&AndroidVideoDecodeAccelerator::OnKeyAdded,
-                     weak_this_factory_.GetWeakPtr())),
-      base::Bind(&base::DoNothing));
-
-  media_drm_bridge_cdm_context_->SetMediaCryptoReadyCB(media::BindToCurrentLoop(
-      base::Bind(&AndroidVideoDecodeAccelerator::OnMediaCryptoReady,
-                 weak_this_factory_.GetWeakPtr())));
-
-// Postpone NotifyInitializationComplete() call till we create the MediaCodec
-// after OnMediaCryptoReady().
-#else
-
-  NOTIMPLEMENTED();
-  NotifyInitializationComplete(false);
-
-#endif  // !defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
-}
-
 void AndroidVideoDecodeAccelerator::DoIOTask(bool start_timer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   TRACE_EVENT0("media", "AVDA::DoIOTask");
@@ -1298,6 +1246,43 @@ void AndroidVideoDecodeAccelerator::PostError(
   state_ = ERROR;
 }
 
+void AndroidVideoDecodeAccelerator::InitializeCdm(int cdm_id) {
+  DVLOG(2) << __FUNCTION__ << ": " << cdm_id;
+
+#if !defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
+  NOTIMPLEMENTED();
+  NotifyInitializationComplete(false);
+#else
+  // Store the CDM to hold a reference to it.
+  cdm_for_reference_holding_only_ = media::MojoCdmService::LegacyGetCdm(cdm_id);
+  DCHECK(cdm_for_reference_holding_only_);
+
+  // On Android platform the CdmContext must be a MediaDrmBridgeCdmContext.
+  media_drm_bridge_cdm_context_ = static_cast<media::MediaDrmBridgeCdmContext*>(
+      cdm_for_reference_holding_only_->GetCdmContext());
+  DCHECK(media_drm_bridge_cdm_context_);
+
+  // Register CDM callbacks. The callbacks registered will be posted back to
+  // this thread via BindToCurrentLoop.
+
+  // Since |this| holds a reference to the |cdm_|, by the time the CDM is
+  // destructed, UnregisterPlayer() must have been called and |this| has been
+  // destructed as well. So the |cdm_unset_cb| will never have a chance to be
+  // called.
+  // TODO(xhwang): Remove |cdm_unset_cb| after it's not used on all platforms.
+  cdm_registration_id_ = media_drm_bridge_cdm_context_->RegisterPlayer(
+      media::BindToCurrentLoop(
+          base::Bind(&AndroidVideoDecodeAccelerator::OnKeyAdded,
+                     weak_this_factory_.GetWeakPtr())),
+      base::Bind(&base::DoNothing));
+
+  // Deferred initialization will continue in OnMediaCryptoReady().
+  media_drm_bridge_cdm_context_->SetMediaCryptoReadyCB(media::BindToCurrentLoop(
+      base::Bind(&AndroidVideoDecodeAccelerator::OnMediaCryptoReady,
+                 weak_this_factory_.GetWeakPtr())));
+#endif  // !defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
+}
+
 void AndroidVideoDecodeAccelerator::OnMediaCryptoReady(
     media::MediaDrmBridgeCdmContext::JavaObjectPtr media_crypto,
     bool needs_protected_surface) {