From a0da1f067b603ae82aaf3b1f8b58c2401b48ecc8 Mon Sep 17 00:00:00 2001 From: Adam Stone Date: Thu, 30 Mar 2017 11:31:51 -0700 Subject: [PATCH] Support GetPropertyByteArray for getting metrics. Adds support for GetPropertyByteArray to return a serialized set of metrics to the caller. This should be the last part of the widevine plugin changes that fix the referenced bug. More changes are coming on the MediaDrm side. This is a merge of wvgerrit/28422 I intend to submit 2048751, 2048750, and 2048509 together. Bug: 36217927 Test: Added additional unit tests for affected code. Change-Id: I2618c2be48d7d780127e35f237e2276efd080879 --- libwvdrmengine/cdm/Android.mk | 3 +- libwvdrmengine/cdm/include/ami_adapter.h | 7 ++-- .../include/wv_content_decryption_module.h | 7 ++-- libwvdrmengine/cdm/src/ami_adapter.cpp | 4 --- .../cdm/src/wv_content_decryption_module.cpp | 35 +++++++++++-------- libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp | 5 ++- .../mediadrm/src_hidl/WVDrmPlugin.cpp | 4 +++ .../mediadrm/test/WVDrmPlugin_test.cpp | 28 ++++++++++++++- .../test/legacy_src/WVDrmPlugin_test.cpp | 24 ++++++++++++- 9 files changed, 85 insertions(+), 32 deletions(-) diff --git a/libwvdrmengine/cdm/Android.mk b/libwvdrmengine/cdm/Android.mk index bf02ae64..9c54844b 100644 --- a/libwvdrmengine/cdm/Android.mk +++ b/libwvdrmengine/cdm/Android.mk @@ -39,8 +39,7 @@ LOCAL_SRC_FILES := \ $(SRC_DIR)/wv_content_decryption_module.cpp \ $(METRICS_SRC_DIR)/distribution.cpp \ $(METRICS_SRC_DIR)/event_metric.cpp \ - $(METRICS_SRC_DIR)/metrics_front_end.cpp \ - $(METRICS_SRC_DIR)/metrics_group.cpp \ + $(METRICS_SRC_DIR)/metrics_collections.cpp \ $(METRICS_SRC_DIR)/timer_metric.cpp \ diff --git a/libwvdrmengine/cdm/include/ami_adapter.h b/libwvdrmengine/cdm/include/ami_adapter.h index 75999233..42bc4b1f 100644 --- a/libwvdrmengine/cdm/include/ami_adapter.h +++ b/libwvdrmengine/cdm/include/ami_adapter.h @@ -4,22 +4,19 @@ #define CDM_AMI_ADAPTER_H_ #include +#include #include -#include "report.h" - namespace wvcdm { -class AmiAdapter : public metrics::Report { +class AmiAdapter { public: AmiAdapter(); AmiAdapter(int64_t parent); ~AmiAdapter(); - metrics::Report* NewReport() const; - void UpdateString(const std::string& metric_id, const std::string& value); void UpdateInt32(const std::string& metric_id, int32_t value); void UpdateInt64(const std::string& metric_id, int64_t value); diff --git a/libwvdrmengine/cdm/include/wv_content_decryption_module.h b/libwvdrmengine/cdm/include/wv_content_decryption_module.h index 8d0b5d27..11cf96cb 100644 --- a/libwvdrmengine/cdm/include/wv_content_decryption_module.h +++ b/libwvdrmengine/cdm/include/wv_content_decryption_module.h @@ -11,7 +11,6 @@ #include "cdm_identifier.h" #include "file_store.h" #include "lock.h" -#include "metrics_front_end.h" #include "timer.h" #include "wv_cdm_types.h" @@ -122,6 +121,9 @@ class WvContentDecryptionModule : public android::RefBase, public TimerHandler { // Validate a passed-in service certificate virtual bool IsValidServiceCertificate(const std::string& certificate); + // Retrieve the serialized metrics from the CDM. + virtual void GetSerializedMetrics(std::string* serialized_metrics); + private: struct CdmInfo { CdmInfo(); @@ -156,9 +158,6 @@ class WvContentDecryptionModule : public android::RefBase, public TimerHandler { // This contains weak pointers to the CDM instances contained in |cdms_|. std::map cdm_by_session_id_; - metrics::Report* report_root_; - metrics::MetricsFrontEnd* front_end_; - CORE_DISALLOW_COPY_AND_ASSIGN(WvContentDecryptionModule); }; diff --git a/libwvdrmengine/cdm/src/ami_adapter.cpp b/libwvdrmengine/cdm/src/ami_adapter.cpp index 73dd4ee9..4c7cfc29 100644 --- a/libwvdrmengine/cdm/src/ami_adapter.cpp +++ b/libwvdrmengine/cdm/src/ami_adapter.cpp @@ -22,10 +22,6 @@ AmiAdapter::~AmiAdapter() { analytics_item_.selfrecord(); } -metrics::Report* AmiAdapter::NewReport() const { - return new AmiAdapter(analytics_item_.getSessionID()); -} - void AmiAdapter::UpdateString(const std::string& metric_id, const std::string& value) { analytics_item_.setCString(metric_id.c_str(), value.c_str()); diff --git a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp index 6668fc60..21fecfe5 100644 --- a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp +++ b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp @@ -7,7 +7,7 @@ #include "initialization_data.h" #include "license.h" #include "log.h" -#include "metrics_front_end.h" +#include "metrics.pb.h" #include "properties.h" #include "service_certificate.h" #include "wv_cdm_constants.h" @@ -21,22 +21,10 @@ namespace wvcdm { Lock WvContentDecryptionModule::session_sharing_id_generation_lock_; -WvContentDecryptionModule::WvContentDecryptionModule() { - // TODO (b/36497276) - // replace call to new AmiAdapter() and remove ami_apdater.* - report_root_ = NULL; // new AmiAdapter(); - front_end_ = new metrics::MetricsFrontEnd(report_root_); - metrics::MetricsFrontEnd::OverrideInstance(front_end_); -} +WvContentDecryptionModule::WvContentDecryptionModule() {} WvContentDecryptionModule::~WvContentDecryptionModule() { DisablePolicyTimer(true); - - metrics::MetricsFrontEnd::OverrideInstance(NULL); - delete front_end_; - delete report_root_; - front_end_ = NULL; - report_root_ = NULL; } bool WvContentDecryptionModule::IsSupported(const std::string& init_data_type) { @@ -406,6 +394,25 @@ bool WvContentDecryptionModule::IsValidServiceCertificate( NO_ERROR; } +void WvContentDecryptionModule::GetSerializedMetrics( + std::string* serialized_metrics) { + drm_metrics::MetricsGroup metric_group; + { + AutoLock auto_lock(cdms_lock_); + for (auto it = cdms_.begin(); it != cdms_.end(); it++) { + metrics::EngineMetrics* engine_metrics = + it->second.cdm_engine->GetMetrics(); + if (engine_metrics) { + // Serialize the metrics from the engine and any completed sessions. + // Clear the metrics from any completed sessions. + engine_metrics->Serialize( + metric_group.add_metric_sub_group(), true, true); + } + } + } + metric_group.SerializeToString(serialized_metrics); +} + WvContentDecryptionModule::CdmInfo::CdmInfo() : cdm_engine(new CdmEngine(&file_system)) {} diff --git a/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp b/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp index 5a8603cc..b5607418 100644 --- a/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp +++ b/libwvdrmengine/mediadrm/src/WVDrmPlugin.cpp @@ -528,13 +528,16 @@ status_t WVDrmPlugin::getPropertyString(const String8& name, status_t WVDrmPlugin::getPropertyByteArray(const String8& name, Vector& value) const { - if (name == "deviceUniqueId") { return queryProperty(QUERY_KEY_DEVICE_ID, value); } else if (name == "provisioningUniqueId") { return queryProperty(QUERY_KEY_PROVISIONING_ID, value); } else if (name == "serviceCertificate") { value = ToVector(mPropertySet.service_certificate()); + } else if (name == "metrics") { + std::string metrics_value; + mCDM->GetSerializedMetrics(&metrics_value); + value = ToVector(metrics_value); } else { ALOGE("App requested unknown byte array property %s", name.string()); return android::ERROR_DRM_CANNOT_HANDLE; diff --git a/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp b/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp index bcbe9903..0c7941bc 100644 --- a/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp +++ b/libwvdrmengine/mediadrm/src_hidl/WVDrmPlugin.cpp @@ -685,6 +685,10 @@ Return WVDrmPlugin::getPropertyByteArray( } } else if (name == "serviceCertificate") { value = StrToVector(mPropertySet.service_certificate()); + } else if (name == "metrics") { + std::string metrics_value; + mCDM->GetSerializedMetrics(&metrics_value); + value = StrToVector(metrics_value); } else { ALOGE("App requested unknown byte array property %s", name.c_str()); status = android::ERROR_DRM_CANNOT_HANDLE; diff --git a/libwvdrmengine/mediadrm/test/WVDrmPlugin_test.cpp b/libwvdrmengine/mediadrm/test/WVDrmPlugin_test.cpp index 9d20462d..e08bb45d 100644 --- a/libwvdrmengine/mediadrm/test/WVDrmPlugin_test.cpp +++ b/libwvdrmengine/mediadrm/test/WVDrmPlugin_test.cpp @@ -114,7 +114,17 @@ const std::string kAppId("com.unittest.mock.app.id"); const uint8_t* const kUnprovisionResponse = reinterpret_cast("unprovision"); const size_t kUnprovisionResponseSize = 11; -} +const std::string kDeviceId = "0123456789ABCDEF"; + +// This is a serialized MetricsGroup message containing a small amount of +// sample data. This ensures we're able to extract it via a property. +const char kSerializedMetrics[] = { + 0x0a, 0x0a, 0x0a, 0x04, 0x74, 0x65, 0x73, 0x74, 0x12, 0x02, 0x08, 0x00, + 0x0a, 0x12, 0x0a, 0x05, 0x74, 0x65, 0x73, 0x74, 0x32, 0x12, 0x09, 0x11, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x15, 0x0a, 0x05, + 0x74, 0x65, 0x73, 0x74, 0x33, 0x12, 0x0c, 0x1a, 0x0a, 0x74, 0x65, 0x73, + 0x74, 0x5f, 0x76, 0x61, 0x6c, 0x75, 0x65}; +} // anonymous namespace class MockCDM : public WvContentDecryptionModule { public: @@ -177,6 +187,8 @@ class MockCDM : public WvContentDecryptionModule { CdmResponseType(const CdmUsageInfoReleaseMessage&)); MOCK_METHOD1(IsValidServiceCertificate, bool(const std::string&)); + + MOCK_METHOD1(GetSerializedMetrics, void(std::string*)); }; class MockCrypto : public WVGenericCryptoInterface { @@ -1040,6 +1052,8 @@ TEST_F(WVDrmPluginTest, ReturnsExpectedPropertyValues) { static const std::string openSessions = "42"; static const std::string maxSessions = "54"; static const std::string oemCryptoApiVersion = "13"; + std::string serializedMetrics( + kSerializedMetrics, kSerializedMetrics + sizeof(kSerializedMetrics)); EXPECT_CALL(*cdm, QueryStatus(_, QUERY_KEY_SECURITY_LEVEL, _)) .WillOnce(DoAll(SetArgPointee<2>(QUERY_VALUE_SECURITY_LEVEL_L1), @@ -1071,6 +1085,9 @@ TEST_F(WVDrmPluginTest, ReturnsExpectedPropertyValues) { .WillOnce(DoAll(SetArgPointee<2>(oemCryptoApiVersion), testing::Return(wvcdm::NO_ERROR))); + EXPECT_CALL(*cdm, GetSerializedMetrics(_)) + .WillOnce(SetArgPointee<0>(serializedMetrics)); + WVDrmPlugin plugin(cdm.get(), appPackageName, &crypto, false); std::string stringResult; std::vector vectorResult; @@ -1157,6 +1174,15 @@ TEST_F(WVDrmPluginTest, ReturnsExpectedPropertyValues) { ASSERT_EQ(Status::OK, status); EXPECT_EQ(oemCryptoApiVersion, stringResult.c_str()); }); + + plugin.getPropertyByteArray( + hidl_string("metrics"), + [&](Status status, hidl_vec vectorResult) { + ASSERT_EQ(Status::OK, status); + std::vector id(vectorResult); + EXPECT_THAT(id, ElementsAreArray(serializedMetrics.data(), + serializedMetrics.size())); + }); } TEST_F(WVDrmPluginTest, DoesNotGetUnknownProperties) { diff --git a/libwvdrmengine/mediadrm/test/legacy_src/WVDrmPlugin_test.cpp b/libwvdrmengine/mediadrm/test/legacy_src/WVDrmPlugin_test.cpp index e00fbb82..49742623 100644 --- a/libwvdrmengine/mediadrm/test/legacy_src/WVDrmPlugin_test.cpp +++ b/libwvdrmengine/mediadrm/test/legacy_src/WVDrmPlugin_test.cpp @@ -32,7 +32,16 @@ const String8 kAppId("com.unittest.mock.app.id"); const uint8_t* const kUnprovisionResponse = reinterpret_cast("unprovision"); const size_t kUnprovisionResponseSize = 11; -} + +// This is a serialized MetricsGroup message containing a small amount of +// sample data. This ensures we're able to extract it via a property. +const char kSerializedMetrics[] = { + 0x0a, 0x0a, 0x0a, 0x04, 0x74, 0x65, 0x73, 0x74, 0x12, 0x02, 0x08, 0x00, + 0x0a, 0x12, 0x0a, 0x05, 0x74, 0x65, 0x73, 0x74, 0x32, 0x12, 0x09, 0x11, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x15, 0x0a, 0x05, + 0x74, 0x65, 0x73, 0x74, 0x33, 0x12, 0x0c, 0x1a, 0x0a, 0x74, 0x65, 0x73, + 0x74, 0x5f, 0x76, 0x61, 0x6c, 0x75, 0x65}; +} // anonymous namespace class MockCDM : public WvContentDecryptionModule { public: @@ -95,6 +104,8 @@ class MockCDM : public WvContentDecryptionModule { CdmResponseType(const CdmUsageInfoReleaseMessage&)); MOCK_METHOD1(IsValidServiceCertificate, bool(const std::string&)); + + MOCK_METHOD1(GetSerializedMetrics, void(std::string*)); }; class MockCrypto : public WVGenericCryptoInterface { @@ -833,6 +844,8 @@ TEST_F(WVDrmPluginTest, ReturnsExpectedPropertyValues) { static const string openSessions = "15"; static const string maxSessions = "18"; static const string oemCryptoApiVersion = "10"; + string serializedMetrics(kSerializedMetrics, + kSerializedMetrics + sizeof(kSerializedMetrics)); EXPECT_CALL(*cdm, QueryStatus(_, QUERY_KEY_SECURITY_LEVEL, _)) .WillOnce(DoAll(SetArgPointee<2>(QUERY_VALUE_SECURITY_LEVEL_L1), @@ -864,6 +877,9 @@ TEST_F(WVDrmPluginTest, ReturnsExpectedPropertyValues) { .WillOnce(DoAll(SetArgPointee<2>(oemCryptoApiVersion), Return(wvcdm::NO_ERROR))); + EXPECT_CALL(*cdm, GetSerializedMetrics(_)) + .WillOnce(SetArgPointee<0>(serializedMetrics)); + String8 stringResult; Vector vectorResult; @@ -915,6 +931,12 @@ TEST_F(WVDrmPluginTest, ReturnsExpectedPropertyValues) { res = plugin.getPropertyString(String8("oemCryptoApiVersion"), stringResult); ASSERT_EQ(OK, res); EXPECT_EQ(oemCryptoApiVersion, stringResult.string()); + + vectorResult.clear(); + res = plugin.getPropertyByteArray(String8("metrics"), vectorResult); + ASSERT_EQ(OK, res); + EXPECT_THAT(vectorResult, ElementsAreArray(serializedMetrics.data(), + serializedMetrics.size())); } TEST_F(WVDrmPluginTest, DoesNotGetUnknownProperties) {