From 52bd76e0e2c51058b4c9cec29e52a9ea3f123696 Mon Sep 17 00:00:00 2001 From: Alex Dale Date: Fri, 17 Mar 2023 15:50:34 -0700 Subject: [PATCH] Fixed test and log formatting for CdmResponseType. [ Merge of http://go/wvgerrit/168397 ] When CdmResponseType (enum) was transformed to CdmResponseType (struct), the test printers where not updated to print the result of failed comparisons. In addition, several logs statements were updated haphazardly, leaving inconsistencies and potential compiler-specific behavior. This CL replaces CdmResponseType std::string operator with a ToString() method. This is to make it consistent with Google's C++ style guide on conversion operators vs methods. The string conversion function is now defined in wv_cdm_types.cpp instead of inline in the header file. The PrintTo function has been implemented along with the other CDM test printers in test_printers.cpp. Bug: 273989359 Test: run_x86_64_tests Test: MediaDrmParameterizedTests on redfin Test: Forrest drm_compliance Change-Id: Ibfaa17029046b75b1c8c278f7bd7e04a24379848 --- .../cdm/core/include/wv_cdm_types.h | 91 +++++++++++-------- libwvdrmengine/cdm/core/src/cdm_engine.cpp | 12 +-- libwvdrmengine/cdm/core/src/cdm_session.cpp | 6 +- .../cdm/core/src/cdm_usage_table.cpp | 16 ++-- .../cdm/core/src/client_identification.cpp | 2 +- libwvdrmengine/cdm/core/src/license.cpp | 2 +- .../cdm/core/src/ota_keybox_provisioner.cpp | 4 +- .../cdm/core/src/system_id_extractor.cpp | 6 +- libwvdrmengine/cdm/core/src/wv_cdm_types.cpp | 29 ++++++ .../cdm/core/test/keybox_ota_test.cpp | 6 +- .../cdm/core/test/test_printers.cpp | 6 +- libwvdrmengine/cdm/core/test/test_printers.h | 3 +- .../cdm/metrics/src/attribute_handler.cpp | 2 +- .../cdm/src/wv_content_decryption_module.cpp | 2 +- .../cdm/test/cdm_extended_duration_test.cpp | 8 +- .../cdm/test/request_license_test.cpp | 4 +- libwvdrmengine/include/WVTypes.h | 2 +- libwvdrmengine/include/mapErrors-inl.h | 4 +- .../mediacrypto/src/WVCryptoPlugin.cpp | 2 +- libwvdrmengine/src/Utils.cpp | 45 +++++---- libwvdrmengine/src/WVDrmFactory.cpp | 5 +- 21 files changed, 149 insertions(+), 108 deletions(-) diff --git a/libwvdrmengine/cdm/core/include/wv_cdm_types.h b/libwvdrmengine/cdm/core/include/wv_cdm_types.h index 01e08450..41499651 100644 --- a/libwvdrmengine/cdm/core/include/wv_cdm_types.h +++ b/libwvdrmengine/cdm/core/include/wv_cdm_types.h @@ -460,56 +460,69 @@ enum CdmResponseEnum : int32_t { // * android/include/mapErrors-inl.h }; -struct CdmResponseType { - explicit CdmResponseType(CdmResponseEnum status = NO_ERROR, - OEMCryptoResult oec_result = OEMCrypto_SUCCESS, - const char* crypto_session_method = nullptr) - : status_(status), - oec_result_(oec_result), - crypto_session_method_(crypto_session_method){}; - explicit operator CdmResponseEnum() const { return status_; }; - explicit operator std::string() { - std::string str = "status = "; - str.append(std::to_string(static_cast(status_))); - if (oec_result_) { - str.append("; oec_result = "); - str.append(std::to_string(static_cast(oec_result_))); - } - if (crypto_session_method_) { - str.append("; crypto_session_method = "); - str.append(crypto_session_method_); - } - return str; - } - CdmResponseEnum Enum() const { return status_; }; - constexpr explicit operator int() const { return status_; }; - OEMCryptoResult getOEMCryptoResult() const { return oec_result_; } - const char* getCryptoSessionMethod() const { return crypto_session_method_; } - bool operator==(CdmResponseEnum other) const { return status_ == other; } - bool operator!=(CdmResponseEnum other) const { return status_ != other; } - bool operator==(const CdmResponseType& other) const { - return status_ == other.status_ && oec_result_ == other.oec_result_ && - crypto_session_method_ == other.crypto_session_method_; - } - bool operator!=(const CdmResponseType& other) const { - return !operator==(other); +class CdmResponseType { + public: + constexpr CdmResponseType() {} + constexpr explicit CdmResponseType(CdmResponseEnum code) : code_(code) {} + constexpr CdmResponseType(CdmResponseEnum code, OEMCryptoResult oemc_result, + const char* crypto_session_method) + : code_(code), + has_oemc_result_(true), + oemc_result_(oemc_result), + crypto_session_method_(crypto_session_method) {} + + constexpr bool IsOk() const { + return (code_ == NO_ERROR) && + (!HasOemcResult() || (oemc_result_ == OEMCrypto_SUCCESS)); } + constexpr explicit operator CdmResponseEnum() const { return code_; }; + constexpr CdmResponseEnum code() const { return code_; }; + // Returns the string representation of |code_|, ignores + // the other fields. + const char* GetCodeString() const; + + constexpr bool HasOemcResult() const { return has_oemc_result_; } + constexpr OEMCryptoResult oemc_result() const { return oemc_result_; } + + constexpr bool HasCryptoSessionMethod() const { + return crypto_session_method_ != nullptr; + } + constexpr const char* crypto_session_method() const { + return crypto_session_method_; + } + + constexpr int ToInt() const { return static_cast(code_); } + constexpr explicit operator int() const { return ToInt(); }; + + bool operator==(CdmResponseEnum other) const { return code_ == other; } + bool operator!=(CdmResponseEnum other) const { return code_ != other; } + bool operator==(const CdmResponseType& other) const; + bool operator!=(const CdmResponseType& other) const { + return !(*this == other); + } + + std::string ToString() const; + private: - CdmResponseEnum status_; - OEMCryptoResult oec_result_; - const char* crypto_session_method_; - // CORE_DISALLOW_COPY_AND_ASSIGN(CdmResponseType); + CdmResponseEnum code_ = NO_ERROR; + // Setting |oemc_result_| is optional, only set if |has_oemc_result_| + // is true. + // TODO(b/254108623): Remove |has_oemc_result_| and replace |oemc_result_| + // with std::optional. + bool has_oemc_result_ = false; + OEMCryptoResult oemc_result_ = OEMCrypto_SUCCESS; + const char* crypto_session_method_ = nullptr; }; static inline bool operator==(const CdmResponseEnum lhs, const CdmResponseType& rhs) { - return lhs == rhs.Enum(); + return lhs == rhs.code(); } static inline bool operator!=(const CdmResponseEnum lhs, const CdmResponseType& rhs) { - return !(lhs == rhs.Enum()); + return lhs != rhs.code(); } enum CdmKeyStatus : int32_t { diff --git a/libwvdrmengine/cdm/core/src/cdm_engine.cpp b/libwvdrmengine/cdm/core/src/cdm_engine.cpp index 05fcc2cb..949f528f 100644 --- a/libwvdrmengine/cdm/core/src/cdm_engine.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_engine.cpp @@ -423,7 +423,7 @@ CdmResponseType CdmEngine::AddKey(const CdmSessionId& session_id, } } - switch (sts.Enum()) { + switch (sts.code()) { case KEY_ADDED: break; case NEED_KEY: @@ -863,11 +863,11 @@ CdmResponseType CdmEngine::QueryStatus(RequestedSecurityLevel security_level, } if (status == NOT_IMPLEMENTED_ERROR || status == PROVISIONING_TYPE_IS_NOT_BOOT_CERTIFICATE_CHAIN_ERROR) { - LOGD("BCC not available: %d", static_cast(status.Enum())); + LOGD("BCC not available: %d", status.ToInt()); *query_response = QUERY_VALUE_NONE; return CdmResponseType(NO_ERROR); } - LOGE("Failed to extract BCC: status = %d", static_cast(status.Enum())); + LOGE("Failed to extract BCC: status = %d", status.ToInt()); return status; } @@ -1471,7 +1471,7 @@ CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, // try L3. CdmResponseType status = GetUsageInfo(app_id, ssid, kLevelDefault, error_detail, usage_info); - switch (status.Enum()) { + switch (status.code()) { case NEED_PROVISIONING: case GET_USAGE_INFO_ERROR_1: case GET_USAGE_INFO_ERROR_2: @@ -1647,7 +1647,7 @@ CdmResponseType CdmEngine::GetUsageInfo( usage_info->clear(); usage_info->push_back(request.message); - switch (status.Enum()) { + switch (status.code()) { case KEY_MESSAGE: break; case KEY_CANCELED: // usage information not present in @@ -1853,7 +1853,7 @@ CdmResponseType CdmEngine::LoadUsageSession(const CdmKeySetId& key_set_id, CdmKeyRequest request; status = session->GenerateReleaseRequest(&request); *release_message = std::move(request.message); - switch (status.Enum()) { + switch (status.code()) { case KEY_MESSAGE: break; case KEY_CANCELED: diff --git a/libwvdrmengine/cdm/core/src/cdm_session.cpp b/libwvdrmengine/cdm/core/src/cdm_session.cpp index 78de664b..c0f20ded 100644 --- a/libwvdrmengine/cdm/core/src/cdm_session.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_session.cpp @@ -323,7 +323,7 @@ CdmResponseType CdmSession::RestoreOfflineSession(const CdmKeySetId& key_set_id, license_data.drm_certificate, key_request_, key_response_); if (result != NO_ERROR) { - SetErrorDetail(error_detail, result.Enum()); + SetErrorDetail(error_detail, result.code()); return CdmResponseType(RELEASE_LICENSE_ERROR_1); } } else { @@ -333,7 +333,7 @@ CdmResponseType CdmSession::RestoreOfflineSession(const CdmKeySetId& key_set_id, license_data.last_playback_time, license_data.grace_period_end_time, this); if (result != NO_ERROR) { - SetErrorDetail(error_detail, result.Enum()); + SetErrorDetail(error_detail, result.code()); return CdmResponseType(RESTORE_OFFLINE_LICENSE_ERROR_2); } } @@ -1227,7 +1227,7 @@ CdmResponseType CdmSession::LoadPrivateKey( crypto_metrics_, crypto_session_load_certificate_private_key_, load_cert_sts); - switch (load_cert_sts.Enum()) { + switch (load_cert_sts.code()) { case NO_ERROR: metrics_->drm_certificate_key_type_.Record( DrmKeyTypeToMetricValue(private_key.type())); diff --git a/libwvdrmengine/cdm/core/src/cdm_usage_table.cpp b/libwvdrmengine/cdm/core/src/cdm_usage_table.cpp index 48871593..5a99c78b 100644 --- a/libwvdrmengine/cdm/core/src/cdm_usage_table.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_usage_table.cpp @@ -205,7 +205,7 @@ bool CdmUsageTable::RestoreTable(CryptoSession* const crypto_session) { const CdmResponseType status = crypto_session->LoadUsageTableHeader(requested_security_level_, header_); if (status != NO_ERROR) { - LOGE("Failed to load usage table header: sts = %d", status.Enum()); + LOGE("Failed to load usage table header: sts = %d", status.ToInt()); return false; } @@ -424,7 +424,7 @@ bool CdmUsageTable::OpenSessionCheck(CryptoSession* const crypto_session) { LOGE( "Cannot initialize usage table header with open crypto session: " "status = %d, count = %zu", - status.Enum(), session_count); + status.ToInt(), session_count); return false; } return true; @@ -458,7 +458,7 @@ bool CdmUsageTable::CapacityCheck(CryptoSession* const crypto_session) { local_crypto_session->Open(requested_security_level_); if (status != NO_ERROR) { LOGE("Failed to open crypto session for capacity test: sts = %d", - status.Enum()); + status.ToInt()); return false; } @@ -466,7 +466,7 @@ bool CdmUsageTable::CapacityCheck(CryptoSession* const crypto_session) { status = AddEntry(local_crypto_session, true, kDummyKeySetId, kEmptyString, kEmptyString, &temporary_entry_index); if (status != NO_ERROR) { - LOGE("Failed to add entry for capacity test: sts = %d", status.Enum()); + LOGE("Failed to add entry for capacity test: sts = %d", status.ToInt()); return false; } @@ -475,7 +475,7 @@ bool CdmUsageTable::CapacityCheck(CryptoSession* const crypto_session) { /* defrag_table = */ true, device_files_.get(), metrics); if (status != NO_ERROR) { LOGE("Failed to invalidate entry for capacity test: sts = %d", - status.Enum()); + status.ToInt()); return false; } if (entry_info_list_.size() > temporary_entry_index) { @@ -1017,7 +1017,7 @@ CdmResponseType CdmUsageTable::DefragTable(DeviceFiles* device_files, status = MoveEntry(from_entry_index, from_entry, to_entry_index, device_files, metrics); - switch (status.Enum()) { + switch (status.code()) { case NO_ERROR: { entries_to_remove.pop_back(); entries_to_move.pop_back(); @@ -1081,7 +1081,7 @@ CdmResponseType CdmUsageTable::DefragTable(DeviceFiles* device_files, // For all other cases, it may not be safe to proceed, even to // shrink the table. LOGE("Unrecoverable error occurred while defragging table: status = %d", - status.Enum()); + status.ToInt()); return status; } } // End switch case. @@ -1149,7 +1149,7 @@ CdmResponseType CdmUsageTable::ReleaseOldestEntry( device_files_.get(), metrics); if (status != NO_ERROR) { - LOGE("Failed to invalidate oldest entry: status = %d", status.Enum()); + LOGE("Failed to invalidate oldest entry: status = %d", status.ToInt()); return status; } diff --git a/libwvdrmengine/cdm/core/src/client_identification.cpp b/libwvdrmengine/cdm/core/src/client_identification.cpp index 62d0a728..a9a363ee 100644 --- a/libwvdrmengine/cdm/core/src/client_identification.cpp +++ b/libwvdrmengine/cdm/core/src/client_identification.cpp @@ -140,7 +140,7 @@ CdmResponseType ClientIdentification::Prepare( CdmResponseType status = crypto_session_->GetProvisioningToken(&token, &additional_token); if (status != NO_ERROR) { - LOGE("Failed to get provisioning token: status = %d", status.Enum()); + LOGE("Failed to get provisioning token: status = %d", status.ToInt()); return status; } client_id->set_token(token); diff --git a/libwvdrmengine/cdm/core/src/license.cpp b/libwvdrmengine/cdm/core/src/license.cpp index 28271d36..6f44a625 100644 --- a/libwvdrmengine/cdm/core/src/license.cpp +++ b/libwvdrmengine/cdm/core/src/license.cpp @@ -328,7 +328,7 @@ CdmResponseType CdmLicense::PrepareKeyRequest( // of the license response. status = crypto_session_->GenerateNonce(&license_nonce_); - switch (status.Enum()) { + switch (status.code()) { case NO_ERROR: break; case SESSION_LOST_STATE_ERROR: diff --git a/libwvdrmengine/cdm/core/src/ota_keybox_provisioner.cpp b/libwvdrmengine/cdm/core/src/ota_keybox_provisioner.cpp index f22a4896..ccc11463 100644 --- a/libwvdrmengine/cdm/core/src/ota_keybox_provisioner.cpp +++ b/libwvdrmengine/cdm/core/src/ota_keybox_provisioner.cpp @@ -155,7 +155,7 @@ CdmResponseType OtaKeyboxProvisioner::GetProvisioningRequest( CleanUp(); return result; } else if (result != NO_ERROR) { - LOGE("Failed to generate OKP request: status = %d", result.Enum()); + LOGE("Failed to generate OKP request: status = %d", result.ToInt()); return result; } // Step 2: Wrap in ProvisioningRequest. @@ -164,7 +164,7 @@ CdmResponseType OtaKeyboxProvisioner::GetProvisioningRequest( result = client_id_.Prepare(kEmptyAppParameters, kEmptyString, client_id); if (result != NO_ERROR) { LOGW("Failed to prepare client ID, continuing without: result = %d", - result.Enum()); + result.ToInt()); client_id->Clear(); } LOGI("OTA request generated"); diff --git a/libwvdrmengine/cdm/core/src/system_id_extractor.cpp b/libwvdrmengine/cdm/core/src/system_id_extractor.cpp index 9c0e3678..4ca2c639 100644 --- a/libwvdrmengine/cdm/core/src/system_id_extractor.cpp +++ b/libwvdrmengine/cdm/core/src/system_id_extractor.cpp @@ -53,7 +53,7 @@ bool SystemIdExtractor::ExtractSystemId(uint32_t* system_id) { crypto_session_->GetProvisioningMethod(security_level_, &type); if (status != NO_ERROR) { LOGE("Failed to get provisioning method: security_level = %s, status = %d", - RequestedSecurityLevelToString(security_level_), status.Enum()); + RequestedSecurityLevelToString(security_level_), status.ToInt()); return false; } bool success = false; @@ -121,7 +121,7 @@ bool SystemIdExtractor::ExtractSystemIdProv20(uint32_t* system_id) { } if (status != NO_ERROR) { LOGE("Failed to get keybox data: security_level = %s, status = %d", - RequestedSecurityLevelToString(security_level_), status.Enum()); + RequestedSecurityLevelToString(security_level_), status.ToInt()); return false; } if (!ExtractSystemIdFromKeyboxData(key_data, system_id)) { @@ -137,7 +137,7 @@ bool SystemIdExtractor::ExtractSystemIdProv30(uint32_t* system_id) { crypto_session_->GetTokenFromOemCert(security_level_, &oem_cert); if (status != NO_ERROR) { LOGE("Failed to get OEM certificate: security_level = %s, status = %d", - RequestedSecurityLevelToString(security_level_), status.Enum()); + RequestedSecurityLevelToString(security_level_), status.ToInt()); return false; } if (!ExtractSystemIdFromOemCert(oem_cert, system_id)) { diff --git a/libwvdrmengine/cdm/core/src/wv_cdm_types.cpp b/libwvdrmengine/cdm/core/src/wv_cdm_types.cpp index 01055be4..168bfc90 100644 --- a/libwvdrmengine/cdm/core/src/wv_cdm_types.cpp +++ b/libwvdrmengine/cdm/core/src/wv_cdm_types.cpp @@ -4,6 +4,7 @@ #include "wv_cdm_types.h" #include +#include #include "wv_cdm_constants.h" @@ -24,6 +25,34 @@ const char* UnknownValueRep(EnumType value) { } } // namespace +const char* CdmResponseType::GetCodeString() const { + return CdmResponseEnumToString(code_); +} + +bool CdmResponseType::operator==(const CdmResponseType& other) const { + if (code_ != other.code_) return false; + if (!HasOemcResult() || !other.HasOemcResult()) + return !HasOemcResult() && !other.HasOemcResult(); + if (oemc_result_ != other.oemc_result_) return false; + if (!HasCryptoSessionMethod() || !other.HasCryptoSessionMethod()) + return !HasCryptoSessionMethod() && !other.HasCryptoSessionMethod(); + return strcmp(crypto_session_method_, other.crypto_session_method_) == 0; +} + +std::string CdmResponseType::ToString() const { + std::string result = CdmResponseEnumToString(code_); + if (!HasOemcResult()) return result; + // TODO(sigquit): Consider using a string representation of OEMCryptoResult. + result.append(" (oemc_result = "); + result.append(std::to_string(oemc_result_)); + if (HasCryptoSessionMethod()) { + result.append(", method = "); + result.append(crypto_session_method_); + } + result.append(")"); + return result; +} + const char* CdmCertificateTypeToString(CdmCertificateType type) { switch (type) { case kCertificateWidevine: diff --git a/libwvdrmengine/cdm/core/test/keybox_ota_test.cpp b/libwvdrmengine/cdm/core/test/keybox_ota_test.cpp index 5669c9c1..1c3ec680 100644 --- a/libwvdrmengine/cdm/core/test/keybox_ota_test.cpp +++ b/libwvdrmengine/cdm/core/test/keybox_ota_test.cpp @@ -111,7 +111,7 @@ TEST_F(CdmOtaKeyboxTest, BasicTest) { } else { std::cout << " " << "Could not find system id before test. "; - PrintTo(system_id_status.Enum(), &std::cout); + PrintTo(system_id_status, &std::cout); std::cout << "\n"; } @@ -138,7 +138,7 @@ TEST_F(CdmOtaKeyboxTest, BasicTest) { } else { std::cout << " " << "Could not find system id after first provisioning. "; - PrintTo(system_id_status.Enum(), &std::cout); + PrintTo(system_id_status, &std::cout); std::cout << "\n"; } @@ -163,7 +163,7 @@ TEST_F(CdmOtaKeyboxTest, BasicTest) { } else { std::cout << " " << "Could not find system id after second provisioning. "; - PrintTo(system_id_status.Enum(), &std::cout); + PrintTo(system_id_status, &std::cout); std::cout << "\n"; } diff --git a/libwvdrmengine/cdm/core/test/test_printers.cpp b/libwvdrmengine/cdm/core/test/test_printers.cpp index c8b2c30c..4df1454d 100644 --- a/libwvdrmengine/cdm/core/test/test_printers.cpp +++ b/libwvdrmengine/cdm/core/test/test_printers.cpp @@ -9,10 +9,14 @@ #include "error_string_util.h" namespace wvcdm { -void PrintTo(const CdmResponseEnum& value, ::std::ostream* os) { +void PrintTo(CdmResponseEnum value, ::std::ostream* os) { *os << CdmResponseEnumToString(value); } +void PrintTo(const CdmResponseType& value, ::std::ostream* os) { + *os << value.ToString(); +} + void PrintTo(const enum CdmLicenseType& value, ::std::ostream* os) { switch (value) { case kLicenseTypeOffline: diff --git a/libwvdrmengine/cdm/core/test/test_printers.h b/libwvdrmengine/cdm/core/test/test_printers.h index 7407a553..174872c7 100644 --- a/libwvdrmengine/cdm/core/test/test_printers.h +++ b/libwvdrmengine/cdm/core/test/test_printers.h @@ -14,7 +14,8 @@ #include "wv_cdm_types.h" namespace wvcdm { -void PrintTo(const enum CdmResponseEnum& value, ::std::ostream* os); +void PrintTo(CdmResponseEnum value, ::std::ostream* os); +void PrintTo(const CdmResponseType& value, ::std::ostream* os); void PrintTo(const enum CdmLicenseType& value, ::std::ostream* os); void PrintTo(const enum CdmSecurityLevel& value, ::std::ostream* os); void PrintTo(const enum CdmCertificateType& value, ::std::ostream* os); diff --git a/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp b/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp index c4b0540f..0c51d574 100644 --- a/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp +++ b/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp @@ -20,7 +20,7 @@ template <> void SetAttributeField(const CdmResponseType& cdm_error, drm_metrics::Attributes* attributes) { - attributes->set_error_code(cdm_error.Enum()); + attributes->set_error_code(cdm_error.code()); } template <> diff --git a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp index 8c95cb8c..745671f7 100644 --- a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp +++ b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp @@ -399,7 +399,7 @@ CdmResponseType WvContentDecryptionModule::GetMetrics( metrics->push_back(metric); } else { LOGD("GetMetrics call failed: cdm identifier=%u, error=%d", - identifier.unique_id, status.Enum()); + identifier.unique_id, status.ToInt()); } } // With no streaming activities, |cdms_| size would be zero, diff --git a/libwvdrmengine/cdm/test/cdm_extended_duration_test.cpp b/libwvdrmengine/cdm/test/cdm_extended_duration_test.cpp index 304ba9a9..38a55f9d 100644 --- a/libwvdrmengine/cdm/test/cdm_extended_duration_test.cpp +++ b/libwvdrmengine/cdm/test/cdm_extended_duration_test.cpp @@ -456,7 +456,7 @@ class WvCdmExtendedDurationTest : public WvCdmTestBase { void Provision() { CdmResponseType status = decryptor_->OpenSession( config_.key_system(), nullptr, kDefaultCdmIdentifier, nullptr, &session_id_); - switch (status.Enum()) { + switch (status.code()) { case NO_ERROR: decryptor_->CloseSession(session_id_); return; @@ -968,7 +968,7 @@ TEST_F(WvCdmExtendedDurationTest, DecryptionCloseSessionConcurrencyTest) { status = decryptor_->Decrypt(session_id_, (data + i)->validate_key_id, decryption_parameters); - switch (status.Enum()) { + switch (status.code()) { case SESSION_NOT_FOUND_FOR_DECRYPT: case SESSION_NOT_FOUND_18: // Session was closed before decrypt was called. This is expected @@ -978,9 +978,7 @@ TEST_F(WvCdmExtendedDurationTest, DecryptionCloseSessionConcurrencyTest) { EXPECT_EQ((data + i)->decrypt_data, decrypt_buffer); break; default: - // common_typos_disable - EXPECT_TRUE(false) << " Unexpected decrypt result: " << status.Enum(); - // common_typos_enable + ADD_FAILURE() << "Unexpected decrypt result: " << status.ToString(); } } } diff --git a/libwvdrmengine/cdm/test/request_license_test.cpp b/libwvdrmengine/cdm/test/request_license_test.cpp index 13a87ac7..729b527c 100644 --- a/libwvdrmengine/cdm/test/request_license_test.cpp +++ b/libwvdrmengine/cdm/test/request_license_test.cpp @@ -2112,7 +2112,7 @@ class WvCdmRequestLicenseTest : public WvCdmTestBase { decryptor_->CloseSession(session_id_); - switch (status.Enum()) { + switch (status.code()) { case wvcdm::NO_ERROR: case KEY_MESSAGE: return true; @@ -2591,7 +2591,7 @@ TEST_F(WvCdmRequestLicenseTest, ProvisioningRevocationTest) { decryptor_->OpenSession(config_.key_system(), nullptr, kDefaultCdmIdentifier, nullptr, &session_id_); - switch (result.Enum()) { + switch (result.code()) { case wvcdm::NO_ERROR: decryptor_->CloseSession(session_id_); return; diff --git a/libwvdrmengine/include/WVTypes.h b/libwvdrmengine/include/WVTypes.h index 0be919c4..4c21c9a7 100644 --- a/libwvdrmengine/include/WVTypes.h +++ b/libwvdrmengine/include/WVTypes.h @@ -43,7 +43,7 @@ struct WvStatus { mCdmErr = cdmErr; return *this; } - wvcdm::CdmResponseType getCdmErr() const { return mCdmErr; } + const wvcdm::CdmResponseType& getCdmErr() const { return mCdmErr; } private: AidlDrmStatus mStatus{}; diff --git a/libwvdrmengine/include/mapErrors-inl.h b/libwvdrmengine/include/mapErrors-inl.h index 270ac1ca..776f14d1 100644 --- a/libwvdrmengine/include/mapErrors-inl.h +++ b/libwvdrmengine/include/mapErrors-inl.h @@ -19,7 +19,7 @@ using ::aidl::android::hardware::drm::Status; static inline WvStatus mapCdmResponseType(wvcdm::CdmResponseType res) { Status err = Status::ERROR_DRM_UNKNOWN; - switch (res.Enum()) { + switch (res.code()) { case wvcdm::KEY_ADDED: case wvcdm::KEY_MESSAGE: case wvcdm::KEY_CANCELED: @@ -116,7 +116,7 @@ static inline WvStatus mapCdmResponseType(wvcdm::CdmResponseType res) { case wvcdm::REWRAP_DEVICE_RSA_KEY_ERROR: case wvcdm::PARSE_OKP_RESPONSE_ERROR: case wvcdm::OKP_ALREADY_PROVISIONED: - ALOGW("Returns UNKNOWN error for legacy status: %u", res.Enum()); + ALOGW("Returns UNKNOWN error for legacy status: %u", res.ToInt()); err = Status::ERROR_DRM_UNKNOWN; break; diff --git a/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp b/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp index 619f56c0..863088f6 100644 --- a/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp +++ b/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp @@ -306,7 +306,7 @@ SharedBufferBase::~SharedBufferBase() { } else { ALOGE("Decrypt error in session %s during a sample %s protected data: %d", mSessionId.c_str(), hasProtectedData ? "with" : "without", static_cast(res)); - switch (res.Enum()) { + switch (res.code()) { case wvcdm::INSUFFICIENT_CRYPTO_RESOURCES: errorDetailMsg->assign( "Error decrypting data: insufficient crypto resources"); diff --git a/libwvdrmengine/src/Utils.cpp b/libwvdrmengine/src/Utils.cpp index 0b5d8627..754f802e 100644 --- a/libwvdrmengine/src/Utils.cpp +++ b/libwvdrmengine/src/Utils.cpp @@ -139,31 +139,28 @@ CryptoSessionApi getCryptoSessionMethodEnum(const std::string& method) { const char* msg) { if (status == ::aidl::android::hardware::drm::Status::OK) { return ::ndk::ScopedAStatus::ok(); - } else { - auto err = static_cast(status); - Json::Value jsonMsg(Json::objectValue); - auto cdmErr = status.getCdmErr(); - auto cdmStatus = cdmErr.Enum(); - if (cdmStatus) { - jsonMsg["cdmError"] = cdmStatus; - } - auto oemErr = cdmErr.getOEMCryptoResult(); - if (oemErr) { - jsonMsg["oemError"] = oemErr; - } - auto cryptoSessionMethod = cdmErr.getCryptoSessionMethod(); - if (cryptoSessionMethod != nullptr) { - jsonMsg["context"] = - getCryptoSessionMethodEnum(std::string(cryptoSessionMethod)); - } - if (msg) { - jsonMsg["errorMessage"] = msg; - } - Json::FastWriter writer; - writer.omitEndingLineFeed(); - return ::ndk::ScopedAStatus::fromServiceSpecificErrorWithMessage( - err, writer.write(jsonMsg).c_str()); } + const int32_t err = static_cast(status); + Json::Value jsonMsg(Json::objectValue); + const auto& cdmErr = status.getCdmErr(); + + if (!cdmErr.IsOk()) { + jsonMsg["cdmError"] = cdmErr.code(); + } + if (cdmErr.HasOemcResult()) { + jsonMsg["oemError"] = cdmErr.oemc_result(); + } + if (cdmErr.HasCryptoSessionMethod()) { + jsonMsg["context"] = + getCryptoSessionMethodEnum(cdmErr.crypto_session_method()); + } + if (msg != nullptr) { + jsonMsg["errorMessage"] = msg; + } + Json::FastWriter writer; + writer.omitEndingLineFeed(); + return ::ndk::ScopedAStatus::fromServiceSpecificErrorWithMessage( + err, writer.write(jsonMsg).c_str()); } } // namespace wvdrm diff --git a/libwvdrmengine/src/WVDrmFactory.cpp b/libwvdrmengine/src/WVDrmFactory.cpp index 2e36b6ca..87ad8483 100644 --- a/libwvdrmengine/src/WVDrmFactory.cpp +++ b/libwvdrmengine/src/WVDrmFactory.cpp @@ -199,9 +199,8 @@ void WVDrmFactory::printCdmMetrics(int fd) { dprintf(fd, "%s\n", formatted.c_str()); } } else { - dprintf(fd, " error_message: %s\n", - wvcdm::CdmResponseEnumToString(result.Enum())); - dprintf(fd, " error_code: %d\n", static_cast(result)); + dprintf(fd, " error_message: %s\n", result.GetCodeString()); + dprintf(fd, " error_code: %d\n", result.ToInt()); } }