Change MediaFilter::Stop() to accept a callback so that Stop() is asynchronous. So far PipelineImpl handles asynchronous MediaFilter::Stop(). The actual change to make MediaFilter::Stop() asynchronous will be in another check in.

BUG=16059
TEST=Unit tests still runs

Review URL: http://codereview.chromium.org/2101015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@48769 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 198 insertions, 141 deletions

diff --git a/media/base/filter_host.h b/media/base/filter_host.h
index e28a957..c21b693 100644
--- a/media/base/filter_host.h
+++ b/media/base/filter_host.h
@@ -26,7 +26,7 @@ namespace media {
 class FilterHost {
  public:
   // Stops execution of the pipeline due to a fatal error.  Do not call this
-  // method with PIPELINE_OK or PIPELINE_STOPPING (used internally by pipeline).
+  // method with PIPELINE_OK.
   virtual void SetError(PipelineError error) = 0;
 
   // Gets the current time in microseconds.
diff --git a/media/base/filters.h b/media/base/filters.h
index c4da5d9..141a753 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -108,9 +108,22 @@ class MediaFilter : public base::RefCountedThreadSafe<MediaFilter> {
     }
   }
 
+  // TODO(boliu): Remove once Stop() is asynchronous in subclasses.
+  virtual void Stop() {}
+
   // The pipeline is being stopped either as a result of an error or because
   // the client called Stop().
-  virtual void Stop() = 0;
+  // TODO(boliu): No implementation in subclasses yet.
+  virtual void Stop(FilterCallback* callback) {
+    // TODO(boliu): Call the synchronous version for now. Remove once
+    // all filters have asynchronous stop.
+    Stop();
+
+    if (callback) {
+      callback->Run();
+      delete callback;
+    }
+  }
 
   // The pipeline playback rate has been changed.  Filters may implement this
   // method if they need to respond to this call.
diff --git a/media/base/mock_filters.cc b/media/base/mock_filters.cc
index 7b854fc..f2110ff 100644
--- a/media/base/mock_filters.cc
+++ b/media/base/mock_filters.cc
@@ -1,6 +1,6 @@
-// Copyright (c) 2009 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.
+// 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.
 
 #include "media/base/mock_filters.h"
 
@@ -15,4 +15,9 @@ void DestroyFilterCallback(::testing::Unused, FilterCallback* callback) {
   delete callback;
 }
 
+void RunStopFilterCallback(FilterCallback* callback) {
+  callback->Run();
+  delete callback;
+}
+
 }  // namespace media
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index 9aeaeec..ac9aab6 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -95,7 +95,7 @@ class MockDataSource : public DataSource {
   MockDataSource() {}
 
   // MediaFilter implementation.
-  MOCK_METHOD0(Stop, void());
+  MOCK_METHOD1(Stop, void(FilterCallback* callback));
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
   MOCK_METHOD0(OnAudioRendererDisabled, void());
@@ -121,7 +121,7 @@ class MockDemuxer : public Demuxer {
   MockDemuxer() {}
 
   // MediaFilter implementation.
-  MOCK_METHOD0(Stop, void());
+  MOCK_METHOD1(Stop, void(FilterCallback* callback));
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
   MOCK_METHOD0(OnAudioRendererDisabled, void());
@@ -162,7 +162,7 @@ class MockVideoDecoder : public VideoDecoder {
   MockVideoDecoder() {}
 
   // MediaFilter implementation.
-  MOCK_METHOD0(Stop, void());
+  MOCK_METHOD1(Stop, void(FilterCallback* callback));
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
   MOCK_METHOD0(OnAudioRendererDisabled, void());
@@ -185,7 +185,7 @@ class MockAudioDecoder : public AudioDecoder {
   MockAudioDecoder() {}
 
   // MediaFilter implementation.
-  MOCK_METHOD0(Stop, void());
+  MOCK_METHOD1(Stop, void(FilterCallback* callback));
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
   MOCK_METHOD0(OnAudioRendererDisabled, void());
@@ -213,7 +213,7 @@ class MockVideoRenderer : public VideoRenderer {
   MockVideoRenderer() {}
 
   // MediaFilter implementation.
-  MOCK_METHOD0(Stop, void());
+  MOCK_METHOD1(Stop, void(FilterCallback* callback));
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
   MOCK_METHOD0(OnAudioRendererDisabled, void());
@@ -235,7 +235,7 @@ class MockAudioRenderer : public AudioRenderer {
   MockAudioRenderer() {}
 
   // MediaFilter implementation.
-  MOCK_METHOD0(Stop, void());
+  MOCK_METHOD1(Stop, void(FilterCallback* callback));
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Seek, void(base::TimeDelta time, FilterCallback* callback));
   MOCK_METHOD0(OnAudioRendererDisabled, void());
@@ -329,6 +329,11 @@ void RunFilterCallback(::testing::Unused, FilterCallback* callback);
 // methods.
 void DestroyFilterCallback(::testing::Unused, FilterCallback* callback);
 
+// Helper gmock function that immediately executes and destroys the
+// FilterCallback on behalf of the provided filter.  Can be used when mocking
+// the Stop() method.
+void RunStopFilterCallback(FilterCallback* callback);
+
 // Helper gmock action that calls SetError() on behalf of the provided filter.
 ACTION_P2(SetError, filter, error) {
   filter->host()->SetError(error);
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index 8e39d06..4af77db 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -22,10 +22,8 @@ namespace media {
 
 // Error definitions for pipeline.  All codes indicate an error except:
 // PIPELINE_OK indicates the pipeline is running normally.
-// PIPELINE_STOPPING is used internally when Pipeline::Stop() is called.
 enum PipelineError {
   PIPELINE_OK,
-  PIPELINE_STOPPING,
   PIPELINE_ERROR_URL_NOT_FOUND,
   PIPELINE_ERROR_NETWORK,
   PIPELINE_ERROR_DECODE,
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 532236d..d7f153d 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -349,9 +349,28 @@ bool PipelineImpl::IsPipelineInitializing() {
          state_ == kInitVideoRenderer;
 }
 
+bool PipelineImpl::IsPipelineStopped() {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+  return state_ == kStopped || state_ == kError;
+}
+
+void PipelineImpl::FinishInitialization() {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+  // Execute the seek callback, if present.  Note that this might be the
+  // initial callback passed into Start().
+  if (seek_callback_.get()) {
+    seek_callback_->Run();
+    seek_callback_.reset();
+  }
+  filter_factory_ = NULL;
+}
+
 // static
-bool PipelineImpl::StateTransitionsToStarted(State state) {
-  return state == kPausing || state == kSeeking || state == kStarting;
+bool PipelineImpl::TransientState(State state) {
+  return state == kPausing ||
+         state == kSeeking ||
+         state == kStarting ||
+         state == kStopping;
 }
 
 // static
@@ -363,6 +382,8 @@ PipelineImpl::State PipelineImpl::FindNextState(State current) {
     return kStarting;
   if (current == kStarting)
     return kStarted;
+  if (current == kStopping)
+    return kStopped;
   return current;
 }
 
@@ -371,8 +392,6 @@ void PipelineImpl::SetError(PipelineError error) {
   DCHECK(error != PIPELINE_OK) << "PIPELINE_OK isn't an error!";
   LOG(INFO) << "Media pipeline error: " << error;
 
-  AutoLock auto_lock(lock_);
-  error_ = error;
   message_loop_->PostTask(FROM_HERE,
      NewRunnableMethod(this, &PipelineImpl::ErrorChangedTask, error));
 }
@@ -531,7 +550,7 @@ void PipelineImpl::InitializeTask() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
   // If we have received the stop or error signal, return immediately.
-  if (state_ == kStopped || state_ == kError)
+  if (state_ == kStopping || IsPipelineStopped())
     return;
 
   DCHECK(state_ == kCreated || IsPipelineInitializing());
@@ -591,10 +610,6 @@ void PipelineImpl::InitializeTask() {
       return;
     }
 
-    // We've successfully created and initialized every filter, so we no longer
-    // need the filter factory.
-    filter_factory_ = NULL;
-
     // Initialization was successful, we are now considered paused, so it's safe
     // to set the initial playback rate and volume.
     PlaybackRateChangedTask(GetPlaybackRate());
@@ -610,69 +625,57 @@ void PipelineImpl::InitializeTask() {
 }
 
 // This method is called as a result of the client calling Pipeline::Stop() or
-// as the result of an error condition.  If there is no error, then set the
-// pipeline's |error_| member to PIPELINE_STOPPING.  We stop the filters in the
-// reverse order.
+// 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_);
-  stop_callback_.reset(stop_callback);
+  PipelineError error = GetError();
 
-  // If we've already stopped, return immediately.
-  if (state_ == kStopped) {
+  if (state_ == kStopped || (state_ == kStopping && error == PIPELINE_OK)) {
+    // If we are already stopped or stopping normally, return immediately.
+    delete stop_callback;
     return;
+  } else if (state_ == kError ||
+             (state_ == kStopping && error != PIPELINE_OK)) {
+    // If we are stopping due to SetError(), stop normally instead of
+    // going to error state.
+    AutoLock auto_lock(lock_);
+    error_ = PIPELINE_OK;
   }
 
-  // Carry out setting the error, notifying the client and destroying filters.
-  ErrorChangedTask(PIPELINE_STOPPING);
-
-  // We no longer need to examine our previous state, set it to stopped.
-  state_ = kStopped;
-
-  // Reset the pipeline.
-  ResetState();
+  stop_callback_.reset(stop_callback);
 
-  // Notify the client that stopping has finished.
-  if (stop_callback_.get()) {
-    stop_callback_->Run();
-    stop_callback_.reset();
+  if (IsPipelineInitializing()) {
+    FinishInitialization();
   }
+
+  StartDestroyingFilters();
 }
 
 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.
-  // TODO(hclam): Remove the condition for kStopped. It is there only because
-  // FFmpegDemuxer submits a read error while reading after it is called to
-  // stop. After FFmpegDemuxer is cleaned up we should remove this condition
-  // and add an extra assert.
-  if (state_ == kError || state_ == kStopped) {
+  // 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() || state_ == kStopping) {
     return;
   }
 
+  AutoLock auto_lock(lock_);
+  error_ = error;
+
   // Notify the client that starting did not complete, if necessary.
-  if (IsPipelineInitializing() && seek_callback_.get()) {
-    seek_callback_->Run();
+  if (IsPipelineInitializing()) {
+    FinishInitialization();
   }
-  seek_callback_.reset();
-  filter_factory_ = NULL;
-
-  // We no longer need to examine our previous state, set it to stopped.
-  state_ = kError;
 
-  // Destroy every filter and reset the pipeline as well.
-  DestroyFilters();
-
-  // If our owner has requested to be notified of an error, execute
-  // |error_callback_| unless we have a "good" error.
-  if (error_callback_.get() && error != PIPELINE_STOPPING) {
-    error_callback_->Run();
-  }
+  StartDestroyingFilters();
 }
 
 void PipelineImpl::PlaybackRateChangedTask(float playback_rate) {
@@ -790,11 +793,11 @@ void PipelineImpl::FilterStateTransitionTask() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
   // No reason transitioning if we've errored or have stopped.
-  if (state_ == kError || state_ == kStopped) {
+  if (IsPipelineStopped()) {
     return;
   }
 
-  if (!StateTransitionsToStarted(state_)) {
+  if (!TransientState(state_)) {
     NOTREACHED() << "Invalid current state: " << state_;
     SetError(PIPELINE_ERROR_ABORT);
     return;
@@ -811,13 +814,13 @@ void PipelineImpl::FilterStateTransitionTask() {
       clock_.SetTime(seek_timestamp_);
     }
 
-    if (StateTransitionsToStarted(state_)) {
+    if (TransientState(state_)) {
       remaining_transitions_ = filters_.size();
     }
   }
 
   // Carry out the action for the current state.
-  if (StateTransitionsToStarted(state_)) {
+  if (TransientState(state_)) {
     MediaFilter* filter = filters_[filters_.size() - remaining_transitions_];
     if (state_ == kPausing) {
       filter->Pause(NewCallback(this, &PipelineImpl::OnFilterStateTransition));
@@ -826,16 +829,13 @@ void PipelineImpl::FilterStateTransitionTask() {
           NewCallback(this, &PipelineImpl::OnFilterStateTransition));
     } else if (state_ == kStarting) {
       filter->Play(NewCallback(this, &PipelineImpl::OnFilterStateTransition));
+    } else if (state_ == kStopping) {
+      filter->Stop(NewCallback(this, &PipelineImpl::OnFilterStateTransition));
     } else {
       NOTREACHED();
     }
   } else if (state_ == kStarted) {
-    // Execute the seek callback, if present.  Note that this might be the
-    // initial callback passed into Start().
-    if (seek_callback_.get()) {
-      seek_callback_->Run();
-      seek_callback_.reset();
-    }
+    FinishInitialization();
 
     // Finally, reset our seeking timestamp back to zero.
     seek_timestamp_ = base::TimeDelta();
@@ -848,11 +848,52 @@ void PipelineImpl::FilterStateTransitionTask() {
         rendered_mime_types_.end();
     if (!waiting_for_clock_update_)
       clock_.Play();
+  } else if (IsPipelineStopped()) {
+    FinishDestroyingFiltersTask();
   } else {
     NOTREACHED();
   }
 }
 
+void PipelineImpl::FinishDestroyingFiltersTask() {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+  DCHECK(IsPipelineStopped());
+
+  // Stop every running filter thread.
+  //
+  // TODO(scherkus): can we watchdog this section to detect wedged threads?
+  for (FilterThreadVector::iterator iter = filter_threads_.begin();
+       iter != filter_threads_.end();
+       ++iter) {
+    (*iter)->Stop();
+  }
+
+  // Reset the pipeline, which will decrement a reference to this object.
+  // We will get destroyed as soon as the remaining tasks finish executing.
+  // To be safe, we'll set our pipeline reference to NULL.
+  filters_.clear();
+  filter_types_.clear();
+  STLDeleteElements(&filter_threads_);
+
+  if (PIPELINE_OK == GetError()) {
+    // Destroying filters due to Stop().
+    ResetState();
+
+    // Notify the client that stopping has finished.
+    if (stop_callback_.get()) {
+      stop_callback_->Run();
+      stop_callback_.reset();
+    }
+  } else {
+    // Destroying filters due to SetError().
+    state_ = kError;
+    // If our owner has requested to be notified of an error.
+    if (error_callback_.get()) {
+      error_callback_->Run();
+    }
+  }
+}
+
 template <class Filter, class Source>
 void PipelineImpl::CreateFilter(FilterFactory* filter_factory,
                                 Source source,
@@ -965,54 +1006,24 @@ void PipelineImpl::GetFilter(scoped_refptr<Filter>* filter_out) const {
   }
 }
 
-void PipelineImpl::DestroyFilters() {
-  // Stop every filter.
-  for (FilterVector::iterator iter = filters_.begin();
-       iter != filters_.end();
-       ++iter) {
-    (*iter)->Stop();
-  }
-
-  // Crude blocking counter implementation.
-  Lock lock;
-  ConditionVariable wait_for_zero(&lock);
-  int count = filter_threads_.size();
-
-  // Post a task to every filter's thread to ensure that they've completed their
-  // stopping logic before stopping the threads themselves.
-  //
-  // TODO(scherkus): again, Stop() should either be synchronous or we should
-  // receive a signal from filters that they have indeed stopped.
-  for (FilterThreadVector::iterator iter = filter_threads_.begin();
-       iter != filter_threads_.end();
-       ++iter) {
-    (*iter)->message_loop()->PostTask(FROM_HERE,
-        NewRunnableFunction(&DecrementCounter, &lock, &wait_for_zero, &count));
-  }
+void PipelineImpl::StartDestroyingFilters() {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+  DCHECK_NE(kStopped, state_);
 
-  // Wait on our "blocking counter".
-  {
-    AutoLock auto_lock(lock);
-    while (count > 0) {
-      wait_for_zero.Wait();
-    }
+  if (state_ == kStopping) {
+    return; // Do not call Stop() on filters twice.
   }
 
-  // Stop every running filter thread.
-  //
-  // TODO(scherkus): can we watchdog this section to detect wedged threads?
-  for (FilterThreadVector::iterator iter = filter_threads_.begin();
-       iter != filter_threads_.end();
-       ++iter) {
-    (*iter)->Stop();
+  remaining_transitions_ = filters_.size();
+  if (remaining_transitions_ > 0) {
+    state_ = kStopping;
+    filters_.front()->Stop(NewCallback(
+        this, &PipelineImpl::OnFilterStateTransition));
+  } else {
+    state_ = kStopped;
+    message_loop_->PostTask(FROM_HERE,
+        NewRunnableMethod(this, &PipelineImpl::FinishDestroyingFiltersTask));
   }
-
-  // Reset the pipeline, which will decrement a reference to this object.
-  // We will get destroyed as soon as the remaining tasks finish executing.
-  // To be safe, we'll set our pipeline reference to NULL.
-  filters_.clear();
-  filter_types_.clear();
-  STLDeleteElements(&filter_threads_);
 }
 
 }  // namespace media
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index 61f360b..836715b 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -46,9 +46,13 @@ namespace media {
 //         `-------------> [ Ended ] ---------------------'
 //
 //                  SetError()
-//   [ Any State ] -------------> [ Error ]
-//         |          Stop()
-//         '--------------------> [ Stopped ]
+//   [ Any State ] -------------> [ Stopping (for each filter)]
+//         | Stop()                             |
+//         V                                    V
+//   [ Stopping (for each filter) ]         [ Error ]
+//         |
+//         V
+//   [ Stopped ]
 //
 // Initialization is a series of state transitions from "Created" through each
 // filter initialization state.  When all filter initialization states have
@@ -115,6 +119,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
     kStarting,
     kStarted,
     kEnded,
+    kStopping,
     kStopped,
     kError,
   };
@@ -131,9 +136,18 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // Helper method to tell whether we are in the state of initializing.
   bool IsPipelineInitializing();
 
-  // Returns true if the given state is one that transitions to the started
-  // state.
-  static bool StateTransitionsToStarted(State state);
+  // Helper method to tell whether we are stopped or in error.
+  bool IsPipelineStopped();
+
+  // Helper method to execute callback from Start() and reset
+  // |filter_factory_|. Called when initialization completes
+  // normally or when pipeline is stopped or error occurs during
+  // initialization.
+  void FinishInitialization();
+
+  // Returns true if the given state is one that transitions to a new state
+  // after iterating through each filter.
+  static bool TransientState(State state);
 
   // Given the current state, returns the next state.
   static State FindNextState(State current);
@@ -166,7 +180,8 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // Callback executed by filters upon completing initialization.
   void OnFilterInitialize();
 
-  // Callback executed by filters upon completing Play(), Pause() or Seek().
+  // Callback executed by filters upon completing Play(), Pause(), Seek(),
+  // or Stop().
   void OnFilterStateTransition();
 
   // The following "task" methods correspond to the public methods, but these
@@ -182,8 +197,7 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // initialization.
   void InitializeTask();
 
-  // Stops and destroys all filters, placing the pipeline in the kStopped state
-  // and setting the error code to PIPELINE_STOPPED.
+  // Stops and destroys all filters, placing the pipeline in the kStopped state.
   void StopTask(PipelineCallback* stop_callback);
 
   // Carries out stopping and destroying all filters, placing the pipeline in
@@ -211,6 +225,12 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // Carries out advancing to the next filter during Play()/Pause()/Seek().
   void FilterStateTransitionTask();
 
+  // Carries out stopping filter threads, deleting filters, running
+  // appropriate callbacks, and setting the appropriate pipeline state
+  // depending on whether we performing Stop() or SetError().
+  // Called after all filters have been stopped.
+  void FinishDestroyingFiltersTask();
+
   // Internal methods used in the implementation of the pipeline thread.  All
   // of these methods are only called on the pipeline thread.
 
@@ -273,9 +293,8 @@ class PipelineImpl : public Pipeline, public FilterHost {
   template <class Filter>
   void GetFilter(scoped_refptr<Filter>* filter_out) const;
 
-  // Stops every filters, filter host and filter thread and releases all
-  // references to them.
-  void DestroyFilters();
+  // Kicks off stopping filters. Called by StopTask() and ErrorChangedTask().
+  void StartDestroyingFilters();
 
   // Message loop used to execute pipeline tasks.
   MessageLoop* message_loop_;
diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc
index 82c9fd9..25f4f3f 100644
--- a/media/base/pipeline_impl_unittest.cc
+++ b/media/base/pipeline_impl_unittest.cc
@@ -98,7 +98,8 @@ class PipelineImplTest : public ::testing::Test {
     EXPECT_CALL(*mocks_->data_source(), SetPlaybackRate(0.0f));
     EXPECT_CALL(*mocks_->data_source(), Seek(base::TimeDelta(), NotNull()))
         .WillOnce(Invoke(&RunFilterCallback));
-    EXPECT_CALL(*mocks_->data_source(), Stop());
+    EXPECT_CALL(*mocks_->data_source(), Stop(NotNull()))
+        .WillOnce(Invoke(&RunStopFilterCallback));
     EXPECT_CALL(*mocks_->data_source(), media_format())
         .WillOnce(ReturnRef(data_source_media_format_));
   }
@@ -116,7 +117,8 @@ class PipelineImplTest : public ::testing::Test {
     EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f));
     EXPECT_CALL(*mocks_->demuxer(), Seek(base::TimeDelta(), NotNull()))
         .WillOnce(Invoke(&RunFilterCallback));
-    EXPECT_CALL(*mocks_->demuxer(), Stop());
+    EXPECT_CALL(*mocks_->demuxer(), Stop(NotNull()))
+        .WillOnce(Invoke(&RunStopFilterCallback));
 
     // Configure the demuxer to return the streams.
     for (size_t i = 0; i < streams->size(); ++i) {
@@ -149,7 +151,8 @@ class PipelineImplTest : public ::testing::Test {
     EXPECT_CALL(*mocks_->video_decoder(), SetPlaybackRate(0.0f));
     EXPECT_CALL(*mocks_->video_decoder(), Seek(base::TimeDelta(), NotNull()))
         .WillOnce(Invoke(&RunFilterCallback));
-    EXPECT_CALL(*mocks_->video_decoder(), Stop());
+    EXPECT_CALL(*mocks_->video_decoder(), Stop(NotNull()))
+        .WillOnce(Invoke(&RunStopFilterCallback));
     EXPECT_CALL(*mocks_->video_decoder(), media_format())
         .WillOnce(ReturnRef(video_decoder_media_format_));
   }
@@ -161,7 +164,8 @@ class PipelineImplTest : public ::testing::Test {
     EXPECT_CALL(*mocks_->audio_decoder(), SetPlaybackRate(0.0f));
     EXPECT_CALL(*mocks_->audio_decoder(), Seek(base::TimeDelta(), NotNull()))
         .WillOnce(Invoke(&RunFilterCallback));
-    EXPECT_CALL(*mocks_->audio_decoder(), Stop());
+    EXPECT_CALL(*mocks_->audio_decoder(), Stop(NotNull()))
+        .WillOnce(Invoke(&RunStopFilterCallback));
     EXPECT_CALL(*mocks_->audio_decoder(), media_format())
         .WillOnce(ReturnRef(audio_decoder_media_format_));
   }
@@ -174,7 +178,8 @@ class PipelineImplTest : public ::testing::Test {
     EXPECT_CALL(*mocks_->video_renderer(), SetPlaybackRate(0.0f));
     EXPECT_CALL(*mocks_->video_renderer(), Seek(base::TimeDelta(), NotNull()))
         .WillOnce(Invoke(&RunFilterCallback));
-    EXPECT_CALL(*mocks_->video_renderer(), Stop());
+    EXPECT_CALL(*mocks_->video_renderer(), Stop(NotNull()))
+        .WillOnce(Invoke(&RunStopFilterCallback));
   }
 
   // Sets up expectations to allow the audio renderer to initialize.
@@ -186,7 +191,8 @@ class PipelineImplTest : public ::testing::Test {
     EXPECT_CALL(*mocks_->audio_renderer(), SetVolume(1.0f));
     EXPECT_CALL(*mocks_->audio_renderer(), Seek(base::TimeDelta(), NotNull()))
         .WillOnce(Invoke(&RunFilterCallback));
-    EXPECT_CALL(*mocks_->audio_renderer(), Stop());
+    EXPECT_CALL(*mocks_->audio_renderer(), Stop(NotNull()))
+        .WillOnce(Invoke(&RunStopFilterCallback));
   }
 
   // Sets up expectations on the callback and initializes the pipeline.  Called
@@ -288,7 +294,8 @@ TEST_F(PipelineImplTest, NotStarted) {
 TEST_F(PipelineImplTest, NeverInitializes) {
   EXPECT_CALL(*mocks_->data_source(), Initialize("", NotNull()))
       .WillOnce(Invoke(&DestroyFilterCallback));
-  EXPECT_CALL(*mocks_->data_source(), Stop());
+  EXPECT_CALL(*mocks_->data_source(), Stop(NotNull()))
+      .WillOnce(Invoke(&RunStopFilterCallback));
 
   // This test hangs during initialization by never calling
   // InitializationComplete().  StrictMock<> will ensure that the callback is
@@ -324,7 +331,8 @@ TEST_F(PipelineImplTest, URLNotFound) {
                                PIPELINE_ERROR_URL_NOT_FOUND),
                       Invoke(&RunFilterCallback)));
   EXPECT_CALL(callbacks_, OnError());
-  EXPECT_CALL(*mocks_->data_source(), Stop());
+  EXPECT_CALL(*mocks_->data_source(), Stop(NotNull()))
+      .WillOnce(Invoke(&RunStopFilterCallback));
 
   InitializePipeline();
   EXPECT_FALSE(pipeline_->IsInitialized());
@@ -336,7 +344,8 @@ TEST_F(PipelineImplTest, NoStreams) {
   // we cannot fully initialize the pipeline.
   EXPECT_CALL(*mocks_->data_source(), Initialize("", NotNull()))
       .WillOnce(Invoke(&RunFilterCallback));
-  EXPECT_CALL(*mocks_->data_source(), Stop());
+  EXPECT_CALL(*mocks_->data_source(), Stop(NotNull()))
+      .WillOnce(Invoke(&RunStopFilterCallback));
   EXPECT_CALL(*mocks_->data_source(), media_format())
       .WillOnce(ReturnRef(data_source_media_format_));
 
@@ -344,7 +353,8 @@ TEST_F(PipelineImplTest, NoStreams) {
       .WillOnce(Invoke(&RunFilterCallback));
   EXPECT_CALL(*mocks_->demuxer(), GetNumberOfStreams())
       .WillRepeatedly(Return(0));
-  EXPECT_CALL(*mocks_->demuxer(), Stop());
+  EXPECT_CALL(*mocks_->demuxer(), Stop(NotNull()))
+      .WillOnce(Invoke(&RunStopFilterCallback));
   EXPECT_CALL(callbacks_, OnError());
 
   InitializePipeline();
diff --git a/webkit/glue/webmediaplayer_impl.cc b/webkit/glue/webmediaplayer_impl.cc
index 0157c80..66cf896 100644
--- a/webkit/glue/webmediaplayer_impl.cc
+++ b/webkit/glue/webmediaplayer_impl.cc
@@ -634,10 +634,6 @@ void WebMediaPlayerImpl::OnPipelineError() {
   DCHECK(MessageLoop::current() == main_loop_);
   switch (pipeline_->GetError()) {
     case media::PIPELINE_OK:
-    case media::PIPELINE_STOPPING:
-      NOTREACHED() << "We shouldn't get called with these non-errors";
-      break;
-
     case media::PIPELINE_ERROR_INITIALIZATION_FAILED:
     case media::PIPELINE_ERROR_REQUIRED_FILTER_MISSING:
     case media::PIPELINE_ERROR_COULD_NOT_RENDER: