Verify MediaKeys parameter values consistently

Source/modules/encryptedmedia/MediaKeys.cpp

 /*
  * Copyright (C) 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 35 matching lines @@
     // From <http://dvcs.w3.org/hg/html-media/raw-file/default/encrypted-media/encrypted-media.html#dom-media-keys-constructor>:
     // The MediaKeys(keySystem) constructor must run the following steps:
 
     // 1. If keySystem is null or an empty string, throw an InvalidAccessError exception and abort these steps.
     if (keySystem.isEmpty()) {
         exceptionState.throwDOMException(InvalidAccessError, "The key system provided is invalid.");
         return nullptr;
     }
 
     // 2. If keySystem is not one of the user agent's supported Key Systems, throw a NotSupportedError and abort these steps.
-    if (!ContentDecryptionModule::supportsKeySystem(keySystem)) {
+    if (!isKeySystemSupported(keySystem)) {
         exceptionState.throwDOMException(NotSupportedError, "The '" + keySystem + "' key system is not supported.");
         return nullptr;
     }
 
     // 3. Let cdm be the content decryption module corresponding to keySystem.
     // 4. Load cdm if necessary.
     OwnPtr<ContentDecryptionModule> cdm = ContentDecryptionModule::create(keySystem);
     if (!cdm) {
         exceptionState.throwDOMException(NotSupportedError, "A content decryption module could not be loaded for the '" + keySystem + "' key system.");
         return nullptr;
@@ skipping 34 matching lines @@
         return nullptr;
     }
 
     if (!initData || !initData->length()) {
         exceptionState.throwDOMException(InvalidAccessError, "The initData provided is null or empty.");
         return nullptr;
     }
 
     // 1. If type contains a MIME type that is not supported or is not supported by the keySystem,
     // throw a NOT_SUPPORTED_ERR exception and abort these steps.
-    if (!m_cdm->supportsMIMEType(contentType)) {
+    if (!isKeySystemSupportedWithContentType(m_keySystem, contentType)) {
         exceptionState.throwDOMException(NotSupportedError, "The type provided ('" + contentType + "') is unsupported.");
         return nullptr;
     }
 
     // 2. Create a new MediaKeySession object.
     RefPtrWillBeRawPtr<MediaKeySession> session = MediaKeySession::create(context, m_cdm.get(), m_weakFactory.createWeakPtr());
     // 2.1 Let the keySystem attribute be keySystem.
     ASSERT(!session->keySystem().isEmpty());
     // FIXME: 2.2 Let the state of the session be CREATED.
 
@@ skipping 12 matching lines @@
 bool MediaKeys::isTypeSupported(const String& keySystem, const String& contentType)
 {
     WTF_LOG(Media, "MediaKeys::isTypeSupported(%s, %s)", keySystem.ascii().data(), contentType.ascii().data());
 
     // 1. If keySystem is null or an empty string, return false and abort these steps.
     if (keySystem.isEmpty())
         return false;
 
     // 2. If keySystem contains an unrecognized or unsupported Key System, return false and abort
     // these steps. Key system string comparison is case-sensitive.
-    if (!MIMETypeRegistry::isSupportedEncryptedMediaMIMEType(keySystem, "", ""))
+    if (!isKeySystemSupported(keySystem))
         return false;
 
     // 3. If contentType is null or an empty string, return true and abort these steps.
     if (contentType.isEmpty())
         return true;
 
     // 4. If the Key System specified by keySystem does not support decrypting the container and/or
     // codec specified by contentType, return false and abort these steps.
+    return isKeySystemSupportedWithContentType(keySystem, contentType);
+}
+
+bool MediaKeys::isKeySystemSupported(const String& keySystem)
+{
+    ASSERT(!keySystem.isEmpty());
+    return MIMETypeRegistry::isSupportedEncryptedMediaMIMEType(keySystem, "", "");
+}
+
+bool MediaKeys::isKeySystemSupportedWithContentType(const String& keySystem, const String& contentType)
+{
+    ASSERT(!keySystem.isEmpty());
+    ASSERT(!contentType.isEmpty());

[acolwell GONE FROM CHROMIUM, 2014/02/28 16:50:40]

nit: I'm not sure there is a lot of value having 2 methods here. I actually find
it a little confusing especially since these both ultimately bottom out in the
same MIMETypeRegistry call. Also the fact that there are parameter checks before
these methods are even called make the ASSERTS() here seem redundant.

[jrummell, 2014/02/28 19:06:52]

Done.

+
     ContentType type(contentType);
     String codecs = type.parameter("codecs");
-
     return MIMETypeRegistry::isSupportedEncryptedMediaMIMEType(keySystem, type.type(), codecs);
 }
 
 void MediaKeys::setMediaElement(HTMLMediaElement* element)
 {
     // FIXME: Cause HTMLMediaElement::setMediaKeys() to throw an exception if m_mediaElement is not 0
     // and remove the code that prevents the assert below in HTMLMediaElement.
     ASSERT(!m_mediaElement != !element);
     m_mediaElement = element;
 }
@@ skipping 26 matching lines @@
 
 void MediaKeys::contextDestroyed()
 {
     ContextLifecycleObserver::contextDestroyed();
 
     // We don't need the CDM anymore.
     m_cdm.clear();
 }
 
 }