|
|
Created:
4 years, 8 months ago by jrummell Modified:
4 years, 7 months ago CC:
asvitkine+watch_chromium.org, blink-reviews, chromium-reviews, eric.carlson_apple.com, feature-media-reviews_chromium.org, haraken, mlamouri+watch-blink_chromium.org, philipj_slow Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionEME: Correctly handle container-only contentType strings
Container-only contentType strings are now only allowed if they
imply a codec. Since none of the supported containers fall into
this category, ignore the configuration if codecs= is not specified.
For compatability with existing applications, allow (and add a
deprecation message) if "audio/webm", "video/webm", "audio/mp4",
or "video/mp4" are used without a codec string. Also add UMA
statistics so we can tell when this exception can be removed.
The UMA will log the the number of successful calls where
audioCapabilities or videoCapabilities are specified, and whether
the contentType string contains codecs= or not.
For compatability with other browsers (which may not enforce this),
the deprecation message is only generated on a successful
requestMediaKeySystemAccess() call (provided the successful
configuration specified a contentType without codecs=). This
will allow clients to attempt requestMediaKeySystemAccess() with
and without codecs= until all browsers enforce this requirement.
BUG=605661
TEST=updated EME layout tests passes
Committed: https://crrev.com/1e7a1cb3a153677ad560ce47e18b4bb2b107ae32
Cr-Commit-Position: refs/heads/master@{#390711}
Patch Set 1 #
Total comments: 30
Patch Set 2 : changes #
Total comments: 7
Patch Set 3 : UseCounter #
Total comments: 1
Patch Set 4 : rebase #
Messages
Total messages: 24 (8 generated)
jrummell@chromium.org changed reviewers: + ddorwin@chromium.org, xhwang@chromium.org
PTAL.
lgtm % nits https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... File third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp (right): https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:114: // Generate warning and report to UMA if |config| contains a container-only Can't find |config| in the declaration :) https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:118: void checkNoContainerOnlyContentTypeStrings(const WebMediaKeySystemConfiguration&); The function name is a bit hard to read.. How about checkEmptyCodecs()? https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:219: if (!config.audioCapabilities.isEmpty()) { The logic here is good. Just to be clear, this check covers both the case where audioCapabilities doesn't exist, and audioCapabilities exists but is empty: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
https://chromiumcodereview.appspot.com/1911953003/diff/1/media/blink/key_syst... File media/blink/key_system_config_selector.cc (right): https://chromiumcodereview.appspot.com/1911953003/diff/1/media/blink/key_syst... media/blink/key_system_config_selector.cc:317: // Since the spec didn't initially require this, add an exemption for If we want to exclude contentTypes without codecs once one capability has been selected, we may need to move this to the call site where we have more context. https://chromiumcodereview.appspot.com/1911953003/diff/1/media/blink/key_syst... media/blink/key_system_config_selector.cc:322: if (container_mime_type != "audio/webm" && This assumes the string is lower case, but that doesn't happen until line 342 below. Per step 3.6, comparison is case-insensitive. Thus, we should move that up to the top of this function, since this function covers steps 3.4-.11 per the comment at the call site. I think the TODO below is incorrect and can be removed. https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/L... File third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-requestmediakeysystemaccess.html (right): https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/L... third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-requestmediakeysystemaccess.html:263: videoCapabilities: [{contentType: 'VIDEO/WEBM; CODECS="vp8"'}], This fails because of a bug in the next file. It should pass. Unfortunately, that means we can't have a test for the deprecation. Since this is specifically testing "CODECS", use the more common "video/webm" used for other tests. It appears we are missing a test where "codecs" is just not present. Please add that too and add a TODO for both of these tests that they should fail with a reference to the bug. https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/L... third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-requestmediakeysystemaccess.html:264: }], 'NotSupportedError', 'VIDEO/WEBM, CODECS'); Update the name too. https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... File third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp (right): https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:114: // Generate warning and report to UMA if |config| contains a container-only nit: I think it's clearer with "to" removed. https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:216: // as users may pass container only contentType strings for compatibility s/users/apps/ https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:217: // with other systems. s/systems/implementations/ https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:218: bool emptyAudioCodecs = false; How about isAudioCodecsEmpty? https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:222: emptyAudioCodecs = true; This will currently trigger if there is capabilities with codecs followed by one without codecs. This seems like it might be a useful mechanism for compatibility with other implementations^ without having to duplicate the entire configuration. Should we not warn in that case? Or maybe we should have Chromium reject codec-less strings once a one supported item has been found. Then, we could leave this code as is. ^ This does not work for Edge (https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/7223299/), and I'm not sure what other user agents there might be an issue with. Perhaps this isn't a useful pattern. Still, the proposal above seems reasonable. https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:228: new EnumerationHistogram("Media.EME.RequestMediaKeySystemAccess.HasEmptyAudioCodecs", 2)); We might want to rename this to something like SelectedAudioContentTypeWithoutCodecs based on how the implementation changes. https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:248: "Container only contentType strings that do not imply a specific set of codecs will not be " Since this is a deprecation message, we can be more specific: contentType strings without codecs will... https://chromiumcodereview.appspot.com/1911953003/diff/1/third_party/WebKit/S... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:249: "supported in the future. Please specify the desired codec as part of the contentType.")); "codec(s)"
Updated. https://codereview.chromium.org/1911953003/diff/1/media/blink/key_system_conf... File media/blink/key_system_config_selector.cc (right): https://codereview.chromium.org/1911953003/diff/1/media/blink/key_system_conf... media/blink/key_system_config_selector.cc:317: // Since the spec didn't initially require this, add an exemption for On 2016/04/25 22:58:51, ddorwin wrote: > If we want to exclude contentTypes without codecs once one capability has been > selected, we may need to move this to the call site where we have more context. Acknowledged. https://codereview.chromium.org/1911953003/diff/1/media/blink/key_system_conf... media/blink/key_system_config_selector.cc:322: if (container_mime_type != "audio/webm" && On 2016/04/25 22:58:51, ddorwin wrote: > This assumes the string is lower case, but that doesn't happen until line 342 > below. > > Per step 3.6, comparison is case-insensitive. Thus, we should move that up to > the top of this function, since this function covers steps 3.4-.11 per the > comment at the call site. I think the TODO below is incorrect and can be > removed. Done. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-requestmediakeysystemaccess.html (right): https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-requestmediakeysystemaccess.html:263: videoCapabilities: [{contentType: 'VIDEO/WEBM; CODECS="vp8"'}], On 2016/04/25 22:58:51, ddorwin wrote: > This fails because of a bug in the next file. It should pass. Unfortunately, > that means we can't have a test for the deprecation. > > Since this is specifically testing "CODECS", use the more common "video/webm" > used for other tests. > > It appears we are missing a test where "codecs" is just not present. Please add > that too and add a TODO for both of these tests that they should fail with a > reference to the bug. Converted to a successful test. There are existing tests without codecs. I've added a comment to those tests. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-requestmediakeysystemaccess.html:264: }], 'NotSupportedError', 'VIDEO/WEBM, CODECS'); On 2016/04/25 22:58:51, ddorwin wrote: > Update the name too. Done. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp (right): https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:114: // Generate warning and report to UMA if |config| contains a container-only On 2016/04/25 22:14:08, xhwang wrote: > Can't find |config| in the declaration :) presubmit checks made me remove |config| as the parameter name. Fixed. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:114: // Generate warning and report to UMA if |config| contains a container-only On 2016/04/25 22:58:52, ddorwin wrote: > nit: I think it's clearer with "to" removed. Done. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:118: void checkNoContainerOnlyContentTypeStrings(const WebMediaKeySystemConfiguration&); On 2016/04/25 22:14:08, xhwang wrote: > The function name is a bit hard to read.. How about checkEmptyCodecs()? Done. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:216: // as users may pass container only contentType strings for compatibility On 2016/04/25 22:58:51, ddorwin wrote: > s/users/apps/ Done. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:217: // with other systems. On 2016/04/25 22:58:51, ddorwin wrote: > s/systems/implementations/ Done. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:218: bool emptyAudioCodecs = false; On 2016/04/25 22:58:51, ddorwin wrote: > How about isAudioCodecsEmpty? Done. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:219: if (!config.audioCapabilities.isEmpty()) { On 2016/04/25 22:14:08, xhwang wrote: > The logic here is good. > > Just to be clear, this check covers both the case where audioCapabilities > doesn't exist, and audioCapabilities exists but is empty: > > https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit... Added additional check to make it clearer. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:222: emptyAudioCodecs = true; On 2016/04/25 22:58:51, ddorwin wrote: > This will currently trigger if there is capabilities with codecs followed by one > without codecs. This seems like it might be a useful mechanism for compatibility > with other implementations^ without having to duplicate the entire > configuration. > > Should we not warn in that case? Or maybe we should have Chromium reject > codec-less strings once a one supported item has been found. Then, we could > leave this code as is. > > > ^ This does not work for Edge > (https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/7223299/), > and I'm not sure what other user agents there might be an issue with. Perhaps > this isn't a useful pattern. Still, the proposal above seems reasonable. Changed to only track if all returned capabilities don't supply codecs=. Thus configurations that have both accepted won't get a message, since they will continue to succeed once the code enforces the check. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:228: new EnumerationHistogram("Media.EME.RequestMediaKeySystemAccess.HasEmptyAudioCodecs", 2)); On 2016/04/25 22:58:51, ddorwin wrote: > We might want to rename this to something like > SelectedAudioContentTypeWithoutCodecs based on how the implementation changes. Done. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:248: "Container only contentType strings that do not imply a specific set of codecs will not be " On 2016/04/25 22:58:51, ddorwin wrote: > Since this is a deprecation message, we can be more specific: > contentType strings without codecs will... Done. https://codereview.chromium.org/1911953003/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:249: "supported in the future. Please specify the desired codec as part of the contentType.")); On 2016/04/25 22:58:51, ddorwin wrote: > "codec(s)" Done. https://codereview.chromium.org/1911953003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp (right): https://codereview.chromium.org/1911953003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:231: == config.audioCapabilities.end(); Tried to split this into meaningful lines so that it's not one long line, but this is all I can get "git clang-format --style Webkit", "git cl format", and presubmit to agree on.
Thanks. Minor stuff. The biggest question is about whether to use countDeprecation(). https://codereview.chromium.org/1911953003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp (right): https://codereview.chromium.org/1911953003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:225: // codecs. This allows apps to provide both (one with codecs, one without) Since we don't think this is useful, perhaps we should just say, "This avoids alerting on configurations that will continue to succeed..." https://codereview.chromium.org/1911953003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:234: new EnumerationHistogram("Media.EME.RequestMediaKeySystemAccess.SelectedAudioContentTypeWithoutCodecs", 2)); Hmm. This isn't strictly accurate since we are checking for _all_. AllSelectedAudioContentTypeMisingCodecs? https://codereview.chromium.org/1911953003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:249: m_resolver->getExecutionContext()->addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel, Should we use countDeprecation() instead of a separate histogram? UseCounter isn't as easy to use as our own histogram, but it does deal with logging consistently.
Thanks. Minor stuff. The biggest question is about whether to use countDeprecation().
Updated to use UseCounters. Side effect is that you only get 1 warning per document, not every time RMKSA() is called incorrectly. https://codereview.chromium.org/1911953003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp (right): https://codereview.chromium.org/1911953003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:225: // codecs. This allows apps to provide both (one with codecs, one without) On 2016/04/27 23:33:48, ddorwin wrote: > Since we don't think this is useful, perhaps we should just say, "This avoids > alerting on configurations that will continue to succeed..." Done. https://codereview.chromium.org/1911953003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:234: new EnumerationHistogram("Media.EME.RequestMediaKeySystemAccess.SelectedAudioContentTypeWithoutCodecs", 2)); On 2016/04/27 23:33:48, ddorwin wrote: > Hmm. This isn't strictly accurate since we are checking for _all_. > > AllSelectedAudioContentTypeMisingCodecs? Switched to UseCounter. https://codereview.chromium.org/1911953003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:249: m_resolver->getExecutionContext()->addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel, On 2016/04/27 23:33:48, ddorwin wrote: > Should we use countDeprecation() instead of a separate histogram? UseCounter > isn't as easy to use as our own histogram, but it does deal with logging > consistently. Done (along with counting the successes).
LGTM with comment https://codereview.chromium.org/1911953003/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp (right): https://codereview.chromium.org/1911953003/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:240: Deprecation::countDeprecation(m_resolver->getExecutionContext(), UseCounter::EncryptedMediaAllSelectedContentTypesMissingCodecs); This name isn't strictly correct since one type could have codecs for all entries, but as long as we know how to interpret it I guess.
jrummell@chromium.org changed reviewers: + asvitkine@chromium.org, dcheng@chromium.org
Adding OWNERS. Just adding 2 UseCounters. +dcheng@ for third_party/WebKit/Source/core/frame/ +asvitkine@ for tools/metrics/histograms/histograms.xml
lgtm
lgtm
The CQ bit was checked by jrummell@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org Link to the patchset: https://codereview.chromium.org/1911953003/#ps40001 (title: "UseCounter")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1911953003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1911953003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...) ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...) mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
The CQ bit was checked by jrummell@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from asvitkine@chromium.org, xhwang@chromium.org, ddorwin@chromium.org, dcheng@chromium.org Link to the patchset: https://codereview.chromium.org/1911953003/#ps60001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1911953003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1911953003/60001
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/1e7a1cb3a153677ad560ce47e18b4bb2b107ae32 Cr-Commit-Position: refs/heads/master@{#390711}
Message was sent while issue was closed.
Description was changed from ========== EME: Correctly handle container-only contentType strings Container-only contentType strings are now only allowed if they imply a codec. Since none of the supported containers fall into this category, ignore the configuration if codecs= is not specified. For compatability with existing applications, allow (and add a deprecation message) if "audio/webm", "video/webm", "audio/mp4", or "video/mp4" are used without a codec string. Also add UMA statistics so we can tell when this exception can be removed. The UMA will log the the number of successful calls where audioCapabilities or videoCapabilities are specified, and whether the contentType string contains codecs= or not. For compatability with other browsers (which may not enforce this), the deprecation message is only generated on a successful requestMediaKeySystemAccess() call (provided the successful configuration specified a contentType without codecs=). This will allow clients to attempt requestMediaKeySystemAccess() with and without codecs= until all browsers enforce this requirement. BUG=605661 TEST=updated EME layout tests passes ========== to ========== EME: Correctly handle container-only contentType strings Container-only contentType strings are now only allowed if they imply a codec. Since none of the supported containers fall into this category, ignore the configuration if codecs= is not specified. For compatability with existing applications, allow (and add a deprecation message) if "audio/webm", "video/webm", "audio/mp4", or "video/mp4" are used without a codec string. Also add UMA statistics so we can tell when this exception can be removed. The UMA will log the the number of successful calls where audioCapabilities or videoCapabilities are specified, and whether the contentType string contains codecs= or not. For compatability with other browsers (which may not enforce this), the deprecation message is only generated on a successful requestMediaKeySystemAccess() call (provided the successful configuration specified a contentType without codecs=). This will allow clients to attempt requestMediaKeySystemAccess() with and without codecs= until all browsers enforce this requirement. BUG=605661 TEST=updated EME layout tests passes Committed: https://crrev.com/1e7a1cb3a153677ad560ce47e18b4bb2b107ae32 Cr-Commit-Position: refs/heads/master@{#390711} ========== |