Use variadic template in media::BindToCurrentLoop()

Use variadic template in media::BindToCurrentLoop()

Stop using pump.y to generate media/base/bind_to_current_loop.h and use
a variadic template instead.

This is a mirror change of
https://crrev.com/c87149e666acfe39d49d7f30a5fd9acc7ef085ea.

TEST=media_unittests

Committed: https://crrev.com/47c5177c37445dacbb134621153e313559186d75
Cr-Commit-Position: refs/heads/master@{#319343}

[wdzierzanowski, 2015-03-05 13:34:56]

wdzierzanowski@opera.com changed reviewers:
+ scherkus@chromium.org, xhwang@chromium.org

[wdzierzanowski, 2015-03-05 13:38:55]

@scherkus, @xhwang: Could you PTAL?

[xhwang, 2015-03-05 17:19:15]

Thanks for handling this! Just a few questions.

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
File media/base/bind_to_current_loop.h (right):

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
media/base/bind_to_current_loop.h:42: template <typename Sig> struct
TrampolineHelper;
What does "sig" mean here?

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
media/base/bind_to_current_loop.h:42: template <typename Sig> struct
TrampolineHelper;
Is this needed here? Can we drop this line?

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
media/base/bind_to_current_loop.h:51: base::Bind(cb,
internal::TrampolineForward(args)...));
internal not needed?

[wdzierzanowski, 2015-03-05 20:45:16]

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
File media/base/bind_to_current_loop.h (right):

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
media/base/bind_to_current_loop.h:42: template <typename Sig> struct
TrampolineHelper;
On 2015/03/05 17:19:15, xhwang wrote:
> What does "sig" mean here?

It stands for "signature", and my intention was to align with the forward
declaration of base::Callback. I can change it to Signature, or just revert to
T.

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
media/base/bind_to_current_loop.h:42: template <typename Sig> struct
TrampolineHelper;
On 2015/03/05 17:19:15, xhwang wrote:
> Is this needed here? Can we drop this line?

We can't, the compiler doesn't like that:

../../media/base/bind_to_current_loop.h:43:8: error: explicit specialization of
non-template struct 'TrampolineHelper'

(where line 43 is the current 45)

I think the switch to a variadic template in the specialization doesn't remove
the reason this forward declaration was put here in the first place. And the
reason is the same as in the case of base::Callback: We first tell the compiler
TrampolineHelper is a struct template with one type parameter, and then define
specializations where the type is a function returning void and taking zero or
more arguments.

Should I add a comment explaining this? Or put a reference to base::Callback?

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
media/base/bind_to_current_loop.h:51: base::Bind(cb,
internal::TrampolineForward(args)...));
On 2015/03/05 17:19:15, xhwang wrote:
> internal not needed?

Of course not, blindly copied from the old version :)

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
media/base/bind_to_current_loop.h:51: base::Bind(cb,
internal::TrampolineForward(args)...));
On 2015/03/05 17:19:15, xhwang wrote:
> internal not needed?

Of course not, blindly copied from the old version :)

[wdzierzanowski, 2015-03-05 20:46:20]

> Of course not, blindly copied from the old version :)

I don't know why this showed up twice, sorry for the noise.

[wdzierzanowski, 2015-03-05 20:47:56]

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
File media/base/bind_to_current_loop.h (right):

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
media/base/bind_to_current_loop.h:51: base::Bind(cb,
internal::TrampolineForward(args)...));
On 2015/03/05 20:45:16, wdzierzanowski wrote:
> On 2015/03/05 17:19:15, xhwang wrote:
> > internal not needed?
> 
> Of course not, blindly copied from the old version :)

Done.

[xhwang, 2015-03-05 20:48:50]

lgtm % nit

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
File media/base/bind_to_current_loop.h (right):

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
media/base/bind_to_current_loop.h:42: template <typename Sig> struct
TrampolineHelper;
On 2015/03/05 20:45:16, wdzierzanowski wrote:
> On 2015/03/05 17:19:15, xhwang wrote:
> > Is this needed here? Can we drop this line?
> 
> We can't, the compiler doesn't like that:
> 
> ../../media/base/bind_to_current_loop.h:43:8: error: explicit specialization
of
> non-template struct 'TrampolineHelper'
> 
> (where line 43 is the current 45)
> 
> I think the switch to a variadic template in the specialization doesn't remove
> the reason this forward declaration was put here in the first place. And the
> reason is the same as in the case of base::Callback: We first tell the
compiler
> TrampolineHelper is a struct template with one type parameter, and then define
> specializations where the type is a function returning void and taking zero or
> more arguments.
> 
> Should I add a comment explaining this? Or put a reference to base::Callback?

A comment would help, thanks!

[wdzierzanowski, 2015-03-05 21:00:06]

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
File media/base/bind_to_current_loop.h (right):

https://codereview.chromium.org/978923004/diff/1/media/base/bind_to_current_l...
media/base/bind_to_current_loop.h:42: template <typename Sig> struct
TrampolineHelper;
On 2015/03/05 20:48:49, xhwang wrote:
> On 2015/03/05 20:45:16, wdzierzanowski wrote:
> > On 2015/03/05 17:19:15, xhwang wrote:
> > > Is this needed here? Can we drop this line?
> > 
> > We can't, the compiler doesn't like that:
> > 
> > ../../media/base/bind_to_current_loop.h:43:8: error: explicit specialization
> of
> > non-template struct 'TrampolineHelper'
> > 
> > (where line 43 is the current 45)
> > 
> > I think the switch to a variadic template in the specialization doesn't
remove
> > the reason this forward declaration was put here in the first place. And the
> > reason is the same as in the case of base::Callback: We first tell the
> compiler
> > TrampolineHelper is a struct template with one type parameter, and then
define
> > specializations where the type is a function returning void and taking zero
or
> > more arguments.
> > 
> > Should I add a comment explaining this? Or put a reference to
base::Callback?
> 
> A comment would help, thanks!

Done.

[wdzierzanowski, 2015-03-05 21:32:10]

The CQ bit was checked by wdzierzanowski@opera.com

[wdzierzanowski, 2015-03-05 21:32:10]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org
Link to the patchset: https://codereview.chromium.org/978923004/#ps40001 (title:
"Explain need to forward-declare TrampolineHelper")

[commit-bot: I haz the power, 2015-03-05 21:33:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/978923004/40001

[commit-bot: I haz the power, 2015-03-05 22:25:19]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-05 22:25:56]

Patchset 3 (id:??) landed as
https://crrev.com/47c5177c37445dacbb134621153e313559186d75
Cr-Commit-Position: refs/heads/master@{#319343}