Update Pepper interface for EME

Update Pepper interface for EME

To support CDM_6, make the following changes:
- add SetServerCertificate
- add GetUsableKeyIds
- rename ReleaseSession to CloseSession
- add RemoveSession
- add SessionKeysChange event
- add SessionExpirationChange event

Includes changes to cdm_adapter for the new functionality. Changes to
use these new interfaces in blink in a future CL.

BUG=358271
TEST=existing EME tests still pass + manual testing

Committed: https://crrev.com/1e9a7b144e268ac9310474352d8f54b0b03fe82a
Cr-Commit-Position: refs/heads/master@{#293672}

[jrummell, 2014-08-21 23:35:01]

PTAL.

[ddorwin, 2014-08-22 20:49:21]

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
File content/renderer/pepper/content_decryptor_delegate.cc (right):

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:682: PP_Var key_ids) {
..._array?

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:687: if (key_ids_array) {
DCHECK? Or can this be null if the array is empty? If the latter, you might
comment that. In either case, I we do need to handle the case in opt builds.

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:691: if (array_buffer) {
DCHECK?

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:692: std::vector<uint8>
item;
How about key_id?

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:700: if (promise) {
DCHECK?
I wonder why existing code does not have this? We shouldn't crash in opt builds,
but don't we want to know if we can't find the promise? Or is this some sort of
teardown issue?

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:701: KeyIdsPromise*
key_ids_promise(static_cast<KeyIdsPromise*>(promise.get()));
This promise ID is coming from a plugin process, which cannot be trusted. Maybe
we should be able to get the promise type.
I'm curious what dmichael has to say from a Pepper POV.

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
File content/renderer/pepper/content_decryptor_delegate.h (right):

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.h:73: void
GetUsableKeyIds(const std::string& web_session_id,
Nit: Keep the lifetime methods together, meaning this should be after Remove.

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

https://codereview.chromium.org/496143002/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:336: // called even if Initialize() failed.
Is this true? This function can only be called by unprefixed on a valid
MediaKeys object, so this should never happen, right?

https://codereview.chromium.org/496143002/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:413: void CdmAdapter::GetUsableKeyIds(uint32_t
promise_id,
Why did this move?

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

https://codereview.chromium.org/496143002/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.h:73: virtual void GetUsableKeyIds(uint32_t
promise_id,
ditto

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

https://codereview.chromium.org/496143002/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:45: virtual bool SetServerCertificate(uint32_t
promise_id,
Is there a reason this has a return value?
Should it be removed when CDM_4/5 are no longer supported?
I may have asked this before, but can we just reject the promise here instead?
(I think the answer is that we don't have a host pointer, so no.)

Ditto on some of the other existing methods.

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
File ppapi/api/private/ppb_content_decryptor_private.idl (right):

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
ppapi/api/private/ppb_content_decryptor_private.idl:49: * A promise has been
resolved by the CDM.
These versions that have specific purposes should probably have a
differentiating part of the comment.

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
ppapi/api/private/ppb_content_decryptor_private.idl:54: *
<code>PP_VARTYPE_ARRAY</code> containing the session's usable key IDs.
Do we need to define the element type?

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
ppapi/api/private/ppb_content_decryptor_private.idl:59: [in] PP_Var key_ids);
key_ids_array?

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
ppapi/api/private/ppb_content_decryptor_private.idl:96: *
<code>PP_VARTYPE_STRING</code> containing the session's ID attribute for
This sentence structure/description is appropriate for
PromiseResolvedWithSession, which is delivering the ID for the attribute, but
probably not these methods.

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
ppapi/api/private/ppb_content_decryptor_private.idl:116: *
<code>PP_VARTYPE_STRING</code> containing the session's ID attribute of
ditto here and below.

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppp_conten...
File ppapi/api/private/ppp_content_decryptor_private.idl (right):

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppp_conten...
ppapi/api/private/ppp_content_decryptor_private.idl:38: * rejected depending
upon the success or failure of setti ng the certificate.
setting

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
File ppapi/cpp/private/content_decryptor_private.cc (right):

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
ppapi/cpp/private/content_decryptor_private.cc:264: &GetUsableKeyIds,
ditto

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
ppapi/cpp/private/content_decryptor_private.cc:318: std::vector<uint8_t> entry =
key_ids[i];
const ref?

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
ppapi/cpp/private/content_decryptor_private.cc:321: key_ids_array.Set(i,
pp::Var(array_buffer));
Is there an Add() or Push()? It seems odd that you can set beyond the current
length.

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
File ppapi/cpp/private/content_decryptor_private.h (right):

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
ppapi/cpp/private/content_decryptor_private.h:49: virtual void
GetUsableKeyIds(uint32_t promise_id,
ditto

https://codereview.chromium.org/496143002/diff/1/ppapi/proxy/ppp_content_decr...
File ppapi/proxy/ppp_content_decryptor_private_proxy.cc (right):

https://codereview.chromium.org/496143002/diff/1/ppapi/proxy/ppp_content_decr...
ppapi/proxy/ppp_content_decryptor_private_proxy.cc:438: &GetUsableKeyIds,
ditto

[jrummell, 2014-08-25 21:54:37]

Updated. Includes a rebase to pick up the recent changes to cdm_adapter.

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
File content/renderer/pepper/content_decryptor_delegate.cc (right):

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:682: PP_Var key_ids) {
On 2014/08/22 20:49:20, ddorwin wrote:
> ..._array?

Done.

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:687: if (key_ids_array) {
On 2014/08/22 20:49:20, ddorwin wrote:
> DCHECK? Or can this be null if the array is empty? If the latter, you might
> comment that. In either case, I we do need to handle the case in opt builds.

Done.

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:691: if (array_buffer) {
On 2014/08/22 20:49:20, ddorwin wrote:
> DCHECK?

Done.

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:692: std::vector<uint8>
item;
On 2014/08/22 20:49:20, ddorwin wrote:
> How about key_id?

Done.

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:700: if (promise) {
On 2014/08/22 20:49:20, ddorwin wrote:
> DCHECK?
> I wonder why existing code does not have this? We shouldn't crash in opt
builds,
> but don't we want to know if we can't find the promise? Or is this some sort
of
> teardown issue?

The other OnPromise...() do check that promise is valid. Probably a good idea to
verify with a DCHECK.

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.cc:701: KeyIdsPromise*
key_ids_promise(static_cast<KeyIdsPromise*>(promise.get()));
On 2014/08/22 20:49:20, ddorwin wrote:
> This promise ID is coming from a plugin process, which cannot be trusted.
Maybe
> we should be able to get the promise type.
> I'm curious what dmichael has to say from a Pepper POV.

Added type on CdmPromise so it can be verified.

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
File content/renderer/pepper/content_decryptor_delegate.h (right):

https://codereview.chromium.org/496143002/diff/1/content/renderer/pepper/cont...
content/renderer/pepper/content_decryptor_delegate.h:73: void
GetUsableKeyIds(const std::string& web_session_id,
On 2014/08/22 20:49:20, ddorwin wrote:
> Nit: Keep the lifetime methods together, meaning this should be after Remove.

Done.

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

https://codereview.chromium.org/496143002/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:336: // called even if Initialize() failed.
On 2014/08/22 20:49:20, ddorwin wrote:
> Is this true? This function can only be called by unprefixed on a valid
> MediaKeys object, so this should never happen, right?

Initialize() is called from PpapiDecryptor constructor, so once MediaKeys adds
SetServerCertificate(), I think it would be possible for this to happen.

https://codereview.chromium.org/496143002/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.cc:413: void CdmAdapter::GetUsableKeyIds(uint32_t
promise_id,
On 2014/08/22 20:49:20, ddorwin wrote:
> Why did this move?

To line up with the order in the .h file. Moved back.

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

https://codereview.chromium.org/496143002/diff/1/media/cdm/ppapi/cdm_adapter....
media/cdm/ppapi/cdm_adapter.h:73: virtual void GetUsableKeyIds(uint32_t
promise_id,
On 2014/08/22 20:49:20, ddorwin wrote:
> ditto

Done.

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

https://codereview.chromium.org/496143002/diff/1/media/cdm/ppapi/cdm_wrapper....
media/cdm/ppapi/cdm_wrapper.h:45: virtual bool SetServerCertificate(uint32_t
promise_id,
On 2014/08/22 20:49:20, ddorwin wrote:
> Is there a reason this has a return value?
> Should it be removed when CDM_4/5 are no longer supported?
> I may have asked this before, but can we just reject the promise here instead?
> (I think the answer is that we don't have a host pointer, so no.)
> 
> Ditto on some of the other existing methods.

You are correct. Added comment to match other methods (which were updated in a
CL over the weekend).

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
File ppapi/api/private/ppb_content_decryptor_private.idl (right):

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
ppapi/api/private/ppb_content_decryptor_private.idl:49: * A promise has been
resolved by the CDM.
On 2014/08/22 20:49:21, ddorwin wrote:
> These versions that have specific purposes should probably have a
> differentiating part of the comment.

Done.

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
ppapi/api/private/ppb_content_decryptor_private.idl:54: *
<code>PP_VARTYPE_ARRAY</code> containing the session's usable key IDs.
On 2014/08/22 20:49:21, ddorwin wrote:
> Do we need to define the element type?

Not sure. I can't find any other IDL files returning PP_VARTYPE_ARRAY. Updated
comment.

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
ppapi/api/private/ppb_content_decryptor_private.idl:59: [in] PP_Var key_ids);
On 2014/08/22 20:49:21, ddorwin wrote:
> key_ids_array?

Done.

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
ppapi/api/private/ppb_content_decryptor_private.idl:96: *
<code>PP_VARTYPE_STRING</code> containing the session's ID attribute for
On 2014/08/22 20:49:21, ddorwin wrote:
> This sentence structure/description is appropriate for
> PromiseResolvedWithSession, which is delivering the ID for the attribute, but
> probably not these methods.

Done.

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppb_conten...
ppapi/api/private/ppb_content_decryptor_private.idl:116: *
<code>PP_VARTYPE_STRING</code> containing the session's ID attribute of
On 2014/08/22 20:49:21, ddorwin wrote:
> ditto here and below.

Done.

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppp_conten...
File ppapi/api/private/ppp_content_decryptor_private.idl (right):

https://codereview.chromium.org/496143002/diff/1/ppapi/api/private/ppp_conten...
ppapi/api/private/ppp_content_decryptor_private.idl:38: * rejected depending
upon the success or failure of setti ng the certificate.
On 2014/08/22 20:49:21, ddorwin wrote:
> setting

Done.

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
File ppapi/cpp/private/content_decryptor_private.cc (right):

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
ppapi/cpp/private/content_decryptor_private.cc:264: &GetUsableKeyIds,
On 2014/08/22 20:49:21, ddorwin wrote:
> ditto

Done.

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
ppapi/cpp/private/content_decryptor_private.cc:318: std::vector<uint8_t> entry =
key_ids[i];
On 2014/08/22 20:49:21, ddorwin wrote:
> const ref?

Done.

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
ppapi/cpp/private/content_decryptor_private.cc:321: key_ids_array.Set(i,
pp::Var(array_buffer));
On 2014/08/22 20:49:21, ddorwin wrote:
> Is there an Add() or Push()? It seems odd that you can set beyond the current
> length.

Nope. Added a call to SetLength() above to make it clearer, but still you can
set any element you want and the length will grow to accommodate it.

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
File ppapi/cpp/private/content_decryptor_private.h (right):

https://codereview.chromium.org/496143002/diff/1/ppapi/cpp/private/content_de...
ppapi/cpp/private/content_decryptor_private.h:49: virtual void
GetUsableKeyIds(uint32_t promise_id,
On 2014/08/22 20:49:21, ddorwin wrote:
> ditto

Done.

https://codereview.chromium.org/496143002/diff/1/ppapi/proxy/ppp_content_decr...
File ppapi/proxy/ppp_content_decryptor_private_proxy.cc (right):

https://codereview.chromium.org/496143002/diff/1/ppapi/proxy/ppp_content_decr...
ppapi/proxy/ppp_content_decryptor_private_proxy.cc:438: &GetUsableKeyIds,
On 2014/08/22 20:49:21, ddorwin wrote:
> ditto

Done.

[ddorwin, 2014-08-25 22:55:02]

https://codereview.chromium.org/496143002/diff/20001/content/renderer/pepper/...
File content/renderer/pepper/content_decryptor_delegate.cc (right):

https://codereview.chromium.org/496143002/diff/20001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.cc:659:
DCHECK(promise->Type() == media::CdmPromise::SIMPLE_TYPE);
promise && ?
If you can't do that, this could crash.

https://codereview.chromium.org/496143002/diff/20001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.cc:675:
DCHECK(promise->Type() == media::CdmPromise::NEW_SESSION_TYPE);
ditto

https://codereview.chromium.org/496143002/diff/20001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.cc:704:
DCHECK(promise->Type() == media::CdmPromise::KEY_IDS_TYPE);
ditto

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.cc
File media/base/cdm_promise.cc (right):

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.c...
media/base/cdm_promise.cc:67: return UNKNOWN_TYPE;
This is a symptom. Either the value should be provided to the constructor (best)
or this should be a pure virtual base method.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.c...
media/base/cdm_promise.cc:141: return SIMPLE_TYPE;
Per previous comments, move these to the constructors.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h
File media/base/cdm_promise.h (right):

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h...
media/base/cdm_promise.h:56: virtual void reject(MediaKeys::Exception
exception_code,
Not now, but this should be _R_eject.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h...
media/base/cdm_promise.h:60: virtual PromiseType Type();
Since this is a getter, it should be type().
Also, const function.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h...
media/base/cdm_promise.h:91: virtual void resolve(const T& result);
Not now, but this should be _R_esolve.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h...
media/base/cdm_promise.h:92: virtual PromiseType Type();
Do we need this and l60? Would it be better to initialize the value in the
constructor? (If so, we can make it non-virtual above.)
It should at least be OVERRIDE here.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h...
media/base/cdm_promise.h:117: virtual PromiseType Type();
ditto

https://codereview.chromium.org/496143002/diff/20001/media/cdm/ppapi/cdm_adap...
File media/cdm/ppapi/cdm_adapter.cc (right):

https://codereview.chromium.org/496143002/diff/20001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.cc:336: // called even if Initialize() failed.
On 2014/08/25 21:54:36, jrummell wrote:
> On 2014/08/22 20:49:20, ddorwin wrote:
> > Is this true? This function can only be called by unprefixed on a valid
> > MediaKeys object, so this should never happen, right?
> 
> Initialize() is called from PpapiDecryptor constructor, so once MediaKeys adds
> SetServerCertificate(), I think it would be possible for this to happen.

Possible in a code sense, but not possible from any path exposed to the user or
app, right?

[jrummell, 2014-08-26 00:19:23]

Updated.

https://codereview.chromium.org/496143002/diff/20001/content/renderer/pepper/...
File content/renderer/pepper/content_decryptor_delegate.cc (right):

https://codereview.chromium.org/496143002/diff/20001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.cc:659:
DCHECK(promise->Type() == media::CdmPromise::SIMPLE_TYPE);
On 2014/08/25 22:55:01, ddorwin wrote:
> promise && ?
> If you can't do that, this could crash.

Done.

https://codereview.chromium.org/496143002/diff/20001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.cc:675:
DCHECK(promise->Type() == media::CdmPromise::NEW_SESSION_TYPE);
On 2014/08/25 22:55:01, ddorwin wrote:
> ditto

Done.

https://codereview.chromium.org/496143002/diff/20001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.cc:704:
DCHECK(promise->Type() == media::CdmPromise::KEY_IDS_TYPE);
On 2014/08/25 22:55:01, ddorwin wrote:
> ditto

Done.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.cc
File media/base/cdm_promise.cc (right):

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.c...
media/base/cdm_promise.cc:67: return UNKNOWN_TYPE;
On 2014/08/25 22:55:01, ddorwin wrote:
> This is a symptom. Either the value should be provided to the constructor
(best)
> or this should be a pure virtual base method.

Done.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.c...
media/base/cdm_promise.cc:141: return SIMPLE_TYPE;
On 2014/08/25 22:55:01, ddorwin wrote:
> Per previous comments, move these to the constructors.

Since these are templates, hard to do. Changed to represent parameter type.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h
File media/base/cdm_promise.h (right):

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h...
media/base/cdm_promise.h:56: virtual void reject(MediaKeys::Exception
exception_code,
On 2014/08/25 22:55:01, ddorwin wrote:
> Not now, but this should be _R_eject.

Acknowledged.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h...
media/base/cdm_promise.h:60: virtual PromiseType Type();
On 2014/08/25 22:55:01, ddorwin wrote:
> Since this is a getter, it should be type().
> Also, const function.

Done.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h...
media/base/cdm_promise.h:91: virtual void resolve(const T& result);
On 2014/08/25 22:55:01, ddorwin wrote:
> Not now, but this should be _R_esolve.

Acknowledged.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h...
media/base/cdm_promise.h:92: virtual PromiseType Type();
On 2014/08/25 22:55:01, ddorwin wrote:
> Do we need this and l60? Would it be better to initialize the value in the
> constructor? (If so, we can make it non-virtual above.)
> It should at least be OVERRIDE here.

Since this is a template, I can't find any easy way to set the type() easily.
But added OVERRIDE.

https://codereview.chromium.org/496143002/diff/20001/media/base/cdm_promise.h...
media/base/cdm_promise.h:117: virtual PromiseType Type();
On 2014/08/25 22:55:01, ddorwin wrote:
> ditto

Acknowledged.

https://codereview.chromium.org/496143002/diff/20001/media/cdm/ppapi/cdm_adap...
File media/cdm/ppapi/cdm_adapter.cc (right):

https://codereview.chromium.org/496143002/diff/20001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.cc:336: // called even if Initialize() failed.
On 2014/08/25 22:55:01, ddorwin wrote:
> On 2014/08/25 21:54:36, jrummell wrote:
> > On 2014/08/22 20:49:20, ddorwin wrote:
> > > Is this true? This function can only be called by unprefixed on a valid
> > > MediaKeys object, so this should never happen, right?
> > 
> > Initialize() is called from PpapiDecryptor constructor, so once MediaKeys
adds
> > SetServerCertificate(), I think it would be possible for this to happen.
> 
> Possible in a code sense, but not possible from any path exposed to the user
or
> app, right?

It appears possible in both prefixed and unprefixed EME. Opened
http://crbug.com/407435 to get Initialize() to report it's failure.

[ddorwin, 2014-08-26 00:42:40]

lgtm with naming fix. Thanks.

https://codereview.chromium.org/496143002/diff/40001/media/base/cdm_promise.h
File media/base/cdm_promise.h (right):

https://codereview.chromium.org/496143002/diff/40001/media/base/cdm_promise.h...
media/base/cdm_promise.h:59: virtual ResolveParameterType resolveParameterType()
const = 0;
This is Blink style. Even though it's not a member, we can probably use
getter_style(). But, you could also just name it GetResolveParameterType().

[jrummell, 2014-08-26 17:02:45]

jrummell@chromium.org changed reviewers:
+ dcheng@chromium.org, dmichael@chromium.org

[jrummell, 2014-08-26 17:02:45]

+dcheng@ for ppapi_messages.h
+dmichael@ for ppapi/* and content/renderer/pepper/*

https://codereview.chromium.org/496143002/diff/40001/media/base/cdm_promise.h
File media/base/cdm_promise.h (right):

https://codereview.chromium.org/496143002/diff/40001/media/base/cdm_promise.h...
media/base/cdm_promise.h:59: virtual ResolveParameterType resolveParameterType()
const = 0;
On 2014/08/26 00:42:40, ddorwin wrote:
> This is Blink style. Even though it's not a member, we can probably use
> getter_style(). But, you could also just name it GetResolveParameterType().

changed to GetResolveParameterType().

[jrummell, 2014-08-26 17:46:23]

jrummell@chromium.org changed reviewers:
+ palmer@chromium.org
- dcheng@chromium.org

[jrummell, 2014-08-26 17:46:23]

+palmer@ for ppapi_messages.h
-dcheng@ since he's OOO

[palmer, 2014-08-26 19:09:21]

https://codereview.chromium.org/496143002/diff/60001/content/renderer/pepper/...
File content/renderer/pepper/content_decryptor_delegate.cc (right):

https://codereview.chromium.org/496143002/diff/60001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.cc:343: certificate_length,
certificate);
Check |certificate_length| for sanity first: minimum size, maximum size?

https://codereview.chromium.org/496143002/diff/60001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.cc:694: DCHECK(key_ids);
Nit: Might as well move this DCHECK up a line, to avoid constructing
|key_ids_vector|. I like to have the check right by the thing it is checking.

https://codereview.chromium.org/496143002/diff/60001/content/renderer/pepper/...
File content/renderer/pepper/content_decryptor_delegate.h (right):

https://codereview.chromium.org/496143002/diff/60001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.h:60: int certificate_length,
http://www.chromium.org/Home/chromium-security/education/security-tips-for-ip....

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

https://codereview.chromium.org/496143002/diff/60001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.cc:330: const uint32_t server_certificate_size =
server_certificate.ByteLength();
Ahh, uint32_t. Use that everywhere for these sizes, then.

https://codereview.chromium.org/496143002/diff/60001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.cc:333: PP_DCHECK(server_certificate_size > 0);
These 2 checks are essentially the same — an unsigned integer is always >= 0.

Define some maximum size (and, is 0 really the minimum size?) in a header file
that both consumer and producer can include and use in their run-time checks.

Rather than PP_CHECK, these should probably result in RejectPromise.

https://codereview.chromium.org/496143002/diff/60001/ppapi/api/private/ppb_co...
File ppapi/api/private/ppb_content_decryptor_private.idl (right):

https://codereview.chromium.org/496143002/diff/60001/ppapi/api/private/ppb_co...
ppapi/api/private/ppb_content_decryptor_private.idl:55: *
<code>PP_VARTYPE_ARRAYBUFFER</code> that are the session's usable key IDs.
What is a "key ID", exactly? Is it a cryptographic hash of a key, a 32-bit
integer, et c.? I'm wondering if there isn't some better way to serialize and
type-check them.

https://codereview.chromium.org/496143002/diff/60001/ppapi/api/private/ppp_co...
File ppapi/api/private/ppp_content_decryptor_private.idl (right):

https://codereview.chromium.org/496143002/diff/60001/ppapi/api/private/ppp_co...
ppapi/api/private/ppp_content_decryptor_private.idl:41: *
<code>PP_VARTYPE_ARRAYBUFFER</code> containing the certificate to be used.
What format is the certificate in?

https://codereview.chromium.org/496143002/diff/60001/ppapi/cpp/private/conten...
File ppapi/cpp/private/content_decryptor_private.h (right):

https://codereview.chromium.org/496143002/diff/60001/ppapi/cpp/private/conten...
ppapi/cpp/private/content_decryptor_private.h:50: const std::string&
web_session_id) = 0;
What is the format of these web session IDs? (Again, my concern is about ideal
serialization format and/or stronger sanity/type checking.)

https://codereview.chromium.org/496143002/diff/60001/ppapi/proxy/ppapi_messag...
File ppapi/proxy/ppapi_messages.h (right):

https://codereview.chromium.org/496143002/diff/60001/ppapi/proxy/ppapi_messag...
ppapi/proxy/ppapi_messages.h:763: ppapi::proxy::SerializedVar /* certificate,
ArrayBuffer */)
http://www.chromium.org/Home/chromium-security/education/security-tips-for-ip....

Generally throughout this CL.

[dmichael (off chromium), 2014-08-26 21:51:46]

pepper stuff lgtm

https://codereview.chromium.org/496143002/diff/60001/ppapi/cpp/private/conten...
File ppapi/cpp/private/content_decryptor_private.cc (right):

https://codereview.chromium.org/496143002/diff/60001/ppapi/cpp/private/conten...
ppapi/cpp/private/content_decryptor_private.cc:322: key_ids_array.Set(i,
pp::Var(array_buffer));
nit(?): I would have thought it was unnecessary to wrap array_buffer in pp::Var,
since it is a pp::Var via inheritance.

https://codereview.chromium.org/496143002/diff/60001/ppapi/cpp/private/conten...
ppapi/cpp/private/content_decryptor_private.cc:362: bool
has_additional_usable_key) {
nit: I think these could both fit on 361. I think git cl format would do it that
way. You might see if you can just run it on this patch.

[jrummell, 2014-08-29 22:35:35]

Updated. PS5 is the changes as requested, PS6 is a rebase to pick up recent
changes to cdm_promise (which were pulled out and submitted separately).

dmichael@ -- can you please re-review the changes in
ppapi/proxy/ppb_instance_proxy and
ppapi/proxy/ppp_content_decryptor_private_proxy as there is a bunch of new
Pepper code to handle the changed IPC message types.

https://codereview.chromium.org/496143002/diff/60001/content/renderer/pepper/...
File content/renderer/pepper/content_decryptor_delegate.cc (right):

https://codereview.chromium.org/496143002/diff/60001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.cc:343: certificate_length,
certificate);
On 2014/08/26 19:09:20, Chromium Palmer wrote:
> Check |certificate_length| for sanity first: minimum size, maximum size?

Done.

https://codereview.chromium.org/496143002/diff/60001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.cc:694: DCHECK(key_ids);
On 2014/08/26 19:09:20, Chromium Palmer wrote:
> Nit: Might as well move this DCHECK up a line, to avoid constructing
> |key_ids_vector|. I like to have the check right by the thing it is checking.

Done.

https://codereview.chromium.org/496143002/diff/60001/content/renderer/pepper/...
File content/renderer/pepper/content_decryptor_delegate.h (right):

https://codereview.chromium.org/496143002/diff/60001/content/renderer/pepper/...
content/renderer/pepper/content_decryptor_delegate.h:60: int certificate_length,
On 2014/08/26 19:09:20, Chromium Palmer wrote:
>
http://www.chromium.org/Home/chromium-security/education/security-tips-for-ip....

Converted to uint32_t.

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

https://codereview.chromium.org/496143002/diff/60001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.cc:330: const uint32_t server_certificate_size =
server_certificate.ByteLength();
On 2014/08/26 19:09:20, Chromium Palmer wrote:
> Ahh, uint32_t. Use that everywhere for these sizes, then.

Done.

https://codereview.chromium.org/496143002/diff/60001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.cc:333: PP_DCHECK(server_certificate_size > 0);
On 2014/08/26 19:09:20, Chromium Palmer wrote:
> These 2 checks are essentially the same — an unsigned integer is always >= 0.
> 
> Define some maximum size (and, is 0 really the minimum size?) in a header file
> that both consumer and producer can include and use in their run-time checks.
> 
> Rather than PP_CHECK, these should probably result in RejectPromise.

Done.

https://codereview.chromium.org/496143002/diff/60001/ppapi/api/private/ppb_co...
File ppapi/api/private/ppb_content_decryptor_private.idl (right):

https://codereview.chromium.org/496143002/diff/60001/ppapi/api/private/ppb_co...
ppapi/api/private/ppb_content_decryptor_private.idl:55: *
<code>PP_VARTYPE_ARRAYBUFFER</code> that are the session's usable key IDs.
On 2014/08/26 19:09:20, Chromium Palmer wrote:
> What is a "key ID", exactly? Is it a cryptographic hash of a key, a 32-bit
> integer, et c.? I'm wondering if there isn't some better way to serialize and
> type-check them.

In
https://dvcs.w3.org/hg/html-media/raw-file/default/encrypted-media/encrypted-...
key IDs are not specified as anything in particular, and get returned to callers
as ArrayBuffers. WebM (http://www.matroska.org/technical/specs/index.html) has
ContentSigKeyID as "This is the ID of the private key the data was signed with."
but other than that is just a binary element. Added a size restriction.

https://codereview.chromium.org/496143002/diff/60001/ppapi/api/private/ppp_co...
File ppapi/api/private/ppp_content_decryptor_private.idl (right):

https://codereview.chromium.org/496143002/diff/60001/ppapi/api/private/ppp_co...
ppapi/api/private/ppp_content_decryptor_private.idl:41: *
<code>PP_VARTYPE_ARRAYBUFFER</code> containing the certificate to be used.
On 2014/08/26 19:09:21, Chromium Palmer wrote:
> What format is the certificate in?

The EME spec states "The contents of serverCertificate are Key System-specific."
Widevine uses a proprietary protobuf-based format.

https://codereview.chromium.org/496143002/diff/60001/ppapi/cpp/private/conten...
File ppapi/cpp/private/content_decryptor_private.cc (right):

https://codereview.chromium.org/496143002/diff/60001/ppapi/cpp/private/conten...
ppapi/cpp/private/content_decryptor_private.cc:322: key_ids_array.Set(i,
pp::Var(array_buffer));
On 2014/08/26 21:51:46, dmichael wrote:
> nit(?): I would have thought it was unnecessary to wrap array_buffer in
pp::Var,
> since it is a pp::Var via inheritance.

Done.

https://codereview.chromium.org/496143002/diff/60001/ppapi/cpp/private/conten...
ppapi/cpp/private/content_decryptor_private.cc:362: bool
has_additional_usable_key) {
On 2014/08/26 21:51:46, dmichael wrote:
> nit: I think these could both fit on 361. I think git cl format would do it
that
> way. You might see if you can just run it on this patch.

They do. However, clang-format puts them back onto separate lines.

https://codereview.chromium.org/496143002/diff/60001/ppapi/cpp/private/conten...
File ppapi/cpp/private/content_decryptor_private.h (right):

https://codereview.chromium.org/496143002/diff/60001/ppapi/cpp/private/conten...
ppapi/cpp/private/content_decryptor_private.h:50: const std::string&
web_session_id) = 0;
On 2014/08/26 19:09:21, Chromium Palmer wrote:
> What is the format of these web session IDs? (Again, my concern is about ideal
> serialization format and/or stronger sanity/type checking.)

From
https://dvcs.w3.org/hg/html-media/raw-file/default/encrypted-media/encrypted-...,
these are "A Session ID is a unique string identifier generated by the user
agent or CDM that can be used by the application to identify MediaKeySession
objects." From the Android IPC we limit them to 512 characters, so included here
as well.

https://codereview.chromium.org/496143002/diff/60001/ppapi/proxy/ppapi_messag...
File ppapi/proxy/ppapi_messages.h (right):

https://codereview.chromium.org/496143002/diff/60001/ppapi/proxy/ppapi_messag...
ppapi/proxy/ppapi_messages.h:763: ppapi::proxy::SerializedVar /* certificate,
ArrayBuffer */)
On 2014/08/26 19:09:21, Chromium Palmer wrote:
>
http://www.chromium.org/Home/chromium-security/education/security-tips-for-ip....
> 
> Generally throughout this CL.

I just copied what is used for the other EME methods. Changed for the messages
touched by this CL.

[palmer, 2014-08-30 00:49:49]

LGTM.

[jrummell, 2014-09-04 20:50:12]

ping dmichael@: please re-review the changes in
ppapi/proxy/ppb_instance_proxy.cc and
ppapi/proxy/ppp_content_decryptor_private_proxy.cc as there is a bunch of new
Pepper code to handle the changed IPC message types.

[dmichael (off chromium), 2014-09-04 21:17:11]

This has some ref-count leaks; otherwise I'm fine with the changes.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppb_instanc...
File ppapi/proxy/ppb_instance_proxy.cc (right):

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppb_instanc...
ppapi/proxy/ppb_instance_proxy.cc:1298: instance, promise_id,
key_ids_array->GetPPVar());
GetPPVar increments the reference count, but you don't decrement it after the
call (which is what we should do when passing a PP_Var parameter to the plugin).

How about using ScopedPPVar? E.g., before calling PromiseResolvedWithKeyIds, do:
ScopedPPVar key_var(ScopedPPVar::PassRef(), key_ids_array->GetPPVar());
Then pass it as "key_var.get()". That way, when the ScopedPPVar goes out of
scope, it will release the reference count for you.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppb_instanc...
ppapi/proxy/ppb_instance_proxy.cc:1346:
StringVar::StringToPPVar(web_session_id),
Same problem here; StringToPPVar increments the ref-count.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppb_instanc...
ppapi/proxy/ppb_instance_proxy.cc:1360: instance,
StringVar::StringToPPVar(web_session_id), new_expiry_time);
ditto

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppp_content...
File ppapi/proxy/ppp_content_decryptor_private_proxy.cc (right):

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppp_content...
ppapi/proxy/ppp_content_decryptor_private_proxy.cc:576:
server_certificate.size(), server_certificate.data());
Same problem as the PPB_Instance proxy; you never release the ref-count on
server_certificate_buffer. Please use ScopedPPVar with PassRef.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppp_content...
ppapi/proxy/ppp_content_decryptor_private_proxy.cc:637:
StringVar::StringToPPVar(web_session_id));
ditto

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppp_content...
ppapi/proxy/ppp_content_decryptor_private_proxy.cc:649:
StringVar::StringToPPVar(web_session_id));
ditto

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppp_content...
ppapi/proxy/ppp_content_decryptor_private_proxy.cc:661:
StringVar::StringToPPVar(web_session_id));
ditto

[jrummell, 2014-09-04 22:56:57]

Patchset #7 (id:120001) has been deleted

[jrummell, 2014-09-04 23:05:03]

Updated.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppb_instanc...
File ppapi/proxy/ppb_instance_proxy.cc (right):

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppb_instanc...
ppapi/proxy/ppb_instance_proxy.cc:1298: instance, promise_id,
key_ids_array->GetPPVar());
On 2014/09/04 21:17:11, dmichael wrote:
> GetPPVar increments the reference count, but you don't decrement it after the
> call (which is what we should do when passing a PP_Var parameter to the
plugin).
> 
> How about using ScopedPPVar? E.g., before calling PromiseResolvedWithKeyIds,
do:
> ScopedPPVar key_var(ScopedPPVar::PassRef(), key_ids_array->GetPPVar());
> Then pass it as "key_var.get()". That way, when the ScopedPPVar goes out of
> scope, it will release the reference count for you.

Done.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppb_instanc...
ppapi/proxy/ppb_instance_proxy.cc:1346:
StringVar::StringToPPVar(web_session_id),
On 2014/09/04 21:17:11, dmichael wrote:
> Same problem here; StringToPPVar increments the ref-count.

Done.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppb_instanc...
ppapi/proxy/ppb_instance_proxy.cc:1360: instance,
StringVar::StringToPPVar(web_session_id), new_expiry_time);
On 2014/09/04 21:17:11, dmichael wrote:
> ditto

Done.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppp_content...
File ppapi/proxy/ppp_content_decryptor_private_proxy.cc (right):

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppp_content...
ppapi/proxy/ppp_content_decryptor_private_proxy.cc:576:
server_certificate.size(), server_certificate.data());
On 2014/09/04 21:17:11, dmichael wrote:
> Same problem as the PPB_Instance proxy; you never release the ref-count on
> server_certificate_buffer. Please use ScopedPPVar with PassRef.

Done.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppp_content...
ppapi/proxy/ppp_content_decryptor_private_proxy.cc:637:
StringVar::StringToPPVar(web_session_id));
On 2014/09/04 21:17:11, dmichael wrote:
> ditto

Done.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppp_content...
ppapi/proxy/ppp_content_decryptor_private_proxy.cc:649:
StringVar::StringToPPVar(web_session_id));
On 2014/09/04 21:17:11, dmichael wrote:
> ditto

Done.

https://codereview.chromium.org/496143002/diff/100001/ppapi/proxy/ppp_content...
ppapi/proxy/ppp_content_decryptor_private_proxy.cc:661:
StringVar::StringToPPVar(web_session_id));
On 2014/09/04 21:17:11, dmichael wrote:
> ditto

Done.

[jrummell, 2014-09-04 23:06:01]

The CQ bit was checked by jrummell@chromium.org

[dmichael (off chromium), 2014-09-04 23:08:42]

lgtm

[commit-bot: I haz the power, 2014-09-04 23:09:45]

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

[commit-bot: I haz the power, 2014-09-05 00:11:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-05 00:11:29]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/51858)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jrummell, 2014-09-05 17:51:50]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-09-05 17:52:06]

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

[commit-bot: I haz the power, 2014-09-05 18:11:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-05 18:11:48]

Try jobs failed on following builders:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jrummell, 2014-09-05 18:40:53]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-09-05 18:43:43]

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

[commit-bot: I haz the power, 2014-09-05 19:05:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-05 19:05:13]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[jrummell, 2014-09-05 22:21:26]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-09-05 22:22:44]

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

[commit-bot: I haz the power, 2014-09-06 00:25:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-06 00:25:43]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[jrummell, 2014-09-06 00:29:54]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-09-06 00:30:47]

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

[commit-bot: I haz the power, 2014-09-06 10:24:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-06 10:24:04]

Exceeded time limit waiting for builds to trigger.

[ddorwin, 2014-09-08 00:37:31]

The CQ bit was checked by ddorwin@chromium.org

[commit-bot: I haz the power, 2014-09-08 00:38:07]

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

[commit-bot: I haz the power, 2014-09-08 00:39:31]

Committed patchset #10 (id:200001) as 653406e625fc3c99bd2e3d6ca847fa976eda5e48

[commit-bot: I haz the power, 2014-09-10 03:44:34]

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