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
This commit is contained in:
Alex Dale
2023-03-17 15:50:34 -07:00
parent bfa8d39a63
commit 52bd76e0e2
21 changed files with 149 additions and 108 deletions

View File

@@ -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<int>(status_)));
if (oec_result_) {
str.append("; oec_result = ");
str.append(std::to_string(static_cast<int>(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<int>(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<OEMCryptoResult>.
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 {

View File

@@ -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<int>(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<int>(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:

View File

@@ -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()));

View File

@@ -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;
}

View File

@@ -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);

View File

@@ -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:

View File

@@ -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");

View File

@@ -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)) {

View File

@@ -4,6 +4,7 @@
#include "wv_cdm_types.h"
#include <stdio.h>
#include <string.h>
#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:

View File

@@ -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";
}

View File

@@ -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:

View File

@@ -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);

View File

@@ -20,7 +20,7 @@ template <>
void SetAttributeField<drm_metrics::Attributes::kErrorCodeFieldNumber,
CdmResponseType>(const CdmResponseType& cdm_error,
drm_metrics::Attributes* attributes) {
attributes->set_error_code(cdm_error.Enum());
attributes->set_error_code(cdm_error.code());
}
template <>

View File

@@ -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,

View File

@@ -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();
}
}
}

View File

@@ -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;

View File

@@ -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{};

View File

@@ -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;

View File

@@ -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<int>(res));
switch (res.Enum()) {
switch (res.code()) {
case wvcdm::INSUFFICIENT_CRYPTO_RESOURCES:
errorDetailMsg->assign(
"Error decrypting data: insufficient crypto resources");

View File

@@ -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<int32_t>(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<int32_t>(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

View File

@@ -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<int>(result));
dprintf(fd, " error_message: %s\n", result.GetCodeString());
dprintf(fd, " error_code: %d\n", result.ToInt());
}
}