Pepper: Add some basic tests for PPB_Audio.

Pepper: Add some basic tests for PPB_Audio.

Also, fix a bug in the proxy (also for audio input) -- it should fail if the
callback provided is null.


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

[dmichael (off chromium), 2011-11-30 22:53:48]

http://codereview.chromium.org/8749017/diff/2001/ppapi/tests/test_audio.cc
File ppapi/tests/test_audio.cc (right):

http://codereview.chromium.org/8749017/diff/2001/ppapi/tests/test_audio.cc#ne...
ppapi/tests/test_audio.cc:9: #include "base/basictypes.h"  // For |arraysize()|.
Is it okay to use 'base' in the tests? I've been avoiding it. Can you check with
brettw or piman? I would love to use arraysize too (and a ton of stuff from
memory and synchronization). I do see this isn't the first include of something
simple from base.

I would think we'd want to keep the tests 'clean' of any chromium dependencies,
if we hope for other browsers to ever implement ppapi.

[viettrungluu, 2011-11-30 23:05:09]

http://codereview.chromium.org/8749017/diff/2001/ppapi/tests/test_audio.cc
File ppapi/tests/test_audio.cc (right):

http://codereview.chromium.org/8749017/diff/2001/ppapi/tests/test_audio.cc#ne...
ppapi/tests/test_audio.cc:9: #include "base/basictypes.h"  // For |arraysize()|.
On 2011/11/30 22:53:49, dmichael wrote:
> Is it okay to use 'base' in the tests? I've been avoiding it. Can you check
with
> brettw or piman? I would love to use arraysize too (and a ton of stuff from
> memory and synchronization). I do see this isn't the first include of
something
> simple from base.
> 
> I would think we'd want to keep the tests 'clean' of any chromium
dependencies,
> if we hope for other browsers to ever implement ppapi.

Right. I think the answer is we should avoid base. I'll file a bug. And clean
stuff up.

[viettrungluu, 2011-11-30 23:47:10]

On 2011/11/30 23:05:09, viettrungluu wrote:
> http://codereview.chromium.org/8749017/diff/2001/ppapi/tests/test_audio.cc
> File ppapi/tests/test_audio.cc (right):
> 
>
http://codereview.chromium.org/8749017/diff/2001/ppapi/tests/test_audio.cc#ne...
> ppapi/tests/test_audio.cc:9: #include "base/basictypes.h"  // For
|arraysize()|.
> On 2011/11/30 22:53:49, dmichael wrote:
> > Is it okay to use 'base' in the tests? I've been avoiding it. Can you check
> with
> > brettw or piman? I would love to use arraysize too (and a ton of stuff from
> > memory and synchronization). I do see this isn't the first include of
> something
> > simple from base.
> > 
> > I would think we'd want to keep the tests 'clean' of any chromium
> dependencies,
> > if we hope for other browsers to ever implement ppapi.
> 
> Right. I think the answer is we should avoid base. I'll file a bug. And clean
> stuff up.

(P.S. This conclusion was reached after talking to brettw.)

[dmichael (off chromium), 2011-11-30 23:49:36]

So, assuming you take that include out, LGTM

(seems easy enough to work around in this case, and there's no need to make
things worse. I just started a CL to exclude 'base' and update the tests that
are doing this wrong)

[viettrungluu, 2011-11-30 23:50:56]

On 2011/11/30 23:49:36, dmichael wrote:
> So, assuming you take that include out, LGTM
> 
> (seems easy enough to work around in this case, and there's no need to make
> things worse. I just started a CL to exclude 'base' and update the tests that
> are doing this wrong)

Thanks. On the bright side, it seemed to me that the majority of base includes
were unneeded anyway. (I don't think base is even linked in.)

[commit-bot: I haz the power, 2011-11-30 23:51:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/viettrungluu@chromium.org/8749017/7001

[viettrungluu, 2011-11-30 23:52:48]

On 2011/11/30 23:51:37, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>
https://chromium-status.appspot.com/cq/viettrungluu%40chromium.org/8749017/7001

Abort, abort! This will conflict with my other CL (in the cq already) when it
tries to commit.... Sigh.

[commit-bot: I haz the power, 2011-12-01 01:01:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/viettrungluu@chromium.org/8749017/6002

[commit-bot: I haz the power, 2011-12-01 02:38:27]

Change committed as 112379