EME: Update MediaKeySystemConfiguration defaults; require non-empty capabilities

third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp

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 a5cbf998856527f4776fd6d110fcb73c80637f94..0febf1b15fd756c3f7271cd25cd17807c6765729 100644
--- a/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp
+++ b/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp
@@ -122,6 +122,13 @@ private:
     // See http://crbug.com/605661.
     void checkEmptyCodecs(const WebMediaKeySystemConfiguration&);
 
+    // Generate deprecation warning and log UseCounter if configuration does
+    // not have at least one of 'audioCapabilities' and 'videoCapabilities'
+    // non-empty.
+    // TODO(jrummell): Remove once this is enforced.
+    // See http://crbug.com/616233.
+    void checkCapabilitiesProvided(const WebMediaKeySystemConfiguration&);
+
     Member<ScriptPromiseResolver> m_resolver;
     const String m_keySystem;
     WebVector<WebMediaKeySystemConfiguration> m_supportedConfigurations;
@@ -135,26 +142,29 @@ MediaKeySystemAccessInitializer::MediaKeySystemAccessInitializer(ScriptState* sc
     for (size_t i = 0; i < supportedConfigurations.size(); ++i) {
         const MediaKeySystemConfiguration& config = supportedConfigurations[i];
         WebMediaKeySystemConfiguration webConfig;
-        if (config.hasInitDataTypes()) {
-            webConfig.hasInitDataTypes = true;
-            webConfig.initDataTypes = convertInitDataTypes(config.initDataTypes());
-        }
-        if (config.hasAudioCapabilities()) {
-            webConfig.hasAudioCapabilities = true;
-            webConfig.audioCapabilities = convertCapabilities(config.audioCapabilities());
-        }
-        if (config.hasVideoCapabilities()) {
-            webConfig.hasVideoCapabilities = true;
-            webConfig.videoCapabilities = convertCapabilities(config.videoCapabilities());
-        }
+
+        DCHECK(config.hasInitDataTypes());
+        webConfig.initDataTypes = convertInitDataTypes(config.initDataTypes());
+
+        DCHECK(config.hasAudioCapabilities());
+        webConfig.audioCapabilities = convertCapabilities(config.audioCapabilities());
+
+        DCHECK(config.hasVideoCapabilities());
+        webConfig.videoCapabilities = convertCapabilities(config.videoCapabilities());
+
+        checkCapabilitiesProvided(webConfig);

[foolip, 2016/09/23 08:18:39]

I can't find where the "If video capabilities is null, return NotSupported" bit
of the spec is implemented, does it already reject?

[jrummell, 2016/09/29 20:40:02]

Most of the steps are done in media/blink/key_system_config_selector.cc, see
line 652 in this CL. It has a TODO to check it once we require it.

+
         DCHECK(config.hasDistinctiveIdentifier());
         webConfig.distinctiveIdentifier = convertMediaKeysRequirement(config.distinctiveIdentifier());
+
         DCHECK(config.hasPersistentState());
         webConfig.persistentState = convertMediaKeysRequirement(config.persistentState());
+
         if (config.hasSessionTypes()) {
             webConfig.hasSessionTypes = true;
             webConfig.sessionTypes = convertSessionTypes(config.sessionTypes());
         }
+
         // If |label| is not present, it will be a null string.
         webConfig.label = config.label();
         m_supportedConfigurations[i] = webConfig;
@@ -187,12 +197,8 @@ void MediaKeySystemAccessInitializer::checkVideoCapabilityRobustness() const
     bool hasEmptyRobustness = false;
 
     for (const auto& config : m_supportedConfigurations) {
-        if (!config.hasVideoCapabilities)
-            continue;
-
-        hasVideoCapabilities = true;
-
         for (const auto& capability : config.videoCapabilities) {
+            hasVideoCapabilities = true;
             if (capability.robustness.isEmpty()) {
                 hasEmptyRobustness = true;
                 break;
@@ -224,13 +230,13 @@ void MediaKeySystemAccessInitializer::checkEmptyCodecs(const WebMediaKeySystemCo
     // codecs. This avoids alerting on configurations that will continue
     // to succeed in the future once strict checking is enforced.
     bool areAllAudioCodecsEmpty = false;
-    if (config.hasAudioCapabilities && !config.audioCapabilities.isEmpty()) {
+    if (!config.audioCapabilities.isEmpty()) {
         areAllAudioCodecsEmpty = std::find_if(config.audioCapabilities.begin(), config.audioCapabilities.end(), AreCodecsSpecified)
             == config.audioCapabilities.end();
     }
 
     bool areAllVideoCodecsEmpty = false;
-    if (config.hasVideoCapabilities && !config.videoCapabilities.isEmpty()) {
+    if (!config.videoCapabilities.isEmpty()) {
         areAllVideoCodecsEmpty = std::find_if(config.videoCapabilities.begin(), config.videoCapabilities.end(), AreCodecsSpecified)
             == config.videoCapabilities.end();
     }
@@ -242,6 +248,18 @@ void MediaKeySystemAccessInitializer::checkEmptyCodecs(const WebMediaKeySystemCo
     }
 }
 
+void MediaKeySystemAccessInitializer::checkCapabilitiesProvided(const WebMediaKeySystemConfiguration& config)
+{
+    bool atLeastOneAudioCapability = config.audioCapabilities.size() > 0;
+    bool atLeastOneVideoCapability = config.videoCapabilities.size() > 0;
+
+    if (atLeastOneAudioCapability || atLeastOneVideoCapability) {
+        UseCounter::count(m_resolver->getExecutionContext(), UseCounter::EncryptedMediaCapabilityProvided);
+    } else {
+        Deprecation::countDeprecation(m_resolver->getExecutionContext(), UseCounter::EncryptedMediaCapabilityNotProvided);

[foolip, 2016/09/23 08:18:39]

Optional nit: refactor so that the deprecation is exactly where the promise
would be rejected with NotSupported. It'd be more obviously like what the spec
if handled in MediaKeySystemAccess::getConfiguration, working with the generated
MediaKeySystemConfiguration instead of the WebMediaKeySystemConfiguration it
populates.

(Then the "removal" CL would be a one-liner replacing the reprecation by a
rejection.)

[jrummell, 2016/09/29 20:40:02]

I wish :). Most of the checks happen outside of blink, where we know more about
what is supported by the key system.

+    }
+}
+
 } // namespace
 
 ScriptPromise NavigatorRequestMediaKeySystemAccess::requestMediaKeySystemAccess(
@@ -269,7 +287,10 @@ ScriptPromise NavigatorRequestMediaKeySystemAccess::requestMediaKeySystemAccess(
             scriptState, DOMException::create(InvalidAccessError, "The supportedConfigurations parameter is empty."));
     }
 
-    // 3-4. 'May Document use powerful features?' check.
+    // Note: This method should only be exposed to secure contexts as indicated
+    // by the [SecureContext] IDL attribute. Since that will break some existing
+    // sites, we simply keep track of sites that aren't secure and output a
+    // deprecation message.
     ExecutionContext* executionContext = scriptState->getExecutionContext();
     String errorMessage;
     if (executionContext->isSecureContext(errorMessage)) {
@@ -280,26 +301,27 @@ ScriptPromise NavigatorRequestMediaKeySystemAccess::requestMediaKeySystemAccess(
         // Reject promise with a new DOMException whose name is NotSupportedError.
     }
 
-    // 5. Let origin be the origin of document.
-    //    (Passed with the execution context in step 7.)
-
-    // 6. Let promise be a new promise.
+    // 3. Let document be the calling context's Document.
     Document* document = toDocument(executionContext);
     if (!document->page()) {
         return ScriptPromise::rejectWithDOMException(
             scriptState, DOMException::create(InvalidStateError, "The context provided is not associated with a page."));
     }
 
+    // 4. Let origin be the origin of document.
+    //    (Passed with the execution context.)
+
+    // 5. Let promise be a new promise.
     MediaKeySystemAccessInitializer* initializer = new MediaKeySystemAccessInitializer(scriptState, keySystem, supportedConfigurations);
     ScriptPromise promise = initializer->promise();
 
-    // 7. Asynchronously determine support, and if allowed, create and
+    // 6. Asynchronously determine support, and if allowed, create and
     //    initialize the MediaKeySystemAccess object.
     MediaKeysController* controller = MediaKeysController::from(document->page());
     WebEncryptedMediaClient* mediaClient = controller->encryptedMediaClient(executionContext);
     mediaClient->requestMediaKeySystemAccess(WebEncryptedMediaRequest(initializer));
 
-    // 8. Return promise.
+    // 7. Return promise.
     return promise;
 }