ReleaseSession() should call RemoveSession()

media/cdm/ppapi/cdm_wrapper.h

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef MEDIA_CDM_PPAPI_CDM_WRAPPER_H_
 #define MEDIA_CDM_PPAPI_CDM_WRAPPER_H_
 
 #include <map>
 #include <queue>
 #include <string>
@@ skipping 38 matching lines @@
                              uint32_t init_data_size,
                              cdm::SessionType session_type) = 0;
   virtual void LoadSession(uint32_t promise_id,
                            const char* web_session_id,
                            uint32_t web_session_id_size) = 0;
   virtual void UpdateSession(uint32_t promise_id,
                              const char* web_session_id,
                              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,
                                const char* web_session_id,
                                uint32_t web_session_id_size) = 0;
   virtual void TimerExpired(void* context) = 0;
   virtual cdm::Status Decrypt(const cdm::InputBuffer& encrypted_buffer,
                               cdm::DecryptedBlock* decrypted_buffer) = 0;
   virtual cdm::Status InitializeAudioDecoder(
       const cdm::AudioDecoderConfig& audio_decoder_config) = 0;
   virtual cdm::Status InitializeVideoDecoder(
       const cdm::VideoDecoderConfig& video_decoder_config) = 0;
@@ skipping 144 matching lines @@
                         response_size);
   }
 
   virtual bool GetUsableKeyIds(uint32_t promise_id,
                                const char* web_session_id,
                                uint32_t web_session_id_size) OVERRIDE {
     cdm_->GetUsableKeyIds(promise_id, web_session_id, web_session_id_size);
     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 {
     cdm_->TimerExpired(context);
   }
 
   virtual cdm::Status Decrypt(const cdm::InputBuffer& encrypted_buffer,
                               cdm::DecryptedBlock* decrypted_buffer) OVERRIDE {
     return cdm_->Decrypt(encrypted_buffer, decrypted_buffer);
   }
@@ skipping 197 matching lines @@
   uint32_t session_id = LookupSessionId(web_session_str);
   RegisterPromise(session_id, promise_id);
   // As CDM_4 doesn't support OnSessionUsableKeysChange(), make sure to generate
   // one when the promise is resolved. This may be overly aggressive.
   SetSessionUsableKeysEventNeeded(
       promise_id, web_session_id, web_session_id_size);
   cdm_->UpdateSession(session_id, response, response_size);
 }
 
 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) {
+  return false;
+}
+
+template <>
+void CdmWrapperImpl<cdm::ContentDecryptionModule_4>::RemoveSession(
     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);
 }

[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 <>
-bool CdmWrapperImpl<cdm::ContentDecryptionModule_4>::RemoveSession(
-    uint32_t promise_id,
-    const char* web_session_id,
-    uint32_t web_session_id_size) {
-  return false;
-}
-
-template <>
 bool CdmWrapperImpl<cdm::ContentDecryptionModule_4>::GetUsableKeyIds(
     uint32_t promise_id,
     const char* web_session_id,
     uint32_t web_session_id_size) {
   return false;
 }
 
 template <>
 cdm::Status CdmWrapperImpl<cdm::ContentDecryptionModule_4>::Decrypt(
     const cdm::InputBuffer& encrypted_buffer,
@@ skipping 79 matching lines @@
     uint32_t response_size) {
   // As CDM_5 doesn't support OnSessionUsableKeysChange(), make sure to generate
   // one when the promise is resolved. This may be overly aggressive.
   SetSessionUsableKeysEventNeeded(
       promise_id, web_session_id, web_session_id_size);
   cdm_->UpdateSession(
       promise_id, web_session_id, web_session_id_size, response, response_size);
 }
 
 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) {
+  return false;
+}
+
+template <>
+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) {
   cdm_->ReleaseSession(promise_id, web_session_id, web_session_id_size);
 }
 
 template <>
-bool CdmWrapperImpl<cdm::ContentDecryptionModule_5>::RemoveSession(
-    uint32_t promise_id,
-    const char* web_session_id,
-    uint32_t web_session_id_size) {
-  return false;
-}
-
-template <>
 bool CdmWrapperImpl<cdm::ContentDecryptionModule_5>::GetUsableKeyIds(
     uint32_t promise_id,
     const char* web_session_id,
     uint32_t web_session_id_size) {
   return false;
 }
 
 template <>
 cdm::Status CdmWrapperImpl<cdm::ContentDecryptionModule_5>::Decrypt(
     const cdm::InputBuffer& encrypted_buffer,
@@ skipping 67 matching lines @@
 // stub implementations for new or modified methods that the older CDM interface
 // does not have.
 // Also update supported_cdm_versions.h.
 COMPILE_ASSERT(cdm::ContentDecryptionModule::kVersion ==
                    cdm::ContentDecryptionModule_6::kVersion,
                ensure_cdm_wrapper_templates_have_old_version_support);
 
 }  // namespace media
 
 #endif  // MEDIA_CDM_PPAPI_CDM_WRAPPER_H_