From 605ff8310366e1d7049c1353d449342d0f435f33 Mon Sep 17 00:00:00 2001 From: Adam Stone Date: Wed, 30 Jan 2019 10:31:24 -0800 Subject: [PATCH 1/2] Add error detail metric to some session methods [ Merge from http://go/wvgerrit/71726 ] Adds an error detail metric attribute to RestoreUsageSession and RestoreOfflineSession. These metrics will now report an additional attribute providing additional error detail for debugging. BUG: http://b/115517916 Test: CDM Unit Tests. Manually tried GPlay. Change-Id: Ib48361ef29d33a16150473d8967e4850bc0c623d --- libwvdrmengine/cdm/core/include/cdm_engine.h | 8 ++++- .../include/cdm_engine_metrics_decorator.h | 18 +++++++--- libwvdrmengine/cdm/core/include/cdm_session.h | 6 ++-- libwvdrmengine/cdm/core/src/cdm_engine.cpp | 33 ++++++++++++------- libwvdrmengine/cdm/core/src/cdm_session.cpp | 18 ++++++++-- .../cdm/metrics/include/metrics_collections.h | 11 +++++-- .../cdm/metrics/src/attribute_handler.cpp | 8 +++++ libwvdrmengine/cdm/metrics/src/metrics.proto | 2 ++ .../test/metrics_collections_unittest.cpp | 15 +++++++-- .../cdm/src/wv_content_decryption_module.cpp | 6 ++-- 10 files changed, 97 insertions(+), 28 deletions(-) diff --git a/libwvdrmengine/cdm/core/include/cdm_engine.h b/libwvdrmengine/cdm/core/include/cdm_engine.h index e516f7c3..5de39cba 100644 --- a/libwvdrmengine/cdm/core/include/cdm_engine.h +++ b/libwvdrmengine/cdm/core/include/cdm_engine.h @@ -225,14 +225,19 @@ class CdmEngine { // Usage related methods for streaming licenses // Retrieve a random usage info from the list of all usage infos for this app - // id. + // id. If |error_detail| is not null, an additional error code may be provided + // in the event of an error. virtual CdmResponseType GetUsageInfo(const std::string& app_id, + CdmResponseType* error_detail, CdmUsageInfo* usage_info); // Retrieve the usage info for the specified pst. // Returns UNKNOWN_ERROR if no usage info was found. + // id. If |error_detail| is not null, an additional error code may be provided + // in the event of an error. virtual CdmResponseType GetUsageInfo(const std::string& app_id, const CdmSecureStopId& ssid, + CdmResponseType* error_detail, CdmUsageInfo* usage_info); // Remove all usage records for the current origin. @@ -358,6 +363,7 @@ class CdmEngine { bool ValidateKeySystem(const CdmKeySystem& key_system); CdmResponseType GetUsageInfo(const std::string& app_id, SecurityLevel requested_security_level, + CdmResponseType* error_detail, CdmUsageInfo* usage_info); void OnKeyReleaseEvent(const CdmKeySetId& key_set_id); diff --git a/libwvdrmengine/cdm/core/include/cdm_engine_metrics_decorator.h b/libwvdrmengine/cdm/core/include/cdm_engine_metrics_decorator.h index 04027b48..3b514af0 100644 --- a/libwvdrmengine/cdm/core/include/cdm_engine_metrics_decorator.h +++ b/libwvdrmengine/cdm/core/include/cdm_engine_metrics_decorator.h @@ -181,20 +181,30 @@ class CdmEngineMetricsImpl : public T { } CdmResponseType GetUsageInfo(const std::string& app_id, + CdmResponseType* error_detail, CdmUsageInfo* usage_info) override { CdmResponseType sts; - M_TIME(sts = T::GetUsageInfo(app_id, usage_info), - metrics_, cdm_engine_get_usage_info_, sts); + CdmResponseType error_detail_alt; + M_TIME(sts = T::GetUsageInfo(app_id, &error_detail_alt, usage_info), + metrics_, cdm_engine_get_usage_info_, sts, error_detail_alt); + if (error_detail != nullptr) { + *error_detail = error_detail_alt; + } return sts; } CdmResponseType GetUsageInfo(const std::string& app_id, const CdmSecureStopId& ssid, + CdmResponseType* error_detail, CdmUsageInfo* usage_info) override { CdmResponseType sts; - M_TIME(sts = T::GetUsageInfo(app_id, ssid, usage_info), - metrics_, cdm_engine_get_usage_info_, sts); + CdmResponseType error_detail_alt; + M_TIME(sts = T::GetUsageInfo(app_id, ssid, &error_detail_alt, usage_info), + metrics_, cdm_engine_get_usage_info_, sts, error_detail_alt); + if (error_detail != nullptr) { + *error_detail = error_detail_alt; + } return sts; } diff --git a/libwvdrmengine/cdm/core/include/cdm_session.h b/libwvdrmengine/cdm/core/include/cdm_session.h index c518c582..6fd290c3 100644 --- a/libwvdrmengine/cdm/core/include/cdm_session.h +++ b/libwvdrmengine/cdm/core/include/cdm_session.h @@ -61,9 +61,11 @@ class CdmSession { WvCdmEventListener* event_listener); virtual CdmResponseType RestoreOfflineSession( - const CdmKeySetId& key_set_id, CdmLicenseType license_type); + const CdmKeySetId& key_set_id, CdmLicenseType license_type, + CdmResponseType* error_detail); virtual CdmResponseType RestoreUsageSession( - const DeviceFiles::CdmUsageData& usage_data); + const DeviceFiles::CdmUsageData& usage_data, + CdmResponseType* error_detail); virtual const CdmSessionId& session_id() { return session_id_; } virtual const CdmKeySetId& key_set_id() { return key_set_id_; } diff --git a/libwvdrmengine/cdm/core/src/cdm_engine.cpp b/libwvdrmengine/cdm/core/src/cdm_engine.cpp index feff5702..69c5f190 100644 --- a/libwvdrmengine/cdm/core/src/cdm_engine.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_engine.cpp @@ -285,8 +285,11 @@ CdmResponseType CdmEngine::GenerateKeyRequest( if (license_type == kLicenseTypeRelease && !session->license_received()) { - sts = session->RestoreOfflineSession(key_set_id, kLicenseTypeRelease); - session->GetMetrics()->cdm_session_restore_offline_session_.Increment(sts); + CdmResponseType error_detail = NO_ERROR; + sts = session->RestoreOfflineSession(key_set_id, kLicenseTypeRelease, + &error_detail); + session->GetMetrics()->cdm_session_restore_offline_session_.Increment( + sts, error_detail); if (sts != KEY_ADDED) { LOGE("CdmEngine::GenerateKeyRequest: key release restoration failed," "sts = %d", static_cast(sts)); @@ -418,8 +421,11 @@ CdmResponseType CdmEngine::RestoreKey(const CdmSessionId& session_id, } CdmResponseType sts; - sts = session->RestoreOfflineSession(key_set_id, kLicenseTypeOffline); - session->GetMetrics()->cdm_session_restore_offline_session_.Increment(sts); + CdmResponseType error_detail = NO_ERROR; + sts = session->RestoreOfflineSession(key_set_id, kLicenseTypeOffline, + &error_detail); + session->GetMetrics()->cdm_session_restore_offline_session_.Increment( + sts, error_detail); if (sts == NEED_PROVISIONING) { cert_provisioning_requested_security_level_ = session->GetRequestedSecurityLevel(); @@ -1206,6 +1212,7 @@ CdmResponseType CdmEngine::RemoveOfflineLicense( CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, const CdmSecureStopId& ssid, + CdmResponseType* error_detail, CdmUsageInfo* usage_info) { LOGI("CdmEngine::GetUsageInfo: %s", ssid.c_str()); if (NULL == usage_property_set_.get()) { @@ -1254,8 +1261,7 @@ CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, } } - status = - usage_session_->RestoreUsageSession(usage_data); + status = usage_session_->RestoreUsageSession(usage_data, error_detail); if (KEY_ADDED != status) { LOGE("CdmEngine::GetUsageInfo: restore usage session error %d", status); @@ -1279,6 +1285,7 @@ CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, } CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, + CdmResponseType* error_detail, CdmUsageInfo* usage_info) { LOGI("CdmEngine::GetUsageInfo: %s", app_id.c_str()); // Return a random usage report from a random security level @@ -1289,7 +1296,7 @@ CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, return PARAMETER_NULL; } do { - status = GetUsageInfo(app_id, security_level, usage_info); + status = GetUsageInfo(app_id, security_level, error_detail, usage_info); if (KEY_MESSAGE == status && !usage_info->empty()) { return status; @@ -1298,7 +1305,7 @@ CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, security_level = (kLevel3 == security_level) ? kLevelDefault : kLevel3; do { - status = GetUsageInfo(app_id, security_level, usage_info); + status = GetUsageInfo(app_id, security_level, error_detail, usage_info); if (NEED_PROVISIONING == status) return NO_ERROR; // Valid scenario that one of the security // levels has not been provisioned @@ -1308,6 +1315,7 @@ CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, SecurityLevel requested_security_level, + CdmResponseType* error_detail, CdmUsageInfo* usage_info) { LOGI("CdmEngine::GetUsageInfo: %s, security level: %d", app_id.c_str(), requested_security_level); @@ -1350,7 +1358,7 @@ CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, usage_info->resize(kUsageReportsPerRequest); uint32_t index = rand() % usage_data.size(); - status = usage_session_->RestoreUsageSession(usage_data[index]); + status = usage_session_->RestoreUsageSession(usage_data[index], error_detail); if (KEY_ADDED != status) { LOGE("CdmEngine::GetUsageInfo: restore usage session (%d) error %ld", index, status); @@ -1644,8 +1652,11 @@ CdmResponseType CdmEngine::LoadUsageSession(const CdmKeySetId& key_set_id, return LOAD_USAGE_INFO_MISSING; } - CdmResponseType status = session->RestoreUsageSession(usage_data); - session->GetMetrics()->cdm_session_restore_usage_session_.Increment(status); + CdmResponseType error_detail = NO_ERROR; + CdmResponseType status = session->RestoreUsageSession(usage_data, + &error_detail); + session->GetMetrics()->cdm_session_restore_usage_session_.Increment( + status, error_detail); if (KEY_ADDED != status) { LOGE("CdmEngine::LoadUsageSession: usage session error %ld", status); return status; diff --git a/libwvdrmengine/cdm/core/src/cdm_session.cpp b/libwvdrmengine/cdm/core/src/cdm_session.cpp index 90260800..650db6d1 100644 --- a/libwvdrmengine/cdm/core/src/cdm_session.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_session.cpp @@ -22,6 +22,15 @@ namespace { const size_t kKeySetIdLength = 14; + +// Helper function for setting the error detail value. +void SetErrorDetail(wvcdm::CdmResponseType* error_detail, + wvcdm::CdmResponseType error_code) { + if (error_detail != nullptr) { + *error_detail = error_code; + } +} + } // namespace namespace wvcdm { @@ -200,7 +209,8 @@ CdmResponseType CdmSession::Init(CdmClientPropertySet* cdm_client_property_set, } CdmResponseType CdmSession::RestoreOfflineSession( - const CdmKeySetId& key_set_id, CdmLicenseType license_type) { + const CdmKeySetId& key_set_id, CdmLicenseType license_type, + CdmResponseType* error_detail) { if (!initialized_) { LOGE("CdmSession::RestoreOfflineSession: not initialized"); return NOT_INITIALIZED_ERROR; @@ -272,6 +282,7 @@ CdmResponseType CdmSession::RestoreOfflineSession( license_parser_->RestoreLicenseForRelease(key_request_, key_response_); if (result != NO_ERROR) { + SetErrorDetail(error_detail, result); return RELEASE_LICENSE_ERROR_1; } } else { @@ -279,6 +290,7 @@ CdmResponseType CdmSession::RestoreOfflineSession( key_request_, key_response_, offline_key_renewal_response_, playback_start_time, last_playback_time, grace_period_end_time, this); if (result != NO_ERROR) { + SetErrorDetail(error_detail, result); return RESTORE_OFFLINE_LICENSE_ERROR_2; } } @@ -308,7 +320,8 @@ CdmResponseType CdmSession::RestoreOfflineSession( } CdmResponseType CdmSession::RestoreUsageSession( - const DeviceFiles::CdmUsageData& usage_data) { + const DeviceFiles::CdmUsageData& usage_data, + CdmResponseType* error_detail) { if (!initialized_) { LOGE("CdmSession::RestoreUsageSession: not initialized"); return NOT_INITIALIZED_ERROR; @@ -338,6 +351,7 @@ CdmResponseType CdmSession::RestoreUsageSession( sts = license_parser_->RestoreLicenseForRelease(key_request_, key_response_); if (sts != NO_ERROR) { + SetErrorDetail(error_detail, sts); return RELEASE_LICENSE_ERROR_2; } diff --git a/libwvdrmengine/cdm/metrics/include/metrics_collections.h b/libwvdrmengine/cdm/metrics/include/metrics_collections.h index 122320d6..03876e93 100644 --- a/libwvdrmengine/cdm/metrics/include/metrics_collections.h +++ b/libwvdrmengine/cdm/metrics/include/metrics_collections.h @@ -67,6 +67,8 @@ const int kErrorCodeFieldNumber = ::drm_metrics::Attributes::kErrorCodeFieldNumber; const int kErrorCodeBoolFieldNumber = ::drm_metrics::Attributes::kErrorCodeBoolFieldNumber; +const int kErrorDetailFieldNumber = + ::drm_metrics::Attributes::kErrorDetailFieldNumber; const int kCdmSecurityLevelFieldNumber = ::drm_metrics::Attributes::kCdmSecurityLevelFieldNumber; const int kSecurityLevelFieldNumber = @@ -276,9 +278,11 @@ class SessionMetrics { // Metrics collected at the session level. ValueMetric cdm_session_life_span_; // Milliseconds. EventMetric cdm_session_renew_key_; - CounterMetric + CounterMetric cdm_session_restore_offline_session_; - CounterMetric + CounterMetric cdm_session_restore_usage_session_; EventMetric @@ -401,7 +405,8 @@ class EngineMetrics { cdm_engine_get_provisioning_request_; CounterMetric cdm_engine_get_secure_stop_ids_; - EventMetric + EventMetric cdm_engine_get_usage_info_; EventMetric cdm_engine_handle_provisioning_response_; diff --git a/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp b/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp index 70d00aa8..f8ccdac5 100644 --- a/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp +++ b/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp @@ -88,6 +88,14 @@ void SetAttributeFieldset_license_type(license_type); } +template <> +void SetAttributeField( + const CdmResponseType &error_detail, + drm_metrics::Attributes *attributes) { + attributes->set_error_detail(error_detail); +} + template <> void SetAttributeField<0, util::Unused>(const util::Unused &, drm_metrics::Attributes *) { diff --git a/libwvdrmengine/cdm/metrics/src/metrics.proto b/libwvdrmengine/cdm/metrics/src/metrics.proto index c4f13a27..f1aa60aa 100644 --- a/libwvdrmengine/cdm/metrics/src/metrics.proto +++ b/libwvdrmengine/cdm/metrics/src/metrics.proto @@ -48,6 +48,8 @@ message Attributes { optional uint32 key_request_type = 16; // Contains the CdmLicenseType defined in wv_cdm_types.h. optional uint32 license_type = 17; + // Error detail supplemental to the error_code field. + optional int32 error_detail = 18; } // The Counter message is used to store a count value with an associated diff --git a/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp b/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp index 2c29907e..60bba090 100644 --- a/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp +++ b/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp @@ -41,7 +41,8 @@ TEST_F(EngineMetricsTest, AllEngineMetrics) { engine_metrics.cdm_engine_find_session_for_key_.Increment(false); engine_metrics.cdm_engine_generate_key_request_.Record(1.0, NO_ERROR, kLicenseTypeRelease); engine_metrics.cdm_engine_get_provisioning_request_.Record(1.0, NO_ERROR); - engine_metrics.cdm_engine_get_usage_info_.Record(1.0, NO_ERROR); + engine_metrics.cdm_engine_get_usage_info_.Record(1.0, NO_ERROR, + UNKNOWN_ERROR); engine_metrics.cdm_engine_handle_provisioning_response_.Record(1.0, NO_ERROR); engine_metrics.cdm_engine_open_key_set_session_.Increment(NO_ERROR); engine_metrics.cdm_engine_open_session_.Increment(NO_ERROR); @@ -87,6 +88,12 @@ TEST_F(EngineMetricsTest, AllEngineMetrics) { EXPECT_EQ(OEMCrypto_INITIALIZED_FORCING_L3, actual_metrics.engine_metrics().oemcrypto_initialization_mode() .int_value()); + ASSERT_EQ( + 1, actual_metrics.engine_metrics() + .cdm_engine_get_usage_info_time_us_size()); + + EXPECT_EQ(UNKNOWN_ERROR, actual_metrics.engine_metrics() + .cdm_engine_get_usage_info_time_us(0).attributes().error_detail()); } TEST_F(EngineMetricsTest, EngineAndCryptoMetrics) { @@ -244,8 +251,10 @@ TEST_F(SessionMetricsTest, AllSessionMetrics) { session_metrics.SetSessionId(kSessionId1); session_metrics.cdm_session_life_span_.Record(1.0); session_metrics.cdm_session_renew_key_.Record(1.0, NO_ERROR); - session_metrics.cdm_session_restore_offline_session_.Increment(NO_ERROR); - session_metrics.cdm_session_restore_usage_session_.Increment(NO_ERROR); + session_metrics.cdm_session_restore_offline_session_.Increment(NO_ERROR, + UNKNOWN_ERROR); + session_metrics.cdm_session_restore_usage_session_.Increment(NO_ERROR, + UNKNOWN_ERROR); session_metrics.cdm_session_license_request_latency_ms_.Record( 2.0, kKeyRequestTypeInitial); diff --git a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp index cba2bf50..260ee29e 100644 --- a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp +++ b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp @@ -239,14 +239,16 @@ CdmResponseType WvContentDecryptionModule::GetUsageInfo( const std::string& app_id, const CdmIdentifier& identifier, CdmUsageInfo* usage_info) { CdmEngine* cdm_engine = EnsureCdmForIdentifier(identifier); - return cdm_engine->GetUsageInfo(app_id, usage_info); + CdmResponseType error_detail = NO_ERROR; + return cdm_engine->GetUsageInfo(app_id, &error_detail, usage_info); } CdmResponseType WvContentDecryptionModule::GetUsageInfo( const std::string& app_id, const CdmSecureStopId& ssid, const CdmIdentifier& identifier, CdmUsageInfo* usage_info) { CdmEngine* cdm_engine = EnsureCdmForIdentifier(identifier); - return cdm_engine->GetUsageInfo(app_id, ssid, usage_info); + CdmResponseType error_detail = NO_ERROR; + return cdm_engine->GetUsageInfo(app_id, ssid, &error_detail, usage_info); } CdmResponseType WvContentDecryptionModule::RemoveAllUsageInfo( From 5b49bf83a2a10528d62a5bf36abe4de28cc9a265 Mon Sep 17 00:00:00 2001 From: Adam Stone Date: Fri, 1 Feb 2019 11:28:03 -0800 Subject: [PATCH 2/2] Add device files error detail to metrics. [ Merge from http://go/wvgerrit/71923 ] Plumb through the device files error detail and add the detail to metrics. Bug: http://b/115382201 Test: Unit tests, manual GPlay. Change-Id: I18139f6712b6670be5fed863a97f9f03440745c7 --- libwvdrmengine/cdm/core/include/cdm_engine.h | 6 +-- .../include/cdm_engine_metrics_decorator.h | 8 ++-- libwvdrmengine/cdm/core/include/cdm_session.h | 16 ++++++-- .../cdm/core/include/device_files.h | 37 ++++++++++--------- libwvdrmengine/cdm/core/src/cdm_engine.cpp | 12 +++--- libwvdrmengine/cdm/core/src/cdm_session.cpp | 36 ++++++++++-------- .../cdm/metrics/include/metrics_collections.h | 6 +-- .../cdm/metrics/src/attribute_handler.cpp | 4 +- .../test/metrics_collections_unittest.cpp | 9 +++-- .../cdm/src/wv_content_decryption_module.cpp | 4 +- 10 files changed, 79 insertions(+), 59 deletions(-) diff --git a/libwvdrmengine/cdm/core/include/cdm_engine.h b/libwvdrmengine/cdm/core/include/cdm_engine.h index 5de39cba..3a94f0c6 100644 --- a/libwvdrmengine/cdm/core/include/cdm_engine.h +++ b/libwvdrmengine/cdm/core/include/cdm_engine.h @@ -228,7 +228,7 @@ class CdmEngine { // id. If |error_detail| is not null, an additional error code may be provided // in the event of an error. virtual CdmResponseType GetUsageInfo(const std::string& app_id, - CdmResponseType* error_detail, + int* error_detail, CdmUsageInfo* usage_info); // Retrieve the usage info for the specified pst. @@ -237,7 +237,7 @@ class CdmEngine { // in the event of an error. virtual CdmResponseType GetUsageInfo(const std::string& app_id, const CdmSecureStopId& ssid, - CdmResponseType* error_detail, + int* error_detail, CdmUsageInfo* usage_info); // Remove all usage records for the current origin. @@ -363,7 +363,7 @@ class CdmEngine { bool ValidateKeySystem(const CdmKeySystem& key_system); CdmResponseType GetUsageInfo(const std::string& app_id, SecurityLevel requested_security_level, - CdmResponseType* error_detail, + int* error_detail, CdmUsageInfo* usage_info); void OnKeyReleaseEvent(const CdmKeySetId& key_set_id); diff --git a/libwvdrmengine/cdm/core/include/cdm_engine_metrics_decorator.h b/libwvdrmengine/cdm/core/include/cdm_engine_metrics_decorator.h index 3b514af0..e4e3053a 100644 --- a/libwvdrmengine/cdm/core/include/cdm_engine_metrics_decorator.h +++ b/libwvdrmengine/cdm/core/include/cdm_engine_metrics_decorator.h @@ -181,10 +181,10 @@ class CdmEngineMetricsImpl : public T { } CdmResponseType GetUsageInfo(const std::string& app_id, - CdmResponseType* error_detail, + int* error_detail, CdmUsageInfo* usage_info) override { CdmResponseType sts; - CdmResponseType error_detail_alt; + int error_detail_alt; M_TIME(sts = T::GetUsageInfo(app_id, &error_detail_alt, usage_info), metrics_, cdm_engine_get_usage_info_, sts, error_detail_alt); if (error_detail != nullptr) { @@ -196,10 +196,10 @@ class CdmEngineMetricsImpl : public T { CdmResponseType GetUsageInfo(const std::string& app_id, const CdmSecureStopId& ssid, - CdmResponseType* error_detail, + int* error_detail, CdmUsageInfo* usage_info) override { CdmResponseType sts; - CdmResponseType error_detail_alt; + int error_detail_alt; M_TIME(sts = T::GetUsageInfo(app_id, ssid, &error_detail_alt, usage_info), metrics_, cdm_engine_get_usage_info_, sts, error_detail_alt); if (error_detail != nullptr) { diff --git a/libwvdrmengine/cdm/core/include/cdm_session.h b/libwvdrmengine/cdm/core/include/cdm_session.h index 6fd290c3..dff41844 100644 --- a/libwvdrmengine/cdm/core/include/cdm_session.h +++ b/libwvdrmengine/cdm/core/include/cdm_session.h @@ -60,12 +60,20 @@ class CdmSession { const CdmSessionId* forced_session_id, WvCdmEventListener* event_listener); + // Restores an offline session identified by the |key_set_id| and + // |license_type|. The |error_detail| will be filled with an internal error + // code. The |error_detail| may be a CdmResponseType or other error code type. + // It is only suitable for additional logging or debugging. virtual CdmResponseType RestoreOfflineSession( const CdmKeySetId& key_set_id, CdmLicenseType license_type, - CdmResponseType* error_detail); + int* error_detail); + // Restores an usage session from the provided |usage_data|. + // The |error_detail| will be filled with an internal error code. The + // |error_detail| may be a CdmResponseType or other error code type. It is + // only suitable for additional logging or debugging. virtual CdmResponseType RestoreUsageSession( const DeviceFiles::CdmUsageData& usage_data, - CdmResponseType* error_detail); + int* error_detail); virtual const CdmSessionId& session_id() { return session_id_; } virtual const CdmKeySetId& key_set_id() { return key_set_id_; } @@ -209,7 +217,9 @@ class CdmSession { bool GenerateKeySetId(CdmKeySetId* key_set_id); CdmResponseType StoreLicense(); - bool StoreLicense(DeviceFiles::LicenseState state); + + bool StoreLicense(DeviceFiles::LicenseState state, + int* error_detail); bool UpdateUsageInfo(); diff --git a/libwvdrmengine/cdm/core/include/device_files.h b/libwvdrmengine/cdm/core/include/device_files.h index 0ffdc77f..737e7700 100644 --- a/libwvdrmengine/cdm/core/include/device_files.h +++ b/libwvdrmengine/cdm/core/include/device_files.h @@ -30,24 +30,27 @@ class DeviceFiles { kLicenseStateUnknown, } LicenseState; + // All error response codes start with 5000 to avoid overlap with other error + // spaces. enum ResponseType { - kNoError = 0, - kObjectNotInitialized = 1, - kParameterNull = 2, - kBasePathUnavailable = 3, - kFileNotFound = 4, - kFileOpenFailed = 5, - kFileWriteError = 6, - kFileReadError = 7, - kInvalidFileSize = 8, - kHashComputationFailed = 9, - kFileHashMismatch = 10, - kFileParseError1 = 11, - kFileParseError2 = 12, - kUnknownLicenseState = 13, - kIncorrectFileType = 14, - kIncorrectFileVersion = 15, - kLicenseNotPresent = 16, + kNoError = NO_ERROR, + kResponseTypeBase = 5000, + kObjectNotInitialized = kResponseTypeBase + 1, + kParameterNull = kResponseTypeBase + 2, + kBasePathUnavailable = kResponseTypeBase + 3, + kFileNotFound = kResponseTypeBase + 4, + kFileOpenFailed = kResponseTypeBase + 5, + kFileWriteError = kResponseTypeBase + 6, + kFileReadError = kResponseTypeBase + 7, + kInvalidFileSize = kResponseTypeBase + 8, + kHashComputationFailed = kResponseTypeBase + 9, + kFileHashMismatch = kResponseTypeBase + 10, + kFileParseError1 = kResponseTypeBase + 11, + kFileParseError2 = kResponseTypeBase + 12, + kUnknownLicenseState = kResponseTypeBase + 13, + kIncorrectFileType = kResponseTypeBase + 14, + kIncorrectFileVersion = kResponseTypeBase + 15, + kLicenseNotPresent = kResponseTypeBase + 16, }; struct CdmUsageData { diff --git a/libwvdrmengine/cdm/core/src/cdm_engine.cpp b/libwvdrmengine/cdm/core/src/cdm_engine.cpp index 69c5f190..8f2a6a30 100644 --- a/libwvdrmengine/cdm/core/src/cdm_engine.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_engine.cpp @@ -285,7 +285,7 @@ CdmResponseType CdmEngine::GenerateKeyRequest( if (license_type == kLicenseTypeRelease && !session->license_received()) { - CdmResponseType error_detail = NO_ERROR; + int error_detail = NO_ERROR; sts = session->RestoreOfflineSession(key_set_id, kLicenseTypeRelease, &error_detail); session->GetMetrics()->cdm_session_restore_offline_session_.Increment( @@ -421,7 +421,7 @@ CdmResponseType CdmEngine::RestoreKey(const CdmSessionId& session_id, } CdmResponseType sts; - CdmResponseType error_detail = NO_ERROR; + int error_detail = NO_ERROR; sts = session->RestoreOfflineSession(key_set_id, kLicenseTypeOffline, &error_detail); session->GetMetrics()->cdm_session_restore_offline_session_.Increment( @@ -1212,7 +1212,7 @@ CdmResponseType CdmEngine::RemoveOfflineLicense( CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, const CdmSecureStopId& ssid, - CdmResponseType* error_detail, + int* error_detail, CdmUsageInfo* usage_info) { LOGI("CdmEngine::GetUsageInfo: %s", ssid.c_str()); if (NULL == usage_property_set_.get()) { @@ -1285,7 +1285,7 @@ CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, } CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, - CdmResponseType* error_detail, + int* error_detail, CdmUsageInfo* usage_info) { LOGI("CdmEngine::GetUsageInfo: %s", app_id.c_str()); // Return a random usage report from a random security level @@ -1315,7 +1315,7 @@ CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, CdmResponseType CdmEngine::GetUsageInfo(const std::string& app_id, SecurityLevel requested_security_level, - CdmResponseType* error_detail, + int* error_detail, CdmUsageInfo* usage_info) { LOGI("CdmEngine::GetUsageInfo: %s, security level: %d", app_id.c_str(), requested_security_level); @@ -1652,7 +1652,7 @@ CdmResponseType CdmEngine::LoadUsageSession(const CdmKeySetId& key_set_id, return LOAD_USAGE_INFO_MISSING; } - CdmResponseType error_detail = NO_ERROR; + int error_detail = NO_ERROR; CdmResponseType status = session->RestoreUsageSession(usage_data, &error_detail); session->GetMetrics()->cdm_session_restore_usage_session_.Increment( diff --git a/libwvdrmengine/cdm/core/src/cdm_session.cpp b/libwvdrmengine/cdm/core/src/cdm_session.cpp index 650db6d1..8ca04acc 100644 --- a/libwvdrmengine/cdm/core/src/cdm_session.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_session.cpp @@ -24,8 +24,8 @@ namespace { const size_t kKeySetIdLength = 14; // Helper function for setting the error detail value. -void SetErrorDetail(wvcdm::CdmResponseType* error_detail, - wvcdm::CdmResponseType error_code) { +template +void SetErrorDetail(int* error_detail, T error_code) { if (error_detail != nullptr) { *error_detail = error_code; } @@ -210,7 +210,7 @@ CdmResponseType CdmSession::Init(CdmClientPropertySet* cdm_client_property_set, CdmResponseType CdmSession::RestoreOfflineSession( const CdmKeySetId& key_set_id, CdmLicenseType license_type, - CdmResponseType* error_detail) { + int* error_detail) { if (!initialized_) { LOGE("CdmSession::RestoreOfflineSession: not initialized"); return NOT_INITIALIZED_ERROR; @@ -237,6 +237,7 @@ CdmResponseType CdmSession::RestoreOfflineSession( "CdmSession::RestoreOfflineSession: failed to retrieve license. " "sub error: %d, key set id = %s", sub_error_code, key_set_id.c_str()); + SetErrorDetail(error_detail, sub_error_code); return GET_LICENSE_ERROR; } @@ -306,7 +307,7 @@ CdmResponseType CdmSession::RestoreOfflineSession( sts); return sts; } - if (!StoreLicense(license_state)) { + if (!StoreLicense(license_state, error_detail)) { LOGW( "CdmSession::RestoreUsageSession: unable to save updated usage " "info"); @@ -320,8 +321,7 @@ CdmResponseType CdmSession::RestoreOfflineSession( } CdmResponseType CdmSession::RestoreUsageSession( - const DeviceFiles::CdmUsageData& usage_data, - CdmResponseType* error_detail) { + const DeviceFiles::CdmUsageData& usage_data, int* error_detail) { if (!initialized_) { LOGE("CdmSession::RestoreUsageSession: not initialized"); return NOT_INITIALIZED_ERROR; @@ -686,7 +686,8 @@ CdmResponseType CdmSession::RenewKey(const CdmKeyResponse& key_response) { if (is_offline_) { offline_key_renewal_response_ = key_response; - if (!StoreLicense(DeviceFiles::kLicenseStateActive)) + if (!StoreLicense(DeviceFiles::kLicenseStateActive, + nullptr /* error_detail */)) return RENEW_KEY_ERROR_2; } return KEY_ADDED; @@ -721,7 +722,7 @@ CdmResponseType CdmSession::GenerateReleaseRequest(CdmKeyRequest* key_request) { } if (is_offline_) { // Mark license as being released - if (!StoreLicense(DeviceFiles::kLicenseStateReleasing)) + if (!StoreLicense(DeviceFiles::kLicenseStateReleasing, nullptr)) return RELEASE_KEY_REQUEST_ERROR; } else if (!usage_provider_session_token_.empty()) { if (usage_support_type_ == kUsageEntrySupport) { @@ -846,7 +847,7 @@ CdmResponseType CdmSession::StoreLicense() { return OFFLINE_LICENSE_PROHIBITED; } - if (!StoreLicense(DeviceFiles::kLicenseStateActive)) { + if (!StoreLicense(DeviceFiles::kLicenseStateActive, nullptr)) { LOGE("CdmSession::StoreLicense: Unable to store license"); return STORE_LICENSE_ERROR_1; } @@ -891,15 +892,20 @@ CdmResponseType CdmSession::StoreLicense() { return NO_ERROR; } -bool CdmSession::StoreLicense(DeviceFiles::LicenseState state) { - DeviceFiles::ResponseType sub_error_code = DeviceFiles::kNoError; - return file_handle_->StoreLicense( +bool CdmSession::StoreLicense(DeviceFiles::LicenseState state, + int* error_detail) { + DeviceFiles::ResponseType error_detail_alt = DeviceFiles::kNoError; + bool result = file_handle_->StoreLicense( key_set_id_, state, offline_init_data_, key_request_, key_response_, offline_key_renewal_request_, offline_key_renewal_response_, offline_release_server_url_, policy_engine_->GetPlaybackStartTime(), policy_engine_->GetLastPlaybackTime(), policy_engine_->GetGracePeriodEndTime(), app_parameters_, usage_entry_, - usage_entry_number_, &sub_error_code); + usage_entry_number_, &error_detail_alt); + if (error_detail != nullptr) { + *error_detail = error_detail_alt; + } + return result; } CdmResponseType CdmSession::RemoveKeys() { @@ -953,7 +959,7 @@ void CdmSession::OnTimerEvent(bool update_usage) { policy_engine_->DecryptionEvent(); has_decrypted_since_last_report_ = false; if (is_offline_ && !is_release_) { - StoreLicense(DeviceFiles::kLicenseStateActive); + StoreLicense(DeviceFiles::kLicenseStateActive, nullptr); } } policy_engine_->OnTimerEvent(); @@ -1030,7 +1036,7 @@ CdmResponseType CdmSession::UpdateUsageEntryInformation() { if (is_offline_) StoreLicense(is_release_ ? DeviceFiles::kLicenseStateReleasing - : DeviceFiles::kLicenseStateActive); + : DeviceFiles::kLicenseStateActive, nullptr); else if (!usage_provider_session_token_.empty()) UpdateUsageInfo(); diff --git a/libwvdrmengine/cdm/metrics/include/metrics_collections.h b/libwvdrmengine/cdm/metrics/include/metrics_collections.h index 03876e93..ba684610 100644 --- a/libwvdrmengine/cdm/metrics/include/metrics_collections.h +++ b/libwvdrmengine/cdm/metrics/include/metrics_collections.h @@ -279,10 +279,10 @@ class SessionMetrics { ValueMetric cdm_session_life_span_; // Milliseconds. EventMetric cdm_session_renew_key_; CounterMetric + kErrorDetailFieldNumber, int32_t> cdm_session_restore_offline_session_; CounterMetric + kErrorDetailFieldNumber, int32_t> cdm_session_restore_usage_session_; EventMetric @@ -406,7 +406,7 @@ class EngineMetrics { CounterMetric cdm_engine_get_secure_stop_ids_; EventMetric + kErrorDetailFieldNumber, int32_t> cdm_engine_get_usage_info_; EventMetric cdm_engine_handle_provisioning_response_; diff --git a/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp b/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp index f8ccdac5..c7dc0d84 100644 --- a/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp +++ b/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp @@ -90,8 +90,8 @@ void SetAttributeField void SetAttributeField( - const CdmResponseType &error_detail, + int32_t>( + const int32_t &error_detail, drm_metrics::Attributes *attributes) { attributes->set_error_detail(error_detail); } diff --git a/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp b/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp index 60bba090..d07d22ff 100644 --- a/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp +++ b/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp @@ -7,6 +7,7 @@ #include +#include "device_files.h" #include "gmock/gmock.h" #include "google/protobuf/text_format.h" #include "gtest/gtest.h" @@ -251,10 +252,10 @@ TEST_F(SessionMetricsTest, AllSessionMetrics) { session_metrics.SetSessionId(kSessionId1); session_metrics.cdm_session_life_span_.Record(1.0); session_metrics.cdm_session_renew_key_.Record(1.0, NO_ERROR); - session_metrics.cdm_session_restore_offline_session_.Increment(NO_ERROR, - UNKNOWN_ERROR); - session_metrics.cdm_session_restore_usage_session_.Increment(NO_ERROR, - UNKNOWN_ERROR); + session_metrics.cdm_session_restore_offline_session_.Increment( + NO_ERROR, DeviceFiles::ResponseType::kObjectNotInitialized); + session_metrics.cdm_session_restore_usage_session_.Increment( + NO_ERROR, DeviceFiles::ResponseType::kObjectNotInitialized); session_metrics.cdm_session_license_request_latency_ms_.Record( 2.0, kKeyRequestTypeInitial); diff --git a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp index 260ee29e..a6eb3b45 100644 --- a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp +++ b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp @@ -239,7 +239,7 @@ CdmResponseType WvContentDecryptionModule::GetUsageInfo( const std::string& app_id, const CdmIdentifier& identifier, CdmUsageInfo* usage_info) { CdmEngine* cdm_engine = EnsureCdmForIdentifier(identifier); - CdmResponseType error_detail = NO_ERROR; + int error_detail = NO_ERROR; return cdm_engine->GetUsageInfo(app_id, &error_detail, usage_info); } @@ -247,7 +247,7 @@ CdmResponseType WvContentDecryptionModule::GetUsageInfo( const std::string& app_id, const CdmSecureStopId& ssid, const CdmIdentifier& identifier, CdmUsageInfo* usage_info) { CdmEngine* cdm_engine = EnsureCdmForIdentifier(identifier); - CdmResponseType error_detail = NO_ERROR; + int error_detail = NO_ERROR; return cdm_engine->GetUsageInfo(app_id, ssid, &error_detail, usage_info); }