Address CE CDM test failures and code review comments

The android CL ag/13947818 was submitted before some CE CDM test
failures were noticed and code review comments were received.

Bug: 184813991
Test: WV unit/integration test
Change-Id: Ic31ca5bc5e46994e01eca56248e6bdffedd779f3

libwvdrmengine/cdm/core/include/cdm_session.h

@@ -230,7 +230,8 @@ class CdmSession {
       const std::string& certificate,
       const CryptoWrappedKey& wrapped_private_key);
 
-  CdmResponseType LoadPrivateKey(const CryptoWrappedKey& wrapped_private_key);
+  CdmResponseType LoadPrivateKey(const std::string& certificate,
+                                 const CryptoWrappedKey& wrapped_private_key);
 
   bool GenerateKeySetId(bool atsc_mode_enabled, CdmKeySetId* key_set_id);
 

libwvdrmengine/cdm/core/src/cdm_session.cpp

@@ -172,6 +172,9 @@ CdmResponseType CdmSession::Init(CdmClientPropertySet* cdm_client_property_set,
   if (cdm_client_property_set != nullptr)
     atsc_mode_enabled_ = cdm_client_property_set->use_atsc_mode();
 
+  // If a DRM certificate does not exist, indicate that provisioning is needed.
+  // The actual validation and loading of a certificate will happen when
+  // a key request is generated or an offline license is loaded.
   if (!file_handle_->HasCertificate(atsc_mode_enabled_))
     return NEED_PROVISIONING;
 
@@ -1251,31 +1254,14 @@ CdmResponseType CdmSession::LoadPrivateKey() {
     return NEED_PROVISIONING;
   }
 
-  CdmResponseType status = LoadPrivateKey(private_key);
+  return LoadPrivateKey(drm_certificate, private_key);
-
-  if (status == NO_ERROR) {
-    drm_certificate_ = drm_certificate;
-    wrapped_private_key_.set_type(private_key.type());
-    wrapped_private_key_.set_key(private_key.key());
-  }
-
-  return status;
 }
 
 CdmResponseType CdmSession::LoadPrivateOrLegacyKey(
     const std::string& certificate, const CryptoWrappedKey& private_key) {
   // Use provided key if valid
-  if (!certificate.empty() && private_key.IsValid()) {
+  if (!certificate.empty() && private_key.IsValid())
-    CdmResponseType status = LoadPrivateKey(private_key);
+    return LoadPrivateKey(certificate, private_key);
-
-    if (status == NO_ERROR) {
-      drm_certificate_ = certificate;
-      wrapped_private_key_.set_type(private_key.type());
-      wrapped_private_key_.set_key(private_key.key());
-    }
-
-    return status;
-  }
 
   // Otherwise use key from legacy certificate
   std::string drm_certificate;
@@ -1285,19 +1271,11 @@ CdmResponseType CdmSession::LoadPrivateOrLegacyKey(
       DeviceFiles::kCertificateValid)
     return NEED_PROVISIONING;
 
-  CdmResponseType status = LoadPrivateKey(wrapped_private_key);
+  return LoadPrivateKey(drm_certificate, wrapped_private_key);
-
-  if (status == NO_ERROR) {
-    drm_certificate_ = drm_certificate;
-    wrapped_private_key_.set_type(wrapped_private_key.type());
-    wrapped_private_key_.set_key(wrapped_private_key.key());
-  }
-
-  return status;
 }
 
 CdmResponseType CdmSession::LoadPrivateKey(
-    const CryptoWrappedKey& private_key) {
+    const std::string& drm_certificate, const CryptoWrappedKey& private_key) {
   CdmResponseType load_cert_sts;
   M_TIME(
       load_cert_sts = crypto_session_->LoadCertificatePrivateKey(private_key),
@@ -1308,6 +1286,9 @@ CdmResponseType CdmSession::LoadPrivateKey(
     case NO_ERROR:
       metrics_->drm_certificate_key_type_.Record(
           DrmKeyTypeToMetricValue(private_key.type()));
+
+      drm_certificate_ = drm_certificate;
+      wrapped_private_key_ = std::move(private_key);
       return NO_ERROR;
     case SESSION_LOST_STATE_ERROR:
     case SYSTEM_INVALIDATED_ERROR:

libwvdrmengine/cdm/core/src/device_files.cpp

@@ -127,8 +127,8 @@ bool ExtractFromDeviceCertificate(const DeviceCertificate& device_certificate,
   RETURN_FALSE_IF_NULL(certificate);
   RETURN_FALSE_IF_NULL(private_key);
 
-  bool has_certificate = device_certificate.has_certificate();
+  const bool has_certificate = device_certificate.has_certificate();
-  bool has_key = device_certificate.has_wrapped_private_key();
+  const bool has_key = device_certificate.has_wrapped_private_key();
 
   // If no certificate information, nothing to be done. DeviceCertificate
   // is a legacy DRM certificate

libwvdrmengine/cdm/core/test/test_base.cpp

@@ -397,16 +397,36 @@ void WvCdmTestBase::Provision() {
 void WvCdmTestBase::EnsureProvisioned() {
   CdmSessionId session_id;
   FileSystem file_system;
+  // OpenSession will check if a DRM certificate exists, while
+  // GenerateKeyRequest will actually load the wrapped private key.
+  // Either may return a NEED_PROVISIONING error, so both have to be checked.
   TestCdmEngine cdm_engine(&file_system,
                            std::shared_ptr<EngineMetrics>(new EngineMetrics));
   CdmResponseType status = cdm_engine.OpenSession(config_.key_system(), nullptr,
                                                   nullptr, &session_id);
+
+  CdmAppParameterMap app_parameters;
+  CdmKeySetId key_set_id;
+  InitializationData init_data(ISO_BMFF_VIDEO_MIME_TYPE, binary_key_id());
+  CdmKeyRequest key_request;
+  if (status == NO_ERROR) {
+    status = cdm_engine.GenerateKeyRequest(session_id, key_set_id, init_data,
+                                           kLicenseTypeStreaming,
+                                           app_parameters, &key_request);
+  }
+
   if (status == NEED_PROVISIONING) {
     Provision();
     status = cdm_engine.OpenSession(config_.key_system(), nullptr, nullptr,
                                     &session_id);
+
+    ASSERT_EQ(NO_ERROR, status);
+    status = cdm_engine.GenerateKeyRequest(session_id, key_set_id, init_data,
+                                           kLicenseTypeStreaming,
+                                           app_parameters, &key_request);
+    ASSERT_EQ(KEY_MESSAGE, status);
   }
-  ASSERT_EQ(NO_ERROR, status);
+  ASSERT_EQ(KEY_MESSAGE, status);
   ASSERT_NE("", session_id) << "Could not open CDM session.";
   ASSERT_TRUE(cdm_engine.IsOpenSession(session_id));
   ASSERT_EQ(NO_ERROR, cdm_engine.CloseSession(session_id));

libwvdrmengine/cdm/test/request_license_test.cpp

@@ -53,7 +53,6 @@ namespace {
 // HTTP response codes.
 const int kHttpOk = 200;
 const int kDrmCertificateExpiryPeriod = 150;
-const std::string kCencMimeType = "video/mp4";
 
 const wvcdm::CdmIdentifier kExampleIdentifier = {
     wvcdm::EMPTY_SPOID, "com.example", "com.example", 7, 9};
@@ -1752,9 +1751,9 @@ class WvCdmRequestLicenseTest : public WvCdmTestBase {
 
   CdmResponseType GenerateKeyRequest() {
     CdmAppParameterMap app_parameters;
-    return GenerateKeyRequest(kCencMimeType, binary_key_id(), app_parameters,
+    return GenerateKeyRequest(ISO_BMFF_VIDEO_MIME_TYPE, binary_key_id(),
-                              kLicenseTypeStreaming, kDefaultCdmIdentifier,
+                              app_parameters, kLicenseTypeStreaming,
-                              nullptr);
+                              kDefaultCdmIdentifier, nullptr);
   }
 
   void GenerateKeyRequest(const std::string& init_data,
@@ -2063,9 +2062,9 @@ class WvCdmRequestLicenseTest : public WvCdmTestBase {
 
     if (status == NO_ERROR) {
       wvcdm::CdmAppParameterMap app_parameters;
-      status =
+      status = GenerateKeyRequest(ISO_BMFF_VIDEO_MIME_TYPE, binary_key_id(),
-          GenerateKeyRequest(kCencMimeType, binary_key_id(), app_parameters,
+                                  app_parameters, kLicenseTypeStreaming,
-                             kLicenseTypeStreaming, identifier, nullptr);
+                                  identifier, nullptr);
     }
 
     decryptor_->CloseSession(session_id_);
@@ -2098,12 +2097,12 @@ class WvCdmRequestLicenseTest : public WvCdmTestBase {
     if (IsProvisioned(identifier, requested_security_level)) return;
 
     std::string provisioning_server;
-    CdmCertificateType cert_type = kCertificateWidevine;
     std::string cert_authority, cert, wrapped_key;
 
-    CdmResponseType status = decryptor_->GetProvisioningRequest(
+    const CdmResponseType status = decryptor_->GetProvisioningRequest(
-        cert_type, cert_authority, identifier, kEmptyServiceCertificate,
+        kCertificateWidevine, cert_authority, identifier,
-        requested_security_level, &key_msg_, &provisioning_server);
+        kEmptyServiceCertificate, requested_security_level, &key_msg_,
+        &provisioning_server);
     EXPECT_EQ(wvcdm::NO_ERROR, status);
     if (NO_ERROR != status) return;
     EXPECT_EQ(provisioning_server, kDefaultProvisioningServerUrl);
@@ -2299,9 +2298,9 @@ TEST_F(WvCdmRequestLicenseTest, PerOriginProvisioningTest) {
 
   CdmAppParameterMap app_parameters;
   EXPECT_EQ(KEY_MESSAGE,
-            GenerateKeyRequest(kCencMimeType, binary_key_id(), app_parameters,
+            GenerateKeyRequest(ISO_BMFF_VIDEO_MIME_TYPE, binary_key_id(),
-                               kLicenseTypeStreaming, kDefaultCdmIdentifier,
+                               app_parameters, kLicenseTypeStreaming,
-                               nullptr));
+                               kDefaultCdmIdentifier, nullptr));
   decryptor_->CloseSession(session_id_);
 
   // The other identifier should not be provisioned.
@@ -2881,7 +2880,7 @@ TEST_F(WvCdmRequestLicenseTest, StreamingWithExpiringCertTest) {
                                     kExampleIdentifier, nullptr, &session_id_));
 
   CdmAppParameterMap app_parameters;
-  GenerateKeyRequest(KEY_MESSAGE, kCencMimeType, binary_key_id(),
+  GenerateKeyRequest(KEY_MESSAGE, ISO_BMFF_VIDEO_MIME_TYPE, binary_key_id(),
                      app_parameters, kLicenseTypeStreaming, kExampleIdentifier,
                      nullptr);
   VerifyKeyRequestResponse(config_.license_server(), config_.client_auth());
@@ -2968,8 +2967,9 @@ TEST_F(WvCdmRequestLicenseTest, RestoreOfflineKeysWithExpiringCertTest) {
                                     kExampleIdentifier, nullptr, &session_id_));
 
   CdmAppParameterMap app_parameters;
-  GenerateKeyRequest(KEY_MESSAGE, kCencMimeType, key_id, app_parameters,
+  GenerateKeyRequest(KEY_MESSAGE, ISO_BMFF_VIDEO_MIME_TYPE, key_id,
-                     kLicenseTypeOffline, kExampleIdentifier, nullptr);
+                     app_parameters, kLicenseTypeOffline, kExampleIdentifier,
+                     nullptr);
   VerifyKeyRequestResponse(config_.license_server(), client_auth);
 
   CdmKeySetId key_set_id = key_set_id_;