Merge "Fixes reporting of dynamic adapter metrics." into oc-mr1-dev

libwvdrmengine/cdm/core/src/oemcrypto_adapter_dynamic.cpp

@@ -434,7 +434,7 @@ class WatchDog {
   }
 
   // Check to see if the failure file was created before that last abort.
-  void CheckForPreviousFailure(wvcdm::metrics::CryptoMetrics* metrics) {
+  void CheckForPreviousFailure(wvcdm::metrics::OemCryptoDynamicAdapterMetrics* metrics) {
     wvcdm::FileSystem file_system;
     std::string filename = FailureFilename();
     if (!file_system.Exists(filename)) return;
@@ -448,7 +448,7 @@ class WatchDog {
       if (size == size_read && flag) {
         LOGE("Previous L3 Init failed.");
         if (metrics == nullptr) return;
-        metrics->oemcrypto_initialization_mode_.Record(
+        metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
             wvcdm::metrics::OEMCrypto_INITIALIZED_L3_INITIALIZATION_FAILED);
       }
     }
@@ -574,15 +574,12 @@ class Adapter {
   OEMCryptoResult Initialize() {
 
     /*
-     * To avoid changing the function signature and function contract - declare
+     * To avoid changing the function signature and function contract, use a
-     * a one-off metrics group to collect detailed information about how
+     * reference to a singleton object for the metrics collected from the
-     * oemcrypto was intialized.
+     * dynamic adapter.
-     *
-     * TODO(blueeyes): Refactor this to allow Initialize to provide the
-     * details to the caller or to use the metrics instance provided by
-     * the caller.
      */
-    wvcdm::metrics::CryptoMetrics metrics;
+    wvcdm::metrics::OemCryptoDynamicAdapterMetrics& metrics =
+        wvcdm::metrics::GetDynamicAdapterMetricsInstance();
 
     level1_ = FunctionPointers();  // start with all null pointers.
     level3_ = FunctionPointers();  // start with all null pointers.
@@ -592,20 +589,20 @@ class Adapter {
     watcher->StartThread();
     OEMCryptoResult result = watcher->WaitForStatusAndCleanUp();
     if (Level3_IsInApp()) {
-      metrics.oemcrypto_initialization_mode_
+      metrics.SetInitializationMode(
-          .Record(wvcdm::metrics::OEMCrypto_INITIALIZED_USING_IN_APP);
+          wvcdm::metrics::OEMCrypto_INITIALIZED_USING_IN_APP);
       return result;
     }
     if (force_level3()) {
       LOGW("Test code. User requested falling back to L3");
-      metrics.oemcrypto_initialization_mode_
+      metrics.SetInitializationMode(
-          .Record(wvcdm::metrics::OEMCrypto_INITIALIZED_FORCING_L3);
+          wvcdm::metrics::OEMCrypto_INITIALIZED_FORCING_L3);
       return result;
     }
     std::string library_name;
     if (!wvcdm::Properties::GetOEMCryptoPath(&library_name)) {
       LOGW("L1 library not specified. Falling back to L3");
-      metrics.oemcrypto_initialization_mode_.Record(
+      metrics.OemCryptoDynamicAdapterMetrics::SetInitializationMode(
           wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_NO_L1_LIBRARY_PATH);
       return result;
     }
@@ -613,7 +610,7 @@ class Adapter {
     if (level1_library_ == NULL) {
       LOGW("Could not load %s. Falling back to L3.  %s", library_name.c_str(),
            dlerror());
-      metrics.oemcrypto_initialization_mode_.Record(
+      metrics.OemCryptoDynamicAdapterMetrics::SetInitializationMode(
           wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_L1_OPEN_FAILED);
       return result;
     }
@@ -628,7 +625,7 @@ class Adapter {
     return result;
   }
 
-  bool LoadLevel1(wvcdm::metrics::CryptoMetrics* metrics) {
+  bool LoadLevel1(wvcdm::metrics::OemCryptoDynamicAdapterMetrics* metrics) {
     if (metrics == nullptr) {
       return false;
     }
@@ -640,25 +637,25 @@ class Adapter {
     LOOKUP_ALL(8, APIVersion, OEMCrypto_APIVersion);
     LOOKUP_ALL(8, Terminate,  OEMCrypto_Terminate);
     if (!level1_valid_) {
-      metrics->oemcrypto_initialization_mode_.Record(
+      metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
           wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_INVALID_L1);
       return false;
     }
     OEMCryptoResult st = level1_.Initialize();
     if (st != OEMCrypto_SUCCESS) {
       LOGW("Could not initialize L1. Falling Back to L3.");
-      metrics->oemcrypto_initialization_mode_.Record(
+      metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
           wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_COULD_NOT_INITIALIZE_L1);
       return false;
     }
     level1_.version = level1_.APIVersion();
-    metrics->oemcrypto_l1_api_version_.Record(level1_.version);
+    metrics->SetL1ApiVersion(level1_.version);
-    metrics->oemcrypto_l1_min_api_version_.Record(kMinimumVersion);
+    metrics->SetL1MinApiVersion(kMinimumVersion);
 
     if (level1_.version < kMinimumVersion) {
       LOGW("liboemcrypto.so is version %d, not %d. Falling Back to L3.",
            level1_.version, kMinimumVersion);
-      metrics->oemcrypto_initialization_mode_.Record(
+      metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
           wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_WRONG_L1_VERSION);
       level1_.Terminate();
       return false;
@@ -733,7 +730,7 @@ class Adapter {
 
     // If we have a valid keybox, initialization is done. We're good.
     if (OEMCrypto_SUCCESS == level1_.IsKeyboxValid()) {
-      metrics->oemcrypto_initialization_mode_.Record(
+      metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
           wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L1_WITH_KEYBOX);
       return true;
     }
@@ -742,7 +739,7 @@ class Adapter {
     // will have to be caught in the future when provisioning fails.
     if (level1_.version > 11 &&
         (level1_.GetProvisioningMethod() == OEMCrypto_OEMCertificate)) {
-      metrics->oemcrypto_initialization_mode_.Record(
+      metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
           wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L1_WITH_PROVISIONING_3_0);
       return true;
     }
@@ -759,10 +756,10 @@ class Adapter {
         // because we still want to test OEMCrypto in that configuration.
         LOGE("OEMCrypto uses cert as identification, but cdm does not!");
         LOGE("This will not work on a production device.");
-        metrics->oemcrypto_initialization_mode_.Record(
+        metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
             wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L1_CERTIFICATE_MIX);
       } else {
-        metrics->oemcrypto_initialization_mode_.Record(
+        metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
             wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L1_WITH_CERTIFICATE);
       }
       return true;
@@ -772,7 +769,7 @@ class Adapter {
     if (!wvcdm::Properties::GetFactoryKeyboxPath(&filename)) {
       LOGW("Bad Level 1 Keybox. Falling Back to L3.");
       level1_.Terminate();
-      metrics->oemcrypto_initialization_mode_.Record(
+      metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
           wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_BAD_KEYBOX);
       return false;
     }
@@ -781,7 +778,7 @@ class Adapter {
     if (size <= 0 || !file) {
       LOGW("Could not open %s. Falling Back to L3.", filename.c_str());
       level1_.Terminate();
-      metrics->oemcrypto_initialization_mode_.Record(
+      metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
           wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_COULD_NOT_OPEN_FACTORY_KEYBOX);
       return false;
     }
@@ -792,12 +789,12 @@ class Adapter {
       LOGE("Could NOT install keybox from %s. Falling Back to L3.",
            filename.c_str());
       level1_.Terminate();
-      metrics->oemcrypto_initialization_mode_.Record(
+      metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
           wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L3_COULD_NOT_INSTALL_KEYBOX);
       return false;
     }
     LOGI("Installed keybox from %s", filename.c_str());
-    metrics->oemcrypto_initialization_mode_.Record(
+    metrics->OemCryptoDynamicAdapterMetrics::SetInitializationMode(
         wvcdm::metrics::OEMCrypto_INITIALIZED_USING_L1_INSTALLED_KEYBOX);
     return true;
   }

libwvdrmengine/cdm/metrics/include/metrics_collections.h

@@ -146,11 +146,6 @@ class CryptoMetrics {
   ValueMetric<bool> oemcrypto_supports_usage_table_;
   CounterMetric<OEMCryptoResult> oemcrypto_update_usage_table_;
   EventMetric<OEMCryptoResult> oemcrypto_wrap_keybox_;
-
-  /* Internal OEMCrypto Metrics */
-  ValueMetric<OEMCryptoInitializationMode> oemcrypto_initialization_mode_;
-  ValueMetric<uint32_t> oemcrypto_l1_api_version_;
-  ValueMetric<uint32_t> oemcrypto_l1_min_api_version_;
 };
 
 // This class contains session-scoped metrics. All properties and
@@ -196,6 +191,45 @@ class SessionMetrics {
   CryptoMetrics crypto_metrics_;
 };
 
+// This class contains metrics for the OEMCrypto Dynamic Adapter. They are
+// separated from other metrics because they need to be encapsulated in a
+// singleton object. This is because the dynamic adapter uses the OEMCrypto
+// function signatures and contract and cannot be extended to inject
+// dependencies.
+//
+// Operations for this metrics class are serialized since these particular
+// metrics may be accessed by a separate thread during intialize even as
+// the metric may be serialized.
+class OemCryptoDynamicAdapterMetrics {
+ public:
+  explicit OemCryptoDynamicAdapterMetrics();
+
+  // Set methods for OEMCrypto metrics.
+  void SetInitializationMode(OEMCryptoInitializationMode mode);
+  void SetL1ApiVersion(uint32_t version);
+  void SetL1MinApiVersion(uint32_t version);
+
+  // Serialize the session metrics to the provided |metric_group|.
+  // |metric_group| is owned by the caller and must not be null.
+  void Serialize(drm_metrics::MetricsGroup* metric_group);
+
+  // Clears the existing metric values.
+  void Clear();
+
+ private:
+  Lock adapter_lock_;
+  ValueMetric<OEMCryptoInitializationMode> oemcrypto_initialization_mode_;
+  ValueMetric<uint32_t> oemcrypto_l1_api_version_;
+  ValueMetric<uint32_t> oemcrypto_l1_min_api_version_;
+};
+
+// This will fetch the singleton instance for dynamic adapter metrics.
+// This method is safe only if we use C++ 11. In C++ 11, static function-local
+// initialization is guaranteed to be threadsafe. We return the reference to
+// avoid non-guaranteed destructor order problems. Effectively, the destructor
+// is never run for the created instance.
+OemCryptoDynamicAdapterMetrics& GetDynamicAdapterMetricsInstance();
+
 // This class contains engine-scoped metrics. All properties and
 // statistics related to operations within the engine, but outside
 // the scope of a session are recorded here.

libwvdrmengine/cdm/metrics/include/value_metric.h

@@ -86,6 +86,12 @@ class ValueMetric : public MetricSerializable {
   // Get the current value of the metric.
   const T& GetValue() { return value_; }
 
+  // Clears the indicators that the metric or error was set.
+  void Clear() {
+    has_value_ = false;
+    has_error_ = false;
+  }
+
  private:
   std::string metric_name_;
   T value_;

libwvdrmengine/cdm/metrics/src/metrics_collections.cpp

@@ -234,14 +234,8 @@ CryptoMetrics::CryptoMetrics() :
         "oemcrypto_error"),
     oemcrypto_wrap_keybox_(
         "/drm/widevine/oemcrypto/wrap_keybox/time",
-        "oemcrypto_error"),
+        "oemcrypto_error") {
-    oemcrypto_initialization_mode_(
+}
-        "/drm/widevine/oemcrypto/initialization_mode"),
-    oemcrypto_l1_api_version_(
-        "/drm/widevine/oemcrypto/l1_api_version"),
-    oemcrypto_l1_min_api_version_(
-        "/drm/widevine/oemcrypto/l1_min_api_version")
-      {}
 
 void CryptoMetrics::Serialize(MetricsGroup* metrics) {
   ProtoMetricSerializer serializer(metrics);
@@ -304,11 +298,6 @@ void CryptoMetrics::Serialize(MetricsGroup* metrics) {
   oemcrypto_supports_usage_table_.Serialize(&serializer);
   oemcrypto_update_usage_table_.Serialize(&serializer);
   oemcrypto_wrap_keybox_.Serialize(&serializer);
-
-  /* Internal OEMCrypto Metrics */
-  oemcrypto_initialization_mode_.Serialize(&serializer);
-  oemcrypto_l1_api_version_.Serialize(&serializer);
-  oemcrypto_l1_min_api_version_.Serialize(&serializer);
 }
 
 SessionMetrics::SessionMetrics() :
@@ -341,6 +330,60 @@ void SessionMetrics::SerializeSessionMetrics(MetricsGroup* metric_group) {
   cdm_session_restore_usage_session_.Serialize(&serializer);
 }
 
+
+OemCryptoDynamicAdapterMetrics::OemCryptoDynamicAdapterMetrics() :
+    oemcrypto_initialization_mode_(
+        "/drm/widevine/oemcrypto/initialization_mode"),
+    oemcrypto_l1_api_version_(
+        "/drm/widevine/oemcrypto/l1_api_version"),
+    oemcrypto_l1_min_api_version_(
+        "/drm/widevine/oemcrypto/l1_min_api_version") {
+}
+
+void OemCryptoDynamicAdapterMetrics::SetInitializationMode(
+    OEMCryptoInitializationMode mode) {
+  AutoLock lock(adapter_lock_);
+  oemcrypto_initialization_mode_.Record(mode);
+}
+
+void OemCryptoDynamicAdapterMetrics::SetL1ApiVersion(uint32_t version) {
+  AutoLock lock(adapter_lock_);
+  oemcrypto_l1_api_version_.Record(version);
+}
+
+void OemCryptoDynamicAdapterMetrics::SetL1MinApiVersion(uint32_t version) {
+  AutoLock lock(adapter_lock_);
+  oemcrypto_l1_min_api_version_.Record(version);
+}
+
+void OemCryptoDynamicAdapterMetrics::Serialize(
+    drm_metrics::MetricsGroup* metric_group) {
+  AutoLock lock(adapter_lock_);
+  ProtoMetricSerializer serializer(metric_group);
+
+  oemcrypto_initialization_mode_.Serialize(&serializer);
+  oemcrypto_l1_api_version_.Serialize(&serializer);
+  oemcrypto_l1_min_api_version_.Serialize(&serializer);
+}
+
+void OemCryptoDynamicAdapterMetrics::Clear() {
+  AutoLock lock(adapter_lock_);
+
+  oemcrypto_initialization_mode_.Clear();
+  oemcrypto_l1_api_version_.Clear();
+  oemcrypto_l1_min_api_version_.Clear();
+}
+
+// This method returns a reference. This means that the destructor is never
+// executed for the returned object.
+OemCryptoDynamicAdapterMetrics& GetDynamicAdapterMetricsInstance() {
+  // This is safe in C++ 11 since the initialization is guaranteed to run
+  // only once regardless of multi-threaded access.
+  static OemCryptoDynamicAdapterMetrics* adapter_metrics =
+      new OemCryptoDynamicAdapterMetrics();
+  return *adapter_metrics;
+}
+
 EngineMetrics::EngineMetrics() :
     cdm_engine_add_key_(
         "/drm/widevine/cdm_engine/add_key/time",
@@ -401,7 +444,7 @@ EngineMetrics::EngineMetrics() :
 }
 
 EngineMetrics::~EngineMetrics() {
-  AutoLock kock(session_metrics_lock_);
+  AutoLock lock(session_metrics_lock_);
   std::vector<SessionMetrics*>::iterator i;
   if (!session_metrics_list_.empty()) {
     LOGV("EngineMetrics::~EngineMetrics. Session count: %d",
@@ -435,6 +478,11 @@ void EngineMetrics::Serialize(drm_metrics::MetricsGroup* metric_group,
                               bool clear_serialized_sessions) {
   AutoLock lock(session_metrics_lock_);
 
+  // Serialize the most recent metrics from the OemCyrpto dynamic adapter.
+  OemCryptoDynamicAdapterMetrics& adapter_metrics =
+      GetDynamicAdapterMetricsInstance();
+  adapter_metrics.Serialize(metric_group);
+
   SerializeEngineMetrics(metric_group);
   std::vector<SessionMetrics*>::iterator i;
   for (i = session_metrics_list_.begin(); i != session_metrics_list_.end();

libwvdrmengine/cdm/test/wv_cdm_metrics_test.cpp

@@ -53,15 +53,17 @@ TEST_F(WvContentDecryptionModuleMetricsTest, EngineOnlyMetrics) {
   ASSERT_TRUE(metrics.ParseFromString(serialized_metrics));
   EXPECT_THAT(metrics.metric_size(), Eq(0));
   ASSERT_THAT(metrics.metric_sub_group_size(), Eq(1));
-  ASSERT_THAT(metrics.metric_sub_group(0).metric_size(), Ge(3));
+  ASSERT_THAT(metrics.metric_sub_group(0).metric_size(), Ge(6));
   EXPECT_THAT(metrics.metric_sub_group(0).metric_sub_group_size(), Eq(0));
   EXPECT_THAT(metrics.metric_sub_group(0).metric(0).name(),
-              StrEq("/drm/widevine/cdm_engine/version"));
+              StrEq("/drm/widevine/oemcrypto/initialization_mode"));
+  // Can't check the initialization mode value. Different devices will have
+  // different values.
   EXPECT_THAT(
-      metrics.metric_sub_group(0).metric(2).name(),
+      metrics.metric_sub_group(0).metric(5).name(),
       StrEq("/drm/widevine/cdm_engine/"
             "get_provisioning_request/time/count{error:0}"));
-  EXPECT_THAT(metrics.metric_sub_group(0).metric(2).value().int_value(), Eq(1));
+  EXPECT_THAT(metrics.metric_sub_group(0).metric(5).value().int_value(), Eq(1));
 }
 
 
@@ -85,14 +87,14 @@ TEST_F(WvContentDecryptionModuleMetricsTest, EngineAndSessionMetrics) {
   // The outer container will never have metrics.
   EXPECT_THAT(metrics.metric_size(), Eq(0));
   ASSERT_THAT(metrics.metric_sub_group_size(), Eq(1));
-  ASSERT_THAT(metrics.metric_sub_group(0).metric_size(), Ge(3));
+  ASSERT_THAT(metrics.metric_sub_group(0).metric_size(), Ge(6));
 
   // Validate engine-level metrics.
   EXPECT_THAT(metrics.metric_sub_group(0).metric(0).name(),
-              StrEq("/drm/widevine/cdm_engine/version"));
+              StrEq("/drm/widevine/oemcrypto/initialization_mode"));
-  EXPECT_THAT(metrics.metric_sub_group(0).metric(2).name(),
+  EXPECT_THAT(metrics.metric_sub_group(0).metric(5).name(),
               StrEq("/drm/widevine/cdm_engine/open_session/count{error:7}"));
-  EXPECT_THAT(metrics.metric_sub_group(0).metric(2).value().int_value(), Eq(1));
+  EXPECT_THAT(metrics.metric_sub_group(0).metric(5).value().int_value(), Eq(1));
 
   // Validate a session-level metric.
   EXPECT_THAT(metrics.metric_sub_group(0).metric_sub_group_size(), Eq(1));
@@ -131,12 +133,12 @@ TEST_F(WvContentDecryptionModuleMetricsTest, MultipleEngineMetric) {
 
   for (int i = 0; i < metrics.metric_sub_group_size(); i++) {
     // Validate the engine-level metric.
-    ASSERT_THAT(metrics.metric_sub_group(i).metric_size(), Ge(3));
+    ASSERT_THAT(metrics.metric_sub_group(i).metric_size(), Ge(6));
     EXPECT_THAT(metrics.metric_sub_group(i).metric(0).name(),
-                StrEq("/drm/widevine/cdm_engine/version"));
+                StrEq("/drm/widevine/oemcrypto/initialization_mode"));
-    EXPECT_THAT(metrics.metric_sub_group(i).metric(2).name(),
+    EXPECT_THAT(metrics.metric_sub_group(i).metric(5).name(),
                 StrEq("/drm/widevine/cdm_engine/open_session/count{error:7}"));
-    EXPECT_THAT(metrics.metric_sub_group(i).metric(2).value().int_value(), Eq(1));
+    EXPECT_THAT(metrics.metric_sub_group(i).metric(5).value().int_value(), Eq(1));
 
     // Validate a session-level metric.
     EXPECT_THAT(metrics.metric_sub_group(i).metric_sub_group_size(), Eq(1));