Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users
Status: Fixed
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment
EME: Correctly handle container-only contentType strings
Project Member Reported by ddorwin@chromium.org, Apr 21 2016 Back to list
The fix for https://github.com/w3c/encrypted-media/issues/151 makes container-only contentType strings "not supported" for all practical purposes (all common encryption-supporting containers).

Chrome currently explicitly allows these. We need to get to a point where it does not, but we need to avoid breaking applications, including Shaka, in the meantime. I propose:

1. Add deprecation message ASAP. (We may want to merge this back to M51.)
2. Add a UMA - maybe RAPPOR - stat.
3. Monitor and deprecate when the numbers are low enough.
  - We might also disable in dev and beta if the deprecation message does not work.


 
Labels: -Pri-2 EME-compat M-52 Pri-1
Owner: jrumm...@chromium.org
Status: Assigned
Bumping up the priority to address #1. We can lower it for 2 (maybe) and certainly 3.
Cc: halliwell@chromium.org dougsteed@chromium.org
The current plan is:
* contentTypes without codecs (codecs.empty() in the code) will be disallowed.
* For now, we will: 
  * Have exceptions for "audio/webm", "video/webm", "audio/mp4", and "video/mp4".^
  * Report these as deprecated in Blink. (This includes UMA.)

We will _try_ to minimize the spam for developers using this as a fallback for compatibility with other implementations.

^ This is all currently supported container types. If any new container types are added, they will not allow container-only values.
Cc: vadimgo@chromium.org
+vadimgo
Project Member Comment 4 by bugdroid1@chromium.org, Apr 29 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e7a1cb3a153677ad560ce47e18b4bb2b107ae32

commit 1e7a1cb3a153677ad560ce47e18b4bb2b107ae32
Author: jrummell <jrummell@chromium.org>
Date: Fri Apr 29 19:10:03 2016

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

Review-Url: https://codereview.chromium.org/1911953003
Cr-Commit-Position: refs/heads/master@{#390711}

[modify] https://crrev.com/1e7a1cb3a153677ad560ce47e18b4bb2b107ae32/media/blink/key_system_config_selector.cc
[modify] https://crrev.com/1e7a1cb3a153677ad560ce47e18b4bb2b107ae32/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-requestmediakeysystemaccess.html
[modify] https://crrev.com/1e7a1cb3a153677ad560ce47e18b4bb2b107ae32/third_party/WebKit/Source/core/frame/Deprecation.cpp
[modify] https://crrev.com/1e7a1cb3a153677ad560ce47e18b4bb2b107ae32/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] https://crrev.com/1e7a1cb3a153677ad560ce47e18b4bb2b107ae32/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp
[modify] https://crrev.com/1e7a1cb3a153677ad560ce47e18b4bb2b107ae32/tools/metrics/histograms/histograms.xml

Labels: -Pri-1 Pri-3
Deprecation message and UMA added. Lowering priority until numbers are low enough to always require codecs.
Labels: -Pri-3 Merge-Request-51 Pri-1
UMA is being reported. Bumping priority back up to get this merged into M51.
Comment 7 by tin...@google.com, May 2 2016
Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Comment 8 Deleted
Please merge your change to M51 branch 2704 by EOD today or latest by tomorrow 4:00 PM PST in order to make it to this week beta release. Thank you.
Project Member Comment 10 by bugdroid1@chromium.org, May 2 2016
Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db386c994fe6ef71f90ecd8546290017cf00acbc

commit db386c994fe6ef71f90ecd8546290017cf00acbc
Author: John Rummell <jrummell@chromium.org>
Date: Mon May 02 20:23:06 2016

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

Review-Url: https://codereview.chromium.org/1911953003
Cr-Commit-Position: refs/heads/master@{#390711}
(cherry picked from commit 1e7a1cb3a153677ad560ce47e18b4bb2b107ae32)

Review URL: https://codereview.chromium.org/1939263002 .

Cr-Commit-Position: refs/branch-heads/2704@{#338}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/db386c994fe6ef71f90ecd8546290017cf00acbc/media/blink/key_system_config_selector.cc
[modify] https://crrev.com/db386c994fe6ef71f90ecd8546290017cf00acbc/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-requestmediakeysystemaccess.html
[modify] https://crrev.com/db386c994fe6ef71f90ecd8546290017cf00acbc/third_party/WebKit/Source/core/frame/Deprecation.cpp
[modify] https://crrev.com/db386c994fe6ef71f90ecd8546290017cf00acbc/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] https://crrev.com/db386c994fe6ef71f90ecd8546290017cf00acbc/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp
[modify] https://crrev.com/db386c994fe6ef71f90ecd8546290017cf00acbc/tools/metrics/histograms/histograms.xml

Labels: -Pri-1 -M-52 M-55 Pri-3
Merged into M51. Lowering priority until it can be disabled.
Project Member Comment 12 by bugdroid1@chromium.org, May 19 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ddc14e3fa1385f02b466336dc60d0e1eda450769

commit ddc14e3fa1385f02b466336dc60d0e1eda450769
Author: jrummell <jrummell@chromium.org>
Date: Thu May 19 22:39:55 2016

EME: Add milestone to deprecation message for container-only contentType strings

Update the message to indicate that support will be removed in M54.

BUG= 605661 
TEST=EME layout tests passes

Review-Url: https://codereview.chromium.org/1992313002
Cr-Commit-Position: refs/heads/master@{#394890}

[modify] https://crrev.com/ddc14e3fa1385f02b466336dc60d0e1eda450769/third_party/WebKit/Source/core/frame/Deprecation.cpp

Cc: faniel@chromium.org ericde@chromium.org
Labels: -M-55 -Pri-3 M-56 Pri-1
The removal date has been updated to M56 in  issue 643826 . We should make sure we are ready to remove support before M56 branch cut so we don't need to slip again.

We should drive compliance with  issue 616233  at the same time.
Project Member Comment 14 by bugdroid1@chromium.org, Nov 29 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4b5bb4969a525126bb416aea5407f413abf0d0c

commit c4b5bb4969a525126bb416aea5407f413abf0d0c
Author: jrummell <jrummell@chromium.org>
Date: Tue Nov 29 02:58:27 2016

Update milestone for EME non-standard functionality

Moving milestone to M58.

BUG= 605661 
TEST=messages logged with EME layout tests

Review-Url: https://codereview.chromium.org/2539573002
Cr-Commit-Position: refs/heads/master@{#434852}

[modify] https://crrev.com/c4b5bb4969a525126bb416aea5407f413abf0d0c/third_party/WebKit/Source/core/frame/Deprecation.cpp

Components: Internals>Media>Encrypted
Labels: Merge-Request-56
Requesting merge approval to merge #14 back into M56 (all it does it update the milestone in the deprecation message).
Comment 16 by dimu@chromium.org, Nov 30 2016
Labels: -Merge-Request-56 Merge-Approved-56
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member Comment 17 by bugdroid1@chromium.org, Nov 30 2016
Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b2bcc911e0a56c4326155bc704c5863d34a0503

commit 9b2bcc911e0a56c4326155bc704c5863d34a0503
Author: John Rummell <jrummell@chromium.org>
Date: Wed Nov 30 18:53:05 2016

Merge "Update milestone for EME non-standard functionality"

Moving milestone to M58.

BUG= 605661 
TEST=messages logged with EME layout tests

Review-Url: https://codereview.chromium.org/2539573002
Cr-Commit-Position: refs/heads/master@{#434852}
(cherry picked from commit c4b5bb4969a525126bb416aea5407f413abf0d0c)

Review URL: https://codereview.chromium.org/2541763004 .

Cr-Commit-Position: refs/branch-heads/2924@{#200}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/9b2bcc911e0a56c4326155bc704c5863d34a0503/third_party/WebKit/Source/core/frame/Deprecation.cpp

Labels: -M-56 M-58
Moving to M58 now that the deprecation message has been changed.
Project Member Comment 19 by bugdroid1@chromium.org, Feb 9 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c5784d408a464915895f9b47385df377a555032e

commit c5784d408a464915895f9b47385df377a555032e
Author: jrummell <jrummell@chromium.org>
Date: Thu Feb 09 00:59:30 2017

EME: Allow case-insensitive parameter names in media MIME types

According to RFC 2045,  "All media type values, subtype values, and
parameter names as defined are case-insensitive." Switch
requestMediaKeySystemAccess() to use ContentType, which is not
case-sensitive.

Added new test that should fail. Prior to this change it would pass as
the CODECS="foo" would be ignored, and it would look like a container
only MIME type, which is allowed.

BUG= 605661 
TEST=new test fails as expected

Review-Url: https://codereview.chromium.org/2681993002
Cr-Commit-Position: refs/heads/master@{#449169}

[modify] https://crrev.com/c5784d408a464915895f9b47385df377a555032e/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-requestmediakeysystemaccess.html
[modify] https://crrev.com/c5784d408a464915895f9b47385df377a555032e/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp

Project Member Comment 20 by bugdroid1@chromium.org, Feb 10 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f31b4ce5af50a9ae0b92d484028c41f20db3420

commit 2f31b4ce5af50a9ae0b92d484028c41f20db3420
Author: jrummell <jrummell@chromium.org>
Date: Fri Feb 10 23:40:32 2017

EME: Require codecs parameter for audio/videoCapabilities

Chrome will no longer allow container-only contentType strings in the
audioCapabilities and videoCapabilities passed to requestMediaKeySystemAccess.
There has been a deprecation message for several releases, so enforce this now.

BUG= 605661 
TEST=modified tests pass

Review-Url: https://codereview.chromium.org/2686133002
Cr-Commit-Position: refs/heads/master@{#449788}

[modify] https://crrev.com/2f31b4ce5af50a9ae0b92d484028c41f20db3420/chrome/browser/media/encrypted_media_supported_types_browsertest.cc
[modify] https://crrev.com/2f31b4ce5af50a9ae0b92d484028c41f20db3420/media/blink/key_system_config_selector.cc
[modify] https://crrev.com/2f31b4ce5af50a9ae0b92d484028c41f20db3420/media/blink/key_system_config_selector_unittest.cc
[modify] https://crrev.com/2f31b4ce5af50a9ae0b92d484028c41f20db3420/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-requestmediakeysystemaccess.html
[modify] https://crrev.com/2f31b4ce5af50a9ae0b92d484028c41f20db3420/third_party/WebKit/Source/core/frame/Deprecation.cpp
[modify] https://crrev.com/2f31b4ce5af50a9ae0b92d484028c41f20db3420/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] https://crrev.com/2f31b4ce5af50a9ae0b92d484028c41f20db3420/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp

Status: Fixed
Project Member Comment 22 by bugdroid1@chromium.org, Mar 3
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/699c0840f87b9839b04a7cca2805d890609e5cf6

commit 699c0840f87b9839b04a7cca2805d890609e5cf6
Author: jrummell <jrummell@chromium.org>
Date: Fri Mar 03 20:46:04 2017

[eme] Simplify parsing of contentTypes

Now that ParsedContentType does case-insensitive parameter lookups
( http://crbug.com/689731 ), use it to extract the type and codecs
parameter.

BUG= 605661 
TEST=existing case-insensitive EME tests pass

Review-Url: https://codereview.chromium.org/2728713003
Cr-Commit-Position: refs/heads/master@{#454664}

[modify] https://crrev.com/699c0840f87b9839b04a7cca2805d890609e5cf6/third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp

Sign in to add a comment