Add support for CDM_6.

Add support for CDM_6.

Changes for CDM_6:
- ReleaseSession() becomes CloseSession()
- add RemoveSession()
- add GetUsableKeyIds()
- simulate UsableKeysChanged event for older CDMs

BUG=358271, 351139
TEST=encrypted-media layout tests and browser_tests for encrypted media pass

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288295

[jrummell, 2014-08-06 22:45:29]

PTAL.

[ddorwin, 2014-08-07 01:34:44]

Thanks.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter.cc
File media/cdm/ppapi/cdm_adapter.cc (right):

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:391: RejectPromise(promise_id,
cdm::kNotSupportedError, 0, "Not supported.");
Maybe "Not implemented." "Not supported" is redundant with the exception name,
and it's really just coming soon.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:406: RejectPromise(promise_id,
cdm::kNotSupportedError, 0, "Not supported.");
ditto

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:688: // cdm::Host_5 methods
and 6?

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:701: std::string session_id;
Although the Wrapper explains these are for 4 & 5, it seems like a comment would
be helpful.
Also, note that passing "true" may result in false positives for retrying.
Same at 713.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:770: PostOnMain(callback_factory_.NewCallback(
Just call OnSessionUsableKeysChange?

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter.h
File media/cdm/ppapi/cdm_adapter.h (right):

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.h:90: virtual cdm::Buffer* Allocate(uint32_t
capacity) OVERRIDE;
We should still note that this is cdm::Host to differentiate it from
PPP_ContentDecryptor_Private above.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.h:92: // TODO(jrummell): Pass these through Pepper.
... and add OVERRIDE.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.h:93: virtual void GetUsableKeyIds(uint32_t
promise_id,
These aren't part of a Host interface. See also line 95.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.h:95: virtual void RemoveSession(uint32_t
promise_id,
Swap order and move to line 73 to match cdm.h.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper.h
File media/cdm/ppapi/cdm_wrapper.h (right):

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:59: virtual bool GetUsableKeyIds(uint32_t
promise_id,
nit: After RemoveSession to match CDM_6 (and keeps lifetime items together).

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:105: // In cdm::Host_6, resolving LoadSession()
and UpdateSession() calls will
Isn't this more like the following?
CDMs using Host_6 will call OnSessionUsableKeys() as necessary when resolving
LoadSession() and UpdateSession().

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:108: virtual bool
SessionUsableKeysEventNeeded(uint32_t promise_id,
// These must not be called for Host_6 and later.

Previously, we had talked about implementing things like this in child
class(es), but we may have decided it was too much effort.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:324: std::map<uint32_t, std::string>
session_usable_keys_event_needed_;
promises_needing_usable_....?

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:329: // Overrides for the cdm::Host_5 methods.
Why does 5 come before 4?

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:349: SetSessionUsableKeysEventNeeded(
This may be overly aggressive in firing events, but oh well.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:356: bool
CdmWrapperImpl<cdm::ContentDecryptionModule_5>::GetUsableKeyIds(
nit: ditto on order

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:412: SetSessionUsableKeysEventNeeded(
ditto here and below.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:433: bool
CdmWrapperImpl<cdm::ContentDecryptionModule_4>::GetUsableKeyIds(
nit: ditto on order

[jrummell, 2014-08-07 20:44:31]

Updated. Should I change DEPS to include
https://codereview.chromium.org/120813004?

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter.cc
File media/cdm/ppapi/cdm_adapter.cc (right):

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:391: RejectPromise(promise_id,
cdm::kNotSupportedError, 0, "Not supported.");
On 2014/08/07 01:34:43, ddorwin wrote:
> Maybe "Not implemented." "Not supported" is redundant with the exception name,
> and it's really just coming soon.

This rejection happens for CDM_4 and CDM_5, which don't support
GetUsableKeyIds().

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:406: RejectPromise(promise_id,
cdm::kNotSupportedError, 0, "Not supported.");
On 2014/08/07 01:34:43, ddorwin wrote:
> ditto

ditto

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:688: // cdm::Host_5 methods
On 2014/08/07 01:34:43, ddorwin wrote:
> and 6?

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:701: std::string session_id;
On 2014/08/07 01:34:43, ddorwin wrote:
> Although the Wrapper explains these are for 4 & 5, it seems like a comment
would
> be helpful.
> Also, note that passing "true" may result in false positives for retrying.
> Same at 713.

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:770: PostOnMain(callback_factory_.NewCallback(
On 2014/08/07 01:34:43, ddorwin wrote:
> Just call OnSessionUsableKeysChange?

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter.h
File media/cdm/ppapi/cdm_adapter.h (right):

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.h:90: virtual cdm::Buffer* Allocate(uint32_t
capacity) OVERRIDE;
On 2014/08/07 01:34:43, ddorwin wrote:
> We should still note that this is cdm::Host to differentiate it from
> PPP_ContentDecryptor_Private above.

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.h:92: // TODO(jrummell): Pass these through Pepper.
On 2014/08/07 01:34:43, ddorwin wrote:
> ... and add OVERRIDE.

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.h:93: virtual void GetUsableKeyIds(uint32_t
promise_id,
On 2014/08/07 01:34:44, ddorwin wrote:
> These aren't part of a Host interface. See also line 95.

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.h:95: virtual void RemoveSession(uint32_t
promise_id,
On 2014/08/07 01:34:44, ddorwin wrote:
> Swap order and move to line 73 to match cdm.h.

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper.h
File media/cdm/ppapi/cdm_wrapper.h (right):

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:59: virtual bool GetUsableKeyIds(uint32_t
promise_id,
On 2014/08/07 01:34:44, ddorwin wrote:
> nit: After RemoveSession to match CDM_6 (and keeps lifetime items together).

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:105: // In cdm::Host_6, resolving LoadSession()
and UpdateSession() calls will
On 2014/08/07 01:34:44, ddorwin wrote:
> Isn't this more like the following?
> CDMs using Host_6 will call OnSessionUsableKeys() as necessary when resolving
> LoadSession() and UpdateSession().

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:108: virtual bool
SessionUsableKeysEventNeeded(uint32_t promise_id,
On 2014/08/07 01:34:44, ddorwin wrote:
> // These must not be called for Host_6 and later.
> 
> Previously, we had talked about implementing things like this in child
> class(es), but we may have decided it was too much effort.

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:324: std::map<uint32_t, std::string>
session_usable_keys_event_needed_;
On 2014/08/07 01:34:44, ddorwin wrote:
> promises_needing_usable_....?

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:329: // Overrides for the cdm::Host_5 methods.
On 2014/08/07 01:34:44, ddorwin wrote:
> Why does 5 come before 4?

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:349: SetSessionUsableKeysEventNeeded(
On 2014/08/07 01:34:44, ddorwin wrote:
> This may be overly aggressive in firing events, but oh well.

Acknowledged.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:356: bool
CdmWrapperImpl<cdm::ContentDecryptionModule_5>::GetUsableKeyIds(
On 2014/08/07 01:34:44, ddorwin wrote:
> nit: ditto on order

Done.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:412: SetSessionUsableKeysEventNeeded(
On 2014/08/07 01:34:44, ddorwin wrote:
> ditto here and below.

Acknowledged.

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:433: bool
CdmWrapperImpl<cdm::ContentDecryptionModule_4>::GetUsableKeyIds(
On 2014/08/07 01:34:44, ddorwin wrote:
> nit: ditto on order

Done.

[jrummell, 2014-08-07 21:40:40]

Updated to include InputBuffer changes.

[ddorwin, 2014-08-07 22:10:43]

LGTM

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter.cc
File media/cdm/ppapi/cdm_adapter.cc (right):

https://codereview.chromium.org/446693004/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:391: RejectPromise(promise_id,
cdm::kNotSupportedError, 0, "Not supported.");
On 2014/08/07 20:44:30, jrummell wrote:
> On 2014/08/07 01:34:43, ddorwin wrote:
> > Maybe "Not implemented." "Not supported" is redundant with the exception
name,
> > and it's really just coming soon.
> 
> This rejection happens for CDM_4 and CDM_5, which don't support
> GetUsableKeyIds().

Yes, but the message goes to JS, so we want it to be meaningful.

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/cdm_adap...
File media/cdm/ppapi/cdm_adapter.h (right):

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.h:96: // cdm::Host_4 and cdm::Host_5 implementation.
and 6?

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/cdm_wrap...
File media/cdm/ppapi/cdm_wrapper.h (right):

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/cdm_wrap...
media/cdm/ppapi/cdm_wrapper.h:120: virtual void ConvertInputBuffer(const
cdm::InputBuffer& in,
s/in/v2/
s/out/v1/
?

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/external...
File media/cdm/ppapi/external_clear_key/clear_key_cdm.h (right):

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/external...
media/cdm/ppapi/external_clear_key/clear_key_cdm.h:60: virtual cdm::Status
Decrypt(const cdm::InputBuffer_1& encrypted_buffer,
This will switch in the other CL?

[jrummell, 2014-08-08 00:15:08]

Updated with latest CDM_6 in DEPS.

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/cdm_adap...
File media/cdm/ppapi/cdm_adapter.h (right):

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.h:96: // cdm::Host_4 and cdm::Host_5 implementation.
On 2014/08/07 22:10:43, ddorwin wrote:
> and 6?

Done.

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/cdm_wrap...
File media/cdm/ppapi/cdm_wrapper.h (right):

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/cdm_wrap...
media/cdm/ppapi/cdm_wrapper.h:120: virtual void ConvertInputBuffer(const
cdm::InputBuffer& in,
On 2014/08/07 22:10:43, ddorwin wrote:
> s/in/v2/
> s/out/v1/
> ?

Done.

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/external...
File media/cdm/ppapi/external_clear_key/clear_key_cdm.h (right):

https://codereview.chromium.org/446693004/diff/40001/media/cdm/ppapi/external...
media/cdm/ppapi/external_clear_key/clear_key_cdm.h:60: virtual cdm::Status
Decrypt(const cdm::InputBuffer_1& encrypted_buffer,
On 2014/08/07 22:10:43, ddorwin wrote:
> This will switch in the other CL?

It will go back to InputBuffer once ClearKey switches to CDM_6.

[ddorwin, 2014-08-08 00:33:02]

lgtm

[xhwang, 2014-08-08 00:47:58]

lgtm

https://codereview.chromium.org/446693004/diff/60001/media/cdm/ppapi/cdm_adap...
File media/cdm/ppapi/cdm_adapter.h (right):

https://codereview.chromium.org/446693004/diff/60001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.h:205: const std::string& web_session_id);
Do we have a bug/TODO to s/web_session_id/session_id/ when we drop CDM_4
support?

https://codereview.chromium.org/446693004/diff/60001/media/cdm/ppapi/cdm_wrap...
File media/cdm/ppapi/cdm_wrapper.h (right):

https://codereview.chromium.org/446693004/diff/60001/media/cdm/ppapi/cdm_wrap...
media/cdm/ppapi/cdm_wrapper.h:110: std::string* web_session_id) = 0;
Document what the parameters are about?

[jrummell, 2014-08-08 01:13:17]

Thanks for the reviews.

https://codereview.chromium.org/446693004/diff/60001/media/cdm/ppapi/cdm_adap...
File media/cdm/ppapi/cdm_adapter.h (right):

https://codereview.chromium.org/446693004/diff/60001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.h:205: const std::string& web_session_id);
On 2014/08/08 00:47:58, xhwang wrote:
> Do we have a bug/TODO to s/web_session_id/session_id/ when we drop CDM_4
> support?

Opened http:://crbug.com/401815

https://codereview.chromium.org/446693004/diff/60001/media/cdm/ppapi/cdm_wrap...
File media/cdm/ppapi/cdm_wrapper.h (right):

https://codereview.chromium.org/446693004/diff/60001/media/cdm/ppapi/cdm_wrap...
media/cdm/ppapi/cdm_wrapper.h:110: std::string* web_session_id) = 0;
On 2014/08/08 00:47:58, xhwang wrote:
> Document what the parameters are about?

Done.

[jrummell, 2014-08-08 01:13:25]

The CQ bit was checked by jrummell@chromium.org

[ddorwin, 2014-08-08 01:16:31]

https://codereview.chromium.org/446693004/diff/80001/media/cdm/ppapi/cdm_wrap...
File media/cdm/ppapi/cdm_wrapper.h (right):

https://codereview.chromium.org/446693004/diff/80001/media/cdm/ppapi/cdm_wrap...
media/cdm/ppapi/cdm_wrapper.h:113: virtual bool
SessionUsableKeysEventNeeded(uint32_t promise_id,
nit: Is...

[commit-bot: I haz the power, 2014-08-08 01:19:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jrummell@chromium.org/446693004/80001

[commit-bot: I haz the power, 2014-08-08 04:36:39]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-08 10:07:22]

Change committed as 288295