Allow null buffer resources to be returned through the PPB_ContentDecryptor_Private proxy.

Allow null buffer resources to be returned through the PPB_ContentDecryptor_Private proxy.

This fixes a bug where DeliverBlock() would never be called when no key was available and running out-of-process.
Also fixes an issue in PpapiDecryptor::InitializeVideoDecoder() and adds logging that is useful in tracking down these issues.

TEST=Run the CDM plugin out-of-process and play before the key is added then add the key. Playback should succeed. Upcoming change to encrypted_media_browsertest will make it run OOP.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=161709

[Tom Finegan, 2012-10-11 23:57:56]

http://codereview.chromium.org/11106004/diff/2001/ppapi/proxy/ppb_instance_pr...
File ppapi/proxy/ppb_instance_proxy.cc (right):

http://codereview.chromium.org/11106004/diff/2001/ppapi/proxy/ppb_instance_pr...
ppapi/proxy/ppb_instance_proxy.cc:560: // TODO(tomfinegan): Handle null
decrypted_frame after landing other patches.
s/decrypted_frame/decrypted_samples/

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi/c...
File webkit/media/crypto/ppapi/cdm_wrapper.cc (right):

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi/c...
webkit/media/crypto/ppapi/cdm_wrapper.cc:624: DVLOG(2) << "DeliverBlock() -
status " << status;
This is going to cause link failures on windows builds without adding a base
dependency in gyp.

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi/c...
webkit/media/crypto/ppapi/cdm_wrapper.cc:660: DVLOG(2) << "DeliverFrame() -
status " << status;
Ditto.

[xhwang, 2012-10-12 00:06:44]

lg % mostly nits. you need to address tom's comments.

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi/c...
File webkit/media/crypto/ppapi/cdm_wrapper.cc (right):

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi/c...
webkit/media/crypto/ppapi/cdm_wrapper.cc:624: DVLOG(2) << "DeliverBlock() -
status " << status;
On 2012/10/11 23:57:56, Tom Finegan wrote:
> This is going to cause link failures on windows builds without adding a base
> dependency in gyp.

Yeah, I think we are trying to remove dependency on base/

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi_d...
File webkit/media/crypto/ppapi_decryptor.cc (right):

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi_d...
webkit/media/crypto/ppapi_decryptor.cc:136: key_added_cb_ = key_added_cb;
thanks!

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/proxy_d...
File webkit/media/crypto/proxy_decryptor.cc (right):

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/proxy_d...
webkit/media/crypto/proxy_decryptor.cc:186: DVLOG(1) << "CancelDecrypt()";
why this is level one?

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/proxy_d...
webkit/media/crypto/proxy_decryptor.cc:306: DVLOG(2) << "OnBufferDecrypted()";
why this is level 2?

[ddorwin, 2012-10-12 00:21:16]

http://codereview.chromium.org/11106004/diff/2001/ppapi/proxy/ppb_instance_pr...
File ppapi/proxy/ppb_instance_proxy.cc (right):

http://codereview.chromium.org/11106004/diff/2001/ppapi/proxy/ppb_instance_pr...
ppapi/proxy/ppb_instance_proxy.cc:560: // TODO(tomfinegan): Handle null
decrypted_frame after landing other patches.
On 2012/10/11 23:57:56, Tom Finegan wrote:
> s/decrypted_frame/decrypted_samples/

Done.

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi/c...
File webkit/media/crypto/ppapi/cdm_wrapper.cc (right):

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi/c...
webkit/media/crypto/ppapi/cdm_wrapper.cc:624: DVLOG(2) << "DeliverBlock() -
status " << status;
On 2012/10/12 00:06:44, xhwang wrote:
> On 2012/10/11 23:57:56, Tom Finegan wrote:
> > This is going to cause link failures on windows builds without adding a base
> > dependency in gyp.
> 
> Yeah, I think we are trying to remove dependency on base/

Done.

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi/c...
webkit/media/crypto/ppapi/cdm_wrapper.cc:624: DVLOG(2) << "DeliverBlock() -
status " << status;
On 2012/10/11 23:57:56, Tom Finegan wrote:
> This is going to cause link failures on windows builds without adding a base
> dependency in gyp.

Done.

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi/c...
webkit/media/crypto/ppapi/cdm_wrapper.cc:660: DVLOG(2) << "DeliverFrame() -
status " << status;
On 2012/10/11 23:57:56, Tom Finegan wrote:
> Ditto.

Done.

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi_d...
File webkit/media/crypto/ppapi_decryptor.cc (right):

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/ppapi_d...
webkit/media/crypto/ppapi_decryptor.cc:136: key_added_cb_ = key_added_cb;
On 2012/10/12 00:06:44, xhwang wrote:
> thanks!

Done.

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/proxy_d...
File webkit/media/crypto/proxy_decryptor.cc (right):

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/proxy_d...
webkit/media/crypto/proxy_decryptor.cc:186: DVLOG(1) << "CancelDecrypt()";
On 2012/10/12 00:06:44, xhwang wrote:
> why this is level one?

Because it is rarer and much more interesting than every block (see below).

http://codereview.chromium.org/11106004/diff/2001/webkit/media/crypto/proxy_d...
webkit/media/crypto/proxy_decryptor.cc:306: DVLOG(2) << "OnBufferDecrypted()";
On 2012/10/12 00:06:44, xhwang wrote:
> why this is level 2?

So we can get level 1 logging without getting spammed for every block.

[xhwang, 2012-10-12 00:24:33]

lgtm

[ddorwin, 2012-10-12 00:25:55]

brettw, Please OWNERS review the two *ppapi/* files.

[Tom Finegan (google), 2012-10-12 00:30:06]

On 2012/10/12 00:25:55, ddorwin wrote:
> brettw, Please OWNERS review the two *ppapi/* files.

lgtm

[brettw, 2012-10-12 19:44:06]

https://codereview.chromium.org/11106004/diff/1013/webkit/media/crypto/proxy_...
File webkit/media/crypto/proxy_decryptor.cc (right):

https://codereview.chromium.org/11106004/diff/1013/webkit/media/crypto/proxy_...
webkit/media/crypto/proxy_decryptor.cc:307: DVLOG(2) << "OnBufferDecrypted() -
status: " << status;
I don't undrestand these DVLOG priorities. They seem random. Should we use (1)
everywhere or is there some logic behind the 1, 2, and 3 that you actually use?

[ddorwin, 2012-10-12 19:50:18]

On 2012/10/12 19:44:06, brettw wrote:
>
https://codereview.chromium.org/11106004/diff/1013/webkit/media/crypto/proxy_...
> File webkit/media/crypto/proxy_decryptor.cc (right):
> 
>
https://codereview.chromium.org/11106004/diff/1013/webkit/media/crypto/proxy_...
> webkit/media/crypto/proxy_decryptor.cc:307: DVLOG(2) << "OnBufferDecrypted() -
> status: " << status;
> I don't undrestand these DVLOG priorities. They seem random. Should we use (1)
> everywhere or is there some logic behind the 1, 2, and 3 that you actually
use?

It's all very intentional (at least the ones in this CL). 1 shouldn't be too
noisy but should capture the important things, such as init and cancel. 2 is
being called for each frame. 3 is some more noise about each frame. The main
reason for 2 and 3 is that there are some additional logs  that may be useful
but usually aren't, so 2 would be sufficient in most cases.
In reality, it seems people seem to enable 1 or 3 when working on something.

[brettw, 2012-10-12 23:25:33]

lgtm rubberstamp. I'm usually very anti-DVLOG so I'd remove them as soon as this
is a bit more stable.

[commit-bot: I haz the power, 2012-10-12 23:35:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11106004/1013

[commit-bot: I haz the power, 2012-10-13 03:55:34]

Change committed as 161709