From 0a64d250672bc316aa381d938668ed3334b47f07 Mon Sep 17 00:00:00 2001 From: Adam Stone Date: Tue, 11 Dec 2018 10:40:32 -0800 Subject: [PATCH 1/2] Add a field indicating online vs offline licenses. Import of http://go/wvgerrit/68188 This adds an attribute to metrics indicating if the license was online or offline. Also, added a unit test for CdmEngineMetricsImpl. Test: Unit tests. GPlay manual. GTS tests. Bug: 115523917 Change-Id: Id315c643048914a2c51904451f9665987bc87eb7 --- libwvdrmengine/cdm/core/include/cdm_engine.h | 4 +++ .../cdm/core/include/wv_cdm_types.h | 1 + libwvdrmengine/cdm/core/src/cdm_engine.cpp | 21 ++++++++++++++- .../cdm/core/test/cdm_engine_test.cpp | 27 +++++++++++++++++-- libwvdrmengine/cdm/core/test/test_base.cpp | 4 ++- .../cdm/metrics/include/metrics_collections.h | 8 ++++-- .../cdm/metrics/src/attribute_handler.cpp | 8 ++++++ libwvdrmengine/cdm/metrics/src/metrics.proto | 2 ++ .../cdm/src/wv_content_decryption_module.cpp | 12 ++++++--- .../src_hidl/hidl_metrics_adapter.cpp | 7 +++++ .../test/hidl_metrics_adapter_unittest.cpp | 5 +++- 11 files changed, 88 insertions(+), 11 deletions(-) diff --git a/libwvdrmengine/cdm/core/include/cdm_engine.h b/libwvdrmengine/cdm/core/include/cdm_engine.h index 1c9f47a3..e3cb89dc 100644 --- a/libwvdrmengine/cdm/core/include/cdm_engine.h +++ b/libwvdrmengine/cdm/core/include/cdm_engine.h @@ -97,6 +97,9 @@ class CdmEngine { // process the response. Should be empty if a release response. // |key_data| is the license, renewal, release response or service // certificate response. + // |license_type| must not be null. If the result is KEY_ADDED, this out + // parameter indicates the type of license containd in + // key_data. For any other return code, no value is provided. // |key_set_id| should be non-null and specified if license release. // If offline license or streaming license associated with // a secure stop, |key_set_id| should be non-null and will @@ -107,6 +110,7 @@ class CdmEngine { // (not associated with a secure stop). virtual CdmResponseType AddKey(const CdmSessionId& session_id, const CdmKeyResponse& key_data, + CdmLicenseType* license_type, CdmKeySetId* key_set_id); virtual CdmResponseType RestoreKey(const CdmSessionId& session_id, diff --git a/libwvdrmengine/cdm/core/include/wv_cdm_types.h b/libwvdrmengine/cdm/core/include/wv_cdm_types.h index 2c17d203..46dd5bb3 100644 --- a/libwvdrmengine/cdm/core/include/wv_cdm_types.h +++ b/libwvdrmengine/cdm/core/include/wv_cdm_types.h @@ -5,6 +5,7 @@ #ifndef WVCDM_CORE_WV_CDM_TYPES_H_ #define WVCDM_CORE_WV_CDM_TYPES_H_ +#include #include #include #include diff --git a/libwvdrmengine/cdm/core/src/cdm_engine.cpp b/libwvdrmengine/cdm/core/src/cdm_engine.cpp index 879c410e..e4e24865 100644 --- a/libwvdrmengine/cdm/core/src/cdm_engine.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_engine.cpp @@ -311,8 +311,13 @@ CdmResponseType CdmEngine::GenerateKeyRequest( CdmResponseType CdmEngine::AddKey(const CdmSessionId& session_id, const CdmKeyResponse& key_data, + CdmLicenseType* license_type, CdmKeySetId* key_set_id) { LOGI("CdmEngine::AddKey"); + if (license_type == nullptr) { + LOGE("CdmEngine::AddKey: license_type cannot be null."); + return PARAMETER_NULL; + } CdmSessionId id = session_id; bool license_type_release = session_id.empty(); @@ -349,7 +354,21 @@ CdmResponseType CdmEngine::AddKey(const CdmSessionId& session_id, return EMPTY_KEY_DATA_1; } - CdmResponseType sts = session->AddKey(key_data); + + CdmResponseType sts = (session->AddKey(key_data)); + + if (sts == KEY_ADDED) { + if (session->is_release()) { + *license_type = kLicenseTypeRelease; + } else if (session->is_temporary()) { + *license_type = kLicenseTypeTemporary; + } else if (session->is_offline()) { + *license_type = kLicenseTypeOffline; + } else { + *license_type = kLicenseTypeStreaming; + } + } + if (key_set_id) { if ((session->is_offline() || session->has_provider_session_token()) && !license_type_release) { diff --git a/libwvdrmengine/cdm/core/test/cdm_engine_test.cpp b/libwvdrmengine/cdm/core/test/cdm_engine_test.cpp index 0efea6b2..50b45b8a 100644 --- a/libwvdrmengine/cdm/core/test/cdm_engine_test.cpp +++ b/libwvdrmengine/cdm/core/test/cdm_engine_test.cpp @@ -159,6 +159,12 @@ class WvCdmEngineTest : public WvCdmEnginePreProvTest { protected: void GenerateKeyRequest(const std::string& key_id, const std::string& init_data_type_string) { + GenerateKeyRequest(key_id, init_data_type_string, kLicenseTypeStreaming); + } + + void GenerateKeyRequest(const std::string& key_id, + const std::string& init_data_type_string, + CdmLicenseType license_type) { CdmAppParameterMap app_parameters; CdmKeySetId key_set_id; @@ -167,7 +173,7 @@ class WvCdmEngineTest : public WvCdmEnginePreProvTest { CdmKeyRequest key_request; CdmResponseType result = cdm_engine_.GenerateKeyRequest( - session_id_, key_set_id, init_data, kLicenseTypeStreaming, + session_id_, key_set_id, init_data, license_type, app_parameters, &key_request); EXPECT_EQ(KEY_MESSAGE, result); @@ -234,9 +240,19 @@ class WvCdmEngineTest : public WvCdmEnginePreProvTest { void VerifyNewKeyResponse(const std::string& server_url, const std::string& client_auth) { + VerifyNewKeyResponse(server_url, client_auth, kLicenseTypeStreaming); + } + void VerifyNewKeyResponse(const std::string& server_url, + const std::string& client_auth, + CdmLicenseType expected_license_type) { std::string resp = GetKeyRequestResponse(server_url, client_auth); CdmKeySetId key_set_id; - EXPECT_EQ(KEY_ADDED, cdm_engine_.AddKey(session_id_, resp, &key_set_id)); + CdmLicenseType license_type; + CdmResponseType status = + cdm_engine_.AddKey(session_id_, resp, &license_type, &key_set_id); + + EXPECT_EQ(KEY_ADDED, status); + EXPECT_EQ(expected_license_type, license_type); VerifyLicenseRequestLatency(kKeyRequestTypeInitial, *cdm_engine_.GetMetrics()); } @@ -335,6 +351,13 @@ TEST_F(WvCdmEngineTest, NormalDecryptionIsoBmff) { VerifyNewKeyResponse(config_.license_server(), config_.client_auth()); } +// TODO(blueeyes): Add tests for different license types. +TEST_F(WvCdmEngineTest, ReturnsLicenseTypeDetailStreaming) { + GenerateKeyRequest(binary_key_id(), kCencMimeType, kLicenseTypeStreaming); + VerifyNewKeyResponse(config_.license_server(), config_.client_auth(), + kLicenseTypeStreaming); +} + // TODO(juce): Set up with correct test data. TEST_F(WvCdmEngineTest, DISABLED_NormalDecryptionWebm) { // Extract the key ID from the PSSH box. diff --git a/libwvdrmengine/cdm/core/test/test_base.cpp b/libwvdrmengine/cdm/core/test/test_base.cpp index f0341bb0..349cec5c 100644 --- a/libwvdrmengine/cdm/core/test/test_base.cpp +++ b/libwvdrmengine/cdm/core/test/test_base.cpp @@ -610,8 +610,10 @@ void TestLicenseHolder::SignAndLoadLicense() { signed_response.SerializeToString(&response_data); CdmKeySetId key_set_id; + CdmLicenseType license_type; // Required for AddKey. Result value ignored. EXPECT_EQ(KEY_ADDED, - cdm_engine_->AddKey(session_id_, response_data, &key_set_id)); + cdm_engine_->AddKey(session_id_, response_data, + &license_type, &key_set_id)); } void TestLicenseHolder::DeriveKeysFromSessionKey() { diff --git a/libwvdrmengine/cdm/metrics/include/metrics_collections.h b/libwvdrmengine/cdm/metrics/include/metrics_collections.h index a65b6e74..8eb1bfc5 100644 --- a/libwvdrmengine/cdm/metrics/include/metrics_collections.h +++ b/libwvdrmengine/cdm/metrics/include/metrics_collections.h @@ -85,6 +85,8 @@ const int kEventTypeFieldNumber = ::drm_metrics::Attributes::kEventTypeFieldNumber; const int kKeyRequestTypeFieldNumber = ::drm_metrics::Attributes::kKeyRequestTypeFieldNumber; +const int kLicenseTypeFieldNumber = + ::drm_metrics::Attributes::kLicenseTypeFieldNumber; } // anonymous namespace @@ -374,7 +376,8 @@ class EngineMetrics { void SetAppPackageName(const std::string &app_package_name); // Metrics recorded at the engine level. - EventMetric cdm_engine_add_key_; + EventMetric cdm_engine_add_key_; ValueMetric cdm_engine_cdm_version_; CounterMetric cdm_engine_close_session_; @@ -384,7 +387,8 @@ class EngineMetrics { cdm_engine_decrypt_; CounterMetric cdm_engine_find_session_for_key_; - EventMetric + EventMetric cdm_engine_generate_key_request_; EventMetric cdm_engine_get_provisioning_request_; diff --git a/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp b/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp index 8de1a641..70d00aa8 100644 --- a/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp +++ b/libwvdrmengine/cdm/metrics/src/attribute_handler.cpp @@ -80,6 +80,14 @@ void SetAttributeFieldset_key_request_type(key_request_type); } +template <> +void SetAttributeField( + const CdmLicenseType &license_type, + drm_metrics::Attributes *attributes) { + attributes->set_license_type(license_type); +} + 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 6fa787dd..2f749968 100644 --- a/libwvdrmengine/cdm/metrics/src/metrics.proto +++ b/libwvdrmengine/cdm/metrics/src/metrics.proto @@ -46,6 +46,8 @@ message Attributes { optional uint32 event_type = 15; // Contains the CdmKeyRequestType defined in wv_cdm_types.h. optional uint32 key_request_type = 16; + // Contains the CdmLicenseType defined in wv_cdm_types.h. + optional uint32 license_type = 17; } // The Counter message is used to store a count value with an associated diff --git a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp index 3cf2de23..99538d16 100644 --- a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp +++ b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp @@ -122,7 +122,8 @@ CdmResponseType WvContentDecryptionModule::GenerateKeyRequest( M_TIME(sts = cdm_engine->GenerateKeyRequest(session_id, key_set_id, initialization_data, license_type, app_parameters, key_request), - cdm_engine->GetMetrics(), cdm_engine_generate_key_request_, sts); + cdm_engine->GetMetrics(), cdm_engine_generate_key_request_, + sts, license_type); switch (license_type) { case kLicenseTypeRelease: if (sts != KEY_MESSAGE) { @@ -149,9 +150,12 @@ CdmResponseType WvContentDecryptionModule::AddKey( release_key_set_id = *key_set_id; } CdmResponseType sts; - M_TIME(sts = cdm_engine->AddKey(session_id, key_data, key_set_id), - cdm_engine->GetMetrics(), cdm_engine_add_key_, sts); - if (sts == KEY_ADDED && session_id.empty()) { // license type release + CdmLicenseType license_type; + M_TIME(sts = cdm_engine->AddKey(session_id, key_data, + &license_type, key_set_id), + cdm_engine->GetMetrics(), cdm_engine_add_key_, sts, license_type); + // Empty session id indicates license type release. + if (sts == KEY_ADDED && session_id.empty()) { cdm_engine->CloseKeySetSession(release_key_set_id); cdm_by_session_id_.erase(release_key_set_id); } diff --git a/libwvdrmengine/mediadrm/src_hidl/hidl_metrics_adapter.cpp b/libwvdrmengine/mediadrm/src_hidl/hidl_metrics_adapter.cpp index d5ebf643..f00ae387 100644 --- a/libwvdrmengine/mediadrm/src_hidl/hidl_metrics_adapter.cpp +++ b/libwvdrmengine/mediadrm/src_hidl/hidl_metrics_adapter.cpp @@ -30,6 +30,7 @@ const char kAttributeOemCryptoResult[] = "oem_crypto_result"; const char kAttributeKeyStatusType[] = "key_status_type"; const char kAttributeEventType[] = "event_type"; const char kAttributeKeyRequestType[] = "key_request_type"; +const char kAttributeLicenseType[] = "license_type"; template void SetValue(const T& value, DrmMetricGroup::Attribute* attribute); @@ -237,6 +238,12 @@ void HidlMetricsGroupBuilder::AddAttributes( DrmMetricGroup::ValueType::INT64_TYPE, attributes_proto.key_request_type(), &attribute_vector); } + if (attributes_proto.has_license_type()) { + AddAttribute( + kAttributeLicenseType, + DrmMetricGroup::ValueType::INT64_TYPE, + attributes_proto.license_type(), &attribute_vector); + } *attributes = attribute_vector; } diff --git a/libwvdrmengine/mediadrm/test/hidl_metrics_adapter_unittest.cpp b/libwvdrmengine/mediadrm/test/hidl_metrics_adapter_unittest.cpp index cdfe0e14..f79b8bce 100644 --- a/libwvdrmengine/mediadrm/test/hidl_metrics_adapter_unittest.cpp +++ b/libwvdrmengine/mediadrm/test/hidl_metrics_adapter_unittest.cpp @@ -281,6 +281,7 @@ TEST(HidlMetricsAdapterTest, AddAllAttrbitues) { attributes->set_key_status_type(43); attributes->set_event_type(47); attributes->set_key_request_type(53); + attributes->set_license_type(59); DrmMetricGroup::Metric expected_counter_metric = { "crypto_session_get_token", @@ -302,7 +303,9 @@ TEST(HidlMetricsAdapterTest, AddAllAttrbitues) { DrmMetricGroup::ValueType::INT64_TYPE, 43, 0, "" }, { "event_type", DrmMetricGroup::ValueType::INT64_TYPE, 47, 0, "" }, { "key_request_type", - DrmMetricGroup::ValueType::INT64_TYPE, 53, 0, "" } }, + DrmMetricGroup::ValueType::INT64_TYPE, 53, 0, "" }, + { "license_type", + DrmMetricGroup::ValueType::INT64_TYPE, 59, 0, "" } }, { { "count", DrmMetricGroup::ValueType::INT64_TYPE, 13, 0, "" } } }; // Confirm that all of the attributes exist in the hidl data. From 27e26110b4d420991f95ab9cc250ef74c0a214fb Mon Sep 17 00:00:00 2001 From: Adam Stone Date: Wed, 12 Dec 2018 16:40:41 -0800 Subject: [PATCH 2/2] Add Oemcrypto build information to metrics. Import from http://go/wvgerrit/68385 Adds the build information returned from OEMCrypto_BuildInformation() to the CDM session metrics. Bug: 117117555 Test: Unit tests. GPlay manual. GTS Tests. Change-Id: I505c46fec61a7c62538f843185ec0358f860da79 --- libwvdrmengine/cdm/core/src/cdm_session.cpp | 6 ++++++ libwvdrmengine/cdm/metrics/include/metrics_collections.h | 1 + libwvdrmengine/cdm/metrics/src/metrics.proto | 3 ++- libwvdrmengine/cdm/metrics/src/metrics_collections.cpp | 2 ++ libwvdrmengine/mediadrm/src_hidl/hidl_metrics_adapter.cpp | 5 +++++ .../mediadrm/test/hidl_metrics_adapter_unittest.cpp | 5 +++-- 6 files changed, 19 insertions(+), 3 deletions(-) diff --git a/libwvdrmengine/cdm/core/src/cdm_session.cpp b/libwvdrmengine/cdm/core/src/cdm_session.cpp index d027861c..eeac3a4d 100644 --- a/libwvdrmengine/cdm/core/src/cdm_session.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_session.cpp @@ -98,6 +98,12 @@ CdmResponseType CdmSession::Init(CdmClientPropertySet* cdm_client_property_set, security_level_ = crypto_session_->GetSecurityLevel(); crypto_metrics_->crypto_session_security_level_.Record(security_level_); + std::string oemcrypto_build; + if(crypto_session_->GetBuildInformation(&oemcrypto_build)) { + metrics_->oemcrypto_build_info_.Record(oemcrypto_build); + } else { + metrics_->oemcrypto_build_info_.SetError(false); + } if (!file_handle_->Init(security_level_)) { LOGE("CdmSession::Init: Unable to initialize file handle"); diff --git a/libwvdrmengine/cdm/metrics/include/metrics_collections.h b/libwvdrmengine/cdm/metrics/include/metrics_collections.h index 8eb1bfc5..d68b4d2d 100644 --- a/libwvdrmengine/cdm/metrics/include/metrics_collections.h +++ b/libwvdrmengine/cdm/metrics/include/metrics_collections.h @@ -278,6 +278,7 @@ class SessionMetrics { EventMetric cdm_session_license_request_latency_ms_; + ValueMetric oemcrypto_build_info_; // Serialize the session metrics to the provided |metric_group|. // |metric_group| is owned by the caller and must not be null. diff --git a/libwvdrmengine/cdm/metrics/src/metrics.proto b/libwvdrmengine/cdm/metrics/src/metrics.proto index 2f749968..c4f13a27 100644 --- a/libwvdrmengine/cdm/metrics/src/metrics.proto +++ b/libwvdrmengine/cdm/metrics/src/metrics.proto @@ -157,7 +157,7 @@ message WvCdmMetrics { // This contains metrics that were captured within a CdmSession. This contains // nested CryptoMetrics that were captured in the context of the session. - // next id: 8 + // next id: 9 message SessionMetrics { optional ValueMetric session_id = 1; optional CryptoMetrics crypto_metrics = 2; @@ -166,6 +166,7 @@ message WvCdmMetrics { repeated CounterMetric cdm_session_restore_offline_session = 5; repeated CounterMetric cdm_session_restore_usage_session = 6; repeated DistributionMetric cdm_session_license_request_latency_ms = 7; + optional ValueMetric oemcrypto_build_info = 8; } // These are metrics recorded at the Engine level. This includes CryptoSession diff --git a/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp b/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp index 7cd3e695..26e85ac7 100644 --- a/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp +++ b/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp @@ -177,6 +177,8 @@ void SessionMetrics::SerializeSessionMetrics( session_metrics->mutable_cdm_session_restore_usage_session()); cdm_session_license_request_latency_ms_.ToProto( session_metrics->mutable_cdm_session_license_request_latency_ms()); + session_metrics->set_allocated_oemcrypto_build_info( + oemcrypto_build_info_.ToProto()); } OemCryptoDynamicAdapterMetrics::OemCryptoDynamicAdapterMetrics() diff --git a/libwvdrmengine/mediadrm/src_hidl/hidl_metrics_adapter.cpp b/libwvdrmengine/mediadrm/src_hidl/hidl_metrics_adapter.cpp index f00ae387..75febd26 100644 --- a/libwvdrmengine/mediadrm/src_hidl/hidl_metrics_adapter.cpp +++ b/libwvdrmengine/mediadrm/src_hidl/hidl_metrics_adapter.cpp @@ -397,6 +397,11 @@ void HidlMetricsAdapter::AddSessionMetrics( group_builder.AddDistributions( "cdm_session_license_request_latency_ms", proto_metrics.cdm_session_license_request_latency_ms()); + if (proto_metrics.has_oemcrypto_build_info()) { + group_builder.AddValue( + "oemcrypto_build_info", + proto_metrics.oemcrypto_build_info()); + } group_vector_.emplace_back(group_builder.Build()); } diff --git a/libwvdrmengine/mediadrm/test/hidl_metrics_adapter_unittest.cpp b/libwvdrmengine/mediadrm/test/hidl_metrics_adapter_unittest.cpp index f79b8bce..f30807ae 100644 --- a/libwvdrmengine/mediadrm/test/hidl_metrics_adapter_unittest.cpp +++ b/libwvdrmengine/mediadrm/test/hidl_metrics_adapter_unittest.cpp @@ -397,6 +397,7 @@ TEST(HidlMetricsAdapterTest, EngineAndSessionAllMetrics) { session_metrics.add_cdm_session_restore_offline_session()->set_count(13); session_metrics.add_cdm_session_restore_usage_session()->set_count(13); session_metrics.add_cdm_session_license_request_latency_ms()->set_min(1.0); + session_metrics.mutable_oemcrypto_build_info()->set_string_value("test"); drm_metrics::WvCdmMetrics::EngineMetrics engine_metrics; *(engine_metrics.mutable_crypto_metrics()) = crypto_metrics; @@ -442,8 +443,8 @@ TEST(HidlMetricsAdapterTest, EngineAndSessionAllMetrics) { hidl_vec hidl_metrics; HidlMetricsAdapter::ToHidlMetrics(metrics_proto, &hidl_metrics); ASSERT_EQ(2U, hidl_metrics.size()); - EXPECT_EQ(85U, hidl_metrics[0].metrics.size()); - EXPECT_EQ(63U, hidl_metrics[1].metrics.size()); + EXPECT_EQ(85U, hidl_metrics[0].metrics.size()) << ToString(hidl_metrics); + EXPECT_EQ(64U, hidl_metrics[1].metrics.size()) << ToString(hidl_metrics); } } // namespace wvcdm