Make canPlayType()'s keySystem argument depend on PrefixedEncryptedMedia

Make canPlayType()'s keySystem argument depend on PrefixedEncryptedMedia

The argument is no longer marked as optional, as that would make it
ambiguous with the single-argument canPlayType().

BUG=452098
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189099

[philipj_slow, 2015-01-27 09:01:43]

philipj@opera.com changed reviewers:
+ ddorwin@chromium.org

[philipj_slow, 2015-01-27 09:01:44]

PTAL

[ddorwin, 2015-01-27 18:40:02]

LGTM with comments. Thank you!

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.idl (right):

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.idl:49: DOMString canPlayType(DOMString type);
Does the generated code automatically call HTMLMediaElement::canPlayType(type,
<isNull() string>)?

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.idl:50:
[RuntimeEnabled=PrefixedEncryptedMedia] DOMString canPlayType(DOMString type,
[TreatUndefinedAs=NullString] DOMString? keySystem);
Do we still need the '?'?

[ddorwin, 2015-01-27 18:40:48]

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.idl (right):

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.idl:50:
[RuntimeEnabled=PrefixedEncryptedMedia] DOMString canPlayType(DOMString type,
[TreatUndefinedAs=NullString] DOMString? keySystem);
Should we move the DeprecateAs here too for consistency? Maybe in a separate CL.

[philipj_slow, 2015-01-28 06:10:27]

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.idl (right):

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.idl:49: DOMString canPlayType(DOMString type);
On 2015/01/27 18:40:01, ddorwin wrote:
> Does the generated code automatically call HTMLMediaElement::canPlayType(type,
> <isNull() string>)?

I'm not sure what the <isNull() string> syntax means, but you get a generated
canPlayType1Method and canPlayType2Method. Both use "V8StringResource<> type"
and the latter has "V8StringResource<TreatNullAndUndefinedAsNullString>
keySystem", so nothing has change when it comes to null/undefined.

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.idl:50:
[RuntimeEnabled=PrefixedEncryptedMedia] DOMString canPlayType(DOMString type,
[TreatUndefinedAs=NullString] DOMString? keySystem);
On 2015/01/27 18:40:02, ddorwin wrote:
> Do we still need the '?'?

It was never needed AFAICT, but it doesn't seem worth changing the behavior of
canPlayType("", null) and canPlayType("", undefined) when this is going to be
removed soonish anyway.

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.idl:50:
[RuntimeEnabled=PrefixedEncryptedMedia] DOMString canPlayType(DOMString type,
[TreatUndefinedAs=NullString] DOMString? keySystem);
On 2015/01/27 18:40:48, ddorwin wrote:
> Should we move the DeprecateAs here too for consistency? Maybe in a separate
CL.

I'll do it here. It's not exactly equivalent, but close enough, and having it in
the IDL wherever possible is nice.

[philipj_slow, 2015-01-28 06:11:23]

Move CanPlayTypeKeySystem deprecation to IDL

[philipj_slow, 2015-01-28 06:12:35]

I'll land this now. For reference, here's the diff of the generated code:

--- a/V8HTMLMediaElement.cpp
+++ b/V8HTMLMediaElement.cpp
@@ -753,12 +753,18 @@ static void loadMethodCallback(const
v8::FunctionCallbackInfo<v8::Value>& info)
     TRACE_EVENT_SET_SAMPLING_STATE("v8", "V8Execution");
 }
 
-static void canPlayTypeMethod(const v8::FunctionCallbackInfo<v8::Value>& info)
+static void canPlayType1Method(const v8::FunctionCallbackInfo<v8::Value>& info)
 {
-    if (UNLIKELY(info.Length() < 1)) {
-       
V8ThrowException::throwException(createMinimumArityTypeErrorForMethod(info.GetIsolate(),
"canPlayType", "HTMLMediaElement", 1, info.Length()), info.GetIsolate());
-        return;
+    HTMLMediaElement* impl = V8HTMLMediaElement::toImpl(info.Holder());
+    V8StringResource<> type;
+    {
+        TOSTRING_VOID_INTERNAL(type, info[0]);
     }
+    v8SetReturnValueString(info, impl->canPlayType(type), info.GetIsolate());
+}
+
+static void canPlayType2Method(const v8::FunctionCallbackInfo<v8::Value>& info)
+{
     HTMLMediaElement* impl = V8HTMLMediaElement::toImpl(info.Holder());
     V8StringResource<> type;
     V8StringResource<TreatNullAndUndefinedAsNullString> keySystem;
@@ -769,6 +775,37 @@ static void canPlayTypeMethod(const
v8::FunctionCallbackInfo<v8::Value>& info)
     v8SetReturnValueString(info, impl->canPlayType(type, keySystem),
info.GetIsolate());
 }
 
+static void canPlayTypeMethod(const v8::FunctionCallbackInfo<v8::Value>& info)
+{
+    ExceptionState exceptionState(ExceptionState::ExecutionContext,
"canPlayType", "HTMLMediaElement", info.Holder(), info.GetIsolate());
+    switch (std::min(2, info.Length())) {
+    case 1:
+        if (true) {
+            canPlayType1Method(info);
+            return;
+        }
+        break;
+    case 2:
+        if (RuntimeEnabledFeatures::prefixedEncryptedMediaEnabled()) {
+            if (true) {
+               
UseCounter::countDeprecationIfNotPrivateScript(info.GetIsolate(),
callingExecutionContext(info.GetIsolate()), UseCounter::CanPlayTypeKeySystem);
+                canPlayType2Method(info);
+                return;
+            }
+        }
+        break;
+    default:
+        break;
+    }
+    if (info.Length() < 1) {
+        exceptionState.throwTypeError(ExceptionMessages::notEnoughArguments(1,
info.Length()));
+        exceptionState.throwIfNeeded();
+        return;
+    }
+    exceptionState.throwTypeError("No function was found that matched the
signature provided.");
+    exceptionState.throwIfNeeded();
+}

[philipj_slow, 2015-01-28 06:46:26]

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.idl (right):

https://codereview.chromium.org/876863006/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.idl:50:
[RuntimeEnabled=PrefixedEncryptedMedia] DOMString canPlayType(DOMString type,
[TreatUndefinedAs=NullString] DOMString? keySystem);
On 2015/01/28 06:10:27, philipj_UTC7 wrote:
> On 2015/01/27 18:40:02, ddorwin wrote:
> > Do we still need the '?'?
> 
> It was never needed AFAICT, but it doesn't seem worth changing the behavior of
> canPlayType("", null) and canPlayType("", undefined) when this is going to be
> removed soonish anyway.

Actually the nullability was per the original spec:
https://github.com/w3c/encrypted-media/commit/04bb2c8775b718b56934484e3b76f98...

In any event I'll leave it alone.

[philipj_slow, 2015-01-28 06:46:32]

The CQ bit was checked by philipj@opera.com

[commit-bot: I haz the power, 2015-01-28 06:47:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/876863006/20001

[commit-bot: I haz the power, 2015-01-28 09:09:59]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189099