EME: Address TODOs related to the removal of prefixed EME

BUG=249976

Review-Url: https://codereview.chromium.org/1920433003
Cr-Commit-Position: refs/heads/master@{#412563}

chrome/browser/chromeos/attestation/platform_verification_flow.cc

@@ -200,8 +200,14 @@ void PlatformVerificationFlow::ChallengePlatformKey(
     return;
   }
 
-  // Note: The following two checks are also checked in GetPermissionStatus.
-  // Checking them here explicitly to report the correct error type.
+  // Note: The following checks are performed when use of the protected media
+  // identifier is indicated. The first two in GetPermissionStatus and the third
+  // in DecidePermission.
+  // In Chrome, the result of the first and third could have changed in the
+  // interim, but the mode cannot change.
+  // TODO(ddorwin): Share more code for the first two checks with
+  // ProtectedMediaIdentifierPermissionContext::
+  // IsProtectedMediaIdentifierEnabled().
 
   if (!IsAttestationAllowedByPolicy()) {
     VLOG(1) << "Platform verification not allowed by device policy.";
@@ -209,11 +215,8 @@ void PlatformVerificationFlow::ChallengePlatformKey(
     return;
   }
 
-  // TODO(xhwang): Change to DCHECK when prefixed EME support is removed.
-  // See http://crbug.com/249976
   if (!delegate_->IsInSupportedMode(web_contents)) {
-    VLOG(1) << "Platform verification denied because it's not supported in the "
-            << "current mode.";
+    LOG(ERROR) << "Platform verification not supported in the current mode.";
     ReportError(callback, PLATFORM_NOT_VERIFIED);
     return;
   }

content/renderer/media/cdm/render_cdm_factory.cc

@@ -59,11 +59,8 @@ void RenderCdmFactory::Create(
   }
 
   if (media::CanUseAesDecryptor(key_system)) {
-    // TODO(sandersd): Address this now that prefixed EME has been removed.
-    // http://crbug.com/249976. The prefixed API always allowed distinctive
-    // identifiers and persistent state. Once that changes we can sanity check
-    // here that neither is allowed for AesDecryptor, since it does not support
-    // them and should never be configured that way. http://crbug.com/455271
+    DCHECK(!cdm_config.allow_distinctive_identifier);
+    DCHECK(!cdm_config.allow_persistent_state);
     scoped_refptr<media::MediaKeys> cdm(
         new media::AesDecryptor(security_origin, session_message_cb,
                                 session_closed_cb, session_keys_change_cb));

media/base/android/java/src/org/chromium/media/MediaDrmBridge.java

@@ -752,9 +752,8 @@ public class MediaDrmBridge {
             return;
         }
 
-        // TODO(xhwang): DCHECK this now that prefixed EME is deprecated.
-        // https://crbug.com/249976
         if (!sessionExists(sessionId)) {
+            assert false; // Should never happen.
             onPromiseRejected(
                     promiseId, "Invalid session in updateSession: " + bytesToHexString(sessionId));
             return;

media/cdm/aes_decryptor.cc

@@ -20,6 +20,7 @@
 #include "media/base/cdm_promise.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/decrypt_config.h"
+#include "media/base/limits.h"
 #include "media/base/video_decoder_config.h"
 #include "media/base/video_frame.h"
 #include "media/cdm/json_web_key.h"
@@ -268,47 +269,46 @@ void AesDecryptor::CreateSessionAndGenerateRequest(
   // TODO(jrummell): Validate |session_type|.
 
   std::vector<uint8_t> message;
-  // TODO(jrummell): Since unprefixed will never send NULL, remove this check
-  // when prefixed EME is removed (http://crbug.com/249976).
-  if (!init_data.empty()) {
-    std::vector<std::vector<uint8_t>> keys;
-    switch (init_data_type) {
-      case EmeInitDataType::WEBM:
-        // |init_data| is simply the key needed.
-        keys.push_back(init_data);
-        break;
-      case EmeInitDataType::CENC:
+  std::vector<std::vector<uint8_t>> keys;
+  switch (init_data_type) {
+    case EmeInitDataType::WEBM:
+      // |init_data| is simply the key needed.
+      if (init_data.size() < limits::kMinKeyIdLength ||
+          init_data.size() > limits::kMaxKeyIdLength) {
+        promise->reject(NOT_SUPPORTED_ERROR, 0, "Incorrect length");
+        return;
+      }
+      keys.push_back(init_data);
+      break;
+    case EmeInitDataType::CENC:
 #if defined(USE_PROPRIETARY_CODECS)
-        // |init_data| is a set of 0 or more concatenated 'pssh' boxes.
-        if (!GetKeyIdsForCommonSystemId(init_data, &keys)) {
-          promise->reject(NOT_SUPPORTED_ERROR, 0,
-                          "No supported PSSH box found.");
-          return;
-        }
-        break;
-#else
-        promise->reject(NOT_SUPPORTED_ERROR, 0,
-                        "Initialization data type CENC is not supported.");
+      // |init_data| is a set of 0 or more concatenated 'pssh' boxes.
+      if (!GetKeyIdsForCommonSystemId(init_data, &keys)) {
+        promise->reject(NOT_SUPPORTED_ERROR, 0, "No supported PSSH box found.");
         return;
-#endif
-      case EmeInitDataType::KEYIDS: {
-        std::string init_data_string(init_data.begin(), init_data.end());
-        std::string error_message;
-        if (!ExtractKeyIdsFromKeyIdsInitData(init_data_string, &keys,
-                                             &error_message)) {
-          promise->reject(NOT_SUPPORTED_ERROR, 0, error_message);
-          return;
-        }
-        break;
       }
-      default:
-        NOTREACHED();
-        promise->reject(NOT_SUPPORTED_ERROR, 0,
-                        "init_data_type not supported.");
+      break;
+#else
+      promise->reject(NOT_SUPPORTED_ERROR, 0,
+                      "Initialization data type CENC is not supported.");
+      return;
+#endif
+    case EmeInitDataType::KEYIDS: {
+      std::string init_data_string(init_data.begin(), init_data.end());
+      std::string error_message;
+      if (!ExtractKeyIdsFromKeyIdsInitData(init_data_string, &keys,
+                                           &error_message)) {
+        promise->reject(NOT_SUPPORTED_ERROR, 0, error_message);
         return;
+      }
+      break;
     }
-    CreateLicenseRequest(keys, session_type, &message);
+    default:
+      NOTREACHED();
+      promise->reject(NOT_SUPPORTED_ERROR, 0, "init_data_type not supported.");
+      return;
   }
+  CreateLicenseRequest(keys, session_type, &message);
 
   promise->resolve(session_id);
 
@@ -423,20 +423,7 @@ void AesDecryptor::CloseSession(const std::string& session_id,
 
 void AesDecryptor::RemoveSession(const std::string& session_id,
                                  std::unique_ptr<SimpleCdmPromise> promise) {
-  // AesDecryptor doesn't keep any persistent data, so this should be
-  // NOT_REACHED().
-  // TODO(jrummell): Make sure persistent session types are rejected.
-  // http://crbug.com/384152.
-  //
-  // However, v0.1b calls to CancelKeyRequest() will call this, so close the
-  // session, if it exists.
-  // TODO(jrummell): Remove the close() call when prefixed EME is removed.
-  // http://crbug.com/249976.
-  if (valid_sessions_.find(session_id) != valid_sessions_.end()) {
-    CloseSession(session_id, std::move(promise));
-    return;
-  }
-
+  NOTIMPLEMENTED() << "Need to address https://crbug.com/616166.";
   promise->reject(INVALID_ACCESS_ERROR, 0, "Session does not exist.");
 }
 

media/cdm/aes_decryptor_unittest.cc

@@ -341,19 +341,10 @@ class AesDecryptorTest : public testing::TestWithParam<std::string> {
     cdm_->CloseSession(session_id, CreatePromise(RESOLVED));
   }
 
-  // Removes the session specified by |session_id|. This should simply do a
-  // CloseSession().
-  // TODO(jrummell): Clean this up when the prefixed API is removed.
-  // http://crbug.com/249976.
+  // Only persistent sessions can be removed.
   void RemoveSession(const std::string& session_id) {
-    if (GetParam() == "AesDecryptor") {
-      EXPECT_CALL(*this, OnSessionClosed(session_id));
-      EXPECT_CALL(*this, OnSessionKeysChangeCalled(session_id, false));
-      cdm_->RemoveSession(session_id, CreatePromise(RESOLVED));
-    } else {
-      // CdmAdapter fails as only persistent sessions can be removed.
-      cdm_->RemoveSession(session_id, CreatePromise(REJECTED));
-    }
+    // TODO(ddorwin): This should be RESOLVED after https://crbug.com/616166.
+    cdm_->RemoveSession(session_id, CreatePromise(REJECTED));
   }
 
   MOCK_METHOD2(OnSessionKeysChangeCalled,
@@ -500,32 +491,59 @@ class AesDecryptorTest : public testing::TestWithParam<std::string> {
   const std::vector<SubsampleEntry> no_subsample_entries_;
 };
 
-TEST_P(AesDecryptorTest, CreateSessionWithNullInitData) {
-  EXPECT_CALL(*this,
-              OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL()));
+TEST_P(AesDecryptorTest, CreateSessionWithEmptyInitData) {
   cdm_->CreateSessionAndGenerateRequest(
       MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
-      std::vector<uint8_t>(), CreateSessionPromise(RESOLVED));
+      std::vector<uint8_t>(), CreateSessionPromise(REJECTED));
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::CENC,
+      std::vector<uint8_t>(), CreateSessionPromise(REJECTED));
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::KEYIDS,
+      std::vector<uint8_t>(), CreateSessionPromise(REJECTED));
+}
+
+TEST_P(AesDecryptorTest, CreateSessionWithVariousLengthInitData_WebM) {
+  std::vector<uint8_t> init_data;
+  init_data.resize(1);
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
+      std::vector<uint8_t>(init_data), CreateSessionPromise(RESOLVED));
+
+  init_data.resize(16);  // The expected size.
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
+      std::vector<uint8_t>(init_data), CreateSessionPromise(RESOLVED));
+
+  init_data.resize(512);
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
+      std::vector<uint8_t>(init_data), CreateSessionPromise(RESOLVED));
+
+  init_data.resize(513);
+  cdm_->CreateSessionAndGenerateRequest(
+      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
+      std::vector<uint8_t>(init_data), CreateSessionPromise(REJECTED));
 }
 
 TEST_P(AesDecryptorTest, MultipleCreateSession) {
-  EXPECT_CALL(*this,
-              OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL()));
+  EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsNotEmpty(),
+                                      GURL::EmptyGURL()));
   cdm_->CreateSessionAndGenerateRequest(
       MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
-      std::vector<uint8_t>(), CreateSessionPromise(RESOLVED));
+      std::vector<uint8_t>(1), CreateSessionPromise(RESOLVED));
 
-  EXPECT_CALL(*this,
-              OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL()));
+  EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsNotEmpty(),
+                                      GURL::EmptyGURL()));
   cdm_->CreateSessionAndGenerateRequest(
       MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
-      std::vector<uint8_t>(), CreateSessionPromise(RESOLVED));
+      std::vector<uint8_t>(1), CreateSessionPromise(RESOLVED));
 
-  EXPECT_CALL(*this,
-              OnSessionMessage(IsNotEmpty(), _, IsEmpty(), GURL::EmptyGURL()));
+  EXPECT_CALL(*this, OnSessionMessage(IsNotEmpty(), _, IsNotEmpty(),
+                                      GURL::EmptyGURL()));
   cdm_->CreateSessionAndGenerateRequest(
       MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
-      std::vector<uint8_t>(), CreateSessionPromise(RESOLVED));
+      std::vector<uint8_t>(1), CreateSessionPromise(RESOLVED));
 }
 
 TEST_P(AesDecryptorTest, CreateSessionWithCencInitData) {
@@ -769,8 +787,6 @@ TEST_P(AesDecryptorTest, CloseSession) {
 }
 
 TEST_P(AesDecryptorTest, RemoveSession) {
-  // TODO(jrummell): Clean this up when the prefixed API is removed.
-  // http://crbug.com/249976.
   std::string session_id = CreateSession(key_id_);
   scoped_refptr<DecoderBuffer> encrypted_buffer = CreateEncryptedBuffer(
       encrypted_data_, key_id_, iv_, no_subsample_entries_);

media/cdm/ppapi/external_clear_key/clear_key_cdm.cc

@@ -346,9 +346,14 @@ void ClearKeyCdm::LoadSession(uint32_t promise_id,
                      promise_id),
           base::Bind(&ClearKeyCdm::OnPromiseFailed, base::Unretained(this),
                      promise_id)));
-  decryptor_->CreateSessionAndGenerateRequest(
-      MediaKeys::TEMPORARY_SESSION, EmeInitDataType::WEBM,
-      std::vector<uint8_t>(), std::move(promise));
+  // AesDecryptor does not support loading, so create a temporary session to
+  // represent it in other session-related methods.
+  std::vector<uint8_t> key_id(
+      kLoadableSessionKeyId,
+      kLoadableSessionKeyId + arraysize(kLoadableSessionKeyId) - 1);
+  decryptor_->CreateSessionAndGenerateRequest(MediaKeys::TEMPORARY_SESSION,
+                                              EmeInitDataType::WEBM, key_id,
+                                              std::move(promise));
 }
 
 void ClearKeyCdm::UpdateSession(uint32_t promise_id,