From 1b9c6ea7890620d957252232cf6d775094fab871 Mon Sep 17 00:00:00 2001 From: Adam Stone Date: Tue, 5 Sep 2017 16:40:35 -0700 Subject: [PATCH] Fix support for app package name. The app package name was not being reported to the media stats. This change adds the package name as part of the report to media stats. This is one of two parts to this change. The other part is in frameworks/av. Bug: 64584568 Test: Unit tests, GTS tests, tried with Google Play Movies. Change-Id: I1ca09db3a59d9a0950f424d977f8774dffd09c2b --- .../cdm/core/include/wv_cdm_constants.h | 1 + libwvdrmengine/cdm/include/cdm_identifier.h | 18 ++++++++++++++---- .../cdm/metrics/include/metrics_collections.h | 3 +++ libwvdrmengine/cdm/metrics/src/metrics.proto | 3 +++ .../cdm/metrics/src/metrics_collections.cpp | 17 ++++++++++++++--- .../cdm/src/wv_content_decryption_module.cpp | 9 +++++++-- .../cdm/test/request_license_test.cpp | 1 + .../cdm/test/wv_cdm_metrics_test.cpp | 5 ++++- .../mediadrm/src_hidl/WVDrmPlugin.cpp | 4 +++- 9 files changed, 50 insertions(+), 11 deletions(-) diff --git a/libwvdrmengine/cdm/core/include/wv_cdm_constants.h b/libwvdrmengine/cdm/core/include/wv_cdm_constants.h index 959f17a1..ea5f61ea 100644 --- a/libwvdrmengine/cdm/core/include/wv_cdm_constants.h +++ b/libwvdrmengine/cdm/core/include/wv_cdm_constants.h @@ -105,6 +105,7 @@ static const std::string HLS_URI_ATTRIBUTE = "URI"; static const char EMPTY_ORIGIN[] = ""; static const char EMPTY_SPOID[] = ""; +static const char EMPTY_APP_PACKAGE_NAME[] = ""; } // namespace wvcdm #endif // WVCDM_CORE_WV_CDM_CONSTANTS_H_ diff --git a/libwvdrmengine/cdm/include/cdm_identifier.h b/libwvdrmengine/cdm/include/cdm_identifier.h index e99b9d71..618c4e5b 100644 --- a/libwvdrmengine/cdm/include/cdm_identifier.h +++ b/libwvdrmengine/cdm/include/cdm_identifier.h @@ -25,11 +25,17 @@ struct CdmIdentifier { // The origin. May be blank if the app does not set an origin, which is // the likely behavior of most non-web-browser apps. std::string origin; + + // The application package name provided by the application. This is used to + // provide a friendly name of the application package for the purposes of + // logging and metrics. + std::string app_package_name; }; // Provide comparison operators inline bool operator==(const CdmIdentifier& lhs, const CdmIdentifier& rhs) { - return lhs.spoid == rhs.spoid && lhs.origin == rhs.origin; + return lhs.spoid == rhs.spoid && lhs.origin == rhs.origin + && lhs.app_package_name == rhs.app_package_name; } inline bool operator!=(const CdmIdentifier& lhs, const CdmIdentifier& rhs) { @@ -37,8 +43,11 @@ inline bool operator!=(const CdmIdentifier& lhs, const CdmIdentifier& rhs) { } inline bool operator<(const CdmIdentifier& lhs, const CdmIdentifier& rhs) { - return (lhs.spoid < rhs.spoid) || - ((lhs.spoid == rhs.spoid) && lhs.origin < rhs.origin); + return (lhs.spoid < rhs.spoid) + || ((lhs.spoid == rhs.spoid) + && (lhs.origin < rhs.origin + || (lhs.origin == rhs.origin + && lhs.app_package_name < rhs.app_package_name))); } inline bool operator>(const CdmIdentifier& lhs, const CdmIdentifier& rhs) { @@ -56,7 +65,8 @@ inline bool operator>=(const CdmIdentifier& lhs, const CdmIdentifier& rhs) { // Provide default static const CdmIdentifier kDefaultCdmIdentifier = { EMPTY_SPOID, - EMPTY_ORIGIN + EMPTY_ORIGIN, + EMPTY_APP_PACKAGE_NAME }; } // namespace wvcdm diff --git a/libwvdrmengine/cdm/metrics/include/metrics_collections.h b/libwvdrmengine/cdm/metrics/include/metrics_collections.h index e720408b..15e25133 100644 --- a/libwvdrmengine/cdm/metrics/include/metrics_collections.h +++ b/libwvdrmengine/cdm/metrics/include/metrics_collections.h @@ -263,6 +263,8 @@ class EngineMetrics { void Serialize(drm_metrics::MetricsGroup* metric_group, bool completed_only, bool clear_serialized_sessions); + void SetAppPackageName(const std::string& app_package_name); + // Metrics recorded at the engine level. EventMetric cdm_engine_add_key_; ValueMetric cdm_engine_cdm_version_; @@ -288,6 +290,7 @@ class EngineMetrics { Lock session_metrics_lock_; std::vector session_metrics_list_; CryptoMetrics crypto_metrics_; + std::string app_package_name_; void SerializeEngineMetrics(drm_metrics::MetricsGroup* out); }; diff --git a/libwvdrmengine/cdm/metrics/src/metrics.proto b/libwvdrmengine/cdm/metrics/src/metrics.proto index 4df1fe53..02c5af24 100644 --- a/libwvdrmengine/cdm/metrics/src/metrics.proto +++ b/libwvdrmengine/cdm/metrics/src/metrics.proto @@ -33,4 +33,7 @@ message MetricsGroup { // Allow multiple sub groups of metrics. repeated MetricsGroup metric_sub_group = 2; + + // Name of the application package associated with the metrics. + optional string app_package_name = 3; } diff --git a/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp b/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp index c70f2da4..2e81d5dd 100644 --- a/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp +++ b/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp @@ -440,7 +440,8 @@ EngineMetrics::EngineMetrics() : cdm_engine_unprovision_( "/drm/widevine/cdm_engine/unprovision", "error", - "security_level") { + "security_level"), + app_package_name_("") { } EngineMetrics::~EngineMetrics() { @@ -482,14 +483,20 @@ void EngineMetrics::Serialize(drm_metrics::MetricsGroup* metric_group, OemCryptoDynamicAdapterMetrics& adapter_metrics = GetDynamicAdapterMetricsInstance(); adapter_metrics.Serialize(metric_group); - + if (!app_package_name_.empty()) { + metric_group->set_app_package_name(app_package_name_); + } SerializeEngineMetrics(metric_group); std::vector::iterator i; for (i = session_metrics_list_.begin(); i != session_metrics_list_.end(); /* no increment */) { bool serialized = false; if (!completed_only || (*i)->IsCompleted()) { - (*i)->Serialize(metric_group->add_metric_sub_group()); + MetricsGroup* metric_sub_group = metric_group->add_metric_sub_group(); + if (!app_package_name_.empty()) { + metric_sub_group->set_app_package_name(app_package_name_); + } + (*i)->Serialize(metric_sub_group); serialized = true; } @@ -502,6 +509,10 @@ void EngineMetrics::Serialize(drm_metrics::MetricsGroup* metric_group, } } +void EngineMetrics::SetAppPackageName(const std::string& app_package_name) { + app_package_name_ = app_package_name; +} + void EngineMetrics::SerializeEngineMetrics(MetricsGroup* metric_group) { ProtoMetricSerializer serializer(metric_group); cdm_engine_add_key_.Serialize(&serializer); diff --git a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp index c8d51753..263b84d1 100644 --- a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp +++ b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp @@ -380,9 +380,14 @@ CdmEngine* WvContentDecryptionModule::EnsureCdmForIdentifier( cdms_[identifier].file_system.SetOrigin(identifier.origin); cdms_[identifier].file_system.SetIdentifier( identifier.spoid + identifier.origin); - } - return cdms_[identifier].cdm_engine.get(); + // Set the app package name for use by metrics. + cdms_[identifier].cdm_engine->GetMetrics() + ->SetAppPackageName(identifier.app_package_name); + } + CdmEngine* cdm_engine = cdms_[identifier].cdm_engine.get(); + + return cdm_engine; } CdmEngine* WvContentDecryptionModule::GetCdmForSessionId( diff --git a/libwvdrmengine/cdm/test/request_license_test.cpp b/libwvdrmengine/cdm/test/request_license_test.cpp index e4eccc16..0e307f52 100644 --- a/libwvdrmengine/cdm/test/request_license_test.cpp +++ b/libwvdrmengine/cdm/test/request_license_test.cpp @@ -51,6 +51,7 @@ const int kHttpInternalServerError = 500; const wvcdm::CdmIdentifier kExampleIdentifier = { wvcdm::EMPTY_SPOID, + "com.example", "com.example" }; diff --git a/libwvdrmengine/cdm/test/wv_cdm_metrics_test.cpp b/libwvdrmengine/cdm/test/wv_cdm_metrics_test.cpp index 169cc280..e9963a90 100644 --- a/libwvdrmengine/cdm/test/wv_cdm_metrics_test.cpp +++ b/libwvdrmengine/cdm/test/wv_cdm_metrics_test.cpp @@ -106,7 +106,7 @@ TEST_F(WvContentDecryptionModuleMetricsTest, EngineAndSessionMetrics) { TEST_F(WvContentDecryptionModuleMetricsTest, MultipleEngineMetric) { CdmSessionId session_id; wvcdm::CdmKeySystem key_system("com.widevine"); - CdmIdentifier identifier = { "foo", "bar" }; + CdmIdentifier identifier = { "foo", "bar", "baz" }; // Openning the session will fail with NEEDS_PROVISIONING error. But it will // still create some session-level stats. @@ -146,6 +146,9 @@ TEST_F(WvContentDecryptionModuleMetricsTest, MultipleEngineMetric) { metrics.metric_sub_group(i).metric_sub_group(0).metric(0).name(), StrEq("/drm/widevine/cdm_session/session_id")); } + + // Verify that the second metrics app package name is set. + EXPECT_THAT(metrics.metric_sub_group(1).app_package_name(), StrEq("baz")); } } diff --git a/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp b/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp index 2bbca746..cc9003d6 100644 --- a/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp +++ b/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp @@ -1375,7 +1375,9 @@ WVDrmPlugin::CdmIdentifierBuilder::CdmIdentifierBuilder( mIsIdentifierSealed(false), mUseSpoid(useSpoid), mAppPackageName(appPackageName), - mParent(parent) {} + mParent(parent) { + mCdmIdentifier.app_package_name = mAppPackageName; +} Status WVDrmPlugin::CdmIdentifierBuilder::getCdmIdentifier( CdmIdentifier* identifier) {