 Chromium Code Reviews
 Chromium Code Reviews Issue 1911953003:
  EME: Correctly handle container-only contentType strings  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 1911953003:
  EME: Correctly handle container-only contentType strings  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp | 
| diff --git a/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp b/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp | 
| index fbeffbbd610019dfd1d6c4bb317d754e1ea229bb..934523e36fb9e728b800db080c92434ee060f0ce 100644 | 
| --- a/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp | 
| +++ b/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp | 
| @@ -27,6 +27,7 @@ | 
| #include "public/platform/WebVector.h" | 
| #include "wtf/Vector.h" | 
| #include "wtf/text/WTFString.h" | 
| +#include <algorithm> | 
| namespace blink { | 
| @@ -79,6 +80,11 @@ static WebVector<WebEncryptedMediaSessionType> convertSessionTypes(const Vector< | 
| return result; | 
| } | 
| +static bool AreCodecsSpecified(const WebMediaKeySystemMediaCapability& capability) | 
| +{ | 
| + return !capability.codecs.isEmpty(); | 
| +} | 
| + | 
| // This class allows capabilities to be checked and a MediaKeySystemAccess | 
| // object to be created asynchronously. | 
| class MediaKeySystemAccessInitializer final : public EncryptedMediaRequest { | 
| @@ -111,6 +117,12 @@ private: | 
| // See http://crbug.com/482277 | 
| void checkVideoCapabilityRobustness() const; | 
| + // Generate warning and report UMA if configuration contains a | 
| + // container-only contentType string. | 
| + // TODO(jrummell): Remove once this is no longer allowed. | 
| + // See http://crbug.com/605661. | 
| + void checkEmptyCodecs(const WebMediaKeySystemConfiguration&); | 
| + | 
| Member<ScriptPromiseResolver> m_resolver; | 
| const String m_keySystem; | 
| WebVector<WebMediaKeySystemConfiguration> m_supportedConfigurations; | 
| @@ -154,6 +166,8 @@ MediaKeySystemAccessInitializer::MediaKeySystemAccessInitializer(ScriptState* sc | 
| void MediaKeySystemAccessInitializer::requestSucceeded(WebContentDecryptionModuleAccess* access) | 
| { | 
| + checkEmptyCodecs(access->getConfiguration()); | 
| + | 
| m_resolver->resolve(new MediaKeySystemAccess(m_keySystem, adoptPtr(access))); | 
| m_resolver.clear(); | 
| } | 
| @@ -202,6 +216,42 @@ void MediaKeySystemAccessInitializer::checkVideoCapabilityRobustness() const | 
| } | 
| } | 
| +void MediaKeySystemAccessInitializer::checkEmptyCodecs(const WebMediaKeySystemConfiguration& config) | 
| +{ | 
| + // This is only checking for empty codecs in the selected configuration, | 
| + // as apps may pass container only contentType strings for compatibility | 
| + // with other implementations. | 
| + // This will only check that all returned capabilities do not contain | 
| + // codecs. This allows apps to provide both (one with codecs, one without) | 
| 
ddorwin
2016/04/27 23:33:48
Since we don't think this is useful, perhaps we sh
 
jrummell
2016/04/28 01:32:47
Done.
 | 
| + // for compatibility reasons, and that will continue to succeed in the | 
| + // future once strict checking is enforced. | 
| + bool areAllAudioCodecsEmpty = false; | 
| + if (config.hasAudioCapabilities && !config.audioCapabilities.isEmpty()) { | 
| + areAllAudioCodecsEmpty = std::find_if(config.audioCapabilities.begin(), config.audioCapabilities.end(), AreCodecsSpecified) | 
| + == config.audioCapabilities.end(); | 
| 
jrummell
2016/04/26 21:23:57
Tried to split this into meaningful lines so that
 | 
| + | 
| + DEFINE_THREAD_SAFE_STATIC_LOCAL(EnumerationHistogram, emptyAudioCodecsHistogram, | 
| + new EnumerationHistogram("Media.EME.RequestMediaKeySystemAccess.SelectedAudioContentTypeWithoutCodecs", 2)); | 
| 
ddorwin
2016/04/27 23:33:48
Hmm. This isn't strictly accurate since we are che
 
jrummell
2016/04/28 01:32:47
Switched to UseCounter.
 | 
| + emptyAudioCodecsHistogram.count(areAllAudioCodecsEmpty); | 
| + } | 
| + | 
| + bool areAllVideoCodecsEmpty = false; | 
| + if (config.hasVideoCapabilities && !config.videoCapabilities.isEmpty()) { | 
| + areAllVideoCodecsEmpty = std::find_if(config.videoCapabilities.begin(), config.videoCapabilities.end(), AreCodecsSpecified) | 
| + == config.videoCapabilities.end(); | 
| + | 
| + DEFINE_THREAD_SAFE_STATIC_LOCAL(EnumerationHistogram, emptyVideoCodecsHistogram, | 
| + new EnumerationHistogram("Media.EME.RequestMediaKeySystemAccess.SelectedVideoContentTypeWithoutCodecs", 2)); | 
| + emptyVideoCodecsHistogram.count(areAllVideoCodecsEmpty); | 
| + } | 
| + | 
| + if (areAllAudioCodecsEmpty || areAllVideoCodecsEmpty) { | 
| + m_resolver->getExecutionContext()->addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel, | 
| 
ddorwin
2016/04/27 23:33:48
Should we use countDeprecation() instead of a sepa
 
jrummell
2016/04/28 01:32:47
Done (along with counting the successes).
 | 
| + "contentType strings without codecs will not be supported in the future." | 
| + "Please specify the desired codec(s) as part of the contentType.")); | 
| + } | 
| +} | 
| + | 
| } // namespace | 
| ScriptPromise NavigatorRequestMediaKeySystemAccess::requestMediaKeySystemAccess( |