[eme] Reject CDM calls after connection error

media/mojo/clients/mojo_cdm.cc

Index: media/mojo/clients/mojo_cdm.cc
diff --git a/media/mojo/clients/mojo_cdm.cc b/media/mojo/clients/mojo_cdm.cc
index 042ef15f58b835a094b41ceecfe67bd106005994..f8ba6544cfaf2c8e819831866b88cc4db813046d 100644
--- a/media/mojo/clients/mojo_cdm.cc
+++ b/media/mojo/clients/mojo_cdm.cc
@@ -25,13 +25,6 @@
 
 namespace media {
 
-template <typename PromiseType>
-static void RejectPromise(std::unique_ptr<PromiseType> promise,
-                          mojom::CdmPromiseResultPtr result) {
-  promise->reject(result->exception, result->system_code,
-                  result->error_message);
-}
-
 // static
 void MojoCdm::Create(
     const std::string& key_system,
@@ -122,14 +115,14 @@ void MojoCdm::InitializeCdm(const std::string& key_system,
       base::Bind(&MojoCdm::OnCdmInitialized, base::Unretained(this)));
 }
 
-// TODO(xhwang): Properly handle CDM calls after connection error.
-// See http://crbug.com/671362
 void MojoCdm::OnConnectionError(uint32_t custom_reason,
                                 const std::string& description) {
   LOG(ERROR) << "Remote CDM connection error: custom_reason=" << custom_reason
              << ", description=\"" << description << "\"";
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  remote_cdm_.reset();
+
   // Handle initial connection error.
   if (pending_init_promise_) {
     DCHECK(!cdm_session_tracker_.HasRemainingSessions());
@@ -140,6 +133,9 @@ void MojoCdm::OnConnectionError(uint32_t custom_reason,
     return;
   }
 
+  // As communication with the remote CDM is broken, reject any outstanding
+  // promises and close all the existing sessions.
+  cdm_promise_adapter_.Clear();

[xhwang, 2016/12/14 18:43:54]

Thanks, I think you already fixed the second problem in the bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=671362#c2

However, there's no test coverage on that case. Also adding test for that would
need the MockCdm. For completeness, could you please remove the
CdmPromiseAdapter related changes from this CL, and add it in another CL with
proper test coverage? Thanks!

[jrummell, 2016/12/15 21:53:56]

Done.

   cdm_session_tracker_.CloseRemainingSessions(session_closed_cb_);
 }
 
@@ -148,9 +144,16 @@ void MojoCdm::SetServerCertificate(const std::vector<uint8_t>& certificate,
   DVLOG(2) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  if (!remote_cdm_) {
+    promise->reject(media::CdmPromise::INVALID_STATE_ERROR, 0,
+                    "CDM connection lost.");
+    return;
+  }
+
+  uint32_t promise_id = cdm_promise_adapter_.SavePromise(std::move(promise));
   remote_cdm_->SetServerCertificate(
       certificate, base::Bind(&MojoCdm::OnSimpleCdmPromiseResult,
-                              base::Unretained(this), base::Passed(&promise)));
+                              base::Unretained(this), promise_id));
 }
 
 void MojoCdm::CreateSessionAndGenerateRequest(
@@ -161,10 +164,17 @@ void MojoCdm::CreateSessionAndGenerateRequest(
   DVLOG(2) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  if (!remote_cdm_) {
+    promise->reject(media::CdmPromise::INVALID_STATE_ERROR, 0,
+                    "CDM connection lost.");
+    return;
+  }
+
+  uint32_t promise_id = cdm_promise_adapter_.SavePromise(std::move(promise));
   remote_cdm_->CreateSessionAndGenerateRequest(
       session_type, init_data_type, init_data,
       base::Bind(&MojoCdm::OnNewSessionCdmPromiseResult, base::Unretained(this),
-                 base::Passed(&promise)));
+                 promise_id));
 }
 
 void MojoCdm::LoadSession(SessionType session_type,
@@ -173,10 +183,16 @@ void MojoCdm::LoadSession(SessionType session_type,
   DVLOG(2) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  remote_cdm_->LoadSession(
-      session_type, session_id,
-      base::Bind(&MojoCdm::OnNewSessionCdmPromiseResult, base::Unretained(this),
-                 base::Passed(&promise)));
+  if (!remote_cdm_) {
+    promise->reject(media::CdmPromise::INVALID_STATE_ERROR, 0,
+                    "CDM connection lost.");
+    return;
+  }
+
+  uint32_t promise_id = cdm_promise_adapter_.SavePromise(std::move(promise));
+  remote_cdm_->LoadSession(session_type, session_id,
+                           base::Bind(&MojoCdm::OnNewSessionCdmPromiseResult,
+                                      base::Unretained(this), promise_id));
 }
 
 void MojoCdm::UpdateSession(const std::string& session_id,
@@ -185,10 +201,16 @@ void MojoCdm::UpdateSession(const std::string& session_id,
   DVLOG(2) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  remote_cdm_->UpdateSession(
-      session_id, response,
-      base::Bind(&MojoCdm::OnSimpleCdmPromiseResult, base::Unretained(this),
-                 base::Passed(&promise)));
+  if (!remote_cdm_) {
+    promise->reject(media::CdmPromise::INVALID_STATE_ERROR, 0,
+                    "CDM connection lost.");
+    return;
+  }
+
+  uint32_t promise_id = cdm_promise_adapter_.SavePromise(std::move(promise));
+  remote_cdm_->UpdateSession(session_id, response,
+                             base::Bind(&MojoCdm::OnSimpleCdmPromiseResult,
+                                        base::Unretained(this), promise_id));
 }
 
 void MojoCdm::CloseSession(const std::string& session_id,
@@ -196,9 +218,16 @@ void MojoCdm::CloseSession(const std::string& session_id,
   DVLOG(2) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  remote_cdm_->CloseSession(
-      session_id, base::Bind(&MojoCdm::OnSimpleCdmPromiseResult,
-                             base::Unretained(this), base::Passed(&promise)));
+  if (!remote_cdm_) {
+    promise->reject(media::CdmPromise::INVALID_STATE_ERROR, 0,
+                    "CDM connection lost.");
+    return;
+  }
+
+  uint32_t promise_id = cdm_promise_adapter_.SavePromise(std::move(promise));
+  remote_cdm_->CloseSession(session_id,
+                            base::Bind(&MojoCdm::OnSimpleCdmPromiseResult,
+                                       base::Unretained(this), promise_id));
 }
 
 void MojoCdm::RemoveSession(const std::string& session_id,
@@ -206,9 +235,16 @@ void MojoCdm::RemoveSession(const std::string& session_id,
   DVLOG(2) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  remote_cdm_->RemoveSession(
-      session_id, base::Bind(&MojoCdm::OnSimpleCdmPromiseResult,
-                             base::Unretained(this), base::Passed(&promise)));
+  if (!remote_cdm_) {
+    promise->reject(media::CdmPromise::INVALID_STATE_ERROR, 0,
+                    "CDM connection lost.");
+    return;
+  }
+
+  uint32_t promise_id = cdm_promise_adapter_.SavePromise(std::move(promise));
+  remote_cdm_->RemoveSession(session_id,
+                             base::Bind(&MojoCdm::OnSimpleCdmPromiseResult,
+                                        base::Unretained(this), promise_id));
 }
 
 CdmContext* MojoCdm::GetCdmContext() {
@@ -305,7 +341,8 @@ void MojoCdm::OnCdmInitialized(mojom::CdmPromiseResultPtr result,
   DCHECK(pending_init_promise_);
 
   if (!result->success) {
-    RejectPromise(std::move(pending_init_promise_), std::move(result));
+    pending_init_promise_->reject(result->exception, result->system_code,
+                                  result->error_message);
     return;
   }
 
@@ -330,24 +367,28 @@ void MojoCdm::OnKeyAdded() {
   decryptor_->OnKeyAdded();
 }
 
-void MojoCdm::OnSimpleCdmPromiseResult(
-    std::unique_ptr<SimpleCdmPromise> promise,
-    mojom::CdmPromiseResultPtr result) {
-  if (result->success)
-    promise->resolve();
-  else
-    RejectPromise(std::move(promise), std::move(result));
+void MojoCdm::OnSimpleCdmPromiseResult(uint32_t promise_id,
+                                       mojom::CdmPromiseResultPtr result) {
+  if (result->success) {
+    cdm_promise_adapter_.ResolvePromise(promise_id);
+  } else {
+    cdm_promise_adapter_.RejectPromise(promise_id, result->exception,
+                                       result->system_code,
+                                       result->error_message);
+  }
 }
 
-void MojoCdm::OnNewSessionCdmPromiseResult(
-    std::unique_ptr<NewSessionCdmPromise> promise,
-    mojom::CdmPromiseResultPtr result,
-    const std::string& session_id) {
+void MojoCdm::OnNewSessionCdmPromiseResult(uint32_t promise_id,
+                                           mojom::CdmPromiseResultPtr result,
+                                           const std::string& session_id) {
   if (result->success) {
     cdm_session_tracker_.AddSession(session_id);
-    promise->resolve(session_id);
-  } else
-    RejectPromise(std::move(promise), std::move(result));
+    cdm_promise_adapter_.ResolvePromise(promise_id, session_id);
+  } else {
+    cdm_promise_adapter_.RejectPromise(promise_id, result->exception,
+                                       result->system_code,
+                                       result->error_message);
+  }
 }
 
 }  // namespace media