Refactor KeySystems code to call a function to populate the info.

content/renderer/media/crypto/key_systems_info.cc

Index: content/renderer/media/crypto/key_systems_info.cc
diff --git a/content/renderer/media/crypto/key_systems_info.cc b/content/renderer/media/crypto/key_systems_info.cc
index 55878b35fe7aa241ef8b7ec39ec0c1fe8c07d29e..59b7a957fea6b6ae7df51390a3a1a09bd711d678 100644
--- a/content/renderer/media/crypto/key_systems_info.cc
+++ b/content/renderer/media/crypto/key_systems_info.cc
@@ -5,7 +5,6 @@
 #include "content/renderer/media/crypto/key_systems_info.h"
 
 #include "base/logging.h"
-#include "content/renderer/media/crypto/key_systems.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 
 #include "widevine_cdm_version.h" // In SHARED_INTERMEDIATE_DIR.
@@ -28,38 +27,25 @@ namespace content {
 
 static const char kClearKeyKeySystem[] = "webkit-org.w3.clearkey";
 
-#if defined(ENABLE_PEPPER_CDMS)
-static const char kExternalClearKeyKeySystem[] =
-    "org.chromium.externalclearkey";
-#elif defined(OS_ANDROID)
-static const uint8 kEmptyUuid[16] =
-    { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
-      0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 };
-#endif
+static const char kAudioWebM[] = "audio/webm";
+static const char kVideoWebM[] = "video/webm";
+static const char kVorbis[] = "vorbis";
+static const char kVorbisVP8[] = "vorbis,vp8,vp8.0";
 
-#if defined(WIDEVINE_CDM_AVAILABLE)
+static const char kAudioMp4[] = "audio/mp4";
+static const char kVideoMp4[] = "video/mp4";
+static const char kMp4a[] = "mp4a";
+static const char kAvc1[] = "avc1";
+static const char kMp4aAvc1[] = "mp4a,avc1";
 
-#if defined(WIDEVINE_CDM_CENC_SUPPORT_AVAILABLE)
-// The supported codecs depend on what the CDM provides.
-static const char kWidevineVideoMp4Codecs[] =
-#if defined(WIDEVINE_CDM_AVC1_SUPPORT_AVAILABLE) && \
-    defined(WIDEVINE_CDM_AAC_SUPPORT_AVAILABLE)
-    "avc1,mp4a";
-#elif defined(WIDEVINE_CDM_AVC1_SUPPORT_AVAILABLE)
-    "avc1";
-#else
-    "";  // No codec strings are supported.
-#endif
-
-static const char kWidevineAudioMp4Codecs[] =
-#if defined(WIDEVINE_CDM_AAC_SUPPORT_AVAILABLE)
-    "mp4a";
-#else
-    "";  // No codec strings are supported.
-#endif
-#endif  // defined(WIDEVINE_CDM_CENC_SUPPORT_AVAILABLE)
+#if defined(WIDEVINE_CDM_AVAILABLE)
+static void AddWidevineForTypes(bool is_webm_vp8_and_vorbis_supported,
+                        bool is_mp4_supported,

[scherkus (not reviewing), 2013/09/03 22:22:08]

fix indenting here

[ddorwin, 2013/09/04 00:41:55]

Done. (4 spaces since aligned makes 46 too long)

+                        bool is_aac_supported,
+                        bool is_avc1_supported,
+                        std::vector<KeySystemInfo>* concrete_key_systems) {
+  static const char kWidevineParentKeySystem[] = "com.widevine";
 
-static void RegisterWidevine() {
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
   Version glibc_version(gnu_get_libc_version());
   DCHECK(glibc_version.IsValid());
@@ -67,72 +53,117 @@ static void RegisterWidevine() {
     return;
 #endif  // defined(OS_LINUX) && !defined(OS_CHROMEOS)
 
-  AddConcreteSupportedKeySystem(
-      kWidevineKeySystem,
-      false,
+  KeySystemInfo info(kWidevineKeySystem);
+
+  if (is_webm_vp8_and_vorbis_supported) {
+    info.supported_types.push_back(std::make_pair(kAudioWebM, kVorbis));
+    info.supported_types.push_back(std::make_pair(kVideoWebM, kVorbisVP8));
+  }
+
+  DCHECK(!is_mp4_supported || (!is_aac_supported && !is_aac_supported));
+#if defined(USE_PROPRIETARY_CODECS)

[scherkus (not reviewing), 2013/09/03 22:22:08]

this seems confusing

if this isn't defined but I decide to pass in true I don't get what I expected

[ddorwin, 2013/09/04 00:41:55]

I can add:
#else
  DCHECK(!is_mp4_supported);

OR duplicate the ifdef at line 43 and the existing ifdef mess at line 95.

The latter is probably more correct (don't compile the code) but is messier.
Preference?

I did the latter.

+  if (is_mp4_supported) {
+    const char* video_codecs = is_avc1_supported ?
+                               (is_aac_supported ? kMp4aAvc1 : kAvc1) :
+                               "";
+    const char* audio_codecs = is_aac_supported ? kMp4a : "";
+
+    info.supported_types.push_back(std::make_pair(kAudioMp4, audio_codecs));
+    info.supported_types.push_back(std::make_pair(kVideoMp4, video_codecs));
+  }
+#endif  // defined(USE_PROPRIETARY_CODECS)
+
+  info.parent_key_system = kWidevineParentKeySystem;
+
 #if defined(ENABLE_PEPPER_CDMS)
-      kWidevineCdmPluginMimeType,
+  info.pepper_type = kWidevineCdmPluginMimeType;
 #elif defined(OS_ANDROID)
+  static const uint8 WidevineUuid[16] =
       { 0xED, 0xEF, 0x8B, 0xA9, 0x79, 0xD6, 0x4A, 0xCE,
-        0xA3, 0xC8, 0x27, 0xDC, 0xD5, 0x1D, 0x21, 0xED },
+        0xA3, 0xC8, 0x27, 0xDC, 0xD5, 0x1D, 0x21, 0xED };
+  info.uuid.assign(WidevineUuid, WidevineUuid + arraysize(WidevineUuid));
 #endif  // defined(ENABLE_PEPPER_CDMS)
-      "com.widevine");
-  AddSupportedType(kWidevineKeySystem, "video/webm", "vorbis,vp8,vp8.0");
-  AddSupportedType(kWidevineKeySystem, "audio/webm", "vorbis");
-#if defined(USE_PROPRIETARY_CODECS) && \
-    defined(WIDEVINE_CDM_CENC_SUPPORT_AVAILABLE)
-  AddSupportedType(kWidevineKeySystem, "video/mp4", kWidevineVideoMp4Codecs);
-  AddSupportedType(kWidevineKeySystem, "audio/mp4", kWidevineAudioMp4Codecs);
-#endif  // defined(USE_PROPRIETARY_CODECS) &&
-        // defined(WIDEVINE_CDM_CENC_SUPPORT_AVAILABLE)
-}
-#endif  // WIDEVINE_CDM_AVAILABLE
 
+  concrete_key_systems->push_back(info);
+}
 
-static void RegisterClearKey() {
-  // Clear Key.
-  AddConcreteSupportedKeySystem(
-      kClearKeyKeySystem,
-      true,
 #if defined(ENABLE_PEPPER_CDMS)
-      std::string(),
-#elif defined(OS_ANDROID)
-      kEmptyUuid,
+// Supported types are determined at compile time.
+static void AddPepperBasedWidevine(
+    std::vector<KeySystemInfo>* concrete_key_systems) {
+  AddWidevineForTypes(true,  // WebM, VP8 & Vorbis.
+#if defined(WIDEVINE_CDM_CENC_SUPPORT_AVAILABLE)
+                      true,  // MP4.
+#if defined(WIDEVINE_CDM_AAC_SUPPORT_AVAILABLE)
+                      true,  // AAC.
+#else
+                      false,  // No AAC.
+#endif
+#if defined(WIDEVINE_CDM_AVC1_SUPPORT_AVAILABLE)
+                      true,  // AVC1.
+#else
+                      false,  // No AVC1.

[scherkus (not reviewing), 2013/09/03 22:25:06]

I still wonder if we can clarify this code somehow...

What irks me is it's a bag of bools requiring auxiliary comments to clarify
what's going on.

It also forces us to handle combinations that don't make sense... for example:
1) is_webm_vp8_and_vorbis_supported is always true. Can we add that parameter
when we have code that passes in false?
2) We have code supporting the following combination:
WIDEVINE_CDM_CENC_SUPPORT_AVAILABLE=1
WIDEVINE_CDM_AAC_SUPPORT_AVAILABLE=0
WIDEVINE_CDM_AVC1_SUPPORT_AVAILABLE=0

... which has to be backed up with a DCHECK() in AddWidevineForTypes() and turns
what I assume is a compile-time failure into a run-time failure.

[ddorwin, 2013/09/04 00:41:55]

We will have this code next week (Android). (I was trying to clean up all the
logic now and just move in the next CL.)
This is theoretically legit. It means the init data format is supported but you
don't have secure decoders (that's how we've chosen to implement canPlayType()
in Chrome).

It will probably never be the case in reality, though, so we could eliminate
WIDEVINE_CDM_CENC_SUPPORT_AVAILABLE and OR the other two where necessary
(probably just is_aac_supported || is_aac_supported in the other function).
I switched to all bools for two reasons:
 1) Replace complex macro logic with code that is easier to write and
understand.
 2) These defines really only need to apply to Pepper CDMs. Android and other
platforms don't need them (and they really would mean something else - "compile
in support" - since availability is a runtime check.

I would rather keep the ifdefs out of the above function (AddWidevineForTypes).
Maybe we should use (mask) enums so that we don't need the comments. We could
then convert them to bools inside the function so that the actual logic is still
easier to follow.

Let me know what you think.

+#endif
+#else
+                      false, false, false,  // No MP4 codecs.
+#endif  // defined(WIDEVINE_CDM_CENC_SUPPORT_AVAILABLE)
+                      concrete_key_systems);
+}
 #endif  // defined(ENABLE_PEPPER_CDMS)
-      std::string());
-  AddSupportedType(kClearKeyKeySystem, "video/webm", "vorbis,vp8,vp8.0");
-  AddSupportedType(kClearKeyKeySystem, "audio/webm", "vorbis");
+#endif  // WIDEVINE_CDM_AVAILABLE

[scherkus (not reviewing), 2013/09/03 22:22:08]

missing defined()

[ddorwin, 2013/09/04 00:41:55]

Done.

+
+static void AddClearKey(std::vector<KeySystemInfo>* concrete_key_systems) {
+  KeySystemInfo info(kClearKeyKeySystem);
+
+  info.supported_types.push_back(std::make_pair(kAudioWebM, kVorbis));
+  info.supported_types.push_back(std::make_pair(kVideoWebM, kVorbisVP8));
 #if defined(USE_PROPRIETARY_CODECS)
-  AddSupportedType(kClearKeyKeySystem, "video/mp4", "avc1,mp4a");
-  AddSupportedType(kClearKeyKeySystem, "audio/mp4", "mp4a");
+  info.supported_types.push_back(std::make_pair(kAudioMp4, kMp4a));
+  info.supported_types.push_back(std::make_pair(kVideoMp4, kMp4aAvc1));
 #endif  // defined(USE_PROPRIETARY_CODECS)
+
+  info.use_aes_decryptor = true;
+
+  concrete_key_systems->push_back(info);
 }
 
 #if defined(ENABLE_PEPPER_CDMS)
-static void RegisterExternalClearKey() {
-  // External Clear Key (used for testing).
-  AddConcreteSupportedKeySystem(kExternalClearKeyKeySystem, false,
-                                "application/x-ppapi-clearkey-cdm",
-                                std::string());
-  AddSupportedType(kExternalClearKeyKeySystem,
-                   "video/webm", "vorbis,vp8,vp8.0");
-  AddSupportedType(kExternalClearKeyKeySystem, "audio/webm", "vorbis");
+// External Clear Key (used for testing).
+static void AddExternalClearKey(
+    std::vector<KeySystemInfo>* concrete_key_systems) {
+  static const char kExternalClearKeyKeySystem[] =
+      "org.chromium.externalclearkey";
+  static const char kExternalClearKeyPepperType[] =
+      "application/x-ppapi-clearkey-cdm";
+
+  KeySystemInfo info(kExternalClearKeyKeySystem);
+
+  info.supported_types.push_back(std::make_pair(kAudioWebM, kVorbis));
+  info.supported_types.push_back(std::make_pair(kVideoWebM, kVorbisVP8));
 #if defined(USE_PROPRIETARY_CODECS)
-  AddSupportedType(kExternalClearKeyKeySystem, "video/mp4", "avc1,mp4a");
-  AddSupportedType(kExternalClearKeyKeySystem, "audio/mp4", "mp4a");
+  info.supported_types.push_back(std::make_pair(kAudioMp4, kMp4a));
+  info.supported_types.push_back(std::make_pair(kVideoMp4, kMp4aAvc1));
 #endif  // defined(USE_PROPRIETARY_CODECS)
+
+  info.pepper_type = kExternalClearKeyPepperType;
+
+  concrete_key_systems->push_back(info);
 }
 #endif  // defined(ENABLE_PEPPER_CDMS)
 
-void RegisterKeySystems() {
-  RegisterClearKey();
+void AddKeySystems(std::vector<KeySystemInfo>* key_systems_info) {
+  AddClearKey(key_systems_info);
 
 #if defined(ENABLE_PEPPER_CDMS)
-  RegisterExternalClearKey();
+  AddExternalClearKey(key_systems_info);
 #endif
 
 #if defined(WIDEVINE_CDM_AVAILABLE)
-  RegisterWidevine();
+#if defined(ENABLE_PEPPER_CDMS)
+  AddPepperBasedWidevine(key_systems_info);
+#else
+#error TODO(ddorwin): Need to check the platform
+#endif
 #endif
 }