Encrypted Media: Allows empty key message to be fired.

Encrypted Media: Allows empty key message to be fired.

This CL also changes the expected behavior of GenerateKeyRequest() of CleryKey key system when init_data is null. Previously it returns MediaKeyException (from render side) or fires KeyError (from plugin side). Now it fires an empty KeyMessage regardless. In the future the spec should be clear about this.

BUG=163785
TEST=media Layout test; media_unittests; content_browsertests


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

[xhwang, 2012-12-04 00:27:12]

PTAL!

[xhwang, 2012-12-04 00:31:11]

FWIW, most of this CL will be gone when we pass back the message in a string
instead of in a scoped_array/pp::Buffer_Dev/RR_Resource.

[ddorwin, 2012-12-04 01:44:09]

LGTM % nits

https://codereview.chromium.org/11348365/diff/1/webkit/media/crypto/ppapi/cle...
File webkit/media/crypto/ppapi/clear_key_cdm.cc (right):

https://codereview.chromium.org/11348365/diff/1/webkit/media/crypto/ppapi/cle...
webkit/media/crypto/ppapi/clear_key_cdm.cc:247: key_request->set_message(NULL);
Is this not the default?

https://codereview.chromium.org/11348365/diff/1/webkit/plugins/ppapi/content_...
File webkit/plugins/ppapi/content_decryptor_delegate.cc (right):

https://codereview.chromium.org/11348365/diff/1/webkit/plugins/ppapi/content_...
webkit/plugins/ppapi/content_decryptor_delegate.cc:670: LOG(ERROR) <<
"ContentDecryptorDelegate::KeyMessage";
Did you mean to keep this here? Should at least not be ERROR.

[xhwang, 2012-12-04 01:58:51]

dmichael: Could you please do an OWNERS review?

https://codereview.chromium.org/11348365/diff/1/webkit/media/crypto/ppapi/cle...
File webkit/media/crypto/ppapi/clear_key_cdm.cc (right):

https://codereview.chromium.org/11348365/diff/1/webkit/media/crypto/ppapi/cle...
webkit/media/crypto/ppapi/clear_key_cdm.cc:247: key_request->set_message(NULL);
On 2012/12/04 01:44:10, ddorwin wrote:
> Is this not the default?

You are right. Added DCHECK and removed "else".

https://codereview.chromium.org/11348365/diff/1/webkit/plugins/ppapi/content_...
File webkit/plugins/ppapi/content_decryptor_delegate.cc (right):

https://codereview.chromium.org/11348365/diff/1/webkit/plugins/ppapi/content_...
webkit/plugins/ppapi/content_decryptor_delegate.cc:670: LOG(ERROR) <<
"ContentDecryptorDelegate::KeyMessage";
On 2012/12/04 01:44:10, ddorwin wrote:
> Did you mean to keep this here? Should at least not be ERROR.

oops, removed.

[dmichael (off chromium), 2012-12-04 15:44:34]

lgtm

https://codereview.chromium.org/11348365/diff/4002/ppapi/proxy/ppb_instance_p...
File ppapi/proxy/ppb_instance_proxy.cc (right):

https://codereview.chromium.org/11348365/diff/4002/ppapi/proxy/ppb_instance_p...
ppapi/proxy/ppb_instance_proxy.cc:543: PP_Resource host_resource = 0;
Accepting 0 for the PP_Resource is a change to your API. You should probably
update your documentation.

[xhwang, 2012-12-04 18:33:36]

https://codereview.chromium.org/11348365/diff/4002/ppapi/proxy/ppb_instance_p...
File ppapi/proxy/ppb_instance_proxy.cc (right):

https://codereview.chromium.org/11348365/diff/4002/ppapi/proxy/ppb_instance_p...
ppapi/proxy/ppb_instance_proxy.cc:543: PP_Resource host_resource = 0;
On 2012/12/04 15:44:34, dmichael wrote:
> Accepting 0 for the PP_Resource is a change to your API. You should probably
> update your documentation.

Done.

[commit-bot: I haz the power, 2012-12-04 20:33:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/11348365/2006

[commit-bot: I haz the power, 2012-12-04 23:39:02]

Change committed as 171081