ReleaseSession() should call RemoveSession()

media/cdm/ppapi/cdm_wrapper.h

Index: media/cdm/ppapi/cdm_wrapper.h
diff --git a/media/cdm/ppapi/cdm_wrapper.h b/media/cdm/ppapi/cdm_wrapper.h
index 72afa90a029b2b06f972b71115d16d1f89f57d3f..9f390a26d760f0c109391f41e9c857fe7c02cf2b 100644
--- a/media/cdm/ppapi/cdm_wrapper.h
+++ b/media/cdm/ppapi/cdm_wrapper.h
@@ -56,10 +56,10 @@ class CdmWrapper {
                              uint32_t web_session_id_size,
                              const uint8_t* response,
                              uint32_t response_size) = 0;
-  virtual void CloseSession(uint32_t promise_id,
+  virtual bool CloseSession(uint32_t promise_id,

[xhwang, 2014/08/23 00:01:48]

We can change the return value back to void once we stop supporting CDM4? In
that case, add a TODO here to track that.

[jrummell, 2014/08/23 00:27:15]

Done.

                             const char* web_session_id,
                             uint32_t web_session_id_size) = 0;
-  virtual bool RemoveSession(uint32_t promise_id,
+  virtual void RemoveSession(uint32_t promise_id,
                              const char* web_session_id,
                              uint32_t web_session_id_size) = 0;

[xhwang, 2014/08/23 00:01:48]

nit: swap the order of CloseSession and RemoveSession to be consistent with
CdmAdapter.

[ddorwin, 2014/08/23 00:14:30]

Actually, the CdmAdapter order is "wrong". I am fine with leaving it, though, to
kep this CL small. Maybe we need a TODO there along with the rename. WDYT?

[jrummell, 2014/08/23 00:27:15]

Fixed CdmAdapter.

[jrummell, 2014/08/23 00:27:15]

Fixed CdmAdapter instead.

   virtual bool GetUsableKeyIds(uint32_t promise_id,
@@ -226,17 +226,17 @@ class CdmWrapperImpl : public CdmWrapper {
     return true;
   }
 
-  virtual void CloseSession(uint32_t promise_id,
+  virtual bool CloseSession(uint32_t promise_id,
                             const char* web_session_id,
                             uint32_t web_session_id_size) OVERRIDE {
     cdm_->CloseSession(promise_id, web_session_id, web_session_id_size);
+    return true;
   }
 
-  virtual bool RemoveSession(uint32_t promise_id,
+  virtual void RemoveSession(uint32_t promise_id,
                              const char* web_session_id,
                              uint32_t web_session_id_size) OVERRIDE {
     cdm_->RemoveSession(promise_id, web_session_id, web_session_id_size);
-    return true;
   }
 
   virtual void TimerExpired(void* context) OVERRIDE {
@@ -454,22 +454,22 @@ void CdmWrapperImpl<cdm::ContentDecryptionModule_4>::UpdateSession(
 }
 
 template <>
-void CdmWrapperImpl<cdm::ContentDecryptionModule_4>::CloseSession(
+bool CdmWrapperImpl<cdm::ContentDecryptionModule_4>::CloseSession(
     uint32_t promise_id,
     const char* web_session_id,
     uint32_t web_session_id_size) {
-  std::string web_session_str(web_session_id, web_session_id_size);
-  uint32_t session_id = LookupSessionId(web_session_str);
-  RegisterPromise(session_id, promise_id);
-  cdm_->ReleaseSession(session_id);
+  return false;
 }
 
 template <>
-bool CdmWrapperImpl<cdm::ContentDecryptionModule_4>::RemoveSession(
+void CdmWrapperImpl<cdm::ContentDecryptionModule_4>::RemoveSession(
     uint32_t promise_id,
     const char* web_session_id,
     uint32_t web_session_id_size) {
-  return false;
+  std::string web_session_str(web_session_id, web_session_id_size);
+  uint32_t session_id = LookupSessionId(web_session_str);
+  RegisterPromise(session_id, promise_id);
+  cdm_->ReleaseSession(session_id);
 }

[xhwang, 2014/08/23 00:01:48]

ditto about order swap

[jrummell, 2014/08/23 00:27:15]

CDM6 (and the EME spec) have CloseSession() first, so copying that.

 
 template <>
@@ -572,19 +572,19 @@ void CdmWrapperImpl<cdm::ContentDecryptionModule_5>::UpdateSession(
 }
 
 template <>
-void CdmWrapperImpl<cdm::ContentDecryptionModule_5>::CloseSession(
+bool CdmWrapperImpl<cdm::ContentDecryptionModule_5>::CloseSession(
     uint32_t promise_id,
     const char* web_session_id,
     uint32_t web_session_id_size) {
-  cdm_->ReleaseSession(promise_id, web_session_id, web_session_id_size);
+  return false;
 }
 
 template <>
-bool CdmWrapperImpl<cdm::ContentDecryptionModule_5>::RemoveSession(
+void CdmWrapperImpl<cdm::ContentDecryptionModule_5>::RemoveSession(

[xhwang, 2014/08/23 00:01:48]

It might be easier if we just drop CDM5 support :)

[jrummell, 2014/08/23 00:27:15]

Acknowledged. That would just make this change bigger :)

     uint32_t promise_id,
     const char* web_session_id,
     uint32_t web_session_id_size) {
-  return false;
+  cdm_->ReleaseSession(promise_id, web_session_id, web_session_id_size);
 }
 
 template <>