Avoid double task trampoline for Pipeline state transitions.

media/filters/renderer_impl.cc

Index: media/filters/renderer_impl.cc
diff --git a/media/filters/renderer_impl.cc b/media/filters/renderer_impl.cc
index e15ad6e061b31dde4fe2974819c3916ae5ec535f..1f1228717b09850819d59861e5e0e09cc3b227ce 100644
--- a/media/filters/renderer_impl.cc
+++ b/media/filters/renderer_impl.cc
@@ -11,6 +11,7 @@
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "media/base/audio_renderer.h"
+#include "media/base/bind_to_current_loop.h"
 #include "media/base/demuxer_stream_provider.h"
 #include "media/base/time_source.h"
 #include "media/base/video_renderer.h"
@@ -78,7 +79,9 @@ void RendererImpl::Initialize(DemuxerStreamProvider* demuxer_stream_provider,
   ended_cb_ = ended_cb;
   error_cb_ = error_cb;
 
-  init_cb_ = init_cb;
+  // Always post the callback because |this| could be destroyed upon failure.
+  init_cb_ = BindToCurrentLoop(init_cb);

[xhwang, 2015/01/16 23:09:07]

This is the tricky part of our rule that callee must "ensure" the callback is
called asynchronously. I think InitializeAudioRenderer() makes sure init_cb_ is
always called asynchronously. We didn't use BindToCurrentLoop here because we
don't want to have too many posts which is unnecessary. But I agree that the
intention is not obvious...

Or are you worrying about calling the callback directly in any public method, to
avoid reentrancy back into the caller?

[DaleCurtis, 2015/01/16 23:15:15]

I realize we post below and init cb still runs asynchronously, but it's not on a
clean stack like it was before. That's likely fine if the caller runs the
init_cb and does nothing after, but may result in some long stacks. I erred on
the side of forcing a clean stack even if it's unnecessary. I'd be okay with not
doing that since the stack still wouldn't be very deep.

[xhwang, 2015/01/16 23:24:04]

I am not too worried about long stack as long as we are not reentering the
caller or the callee in the middle of a previous call. I am trying to think of
some more general rule on this but apparently I need to think more on this :(

[DaleCurtis, 2015/01/16 23:33:49]

I think just adding a note to the demuxer and renderer interfaces saying the
init_cb/status_cb "must be run asynchronously (i.e. not during the context of
the initialize()) call." sounds fine. Is there something else you're thinking
of?

[DaleCurtis, 2015/01/20 21:37:43]

Done.

+
   state_ = STATE_INITIALIZING;
   InitializeAudioRenderer();
 }