Remove media::BindToLoop() in favour of media::BindToCurrentLoop().

Remove media::BindToLoop() in favour of media::BindToCurrentLoop().

Nearly all instances of BindToLoop() can be replaced with BindToCurrentLoop(). The three instances of BindToLoop() that can't be replaced with BindToCurrentLoop() are better off not using BindToLoop() in the first place.

BUG=167240
TBR=jam

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

[scherkus (not reviewing), 2014-01-08 03:15:13]

fischman: this is ready for review

I decided to remove BindToLoop() after all. The 3 uses of BTL() seem to be used
in classes that either have unclear threading (VaapiVideoDecodeAccelerator,
MediaSourceDelegate) or don't really need to use it (FFmpegDemuxer).

I'd rather have BTL() removed entirely so it can't be (mis)used in the future
and get those classes to cleaned up.

[Ami GONE FROM CHROMIUM, 2014-01-08 21:37:41]

LGTM % nits
+posciak to cc as FYI for VAVDA change.

https://codereview.chromium.org/66953005/diff/300001/content/renderer/media/r...
File content/renderer/media/rtc_video_decoder_factory_tv.cc (right):

https://codereview.chromium.org/66953005/diff/300001/content/renderer/media/r...
content/renderer/media/rtc_video_decoder_factory_tv.cc:106:
media::BindToCurrentLoop(read_cb).Run(DemuxerStream::kAborted, NULL);
y u no
PostTask(FROM_HERE, MessageLoop::current(), read_cb, kAborted, NULL);

https://codereview.chromium.org/66953005/diff/300001/content/renderer/media/w...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/66953005/diff/300001/content/renderer/media/w...
content/renderer/media/webmediaplayer_impl.cc:120: #define
BIND_TO_CURRENT_LOOP(function) \
"CURRENT" is less informative at the callsite than "RENDER".
WDYT of leaving the macro named BIND_TO_RENDER_LOOP and making the impl of the
macro (here) DCHECK that we're on the main_loop_ before BTCL'ing?

(if you don't take that suggestion, I think you need to do _something_ to avoid
B_T_C_L being an uppercasing+underscoring of BTCL - e.g. incorporate the fact of
the AsWeakPtr into the macro's name)

https://codereview.chromium.org/66953005/diff/300001/media/audio/win/audio_ma...
File media/audio/win/audio_manager_win.cc (right):

https://codereview.chromium.org/66953005/diff/300001/media/audio/win/audio_ma...
media/audio/win/audio_manager_win.cc:164: void
AudioManagerWin::CreateDeviceListener() {
DCHECK(GetTaskRunner()->IsCurrent());
or whatever?

https://codereview.chromium.org/66953005/diff/300001/media/base/android/media...
File media/base/android/media_source_player_unittest.cc (right):

https://codereview.chromium.org/66953005/diff/300001/media/base/android/media...
media/base/android/media_source_player_unittest.cc:207: void
OnNextTestDecodeCallbackPostTaskToReleasePlayer() {
DCHECK already on message_loop_

[scherkus (not reviewing), 2014-01-09 01:52:02]

can you sanity check what I did in content/renderer/media/webmediaplayer_impl.cc
?

https://codereview.chromium.org/66953005/diff/300001/content/renderer/media/r...
File content/renderer/media/rtc_video_decoder_factory_tv.cc (right):

https://codereview.chromium.org/66953005/diff/300001/content/renderer/media/r...
content/renderer/media/rtc_video_decoder_factory_tv.cc:106:
media::BindToCurrentLoop(read_cb).Run(DemuxerStream::kAborted, NULL);
On 2014/01/08 21:37:41, Ami Fischman wrote:
> y u no
> PostTask(FROM_HERE, MessageLoop::current(), read_cb, kAborted, NULL);

done

https://codereview.chromium.org/66953005/diff/300001/content/renderer/media/w...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/66953005/diff/300001/content/renderer/media/w...
content/renderer/media/webmediaplayer_impl.cc:120: #define
BIND_TO_CURRENT_LOOP(function) \
On 2014/01/08 21:37:41, Ami Fischman wrote:
> "CURRENT" is less informative at the callsite than "RENDER".
> WDYT of leaving the macro named BIND_TO_RENDER_LOOP and making the impl of the
> macro (here) DCHECK that we're on the main_loop_ before BTCL'ing?
> 
> (if you don't take that suggestion, I think you need to do _something_ to
avoid
> B_T_C_L being an uppercasing+underscoring of BTCL - e.g. incorporate the fact
of
> the AsWeakPtr into the macro's name)

I really like the DCHECK() idea, but since this macro is typically used inside
param lists, etc.. I had to use a template static helper function.

Let me know what you think.

https://codereview.chromium.org/66953005/diff/300001/media/audio/win/audio_ma...
File media/audio/win/audio_manager_win.cc (right):

https://codereview.chromium.org/66953005/diff/300001/media/audio/win/audio_ma...
media/audio/win/audio_manager_win.cc:164: void
AudioManagerWin::CreateDeviceListener() {
On 2014/01/08 21:37:41, Ami Fischman wrote:
> DCHECK(GetTaskRunner()->IsCurrent());
> or whatever?

indeed!

https://codereview.chromium.org/66953005/diff/300001/media/base/android/media...
File media/base/android/media_source_player_unittest.cc (right):

https://codereview.chromium.org/66953005/diff/300001/media/base/android/media...
media/base/android/media_source_player_unittest.cc:207: void
OnNextTestDecodeCallbackPostTaskToReleasePlayer() {
On 2014/01/08 21:37:41, Ami Fischman wrote:
> DCHECK already on message_loop_

Done.

[Ami GONE FROM CHROMIUM, 2014-01-09 01:57:06]

LGTM

https://codereview.chromium.org/66953005/diff/440021/content/renderer/media/w...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/66953005/diff/440021/content/renderer/media/w...
content/renderer/media/webmediaplayer_impl.cc:120: template <class T>
That's some nice clevermeistering, but you can even cleverERmeister!
See HANDLE_EINTR for how to make a multi-statement macro that still acts as a
value at the callsite.

[scherkus (not reviewing), 2014-01-09 02:14:45]

https://codereview.chromium.org/66953005/diff/440021/content/renderer/media/w...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/66953005/diff/440021/content/renderer/media/w...
content/renderer/media/webmediaplayer_impl.cc:120: template <class T>
On 2014/01/09 01:57:07, Ami Fischman wrote:
> That's some nice clevermeistering, but you can even cleverERmeister!
> See HANDLE_EINTR for how to make a multi-statement macro that still acts as a
> value at the callsite.

it feels like I'm practicing some form of dark sorcery, but done

take a look -- it's funky looking but hey it works!

[Ami GONE FROM CHROMIUM, 2014-01-09 02:34:43]

Ship it!
On Jan 8, 2014 6:14 PM, <scherkus@chromium.org> wrote:

>
> https://codereview.chromium.org/66953005/diff/440021/
> content/renderer/media/webmediaplayer_impl.cc
> File content/renderer/media/webmediaplayer_impl.cc (right):
>
> https://codereview.chromium.org/66953005/diff/440021/
> content/renderer/media/webmediaplayer_impl.cc#newcode120
> content/renderer/media/webmediaplayer_impl.cc:120: template <class T>
> On 2014/01/09 01:57:07, Ami Fischman wrote:
>
>> That's some nice clevermeistering, but you can even cleverERmeister!
>> See HANDLE_EINTR for how to make a multi-statement macro that still
>>
> acts as a
>
>> value at the callsite.
>>
>
> it feels like I'm practicing some form of dark sorcery, but done
>
> take a look -- it's funky looking but hey it works!
>
> https://codereview.chromium.org/66953005/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[scherkus (not reviewing), 2014-01-09 03:05:01]

On 2014/01/09 02:34:43, Ami Fischman wrote:
> Ship it!
> On Jan 8, 2014 6:14 PM, <mailto:scherkus@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/66953005/diff/440021/
> > content/renderer/media/webmediaplayer_impl.cc
> > File content/renderer/media/webmediaplayer_impl.cc (right):
> >
> > https://codereview.chromium.org/66953005/diff/440021/
> > content/renderer/media/webmediaplayer_impl.cc#newcode120
> > content/renderer/media/webmediaplayer_impl.cc:120: template <class T>
> > On 2014/01/09 01:57:07, Ami Fischman wrote:
> >
> >> That's some nice clevermeistering, but you can even cleverERmeister!
> >> See HANDLE_EINTR for how to make a multi-statement macro that still
> >>
> > acts as a
> >
> >> value at the callsite.
> >>
> >
> > it feels like I'm practicing some form of dark sorcery, but done
> >
> > take a look -- it's funky looking but hey it works!
> >
> > https://codereview.chromium.org/66953005/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Hmm.. not sure if this magic trick works on MSVC :~(

[scherkus (not reviewing), 2014-01-09 03:05:02]

On 2014/01/09 02:34:43, Ami Fischman wrote:
> Ship it!
> On Jan 8, 2014 6:14 PM, <mailto:scherkus@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/66953005/diff/440021/
> > content/renderer/media/webmediaplayer_impl.cc
> > File content/renderer/media/webmediaplayer_impl.cc (right):
> >
> > https://codereview.chromium.org/66953005/diff/440021/
> > content/renderer/media/webmediaplayer_impl.cc#newcode120
> > content/renderer/media/webmediaplayer_impl.cc:120: template <class T>
> > On 2014/01/09 01:57:07, Ami Fischman wrote:
> >
> >> That's some nice clevermeistering, but you can even cleverERmeister!
> >> See HANDLE_EINTR for how to make a multi-statement macro that still
> >>
> > acts as a
> >
> >> value at the callsite.
> >>
> >
> > it feels like I'm practicing some form of dark sorcery, but done
> >
> > take a look -- it's funky looking but hey it works!
> >
> > https://codereview.chromium.org/66953005/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Hmm.. not sure if this magic trick works on MSVC :~(

[Ami GONE FROM CHROMIUM, 2014-01-09 19:03:59]

On 2014/01/09 03:05:02, scherkus wrote:
> On 2014/01/09 02:34:43, Ami Fischman wrote:
> > Ship it!
> > On Jan 8, 2014 6:14 PM, <mailto:scherkus@chromium.org> wrote:
> > 
> > >
> > > https://codereview.chromium.org/66953005/diff/440021/
> > > content/renderer/media/webmediaplayer_impl.cc
> > > File content/renderer/media/webmediaplayer_impl.cc (right):
> > >
> > > https://codereview.chromium.org/66953005/diff/440021/
> > > content/renderer/media/webmediaplayer_impl.cc#newcode120
> > > content/renderer/media/webmediaplayer_impl.cc:120: template <class T>
> > > On 2014/01/09 01:57:07, Ami Fischman wrote:
> > >
> > >> That's some nice clevermeistering, but you can even cleverERmeister!
> > >> See HANDLE_EINTR for how to make a multi-statement macro that still
> > >>
> > > acts as a
> > >
> > >> value at the callsite.
> > >>
> > >
> > > it feels like I'm practicing some form of dark sorcery, but done
> > >
> > > take a look -- it's funky looking but hey it works!
> > >
> > > https://codereview.chromium.org/66953005/
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> 
> Hmm.. not sure if this magic trick works on MSVC :~(

Per off-line convo, indeed statement expressions are a gcc extensions
(http://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/Statement-Exprs.html#Statement-Exprs)
but happily the comma operator works just as well here, and has been with us
since the origin of C.

[commit-bot: I haz the power, 2014-01-09 20:05:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/66953005/810001

[commit-bot: I haz the power, 2014-01-09 21:45:33]

Change committed as 243983