Add warning message for empty robustness.

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.h"
 
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/frame/OriginsUsingFeatures.h"
 #include "core/frame/UseCounter.h"
+#include "core/inspector/ConsoleMessage.h"
 #include "modules/encryptedmedia/EncryptedMediaUtils.h"
 #include "modules/encryptedmedia/MediaKeySession.h"
 #include "modules/encryptedmedia/MediaKeySystemAccess.h"
 #include "modules/encryptedmedia/MediaKeysController.h"
 #include "platform/EncryptedMediaRequest.h"
 #include "platform/Logging.h"
 #include "platform/network/ParsedContentType.h"
 #include "public/platform/WebEncryptedMediaClient.h"
 #include "public/platform/WebEncryptedMediaRequest.h"
 #include "public/platform/WebMediaKeySystemConfiguration.h"
@@ skipping 46 matching lines @@
 }
 
 static WebVector<WebEncryptedMediaSessionType> convertSessionTypes(const Vector<String>& sessionTypes)
 {
     WebVector<WebEncryptedMediaSessionType> result(sessionTypes.size());
     for (size_t i = 0; i < sessionTypes.size(); ++i)
         result[i] = EncryptedMediaUtils::convertToSessionType(sessionTypes[i]);
     return result;
 }
 
+static bool isWidevine(const String& keySystem)
+{
+    return keySystem == "com.widevine.alpha";

[ddorwin, 2015/10/01 21:23:47]

This should be in Chromium, though I don't know how easy it is to write to the
console. Also, we wouldn't go to the trouble to abstract it correctly. Since
this is temporary, it's probably fine.

[xhwang, 2015/10/01 22:49:43]

Acknowledged.

+}
+
+// Returns true if the |config| has only one audio or video capability and the robustness in that capability is empty.
+static bool hasOneCapabilityAndRobustnessIsEmpty(const WebMediaKeySystemConfiguration& config)
+{
+    return (config.audioCapabilities.size() == 1 && config.audioCapabilities[0].robustness.isEmpty())

[sandersd (OOO until July 31), 2015/10/01 20:40:24]

I'm not certain that we should restrict to the size() == 1 case. I see three
other options that make more sense:
  - Warn if the first returned capability has no robustness.
  - Warn if any returned capability has no robustness.
  - Warn if there are capabilities, but none of them have robustness.

The last one is probably the right choice, since it's a close match for where
behavior will change.

[ddorwin, 2015/10/01 21:23:47]

Good point. My assumption was that the empty one would be last, but there's no
guarantee of that. However, that is probably the _only_ case where we might want
to not warn about the empty string. (To do this entirely correctly, we'd have to
check the input array, not just the output, to see if we rejected subsequent
entries.)

The real question is whether the empty string should _ever_ be provided for
video and Widevine. The only use case I can think of is supporting some legacy
system that didn't implement robustness values. However, Chromium implemented
these in ~M43, so I would expect most UAs to support them.

Thus, perhaps we should just check for empty string in any entry. If that turns
out to be too noisy (i.e. there is a legit reason of which we're not aware), we
can always fix it. WDYT?

[ddorwin, 2015/10/01 21:23:47]

Empty might be legit for audio. For example, it's unclear what value to use if
the audio is not encrypted. Empty string seems appropriate rather than requiring
use of a minimal decryption capability. Thus, we should probably drop this one.
WDYT?

[sandersd (OOO until July 31), 2015/10/01 21:34:45]

The ideal design for compatibility would be to list the empty capabilities in a
separate configuration entirely, so we can reasonably expect that all accepted
capabilities should have a robustness. That said, other browsers are broken and
I don't know what gymnastics providers will actually implement.

I agree that there are no obvious use cases for video without a robustness, but
your audio use case makes sense to me and is even likely.

You get to make the call on 'any' vs 'all' though, I have no data on which to
make a call. (There may even be an argument for continuing to warn for the
'none' case, even after the change.)

[xhwang, 2015/10/01 22:49:43]

I am going to do "any". 

+        || (config.videoCapabilities.size() == 1 && config.videoCapabilities[0].robustness.isEmpty());
+}
+
 // This class allows capabilities to be checked and a MediaKeySystemAccess
 // object to be created asynchronously.
 class MediaKeySystemAccessInitializer final : public EncryptedMediaRequest {
     WTF_MAKE_NONCOPYABLE(MediaKeySystemAccessInitializer);
 
 public:
     MediaKeySystemAccessInitializer(ScriptState*, const String& keySystem, const HeapVector<MediaKeySystemConfiguration>& supportedConfigurations);
     ~MediaKeySystemAccessInitializer() override { }
 
     // EncryptedMediaRequest implementation.
@@ skipping 46 matching lines @@
             webConfig.sessionTypes = convertSessionTypes(config.sessionTypes());
         }
         // If |label| is not present, it will be a null string.
         webConfig.label = config.label();
         m_supportedConfigurations[i] = webConfig;
     }
 }
 
 void MediaKeySystemAccessInitializer::requestSucceeded(WebContentDecryptionModuleAccess* access)
 {
+    // Check the config and generate warnings on empty robustness string.
+    // TODO(xhwang): Remove after we handle empty robustness correctly. See http://crbug.com/482277
+    if (isWidevine(keySystem()) && hasOneCapabilityAndRobustnessIsEmpty(access->getConfiguration())) {
+        m_resolver->scriptState()->executionContext()->addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel,

[ddorwin, 2015/10/01 21:23:47]

Are we guaranteed that the EC will be valid?

[xhwang, 2015/10/01 22:49:43]

Good question. Actually I don't know. But we have a precedent at l.106.

+            "It is recommended that a robustness level be specified. In a future version, Chromium will be made "

[sandersd (OOO until July 31), 2015/10/01 20:40:24]

While this message is the one recommended by ddorwin@, it seems overly long and
doesn't have a clear message. In fact, the real result is that we may provide a
*higher* robustness level after the change than we do now. I don't know what the
proposed spec change is, so it's hard to recommend a precise wording.

[ddorwin, 2015/10/01 21:23:47]

I think we generally use "Chrome" in console messages. At least for deprecation
messages in Blink.

[ddorwin, 2015/10/01 21:23:47]

There aren't any plans to change the spec beyond what is there today: "The empty
string indicates that any ability to decrypt and decode the content type is
acceptable." Of course, suggestions welcome.

I agree with your points, though, and the wording was just a suggestion. :)
Perhaps the real message is that not specifying the robustness level could
result in unexpected behavior, potentially including failure to play in future
versions. Of course, what we really want is "you should specify a robustness
along with each contentType." Those are both clearer messages, but I'm not sure
how to shrink that message.

[sandersd (OOO until July 31), 2015/10/01 21:34:45]

Perhaps we should just say that the behavior will change, without clarification.

[xhwang, 2015/10/01 22:49:43]

Updated the text... 

+            "spec compliant such that the empty string is interpreted as the minimum robustness being acceptable."));
+    }
+
     m_resolver->resolve(new MediaKeySystemAccess(m_keySystem, adoptPtr(access)));
     m_resolver.clear();
 }
 
 void MediaKeySystemAccessInitializer::requestNotSupported(const WebString& errorMessage)
 {
     m_resolver->reject(DOMException::create(NotSupportedError, errorMessage));
     m_resolver.clear();
 }
 
@@ skipping 53 matching lines @@
     //    initialize the MediaKeySystemAccess object.
     MediaKeysController* controller = MediaKeysController::from(document->page());
     WebEncryptedMediaClient* mediaClient = controller->encryptedMediaClient(executionContext);
     mediaClient->requestMediaKeySystemAccess(WebEncryptedMediaRequest(initializer));
 
     // 8. Return promise.
     return promise;
 }
 
 } // namespace blink