Handle plugin instance crash in ContentDecryptorDelegate.

BUG=chrome-os-partner:17801
TEST=Kill Widevine CDM process and we get decode error.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243762 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/media/encrypted_media_browsertest.cc

@@ -40,6 +40,8 @@ const char kExternalClearKeyFileIOTestKeySystem[] =
     "org.chromium.externalclearkey.fileiotest";
 const char kExternalClearKeyInitializeFailKeySystem[] =
     "org.chromium.externalclearkey.initializefail";
+const char kExternalClearKeyCrashKeySystem[] =
+    "org.chromium.externalclearkey.crash";
 
 // Supported media types.
 const char kWebMAudioOnly[] = "audio/webm; codecs=\"vorbis\"";
@@ -400,6 +402,16 @@ IN_PROC_BROWSER_TEST_F(ECKEncryptedMediaTest, InitializeCDMFail) {
                         kEmeKeyError);
 }
 
+// When CDM crashes, we should still get a decode error.
+IN_PROC_BROWSER_TEST_F(ECKEncryptedMediaTest, CDMCrashDuringDecode) {
+  RunEncryptedMediaTest("encrypted_media_player.html",
+                        "bear-a-enc_a.webm",
+                        kWebMAudioOnly,
+                        kExternalClearKeyCrashKeySystem,
+                        SRC,
+                        kError);
+}
+
 IN_PROC_BROWSER_TEST_F(ECKEncryptedMediaTest, FileIOTest) {
   RunEncryptedMediaTest("encrypted_media_player.html",
                         "bear-a-enc_a.webm",

chrome/browser/media/media_browsertest.h

@@ -23,6 +23,7 @@ class MediaBrowserTest : public InProcessBrowserTest {
 
   // Common test results.
   static const char kEnded[];
+  // TODO(xhwang): Report detailed errors, e.g. "ERROR-3".
   static const char kError[];
   static const char kFailed[];
 

chrome/renderer/media/chrome_key_systems.cc

@@ -70,6 +70,8 @@ static void AddExternalClearKey(
       "org.chromium.externalclearkey.fileiotest";
   static const char kExternalClearKeyInitializeFailKeySystem[] =
       "org.chromium.externalclearkey.initializefail";
+  static const char kExternalClearKeyCrashKeySystem[] =
+      "org.chromium.externalclearkey.crash";
   static const char kExternalClearKeyPepperType[] =
       "application/x-ppapi-clearkey-cdm";
 
@@ -106,6 +108,10 @@ static void AddExternalClearKey(
   // failure case.
   info.key_system = kExternalClearKeyInitializeFailKeySystem;
   concrete_key_systems->push_back(info);
+
+  // A key system that triggers a crash in ClearKeyCdm.
+  info.key_system = kExternalClearKeyCrashKeySystem;
+  concrete_key_systems->push_back(info);
 }
 #endif  // defined(ENABLE_PEPPER_CDMS)
 

content/renderer/media/crypto/ppapi_decryptor.cc

@@ -38,9 +38,8 @@ scoped_ptr<PpapiDecryptor> PpapiDecryptor::Create(
     return scoped_ptr<PpapiDecryptor>();
   }
 
-  plugin_cdm_delegate->Initialize(key_system);
-
-  return scoped_ptr<PpapiDecryptor>(new PpapiDecryptor(plugin_instance,
+  return scoped_ptr<PpapiDecryptor>(new PpapiDecryptor(key_system,
+                                                       plugin_instance,
                                                        plugin_cdm_delegate,
                                                        session_created_cb,
                                                        session_message_cb,
@@ -51,6 +50,7 @@ scoped_ptr<PpapiDecryptor> PpapiDecryptor::Create(
 }
 
 PpapiDecryptor::PpapiDecryptor(
+    const std::string& key_system,
     const scoped_refptr<PepperPluginInstanceImpl>& plugin_instance,
     ContentDecryptorDelegate* plugin_cdm_delegate,
     const media::SessionCreatedCB& session_created_cb,
@@ -79,18 +79,21 @@ PpapiDecryptor::PpapiDecryptor(
 
   weak_this_ = weak_ptr_factory_.GetWeakPtr();
 
-  plugin_cdm_delegate_->SetSessionEventCallbacks(
+  plugin_cdm_delegate_->Initialize(
+      key_system,
       base::Bind(&PpapiDecryptor::OnSessionCreated, weak_this_),
       base::Bind(&PpapiDecryptor::OnSessionMessage, weak_this_),
       base::Bind(&PpapiDecryptor::OnSessionReady, weak_this_),
       base::Bind(&PpapiDecryptor::OnSessionClosed, weak_this_),
-      base::Bind(&PpapiDecryptor::OnSessionError, weak_this_));
+      base::Bind(&PpapiDecryptor::OnSessionError, weak_this_),
+      base::Bind(&PpapiDecryptor::OnFatalPluginError, weak_this_));
 }
 
 PpapiDecryptor::~PpapiDecryptor() {
   plugin_cdm_delegate_ = NULL;
   plugin_instance_ = NULL;
-  destroy_plugin_cb_.Run();
+  if (!destroy_plugin_cb_.is_null())
+    base::ResetAndReturn(&destroy_plugin_cb_).Run();
 }
 
 bool PpapiDecryptor::CreateSession(uint32 session_id,
@@ -101,8 +104,9 @@ bool PpapiDecryptor::CreateSession(uint32 session_id,
   DCHECK(render_loop_proxy_->BelongsToCurrentThread());
   DCHECK(plugin_cdm_delegate_);
 
-  if (!plugin_cdm_delegate_->CreateSession(
-      session_id, type, init_data, init_data_length)) {
+  if (!plugin_cdm_delegate_ ||
+      !plugin_cdm_delegate_->CreateSession(
+           session_id, type, init_data, init_data_length)) {
     ReportFailureToCallPlugin(session_id);
     return false;
   }
@@ -116,9 +120,10 @@ void PpapiDecryptor::UpdateSession(uint32 session_id,
   DVLOG(2) << __FUNCTION__;
   DCHECK(render_loop_proxy_->BelongsToCurrentThread());
 
-  if (!plugin_cdm_delegate_->UpdateSession(
-           session_id, response, response_length))
+  if (!plugin_cdm_delegate_ || !plugin_cdm_delegate_->UpdateSession(
+                                    session_id, response, response_length)) {
     ReportFailureToCallPlugin(session_id);
+  }
 
   if (!new_audio_key_cb_.is_null())
     new_audio_key_cb_.Run();
@@ -131,8 +136,10 @@ void PpapiDecryptor::ReleaseSession(uint32 session_id) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(render_loop_proxy_->BelongsToCurrentThread());
 
-  if (!plugin_cdm_delegate_->ReleaseSession(session_id))
+  if (!plugin_cdm_delegate_ ||
+      !plugin_cdm_delegate_->ReleaseSession(session_id)) {
     ReportFailureToCallPlugin(session_id);
+  }
 }
 
 media::Decryptor* PpapiDecryptor::GetDecryptor() {
@@ -184,8 +191,10 @@ void PpapiDecryptor::Decrypt(
   }
 
   DVLOG(3) << __FUNCTION__ << " - stream_type: " << stream_type;
-  if (!plugin_cdm_delegate_->Decrypt(stream_type, encrypted, decrypt_cb))
+  if (!plugin_cdm_delegate_ ||
+      !plugin_cdm_delegate_->Decrypt(stream_type, encrypted, decrypt_cb)) {
     decrypt_cb.Run(kError, NULL);
+  }
 }
 
 void PpapiDecryptor::CancelDecrypt(StreamType stream_type) {
@@ -196,7 +205,8 @@ void PpapiDecryptor::CancelDecrypt(StreamType stream_type) {
   }
 
   DVLOG(1) << __FUNCTION__ << " - stream_type: " << stream_type;
-  plugin_cdm_delegate_->CancelDecrypt(stream_type);
+  if (plugin_cdm_delegate_)
+    plugin_cdm_delegate_->CancelDecrypt(stream_type);
 }
 
 void PpapiDecryptor::InitializeAudioDecoder(
@@ -213,8 +223,9 @@ void PpapiDecryptor::InitializeAudioDecoder(
   DCHECK(config.IsValidConfig());
 
   audio_decoder_init_cb_ = init_cb;
-  if (!plugin_cdm_delegate_->InitializeAudioDecoder(config, base::Bind(
-      &PpapiDecryptor::OnDecoderInitialized, weak_this_, kAudio))) {
+  if (!plugin_cdm_delegate_ ||
+      !plugin_cdm_delegate_->InitializeAudioDecoder(config, base::Bind(
+           &PpapiDecryptor::OnDecoderInitialized, weak_this_, kAudio))) {
     base::ResetAndReturn(&audio_decoder_init_cb_).Run(false);
     return;
   }
@@ -234,8 +245,9 @@ void PpapiDecryptor::InitializeVideoDecoder(
   DCHECK(config.IsValidConfig());
 
   video_decoder_init_cb_ = init_cb;
-  if (!plugin_cdm_delegate_->InitializeVideoDecoder(config, base::Bind(
-      &PpapiDecryptor::OnDecoderInitialized, weak_this_, kVideo))) {
+  if (!plugin_cdm_delegate_ ||
+      !plugin_cdm_delegate_->InitializeVideoDecoder(config, base::Bind(
+           &PpapiDecryptor::OnDecoderInitialized, weak_this_, kVideo))) {
     base::ResetAndReturn(&video_decoder_init_cb_).Run(false);
     return;
   }
@@ -252,8 +264,10 @@ void PpapiDecryptor::DecryptAndDecodeAudio(
   }
 
   DVLOG(3) << __FUNCTION__;
-  if (!plugin_cdm_delegate_->DecryptAndDecodeAudio(encrypted, audio_decode_cb))
+  if (!plugin_cdm_delegate_ || !plugin_cdm_delegate_->DecryptAndDecodeAudio(
+                                    encrypted, audio_decode_cb)) {
     audio_decode_cb.Run(kError, AudioBuffers());
+  }
 }
 
 void PpapiDecryptor::DecryptAndDecodeVideo(
@@ -267,8 +281,10 @@ void PpapiDecryptor::DecryptAndDecodeVideo(
   }
 
   DVLOG(3) << __FUNCTION__;
-  if (!plugin_cdm_delegate_->DecryptAndDecodeVideo(encrypted, video_decode_cb))
+  if (!plugin_cdm_delegate_ || !plugin_cdm_delegate_->DecryptAndDecodeVideo(
+                                    encrypted, video_decode_cb)) {
     video_decode_cb.Run(kError, NULL);
+  }
 }
 
 void PpapiDecryptor::ResetDecoder(StreamType stream_type) {
@@ -279,7 +295,8 @@ void PpapiDecryptor::ResetDecoder(StreamType stream_type) {
   }
 
   DVLOG(2) << __FUNCTION__ << " - stream_type: " << stream_type;
-  plugin_cdm_delegate_->ResetDecoder(stream_type);
+  if (plugin_cdm_delegate_)
+    plugin_cdm_delegate_->ResetDecoder(stream_type);
 }
 
 void PpapiDecryptor::DeinitializeDecoder(StreamType stream_type) {
@@ -290,7 +307,8 @@ void PpapiDecryptor::DeinitializeDecoder(StreamType stream_type) {
   }
 
   DVLOG(2) << __FUNCTION__ << " - stream_type: " << stream_type;
-  plugin_cdm_delegate_->DeinitializeDecoder(stream_type);
+  if (plugin_cdm_delegate_)
+    plugin_cdm_delegate_->DeinitializeDecoder(stream_type);
 }
 
 void PpapiDecryptor::ReportFailureToCallPlugin(uint32 session_id) {
@@ -346,4 +364,12 @@ void PpapiDecryptor::OnSessionError(uint32 session_id,
   session_error_cb_.Run(session_id, error_code, system_code);
 }
 
+void PpapiDecryptor::OnFatalPluginError() {
+  DCHECK(render_loop_proxy_->BelongsToCurrentThread());
+  DCHECK(plugin_cdm_delegate_);
+  plugin_cdm_delegate_ = NULL;
+  plugin_instance_ = NULL;
+  base::ResetAndReturn(&destroy_plugin_cb_).Run();
+}
+
 }  // namespace content

content/renderer/media/crypto/ppapi_decryptor.h

@@ -73,7 +73,8 @@ class PpapiDecryptor : public media::MediaKeys, public media::Decryptor {
   virtual void DeinitializeDecoder(StreamType stream_type) OVERRIDE;
 
  private:
-  PpapiDecryptor(const scoped_refptr<PepperPluginInstanceImpl>& plugin_instance,
+  PpapiDecryptor(const std::string& key_system,
+                 const scoped_refptr<PepperPluginInstanceImpl>& plugin_instance,
                  ContentDecryptorDelegate* plugin_cdm_delegate,
                  const media::SessionCreatedCB& session_created_cb,
                  const media::SessionMessageCB& session_message_cb,
@@ -97,6 +98,11 @@ class PpapiDecryptor : public media::MediaKeys, public media::Decryptor {
                       media::MediaKeys::KeyError error_code,
                       int system_code);
 
+  // Callback to notify that a fatal error happened in |plugin_cdm_delegate_|.
+  // The error is terminal and |plugin_cdm_delegate_| should not be used after
+  // this call.
+  void OnFatalPluginError();
+
   base::WeakPtr<PpapiDecryptor> weak_this_;
 
   // Hold a reference of the plugin instance to make sure the plugin outlives

content/renderer/pepper/content_decryptor_delegate.cc

@@ -256,29 +256,35 @@ ContentDecryptorDelegate::ContentDecryptorDelegate(
 }
 
 ContentDecryptorDelegate::~ContentDecryptorDelegate() {
+  SatisfyAllPendingCallbacksOnError();
 }
 
-void ContentDecryptorDelegate::Initialize(const std::string& key_system) {
-  DCHECK(!key_system.empty());
-  DCHECK(key_system_.empty());
-  key_system_ = key_system;
-
-  plugin_decryption_interface_->Initialize(
-      pp_instance_,
-      StringVar::StringToPPVar(key_system_));
-}
-
-void ContentDecryptorDelegate::SetSessionEventCallbacks(
+void ContentDecryptorDelegate::Initialize(
+    const std::string& key_system,
     const media::SessionCreatedCB& session_created_cb,
     const media::SessionMessageCB& session_message_cb,
     const media::SessionReadyCB& session_ready_cb,
     const media::SessionClosedCB& session_closed_cb,
-    const media::SessionErrorCB& session_error_cb) {
+    const media::SessionErrorCB& session_error_cb,
+    const base::Closure& fatal_plugin_error_cb) {
+  DCHECK(!key_system.empty());
+  DCHECK(key_system_.empty());
+  key_system_ = key_system;
+
   session_created_cb_ = session_created_cb;
   session_message_cb_ = session_message_cb;
   session_ready_cb_ = session_ready_cb;
   session_closed_cb_ = session_closed_cb;
   session_error_cb_ = session_error_cb;
+  fatal_plugin_error_cb_ = fatal_plugin_error_cb;
+
+  plugin_decryption_interface_->Initialize(
+      pp_instance_, StringVar::StringToPPVar(key_system_));
+}
+
+void ContentDecryptorDelegate::InstanceCrashed() {
+  fatal_plugin_error_cb_.Run();
+  SatisfyAllPendingCallbacksOnError();
 }
 
 bool ContentDecryptorDelegate::CreateSession(uint32 session_id,
@@ -319,6 +325,7 @@ bool ContentDecryptorDelegate::Decrypt(
     const scoped_refptr<media::DecoderBuffer>& encrypted_buffer,
     const Decryptor::DecryptCB& decrypt_cb) {
   DVLOG(3) << "Decrypt() - stream_type: " << stream_type;
+
   // |{audio|video}_input_resource_| is not being used by the plugin
   // now because there is only one pending audio/video decrypt request at any
   // time. This is enforced by the media pipeline.
@@ -1022,4 +1029,30 @@ bool ContentDecryptorDelegate::DeserializeAudioFrames(
   return true;
 }
 
+void ContentDecryptorDelegate::SatisfyAllPendingCallbacksOnError() {
+  if (!audio_decoder_init_cb_.is_null())
+    audio_decoder_init_cb_.ResetAndReturn().Run(false);
+
+  if (!video_decoder_init_cb_.is_null())
+    video_decoder_init_cb_.ResetAndReturn().Run(false);
+
+  audio_input_resource_ = NULL;
+  video_input_resource_ = NULL;
+
+  if (!audio_decrypt_cb_.is_null())
+    audio_decrypt_cb_.ResetAndReturn().Run(media::Decryptor::kError, NULL);
+
+  if (!video_decrypt_cb_.is_null())
+    video_decrypt_cb_.ResetAndReturn().Run(media::Decryptor::kError, NULL);
+
+  if (!audio_decode_cb_.is_null()) {
+    const media::Decryptor::AudioBuffers empty_frames;
+    audio_decode_cb_.ResetAndReturn().Run(media::Decryptor::kError,
+                                          empty_frames);
+  }
+
+  if (!video_decode_cb_.is_null())
+    video_decode_cb_.ResetAndReturn().Run(media::Decryptor::kError, NULL);
+}
+
 }  // namespace content

content/renderer/pepper/content_decryptor_delegate.h

@@ -40,14 +40,16 @@ class ContentDecryptorDelegate {
       const PPP_ContentDecryptor_Private* plugin_decryption_interface);
   ~ContentDecryptorDelegate();
 
-  void Initialize(const std::string& key_system);
+  // This object should not be accessed after |fatal_plugin_error_cb| is called.
+  void Initialize(const std::string& key_system,
+                  const media::SessionCreatedCB& session_created_cb,
+                  const media::SessionMessageCB& session_message_cb,
+                  const media::SessionReadyCB& session_ready_cb,
+                  const media::SessionClosedCB& session_closed_cb,
+                  const media::SessionErrorCB& session_error_cb,
+                  const base::Closure& fatal_plugin_error_cb);
 
-  void SetSessionEventCallbacks(
-      const media::SessionCreatedCB& session_created_cb,
-      const media::SessionMessageCB& session_message_cb,
-      const media::SessionReadyCB& session_ready_cb,
-      const media::SessionClosedCB& session_closed_cb,
-      const media::SessionErrorCB& session_error_cb);
+  void InstanceCrashed();
 
   // Provides access to PPP_ContentDecryptor_Private.
   bool CreateSession(uint32 session_id,
@@ -109,8 +111,11 @@ class ContentDecryptorDelegate {
   class TrackableCallback {
    public:
     TrackableCallback() : id_(0u) {}
-    // TODO(xhwang): Check that no callback is pending in dtor.
-    ~TrackableCallback() {};
+    ~TrackableCallback() {
+      // Callbacks must be satisfied.
+      DCHECK_EQ(id_, 0u);
+      DCHECK(is_null());
+    };
 
     bool Matches(uint32_t id) const { return id == id_; }
 
@@ -162,6 +167,8 @@ class ContentDecryptorDelegate {
                               media::SampleFormat sample_format,
                               media::Decryptor::AudioBuffers* frames);
 
+  void SatisfyAllPendingCallbacksOnError();
+
   const PP_Instance pp_instance_;
   const PPP_ContentDecryptor_Private* const plugin_decryption_interface_;
 
@@ -175,6 +182,10 @@ class ContentDecryptorDelegate {
   media::SessionClosedCB session_closed_cb_;
   media::SessionErrorCB session_error_cb_;
 
+  // Callback to notify that unexpected error happened and |this| should not
+  // be used anymore.
+  base::Closure fatal_plugin_error_cb_;
+
   gfx::Size natural_size_;
 
   // Request ID for tracking pending content decryption callbacks.

content/renderer/pepper/pepper_plugin_instance_impl.cc

@@ -718,6 +718,11 @@ void PepperPluginInstanceImpl::InstanceCrashed() {
   BindGraphics(pp_instance(), 0);
   InvalidateRect(gfx::Rect());
 
+  if (content_decryptor_delegate_) {
+    content_decryptor_delegate_->InstanceCrashed();
+    content_decryptor_delegate_.reset();
+  }
+
   render_frame_->PluginCrashed(module_->path(), module_->GetPeerProcessId());
   UnSetAndDeleteLockTargetAdapter();
 }

media/cdm/ppapi/external_clear_key/clear_key_cdm.cc

@@ -66,6 +66,9 @@ const char kExternalClearKeyDecryptOnlyKeySystem[] =
     "org.chromium.externalclearkey.decryptonly";
 const char kExternalClearKeyFileIOTestKeySystem[] =
     "org.chromium.externalclearkey.fileiotest";
+const char kExternalClearKeyCrashKeySystem[] =
+    "org.chromium.externalclearkey.crash";
+
 const int64 kSecondsPerMinute = 60;
 const int64 kMsPerSecond = 1000;
 const int64 kInitialTimerDelayMs = 200;
@@ -147,7 +150,8 @@ void* CreateCdmInstance(int cdm_interface_version,
   std::string key_system_string(key_system, key_system_size);
   if (key_system_string != kExternalClearKeyKeySystem &&
       key_system_string != kExternalClearKeyDecryptOnlyKeySystem &&
-      key_system_string != kExternalClearKeyFileIOTestKeySystem) {
+      key_system_string != kExternalClearKeyFileIOTestKeySystem &&
+      key_system_string != kExternalClearKeyCrashKeySystem) {
     DVLOG(1) << "Unsupported key system:" << key_system_string;
     return NULL;
   }
@@ -391,6 +395,10 @@ cdm::Status ClearKeyCdm::DecryptAndDecodeSamples(
     cdm::AudioFrames* audio_frames) {
   DVLOG(1) << "DecryptAndDecodeSamples()";
 
+  // Trigger a crash on purpose for testing purpose.
+  if (key_system_ == kExternalClearKeyCrashKeySystem)
+    CHECK(false);
+
   scoped_refptr<media::DecoderBuffer> buffer;
   cdm::Status status = DecryptToMediaDecoderBuffer(encrypted_buffer, &buffer);