Check InitDateType is supported by key system when creating session

Check InitDataType is supported by key system when creating session

BUG=385874
TEST=existing EME tests pass

Committed: https://crrev.com/6599e7e9097f42389af22231dbc1978f5ec2966b
Cr-Commit-Position: refs/heads/master@{#314886}

[jrummell, 2015-02-04 21:40:11]

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

[jrummell, 2015-02-04 21:40:12]

PTAL. I'll update the blink code once this has landed.

[ddorwin, 2015-02-04 23:42:23]

I have some concerns about how the complete fix will play out. Other than that,
LGTM.

https://codereview.chromium.org/897133002/diff/1/media/blink/webcontentdecryp...
File media/blink/webcontentdecryptionmodulesession_impl.cc (right):

https://codereview.chromium.org/897133002/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmodulesession_impl.cc:100: // TODO(jrummell):
Remove once blink updated.
Done? Ditto for two below.

https://codereview.chromium.org/897133002/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmodulesession_impl.cc:126: NOTREACHED();
Do we have LTs that test non-ascii handling?

I think we will hit this when the isKeySystemSupportedWithInitDataType() call is
removed in Blink. Will we implement the check in Blink or should we just handle
it here? Ideally, we'd use enums for IDT (418239), which would address this
specific TODO..

[jrummell, 2015-02-05 01:01:28]

Comments only.

https://codereview.chromium.org/897133002/diff/1/media/blink/webcontentdecryp...
File media/blink/webcontentdecryptionmodulesession_impl.cc (right):

https://codereview.chromium.org/897133002/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmodulesession_impl.cc:100: // TODO(jrummell):
Remove once blink updated.
On 2015/02/04 23:42:23, ddorwin wrote:
> Done? Ditto for two below.

I was going to do some cleanups in a separate CL.

https://codereview.chromium.org/897133002/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmodulesession_impl.cc:126: NOTREACHED();
On 2015/02/04 23:42:23, ddorwin wrote:
> Do we have LTs that test non-ascii handling?
> 
> I think we will hit this when the isKeySystemSupportedWithInitDataType() call
is
> removed in Blink. Will we implement the check in Blink or should we just
handle
> it here? Ideally, we'd use enums for IDT (418239), which would address this
> specific TODO..

No LTs since we typically don't mess with InitDataType. But I can add some in
encrypted-media-syntax.html when I do the blink change.

[ddorwin, 2015-02-05 01:09:31]

https://codereview.chromium.org/897133002/diff/1/media/blink/webcontentdecryp...
File media/blink/webcontentdecryptionmodulesession_impl.cc (right):

https://codereview.chromium.org/897133002/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmodulesession_impl.cc:126: NOTREACHED();
On 2015/02/05 01:01:28, jrummell wrote:
> On 2015/02/04 23:42:23, ddorwin wrote:
> > Do we have LTs that test non-ascii handling?
> > 
> > I think we will hit this when the isKeySystemSupportedWithInitDataType()
call
> is
> > removed in Blink. Will we implement the check in Blink or should we just
> handle
> > it here? Ideally, we'd use enums for IDT (418239), which would address this
> > specific TODO..
> 
> No LTs since we typically don't mess with InitDataType. But I can add some in
> encrypted-media-syntax.html when I do the blink change.

SG, but what is the plan to avoid hitting the NOTREACHED()? I guess this CL is
fine, but you might need another Chromium CL before the Blink one.

[jrummell, 2015-02-05 20:42:15]

Thanks for the review.

https://codereview.chromium.org/897133002/diff/1/media/blink/webcontentdecryp...
File media/blink/webcontentdecryptionmodulesession_impl.cc (right):

https://codereview.chromium.org/897133002/diff/1/media/blink/webcontentdecryp...
media/blink/webcontentdecryptionmodulesession_impl.cc:126: NOTREACHED();
On 2015/02/05 01:09:31, ddorwin wrote:
> On 2015/02/05 01:01:28, jrummell wrote:
> > On 2015/02/04 23:42:23, ddorwin wrote:
> > > Do we have LTs that test non-ascii handling?
> > > 
> > > I think we will hit this when the isKeySystemSupportedWithInitDataType()
> call
> > is
> > > removed in Blink. Will we implement the check in Blink or should we just
> > handle
> > > it here? Ideally, we'd use enums for IDT (418239), which would address
this
> > > specific TODO..
> > 
> > No LTs since we typically don't mess with InitDataType. But I can add some
in
> > encrypted-media-syntax.html when I do the blink change.
> 
> SG, but what is the plan to avoid hitting the NOTREACHED()? I guess this CL is
> fine, but you might need another Chromium CL before the Blink one.

Where step 5 is on the blink side I may need to check that it is ASCII only.
I'll add a test case first and see if it hits this.

[jrummell, 2015-02-05 20:43:18]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2015-02-05 20:44:19]

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

[commit-bot: I haz the power, 2015-02-05 21:46:25]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-02-05 21:47:49]

Patchset 1 (id:??) landed as
https://crrev.com/6599e7e9097f42389af22231dbc1978f5ec2966b
Cr-Commit-Position: refs/heads/master@{#314886}