Fix flaky behavior in pipeline teardown.

media/base/pipeline_impl.cc

 // Copyright (c) 2010 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.
 //
 // TODO(scherkus): clean up PipelineImpl... too many crazy function names,
 // potential deadlocks, etc...
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/stl_util-inl.h"
@@ skipping 282 matching lines @@
   return current_bytes_;
 }
 
 void PipelineImpl::ResetState() {
   AutoLock auto_lock(lock_);
   const base::TimeDelta kZero;
   running_          = false;
   stop_pending_     = false;
   seek_pending_     = false;
   tearing_down_     = false;
+  error_caused_teardown_ = false;
   duration_         = kZero;
   buffered_time_    = kZero;
   buffered_bytes_   = 0;
   streaming_        = false;
   loaded_           = false;
   total_bytes_      = 0;
   video_width_      = 0;
   video_height_     = 0;
   volume_           = 1.0f;
   playback_rate_    = 0.0f;
   error_            = PIPELINE_OK;
   waiting_for_clock_update_ = false;
   audio_disabled_   = false;
   clock_->SetTime(kZero);
   rendered_mime_types_.clear();
 }
 
 void PipelineImpl::set_state(State next_state) {
   state_ = next_state;
 }
 
 bool PipelineImpl::IsPipelineOk() {
   return PIPELINE_OK == GetError();
 }
 
-bool PipelineImpl::IsPipelineInitializing() {
-  DCHECK_EQ(MessageLoop::current(), message_loop_);
-  return state_ == kInitDataSource ||
-         state_ == kInitDemuxer ||
-         state_ == kInitAudioDecoder ||
-         state_ == kInitAudioRenderer ||
-         state_ == kInitVideoDecoder ||
-         state_ == kInitVideoRenderer;
-}
-
 bool PipelineImpl::IsPipelineStopped() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   return state_ == kStopped || state_ == kError;
 }
 
 bool PipelineImpl::IsPipelineTearingDown() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   return tearing_down_;
 }
 
@@ skipping 38 matching lines @@
   } else if (current == kFlushing) {
     // We will always honor Seek() before Stop(). This is based on the
     // assumption that we never accept Seek() after Stop().
     DCHECK(IsPipelineSeeking() || IsPipelineStopPending());
     return IsPipelineSeeking() ? kSeeking : kStopping;
   } else if (current == kSeeking) {
     return kStarting;
   } else if (current == kStarting) {
     return kStarted;
   } else if (current == kStopping) {
-    return kStopped;
+    return error_caused_teardown_ ? kError : kStopped;
   } else {
     return current;
   }
 }
 
 void PipelineImpl::SetError(PipelineError error) {
   DCHECK(IsRunning());
   DCHECK(error != PIPELINE_OK) << "PIPELINE_OK isn't an error!";
   VLOG(1) << "Media pipeline error: " << error;
 
@@ skipping 158 matching lines @@
 void PipelineImpl::InitializeTask() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
   // If we have received the stop or error signal, return immediately.
   if (IsPipelineStopPending() ||
       IsPipelineStopped() ||
       PIPELINE_OK != GetError()) {
     return;
   }
 
-  DCHECK(state_ == kCreated || IsPipelineInitializing());
+  DCHECK(state_ == kCreated ||
+         state_ == kInitDataSource ||
+         state_ == kInitDemuxer ||
+         state_ == kInitAudioDecoder ||
+         state_ == kInitAudioRenderer ||
+         state_ == kInitVideoDecoder ||
+         state_ == kInitVideoRenderer);
 
   // Just created, create data source.
   if (state_ == kCreated) {
     set_state(kInitDataSource);
     pipeline_init_state_.reset(new PipelineInitState());
     pipeline_init_state_->composite_ = new CompositeFilter(message_loop_);
     pipeline_init_state_->composite_->set_host(this);
 
     InitializeDataSource();
     return;
@@ skipping 77 matching lines @@
 
 // This method is called as a result of the client calling Pipeline::Stop() or
 // as the result of an error condition.
 // We stop the filters in the reverse order.
 //
 // TODO(scherkus): beware!  this can get posted multiple times since we post
 // Stop() tasks even if we've already stopped.  Perhaps this should no-op for
 // additional calls, however most of this logic will be changing.
 void PipelineImpl::StopTask(PipelineCallback* stop_callback) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-  PipelineError error = GetError();
+  DCHECK(!IsPipelineStopPending());
+  DCHECK_NE(state_, kStopped);
 
-  if (state_ == kStopped || (IsPipelineStopPending() && error == PIPELINE_OK)) {
-    // If we are already stopped or stopping normally, return immediately.
+  if (state_ == kStopped) {
+    // Already stopped so just run callback.
+    stop_callback->Run();
     delete stop_callback;
     return;
-  } else if (state_ == kError ||
-             (IsPipelineStopPending() && error != PIPELINE_OK)) {
+  }
+
+  if (IsPipelineTearingDown() && error_caused_teardown_) {
     // If we are stopping due to SetError(), stop normally instead of
-    // going to error state.
+    // going to error state and calling |error_callback_|. This converts
+    // the teardown in progress from an error teardown into one that acts
+    // like the error never occurred.
     AutoLock auto_lock(lock_);
     error_ = PIPELINE_OK;
+    error_caused_teardown_ = false;
   }
 
   stop_callback_.reset(stop_callback);
 
   stop_pending_ = true;
-  if (!IsPipelineSeeking()) {
+  if (!IsPipelineSeeking() && !IsPipelineTearingDown()) {
     // We will tear down pipeline immediately when there is no seek operation
-    // pending. This should include the case where we are partially initialized.
-    // Ideally this case should use SetError() rather than Stop() to tear down.
-    DCHECK(!IsPipelineTearingDown());
+    // pending and no teardown in progress. This should include the case where
+    // we are partially initialized.
     TearDownPipeline();
   }
 }
 
 void PipelineImpl::ErrorChangedTask(PipelineError error) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK_NE(PIPELINE_OK, error) << "PIPELINE_OK isn't an error!";
 
   // Suppress executing additional error logic. Note that if we are currently
   // performing a normal stop, then we return immediately and continue the
   // normal stop.
   if (IsPipelineStopped() || IsPipelineTearingDown()) {
     return;
   }
 
   AutoLock auto_lock(lock_);
   error_ = error;
 
+  error_caused_teardown_ = true;
   TearDownPipeline();
 }
 
 void PipelineImpl::PlaybackRateChangedTask(float playback_rate) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   {
     AutoLock auto_lock(lock_);
     clock_->SetPlaybackRate(playback_rate);
   }
 
@@ skipping 185 matching lines @@
 void PipelineImpl::FinishDestroyingFiltersTask() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(IsPipelineStopped());
 
   // Clear renderer references.
   audio_renderer_ = NULL;
   video_renderer_ = NULL;
 
   pipeline_filter_ = NULL;
 
-  stop_pending_ = false;
-  tearing_down_ = false;
+  if (error_caused_teardown_ && GetError() != PIPELINE_OK &&
+      error_callback_.get()) {
+    error_callback_->Run();
+  }
 
-  if (PIPELINE_OK == GetError()) {
-    // Destroying filters due to Stop().
+  if (stop_pending_) {
     ResetState();
 
     // Notify the client that stopping has finished.
     if (stop_callback_.get()) {
       stop_callback_->Run();
       stop_callback_.reset();
     }
-  } else {
-    // Destroying filters due to SetError().
-    set_state(kError);
-    // If our owner has requested to be notified of an error.
-    if (error_callback_.get()) {
-      error_callback_->Run();
-    }
   }
+
+  stop_pending_ = false;
+  tearing_down_ = false;
+  error_caused_teardown_ = false;
 }
 
 bool PipelineImpl::PrepareFilter(scoped_refptr<Filter> filter) {
   bool ret = pipeline_init_state_->composite_->AddFilter(filter.get());
 
   if (!ret) {
     SetError(PIPELINE_ERROR_INITIALIZATION_FAILED);
   }
   return ret;
 }
@@ skipping 164 matching lines @@
   return NULL;
 }
 
 void PipelineImpl::TearDownPipeline() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK_NE(kStopped, state_);
 
   // Mark that we already start tearing down operation.
   tearing_down_ = true;
 
-  if (IsPipelineInitializing()) {
-    // Make it look like initialization was successful.
-    pipeline_filter_ = pipeline_init_state_->composite_;
-    pipeline_init_state_.reset();
+  switch(state_) {
+    case kCreated:
+    case kError:
+      set_state(kStopped);
+      message_loop_->PostTask(FROM_HERE,
+          NewRunnableMethod(this, &PipelineImpl::FinishDestroyingFiltersTask));
+      break;
 
-    set_state(kStopping);
-    pipeline_filter_->Stop(NewCallback(
-        this, &PipelineImpl::OnFilterStateTransition));
+    case kInitDataSource:
+    case kInitDemuxer:
+    case kInitAudioDecoder:
+    case kInitAudioRenderer:
+    case kInitVideoDecoder:
+    case kInitVideoRenderer:
+      // Make it look like initialization was successful.
+      pipeline_filter_ = pipeline_init_state_->composite_;
+      pipeline_init_state_.reset();
 
-    FinishInitialization();
-  } else if (pipeline_filter_.get()) {
-    set_state(kPausing);
-    pipeline_filter_->Pause(NewCallback(
-        this, &PipelineImpl::OnFilterStateTransition));
-  } else {
-    set_state(kStopped);
-    message_loop_->PostTask(FROM_HERE,
-        NewRunnableMethod(this, &PipelineImpl::FinishDestroyingFiltersTask));
-  }
+      set_state(kStopping);
+      pipeline_filter_->Stop(
+          NewCallback(this, &PipelineImpl::OnFilterStateTransition));
+
+      FinishInitialization();
+      break;
+
+    case kPausing:
+    case kSeeking:
+    case kFlushing:
+    case kStarting:
+      set_state(kStopping);
+      pipeline_filter_->Stop(
+          NewCallback(this, &PipelineImpl::OnFilterStateTransition));
+      break;
+
+    case kStarted:
+    case kEnded:
+      set_state(kPausing);
+      pipeline_filter_->Pause(
+          NewCallback(this, &PipelineImpl::OnFilterStateTransition));
+      break;
+
+    case kStopping:
+    case kStopped:
+      NOTREACHED() << "Unexpected state for teardown: " << state_;
+      break;
+    // default: intentionally left out to force new states to cause compiler
+    // errors.
+  };
 }
 
 }  // namespace media