Add RunCallback to invoke a callback parameter in unittests.

Add RunCallback to invoke a callback parameter in unittests.

In gmock, ::testing::InvokeArgument doesn't support chromium base::Callback. Adding customized templated action RunCallback to support this.

Also added new matchers for base::callback.

BUG=none
TEST=Added unittests to test the new matchers and actions. Updated media_unittests to use the new matchers and actions.


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

[xhwang, 2012-11-08 09:16:24]

As discussed in
https://groups.google.com/a/chromium.org/forum/?fromgroups=#!topic/chromium-d...

I'll update more tests if this CL looks good. PTAL!

[scherkus (not reviewing), 2012-11-08 17:58:26]

super cool!

Is it possible to drop the template parameter for RunClosure<T>, perhaps by
using a default template parameter of 0?

[xhwang, 2012-11-08 18:15:44]

On 2012/11/08 17:58:26, scherkus wrote:
> super cool!
> 
> Is it possible to drop the template parameter for RunClosure<T>, perhaps by
> using a default template parameter of 0?

RunClosure<N>() is saying "Call Run() on the N-th parameter which is a closure".
The Closure is not always the first parameter in a function so I feel we should
still keep the template parameter.

[scherkus (not reviewing), 2012-11-08 18:17:07]

On 2012/11/08 18:15:44, xhwang wrote:
> On 2012/11/08 17:58:26, scherkus wrote:
> > super cool!
> > 
> > Is it possible to drop the template parameter for RunClosure<T>, perhaps by
> > using a default template parameter of 0?
> 
> RunClosure<N>() is saying "Call Run() on the N-th parameter which is a
closure".
> The Closure is not always the first parameter in a function so I feel we
should
> still keep the template parameter.

My bad! For some reason I interpreted it as the # of args to be bound while
running the closure, but that makes no sense :)

LGTM

[awong, 2012-11-08 20:44:41]

For something so simple, can we avoid the pump file?  People on chrome team seem
to hate these things, and having 3 is already pretty annoying...

-Albert

On 2012/11/08 18:17:07, scherkus wrote:
> On 2012/11/08 18:15:44, xhwang wrote:
> > On 2012/11/08 17:58:26, scherkus wrote:
> > > super cool!
> > > 
> > > Is it possible to drop the template parameter for RunClosure<T>, perhaps
by
> > > using a default template parameter of 0?
> > 
> > RunClosure<N>() is saying "Call Run() on the N-th parameter which is a
> closure".
> > The Closure is not always the first parameter in a function so I feel we
> should
> > still keep the template parameter.
> 
> My bad! For some reason I interpreted it as the # of args to be bound while
> running the closure, but that makes no sense :)
> 
> LGTM

[awong, 2012-11-08 20:46:12]

Also, can we move callback_util.h into something appropriate in testing? 
Ideally, it should replace gmock_mutant.h

On 2012/11/08 20:44:41, awong wrote:
> For something so simple, can we avoid the pump file?  People on chrome team
seem
> to hate these things, and having 3 is already pretty annoying...
> 
> -Albert
> 
> On 2012/11/08 18:17:07, scherkus wrote:
> > On 2012/11/08 18:15:44, xhwang wrote:
> > > On 2012/11/08 17:58:26, scherkus wrote:
> > > > super cool!
> > > > 
> > > > Is it possible to drop the template parameter for RunClosure<T>, perhaps
> by
> > > > using a default template parameter of 0?
> > > 
> > > RunClosure<N>() is saying "Call Run() on the N-th parameter which is a
> > closure".
> > > The Closure is not always the first parameter in a function so I feel we
> > should
> > > still keep the template parameter.
> > 
> > My bad! For some reason I interpreted it as the # of args to be bound while
> > running the closure, but that makes no sense :)
> > 
> > LGTM

[awong, 2012-11-08 20:46:48]

...with a better name...

On Thu, Nov 8, 2012 at 12:46 PM, <ajwong@chromium.org> wrote:

> Also, can we move callback_util.h into something appropriate in testing?
> Ideally, it should replace gmock_mutant.h
>
>
> On 2012/11/08 20:44:41, awong wrote:
>
>> For something so simple, can we avoid the pump file?  People on chrome
>> team
>>
> seem
>
>> to hate these things, and having 3 is already pretty annoying...
>>
>
>  -Albert
>>
>
>  On 2012/11/08 18:17:07, scherkus wrote:
>> > On 2012/11/08 18:15:44, xhwang wrote:
>> > > On 2012/11/08 17:58:26, scherkus wrote:
>> > > > super cool!
>> > > >
>> > > > Is it possible to drop the template parameter for RunClosure<T>,
>> perhaps
>> by
>> > > > using a default template parameter of 0?
>> > >
>> > > RunClosure<N>() is saying "Call Run() on the N-th parameter which is a
>> > closure".
>> > > The Closure is not always the first parameter in a function so I feel
>> we
>> > should
>> > > still keep the template parameter.
>> >
>> > My bad! For some reason I interpreted it as the # of args to be bound
>> while
>> > running the closure, but that makes no sense :)
>> >
>> > LGTM
>>
>
>
>
>
http://codereview.chromium.**org/11359100/<http://codereview.chromium.org/113...
>

[xhwang, 2012-11-09 02:54:51]

As chatted offline w/ awong@, I'll make this into media/ first then see
opportunities to push it to base/.

Dropped the .pump file and updated more tests.

PTAL!

[scherkus (not reviewing), 2012-11-12 17:26:03]

http://codereview.chromium.org/11359100/diff/6002/media/base/callback_util.h
File media/base/callback_util.h (right):

http://codereview.chromium.org/11359100/diff/6002/media/base/callback_util.h#...
media/base/callback_util.h:2: //     pump.py callback_util.h.pump
I don't see us checking in the .pump file -- is this still an autogenerated
file?

[xhwang, 2012-11-12 18:30:34]

PTAL again!

http://codereview.chromium.org/11359100/diff/6002/media/base/callback_util.h
File media/base/callback_util.h (right):

http://codereview.chromium.org/11359100/diff/6002/media/base/callback_util.h#...
media/base/callback_util.h:2: //     pump.py callback_util.h.pump
On 2012/11/12 17:26:03, scherkus wrote:
> I don't see us checking in the .pump file -- is this still an autogenerated
> file?

Done.

[scherkus (not reviewing), 2012-11-12 18:35:38]

lgtm but I would like a better name

callback_util contains no hint that this is testing-specific code

gmock_callback_support.h?

[awong, 2012-11-12 18:39:31]

LGTM for callback, but agree with scherkus that callback_util is not informative
enough.

Passing review lock to scherkus :)

[xhwang, 2012-11-12 19:34:19]

On 2012/11/12 18:35:38, scherkus wrote:
> lgtm but I would like a better name
> 
> callback_util contains no hint that this is testing-specific code
> 
> gmock_callback_support.h?

Done.

[commit-bot: I haz the power, 2012-11-12 19:52:00]

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

[commit-bot: I haz the power, 2012-11-13 05:49:49]

Change committed as 167309