Restrict WEBM Key ID data to 128-bits for Android

Restrict WebM initData to 128 bits on Android

WebM initData is a key ID, and Android only supports 128-bit key IDs, so fail if a larger |init_data| is
passed to CreateSessionAndGenerateRequest().

BUG=456397
TEST=manual test on Android

Committed: https://crrev.com/87cee08474441eb2e58bfac8bcbfe5afaffd75ae
Cr-Commit-Position: refs/heads/master@{#330154}

[jrummell, 2015-04-29 21:06:07]

jrummell@chromium.org changed reviewers:
+ ddorwin@chromium.org, xhwang@chromium.org

[jrummell, 2015-04-29 21:06:07]

PTAL.

[ddorwin, 2015-04-29 21:19:56]

https://codereview.chromium.org/1113883003/diff/1/content/browser/media/cdm/b...
File content/browser/media/cdm/browser_cdm_manager.cc (right):

https://codereview.chromium.org/1113883003/diff/1/content/browser/media/cdm/b...
content/browser/media/cdm/browser_cdm_manager.cc:41: const size_t
kMaxKeyIdLength = 128 / 8;
We could always have a limit (i.e. 1024 or whatever we do elsewhere) and set it
to 128 on Android. Then, we would always have some checks, not have to ifdef
below, and allow other devices to add similar limits.

If left this way, we should rename the constant since it is the AndroidMax...

https://codereview.chromium.org/1113883003/diff/1/content/browser/media/cdm/b...
content/browser/media/cdm/browser_cdm_manager.cc:356: "Init data for WEBM too
long.");
"'webm' initData is too long."

[jrummell, 2015-05-15 00:52:59]

Updated.

https://codereview.chromium.org/1113883003/diff/1/content/browser/media/cdm/b...
File content/browser/media/cdm/browser_cdm_manager.cc (right):

https://codereview.chromium.org/1113883003/diff/1/content/browser/media/cdm/b...
content/browser/media/cdm/browser_cdm_manager.cc:41: const size_t
kMaxKeyIdLength = 128 / 8;
On 2015/04/29 21:19:56, ddorwin wrote:
> We could always have a limit (i.e. 1024 or whatever we do elsewhere) and set
it
> to 128 on Android. Then, we would always have some checks, not have to ifdef
> below, and allow other devices to add similar limits.
> 
> If left this way, we should rename the constant since it is the AndroidMax...

Renamed. The existing global limit is not currently based on content type.

https://codereview.chromium.org/1113883003/diff/1/content/browser/media/cdm/b...
content/browser/media/cdm/browser_cdm_manager.cc:356: "Init data for WEBM too
long.");
On 2015/04/29 21:19:56, ddorwin wrote:
> "'webm' initData is too long."

Done.

[ddorwin, 2015-05-15 04:12:35]

https://codereview.chromium.org/1113883003/diff/20001/content/browser/media/c...
File content/browser/media/cdm/browser_cdm_manager.cc (right):

https://codereview.chromium.org/1113883003/diff/20001/content/browser/media/c...
content/browser/media/cdm/browser_cdm_manager.cc:41: const size_t
kAndroidMaxKeyIdLengthForWebM = 128 / 8;
I believe this key ID length applies to all containers, not just WebM. And, it's
an exact length, not a max.
Also, it's nice to include the units in the name.
Thus, something like kAndroidKeyIdBytes.

https://codereview.chromium.org/1113883003/diff/20001/content/browser/media/c...
content/browser/media/cdm/browser_cdm_manager.cc:355: init_data.size() >
kAndroidMaxKeyIdLengthForWebM) {
s/>/!=/

Then update the message too.

[jrummell, 2015-05-15 16:41:19]

Updated.

https://codereview.chromium.org/1113883003/diff/20001/content/browser/media/c...
File content/browser/media/cdm/browser_cdm_manager.cc (right):

https://codereview.chromium.org/1113883003/diff/20001/content/browser/media/c...
content/browser/media/cdm/browser_cdm_manager.cc:41: const size_t
kAndroidMaxKeyIdLengthForWebM = 128 / 8;
On 2015/05/15 04:12:35, ddorwin wrote:
> I believe this key ID length applies to all containers, not just WebM. And,
it's
> an exact length, not a max.
> Also, it's nice to include the units in the name.
> Thus, something like kAndroidKeyIdBytes.

Done.

https://codereview.chromium.org/1113883003/diff/20001/content/browser/media/c...
content/browser/media/cdm/browser_cdm_manager.cc:355: init_data.size() >
kAndroidMaxKeyIdLengthForWebM) {
On 2015/05/15 04:12:35, ddorwin wrote:
> s/>/!=/
> 
> Then update the message too.

Done.

[ddorwin, 2015-05-15 16:54:35]

https://codereview.chromium.org/1113883003/diff/40001/content/browser/media/c...
File content/browser/media/cdm/browser_cdm_manager.cc (right):

https://codereview.chromium.org/1113883003/diff/40001/content/browser/media/c...
content/browser/media/cdm/browser_cdm_manager.cc:354: // On Android 'webm'
initData should contain exactly 1 key.
Remove "On Android ". This is always true for WebM.

Even better, something like:
'webm' initData is a single key ID. On Android the length is restricted.

https://codereview.chromium.org/1113883003/diff/40001/content/browser/media/c...
content/browser/media/cdm/browser_cdm_manager.cc:358: "'webm' initData does not
contain a single key.");
By definition, it is a single key ID. The problem is the length.

[jrummell, 2015-05-15 17:52:21]

Updated.

https://codereview.chromium.org/1113883003/diff/40001/content/browser/media/c...
File content/browser/media/cdm/browser_cdm_manager.cc (right):

https://codereview.chromium.org/1113883003/diff/40001/content/browser/media/c...
content/browser/media/cdm/browser_cdm_manager.cc:354: // On Android 'webm'
initData should contain exactly 1 key.
On 2015/05/15 16:54:35, ddorwin wrote:
> Remove "On Android ". This is always true for WebM.
> 
> Even better, something like:
> 'webm' initData is a single key ID. On Android the length is restricted.

Done.

https://codereview.chromium.org/1113883003/diff/40001/content/browser/media/c...
content/browser/media/cdm/browser_cdm_manager.cc:358: "'webm' initData does not
contain a single key.");
On 2015/05/15 16:54:35, ddorwin wrote:
> By definition, it is a single key ID. The problem is the length.

Done.

[ddorwin, 2015-05-15 18:04:35]

lgtm

Thanks.

[jrummell, 2015-05-15 18:04:57]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2015-05-15 18:06:36]

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

[commit-bot: I haz the power, 2015-05-15 19:00:06]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-05-15 19:01:06]

Patchset 4 (id:??) landed as
https://crrev.com/87cee08474441eb2e58bfac8bcbfe5afaffd75ae
Cr-Commit-Position: refs/heads/master@{#330154}