Require Renderer::Initialize() to return a status code via callback.

media/filters/renderer_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/renderer_impl.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/compiler_specific.h"
@@ skipping 32 matching lines @@
 
 RendererImpl::~RendererImpl() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // Tear down in opposite order of construction as |video_renderer_| can still
   // need |time_source_| (which can be |audio_renderer_|) to be alive.
   video_renderer_.reset();
   audio_renderer_.reset();
 
+  DCHECK(init_cb_.is_null());

[xhwang, 2015/01/23 00:22:01]

hmm, what if Pipeline::StopTask() is called during the initializetion? Is that
not possible?

[DaleCurtis, 2015/01/23 20:31:21]

Right, this is possible. Changed this to run init_cb_ with abort status.

   FireAllPendingCallbacks();
 }
 
 void RendererImpl::Initialize(DemuxerStreamProvider* demuxer_stream_provider,
-                              const base::Closure& init_cb,
+                              const PipelineStatusCB& init_cb,
                               const StatisticsCB& statistics_cb,
                               const BufferingStateCB& buffering_state_cb,
                               const PaintCB& paint_cb,
                               const base::Closure& ended_cb,
                               const PipelineStatusCB& error_cb) {
   DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STATE_UNINITIALIZED);
   DCHECK(!init_cb.is_null());
   DCHECK(!statistics_cb.is_null());
@@ skipping 187 matching lines @@
       base::Bind(&RendererImpl::OnBufferingStateChanged, weak_this_,
                  &audio_buffering_state_),
       base::Bind(&RendererImpl::OnAudioRendererEnded, weak_this_),
       base::Bind(&RendererImpl::OnError, weak_this_));
 }
 
 void RendererImpl::OnAudioRendererInitializeDone(PipelineStatus status) {
   DVLOG(1) << __FUNCTION__ << ": " << status;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
-  if (status != PIPELINE_OK)
-    OnError(status);

[xhwang, 2015/01/23 00:22:01]

I agree this is confusing! :)

-
   // OnError() may be fired at any time by the renderers, even if they thought
   // they initialized successfully (due to delayed output device setup).

[xhwang, 2015/01/23 00:22:01]

Ouch, this is painful :(

   if (state_ != STATE_INITIALIZING) {
+    DCHECK(init_cb_.is_null());
     audio_renderer_.reset();
     return;
   }
 
+  if (status != PIPELINE_OK) {
+    base::ResetAndReturn(&init_cb_).Run(status);
+    return;
+  }
+
   DCHECK(!init_cb_.is_null());
   InitializeVideoRenderer();
 }
 
 void RendererImpl::InitializeVideoRenderer() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STATE_INITIALIZING);
   DCHECK(!init_cb_.is_null());
 
@@ skipping 16 matching lines @@
       base::Bind(&RendererImpl::OnVideoRendererEnded, weak_this_),
       base::Bind(&RendererImpl::OnError, weak_this_),
       base::Bind(&RendererImpl::GetMediaTimeForSyncingVideo,
                  base::Unretained(this)));
 }
 
 void RendererImpl::OnVideoRendererInitializeDone(PipelineStatus status) {
   DVLOG(1) << __FUNCTION__ << ": " << status;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
-  if (status != PIPELINE_OK)
-    OnError(status);
-
   // OnError() may be fired at any time by the renderers, even if they thought
   // they initialized successfully (due to delayed output device setup).
   if (state_ != STATE_INITIALIZING) {
+    DCHECK(init_cb_.is_null());
     audio_renderer_.reset();
     video_renderer_.reset();
     return;
   }
 
   DCHECK(!init_cb_.is_null());
 
+  if (status != PIPELINE_OK) {
+    base::ResetAndReturn(&init_cb_).Run(status);
+    return;
+  }
+
   if (audio_renderer_) {
     time_source_ = audio_renderer_->GetTimeSource();
   } else {
     wall_clock_time_source_.reset(new WallClockTimeSource());
     time_source_ = wall_clock_time_source_.get();
   }
 
   state_ = STATE_PLAYING;
   DCHECK(time_source_);
   DCHECK(audio_renderer_ || video_renderer_);
-  base::ResetAndReturn(&init_cb_).Run();
+  base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
 void RendererImpl::FlushAudioRenderer() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STATE_FLUSHING);
   DCHECK(!flush_cb_.is_null());
 
   if (!audio_renderer_) {
     OnAudioRendererFlushDone();
@@ skipping 197 matching lines @@
 
 void RendererImpl::OnError(PipelineStatus error) {
   DVLOG(1) << __FUNCTION__ << "(" << error << ")";
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_NE(PIPELINE_OK, error) << "PIPELINE_OK isn't an error!";
 
   // An error has already been delivered.
   if (state_ == STATE_ERROR)
     return;
 
+  const State old_state = state_;
   state_ = STATE_ERROR;
 
+  if (old_state == STATE_INITIALIZING) {
+    base::ResetAndReturn(&init_cb_).Run(error);
+    return;
+  }
+
   // Pipeline will destroy |this| as the result of error.
   base::ResetAndReturn(&error_cb_).Run(error);
-
   FireAllPendingCallbacks();
 }
 
 void RendererImpl::FireAllPendingCallbacks() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-
-  if (!init_cb_.is_null())
-    base::ResetAndReturn(&init_cb_).Run();
-
   if (!flush_cb_.is_null())
     base::ResetAndReturn(&flush_cb_).Run();
 }
 
 }  // namespace media