Avoid flooding main thread by caching state on media thread.

media/base/pipeline_impl.cc

 // Copyright (c) 2012 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/pipeline_impl.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 33 matching lines @@
 
   void Start(Demuxer* demuxer,
              std::unique_ptr<Renderer> renderer,
              std::unique_ptr<TextRenderer> text_renderer);
   void Stop(const base::Closure& stop_cb);
   void Seek(base::TimeDelta time);
   void Suspend();
   void Resume(std::unique_ptr<Renderer> renderer, base::TimeDelta time);
   void SetPlaybackRate(double playback_rate);
   void SetVolume(float volume);
-  base::TimeDelta GetMediaTime();
+  base::TimeDelta GetMediaTime() const;
+  Ranges<base::TimeDelta> GetBufferedTimeRanges() const;
+  bool DidLoadingProgress();
+  PipelineStatistics GetStatistics() const;
   void SetCdm(CdmContext* cdm_context, const CdmAttachedCB& cdm_attached_cb);
 
  private:
+  // Contains state shared between main and media thread.
+  // Main thread can only read. Media thread can both - read and write.
+  // So it is not necessary to lock when reading from the media thread.
+  // This struct should only contain state that is not immediately needed by
+  // PipelineClient and can be cached on the media thread until queried.
+  // Alternatively we could cache it on the main thread by posting the
+  // notification to the main thread. But some of the state change notifications
+  // (OnStatisticsUpdate and OnBufferedTimeRangesChanged) arrive much more
+  // frequently than needed. Posting all those notifications to the main thread
+  // causes performance issues: crbug.com/619975.
+  struct SharedState {
+    SharedState()

[DaleCurtis, 2016/06/22 15:21:15]

Just use = initialization.

[alokp, 2016/06/22 16:27:54]

Done.

+        : did_loading_progress(false), suspend_timestamp(kNoTimestamp()) {}
+
+    // TODO(scherkus): Enforce that Renderer is only called on a single thread,
+    // even for accessing media time http://crbug.com/370634
+    std::unique_ptr<Renderer> renderer;
+
+    // True when OnBufferedTimeRangesChanged() has been called more recently
+    // than DidLoadingProgress().
+    bool did_loading_progress;
+
+    // Amount of available buffered data as reported by Demuxer.
+    Ranges<base::TimeDelta> buffered_time_ranges;
+
+    // Accumulated statistics reported by the renderer.
+    PipelineStatistics statistics;
+
+    // The media timestamp to return while the pipeline is suspended.
+    // Otherwise set to kNoTimestamp().
+    base::TimeDelta suspend_timestamp;
+  };
+
   // DemuxerHost implementaion.
   void OnBufferedTimeRangesChanged(const Ranges<base::TimeDelta>& ranges) final;
   void SetDuration(base::TimeDelta duration) final;
   void OnDemuxerError(PipelineStatus error) final;
   void AddTextStream(DemuxerStream* text_stream,
                      const TextTrackConfig& config) final;
   void RemoveTextStream(DemuxerStream* text_stream) final;
 
   // RendererClient implementation.
   void OnError(PipelineStatus error) final;
@@ skipping 13 matching lines @@
   State GetNextState() const;
   void StateTransitionTask(PipelineStatus status);
   void OnCdmAttached(const CdmAttachedCB& cdm_attached_cb,
                      CdmContext* cdm_context,
                      bool success);
   void AddTextStreamTask(DemuxerStream* text_stream,
                          const TextTrackConfig& config);
   void RemoveTextStreamTask(DemuxerStream* text_stream);
   void InitializeDemuxer(const PipelineStatusCB& done_cb);
   void InitializeRenderer(const PipelineStatusCB& done_cb);
+  void DestroyRenderer();
   void ReportMetadata();
 
   base::WeakPtr<PipelineImpl> weak_pipeline_;
   const scoped_refptr<base::SingleThreadTaskRunner> media_task_runner_;
   const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
   const scoped_refptr<MediaLog> media_log_;
 
   Demuxer* demuxer_;
-  std::unique_ptr<Renderer> renderer_;
   std::unique_ptr<TextRenderer> text_renderer_;
   double playback_rate_;
   float volume_;
   CdmContext* cdm_context_;
 
-  // Lock used to serialize members - |renderer_| and |suspend_timestamp_| -
-  // accessed in GetMediaTime(), which is the only member function called on
-  // the main thread.
-  mutable base::Lock media_time_lock_;
+  // Lock used to serialize |shared_state_|.
+  mutable base::Lock shared_state_lock_;
+
+  // State shared between main and media thread.
+  SharedState shared_state_;
 
   // Current state of the pipeline.
   State state_;
 
   // Status of the pipeline.  Initialized to PIPELINE_OK which indicates that
   // the pipeline is operating correctly. Any other value indicates that the
   // pipeline is stopped or is stopping.  Clients can call the Stop() method to
   // reset the pipeline state, and restore this to PIPELINE_OK.
   PipelineStatus status_;
 
   // The timestamp to start playback from after starting/seeking/resuming has
   // completed.
   base::TimeDelta start_timestamp_;
 
-  // The media timestamp to return while the pipeline is suspended.
-  // Otherwise set to kNoTimestamp().
-  base::TimeDelta suspend_timestamp_;
-
   // Whether we've received the audio/video/text ended events.
   bool renderer_ended_;
   bool text_renderer_ended_;
 
   // Series of tasks to Start(), Seek(), and Resume().
   std::unique_ptr<SerialRunner> pending_callbacks_;
 
   base::WeakPtr<RendererWrapper> weak_this_;
   base::WeakPtrFactory<RendererWrapper> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(RendererWrapper);
 };
 
 PipelineImpl::PipelineImpl(
     const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
     MediaLog* media_log)
     : media_task_runner_(media_task_runner),
       media_log_(media_log),
       client_(nullptr),
       playback_rate_(kDefaultPlaybackRate),
       volume_(kDefaultVolume),
-      did_loading_progress_(false),
       weak_factory_(this) {
   DVLOG(2) << __FUNCTION__;
   renderer_wrapper_.reset(new RendererWrapper(weak_factory_.GetWeakPtr(),
                                               media_task_runner_, media_log_));
 }
 
 PipelineImpl::~PipelineImpl() {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!client_) << "Stop() must complete before destroying object";
   DCHECK(seek_cb_.is_null());
   DCHECK(suspend_cb_.is_null());
 
-  // Invalidate self weak pointers effectively canceling all pending
-  // notifications in the message queue.
-  // Do this before posting the DeleteSoon task to the media thread to
-  // guarantee that the WeakPtr<PipelineImpl> passed to the RendererWrapper
-  // is not invalidated on the media thread.
-  weak_factory_.InvalidateWeakPtrs();
-
   // RendererWrapper is deleted on the media thread.
   media_task_runner_->DeleteSoon(FROM_HERE, renderer_wrapper_.release());
 }
 
 void PipelineImpl::Start(Demuxer* demuxer,
                          std::unique_ptr<Renderer> renderer,
                          Client* client,
                          const PipelineStatusCB& seek_cb) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 25 matching lines @@
 
 void PipelineImpl::Stop() {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!IsRunning()) {
     DVLOG(2) << "Media pipeline isn't running. Ignoring Stop()";
     return;
   }
 
-  if (statistics_.video_frames_decoded > 0) {
-    UMA_HISTOGRAM_COUNTS("Media.DroppedFrameCount",
-                         statistics_.video_frames_dropped);
-  }
+  // Once the pipeline is stopped, nothing is reported back to the client.
+  // Reset all callbacks and client handle.
+  // Invalidate self weak pointers effectively canceling all pending
+  // notifications in the message queue.
+  weak_factory_.InvalidateWeakPtrs();

[DaleCurtis, 2016/06/22 15:21:15]

Should be okay, we call Stop() and destruction on the same thread IIRC. Why did
you move this? I think this should go after the else clause (where it is
currently) otherwise the posted task will be triggered before.

[alokp, 2016/06/22 16:27:54]

It does not matter because we use WaitableEvent here. I find it cleaner to
shutdown all notifications from the media thread before posting the stop task to
the media thread (note that stop_cb is not posted back to the main thread; it
gets Run on the media thread).

I did not understand your "otherwise the posted task will be triggered before"
comment. Which posted task did you mean?

[DaleCurtis, 2016/06/22 20:40:39]

I just meant that you're now invalidating WeakPtrs to tasks which may not have
completed yet. You're also resetting these values before stop completes. This
seems like a risky change.

[alokp, 2016/06/22 20:52:03]

And those tasks would not complete anyways. We start waiting here after posting
the stop task during which time the main thread will be suspended and none of
the tasks posted on the main thread will run. After stop task completes, we
reset the WeakPtr and callbacks. So it is functionally equivalent (except for
unittests where we do not wait).

That said this change is not needed. I will revert.

+  seek_cb_.Reset();
+  suspend_cb_.Reset();
+  client_ = nullptr;
 
   if (media_task_runner_->BelongsToCurrentThread()) {
     // This path is executed by unittests that share media and main threads.
     base::Closure stop_cb = base::Bind(&base::DoNothing);
     media_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&RendererWrapper::Stop,
                    base::Unretained(renderer_wrapper_.get()), stop_cb));
   } else {
     // This path is executed by production code where the two task runners -
@@ skipping 11 matching lines @@
         base::Bind(&base::WaitableEvent::Signal, base::Unretained(&waiter));
     // If posting the task fails or the posted task fails to run,
     // we will wait here forever. So add a CHECK to make sure we do not run
     // into those situations.
     CHECK(media_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&RendererWrapper::Stop,
                    base::Unretained(renderer_wrapper_.get()), stop_cb)));
     waiter.Wait();
   }
-
-  // Once the pipeline is stopped, nothing is reported back to the client.
-  // Reset all callbacks and client handle.
-  seek_cb_.Reset();
-  suspend_cb_.Reset();
-  client_ = nullptr;
 }
 
 void PipelineImpl::Seek(base::TimeDelta time, const PipelineStatusCB& seek_cb) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!seek_cb.is_null());
 
   if (!IsRunning()) {
     DLOG(ERROR) << "Media pipeline isn't running. Ignoring Seek().";
     return;
@@ skipping 80 matching lines @@
                  base::Unretained(renderer_wrapper_.get()), volume_));
 }
 
 base::TimeDelta PipelineImpl::GetMediaTime() const {
   DCHECK(thread_checker_.CalledOnValidThread());
   return renderer_wrapper_->GetMediaTime();
 }
 
 Ranges<base::TimeDelta> PipelineImpl::GetBufferedTimeRanges() const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  return buffered_time_ranges_;
+  return renderer_wrapper_->GetBufferedTimeRanges();
 }
 
 base::TimeDelta PipelineImpl::GetMediaDuration() const {
   DCHECK(thread_checker_.CalledOnValidThread());
   return duration_;
 }
 
 bool PipelineImpl::DidLoadingProgress() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  bool ret = did_loading_progress_;
-  did_loading_progress_ = false;
-  return ret;
+  return renderer_wrapper_->DidLoadingProgress();
 }
 
 PipelineStatistics PipelineImpl::GetStatistics() const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  return statistics_;
+  return renderer_wrapper_->GetStatistics();
 }
 
 void PipelineImpl::SetCdm(CdmContext* cdm_context,
                           const CdmAttachedCB& cdm_attached_cb) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(cdm_context);
   DCHECK(!cdm_attached_cb.is_null());
 
   media_task_runner_->PostTask(
@@ skipping 112 matching lines @@
   if (state_ != kPlaying)
     return;
 
   DCHECK(!renderer_ended_);
   renderer_ended_ = true;
   RunEndedCallbackIfNeeded();
 }
 
 void PipelineImpl::RendererWrapper::OnStatisticsUpdate(
     const PipelineStatistics& stats) {
+  DVLOG(3) << __FUNCTION__;
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
-  main_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&PipelineImpl::OnStatisticsUpdate, weak_pipeline_, stats));
+  base::AutoLock auto_lock(shared_state_lock_);
+  shared_state_.statistics.audio_bytes_decoded += stats.audio_bytes_decoded;
+  shared_state_.statistics.video_bytes_decoded += stats.video_bytes_decoded;
+  shared_state_.statistics.video_frames_decoded += stats.video_frames_decoded;
+  shared_state_.statistics.video_frames_dropped += stats.video_frames_dropped;
+  shared_state_.statistics.audio_memory_usage += stats.audio_memory_usage;
+  shared_state_.statistics.video_memory_usage += stats.video_memory_usage;
 }
 
 void PipelineImpl::RendererWrapper::OnBufferingStateChange(
     BufferingState state) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
   DVLOG(2) << __FUNCTION__ << "(" << state << ") ";
 
   main_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&PipelineImpl::OnBufferingStateChange, weak_pipeline_, state));
@@ skipping 71 matching lines @@
       // meeting 'decode' error when passing media segment but WebMediaPlayer's
       // ready_state_ is still ReadyStateHaveNothing. In that case, it will
       // treat it as NetworkStateFormatError not NetworkStateDecodeError.
       ReportMetadata();
       start_timestamp_ = demuxer_->GetStartTime();
 
       return InitializeRenderer(done_cb);
 
     case kPlaying:
       DCHECK(start_timestamp_ >= base::TimeDelta());
-      renderer_->StartPlayingFrom(start_timestamp_);
+      shared_state_.renderer->StartPlayingFrom(start_timestamp_);
       {
-        base::AutoLock auto_lock(media_time_lock_);
-        suspend_timestamp_ = kNoTimestamp();
+        base::AutoLock auto_lock(shared_state_lock_);
+        shared_state_.suspend_timestamp = kNoTimestamp();
       }
 
       if (text_renderer_)
         text_renderer_->StartPlaying();
 
       main_task_runner_->PostTask(
           FROM_HERE, base::Bind(&PipelineImpl::OnSeekDone, weak_pipeline_,
                                 start_timestamp_));
 
-      renderer_->SetPlaybackRate(playback_rate_);
-      renderer_->SetVolume(volume_);
+      shared_state_.renderer->SetPlaybackRate(playback_rate_);
+      shared_state_.renderer->SetVolume(volume_);
       return;
 
     case kSuspended:
-      renderer_.reset();
+      DestroyRenderer();
+      {
+        base::AutoLock auto_lock(shared_state_lock_);
+        shared_state_.statistics.audio_memory_usage = 0;
+        shared_state_.statistics.video_memory_usage = 0;
+      }
       main_task_runner_->PostTask(
           FROM_HERE, base::Bind(&PipelineImpl::OnSuspendDone, weak_pipeline_,
-                                suspend_timestamp_));
+                                shared_state_.suspend_timestamp));
       return;
 
     case kStopping:
     case kStopped:
     case kCreated:
     case kSeeking:
     case kSuspending:
     case kResuming:
       NOTREACHED() << "State has no transition: " << state_;
       return;
@@ skipping 12 matching lines @@
   DCHECK_EQ(state_, kSeeking);
   SerialRunner::Queue bound_fns;
 
   // Pause.
   if (text_renderer_) {
     bound_fns.Push(base::Bind(&TextRenderer::Pause,
                               base::Unretained(text_renderer_.get())));
   }
 
   // Flush.
-  DCHECK(renderer_);
-  bound_fns.Push(
-      base::Bind(&Renderer::Flush, base::Unretained(renderer_.get())));
+  DCHECK(shared_state_.renderer);
+  bound_fns.Push(base::Bind(&Renderer::Flush,
+                            base::Unretained(shared_state_.renderer.get())));
 
   if (text_renderer_) {
     bound_fns.Push(base::Bind(&TextRenderer::Flush,
                               base::Unretained(text_renderer_.get())));
   }
 
   // Seek demuxer.
   bound_fns.Push(
       base::Bind(&Demuxer::Seek, base::Unretained(demuxer_), seek_timestamp));
 
   pending_callbacks_ = SerialRunner::Run(bound_fns, done_cb);
 }
 
 void PipelineImpl::RendererWrapper::DoStop(const base::Closure& done_cb) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(media_task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(state_, kStopping);
   DCHECK(!pending_callbacks_.get());
 
-  // TODO(scherkus): Enforce that Renderer is only called on a single thread,
-  // even for accessing media time http://crbug.com/370634
-  std::unique_ptr<Renderer> renderer;
-  {
-    base::AutoLock auto_lock(media_time_lock_);
-    renderer.swap(renderer_);
-  }
-  renderer.reset();
+  DestroyRenderer();
   text_renderer_.reset();
 
   if (demuxer_) {
     demuxer_->Stop();
     demuxer_ = NULL;
   }
 
   SetState(kStopped);
 
   // Post the stop callback to enqueue it after the tasks that may have been
   // posted by Demuxer and Renderer during stopping. Note that in theory the
   // tasks posted by Demuxer/Renderer may post even more tasks that will get
   // enqueued after |done_cb|. This may be problematic because Demuxer may
   // get destroyed as soon as |done_cb| is run. In practice this is not a
   // problem, but ideally Demuxer should be destroyed on the media thread.
   media_task_runner_->PostTask(FROM_HERE, done_cb);
 }
 
 void PipelineImpl::RendererWrapper::OnBufferedTimeRangesChanged(
     const Ranges<base::TimeDelta>& ranges) {
   // TODO(alokp): Add thread DCHECK after ensuring that all Demuxer
   // implementations call DemuxerHost on the media thread.
-  main_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&PipelineImpl::OnBufferedTimeRangesChange,
-                            weak_pipeline_, ranges));
+  base::AutoLock auto_lock(shared_state_lock_);
+  shared_state_.did_loading_progress = true;
+  shared_state_.buffered_time_ranges = ranges;
 }
 
 void PipelineImpl::RendererWrapper::Start(
     Demuxer* demuxer,
     std::unique_ptr<Renderer> renderer,
     std::unique_ptr<TextRenderer> text_renderer) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(kCreated, state_) << "Received start in unexpected state: "
                               << state_;
 
   DCHECK(!demuxer_);
-  DCHECK(!renderer_);
+  DCHECK(!shared_state_.renderer);
   DCHECK(!text_renderer_);
   DCHECK(!renderer_ended_);
   DCHECK(!text_renderer_ended_);
   demuxer_ = demuxer;
   {
-    base::AutoLock auto_lock(media_time_lock_);
-    renderer_ = std::move(renderer);
+    base::AutoLock auto_lock(shared_state_lock_);
+    shared_state_.renderer = std::move(renderer);
   }
   text_renderer_ = std::move(text_renderer);
   if (text_renderer_) {
     text_renderer_->Initialize(
         base::Bind(&RendererWrapper::OnTextRendererEnded, weak_this_));
   }
 
   StateTransitionTask(PIPELINE_OK);
 }
 
 void PipelineImpl::RendererWrapper::Stop(const base::Closure& stop_cb) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
   DCHECK(state_ != kStopping && state_ != kStopped);
 
   SetState(kStopping);
 
+  if (shared_state_.statistics.video_frames_decoded > 0) {
+    UMA_HISTOGRAM_COUNTS("Media.DroppedFrameCount",
+                         shared_state_.statistics.video_frames_dropped);
+  }
+
   // If we stop during starting/seeking/suspending/resuming we don't want to
   // leave outstanding callbacks around. The callbacks also do not get run if
   // the pipeline is stopped before it had a chance to complete outstanding
   // tasks.
   pending_callbacks_.reset();
 
   DoStop(stop_cb);
 }
 
 void PipelineImpl::RendererWrapper::OnPipelineError(PipelineStatus error) {
@@ skipping 18 matching lines @@
   status_ = error;
   main_task_runner_->PostTask(
       FROM_HERE, base::Bind(&PipelineImpl::OnError, weak_pipeline_, error));
 }
 
 void PipelineImpl::RendererWrapper::SetPlaybackRate(double playback_rate) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
   playback_rate_ = playback_rate;
   if (state_ == kPlaying)
-    renderer_->SetPlaybackRate(playback_rate_);
+    shared_state_.renderer->SetPlaybackRate(playback_rate_);
 }
 
 void PipelineImpl::RendererWrapper::SetVolume(float volume) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
   volume_ = volume;
   if (state_ == kPlaying)
-    renderer_->SetVolume(volume_);
+    shared_state_.renderer->SetVolume(volume_);
 }
 
 void PipelineImpl::RendererWrapper::Seek(base::TimeDelta time) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
   // Suppress seeking if we're not fully started.
   if (state_ != kPlaying) {
     DCHECK(state_ == kStopping || state_ == kStopped)
         << "Receive seek in unexpected state: " << state_;
     OnPipelineError(PIPELINE_ERROR_INVALID_STATE);
@@ skipping 15 matching lines @@
 void PipelineImpl::RendererWrapper::Suspend() {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
   // Suppress suspending if we're not playing.
   if (state_ != kPlaying) {
     DCHECK(state_ == kStopping || state_ == kStopped)
         << "Receive suspend in unexpected state: " << state_;
     OnPipelineError(PIPELINE_ERROR_INVALID_STATE);
     return;
   }
-  DCHECK(renderer_);
+  DCHECK(shared_state_.renderer);
   DCHECK(!pending_callbacks_.get());
 
   SetState(kSuspending);
 
   // Freeze playback and record the media time before flushing. (Flushing clears
   // the value.)
-  renderer_->SetPlaybackRate(0.0);
+  shared_state_.renderer->SetPlaybackRate(0.0);
   {
-    base::AutoLock auto_lock(media_time_lock_);
-    suspend_timestamp_ = renderer_->GetMediaTime();
-    DCHECK(suspend_timestamp_ != kNoTimestamp());
+    base::AutoLock auto_lock(shared_state_lock_);
+    shared_state_.suspend_timestamp = shared_state_.renderer->GetMediaTime();
+    DCHECK(shared_state_.suspend_timestamp != kNoTimestamp());
   }
 
   // Queue the asynchronous actions required to stop playback. (Matches setup in
   // DoSeek().)
   // TODO(sandersd): Share implementation with DoSeek().
   SerialRunner::Queue fns;
 
   if (text_renderer_) {
     fns.Push(base::Bind(&TextRenderer::Pause,
                         base::Unretained(text_renderer_.get())));
   }
 
-  fns.Push(base::Bind(&Renderer::Flush, base::Unretained(renderer_.get())));
+  fns.Push(base::Bind(&Renderer::Flush,
+                      base::Unretained(shared_state_.renderer.get())));
 
   if (text_renderer_) {
     fns.Push(base::Bind(&TextRenderer::Flush,
                         base::Unretained(text_renderer_.get())));
   }
 
   pending_callbacks_ = SerialRunner::Run(
       fns, base::Bind(&RendererWrapper::StateTransitionTask, weak_this_));
 }
 
 void PipelineImpl::RendererWrapper::Resume(std::unique_ptr<Renderer> renderer,
                                            base::TimeDelta timestamp) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
   // Suppress resuming if we're not suspended.
   if (state_ != kSuspended) {
     DCHECK(state_ == kStopping || state_ == kStopped)
         << "Receive resume in unexpected state: " << state_;
     OnPipelineError(PIPELINE_ERROR_INVALID_STATE);
     return;
   }
-  DCHECK(!renderer_);
+  DCHECK(!shared_state_.renderer);
   DCHECK(!pending_callbacks_.get());
 
   SetState(kResuming);
 
   {
-    base::AutoLock auto_lock(media_time_lock_);
-    renderer_ = std::move(renderer);
+    base::AutoLock auto_lock(shared_state_lock_);
+    shared_state_.renderer = std::move(renderer);
   }
 
   // Set up for a seek. (Matches setup in SeekTask().)
   // TODO(sandersd): Share implementation with SeekTask().
   renderer_ended_ = false;
   text_renderer_ended_ = false;
   start_timestamp_ = std::max(timestamp, demuxer_->GetStartTime());
 
   // Queue the asynchronous actions required to start playback. Unlike DoSeek(),
   // we need to initialize the renderer ourselves (we don't want to enter state
   // kInitDemuxer, and even if we did the current code would seek to the start
   // instead of |timestamp|).
   SerialRunner::Queue fns;
 
   fns.Push(
       base::Bind(&Demuxer::Seek, base::Unretained(demuxer_), start_timestamp_));
 
   fns.Push(base::Bind(&RendererWrapper::InitializeRenderer, weak_this_));
 
   pending_callbacks_ = SerialRunner::Run(
       fns, base::Bind(&RendererWrapper::StateTransitionTask, weak_this_));
 }
 
+Ranges<base::TimeDelta> PipelineImpl::RendererWrapper::GetBufferedTimeRanges()
+    const {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+
+  base::AutoLock auto_lock(shared_state_lock_);
+  return shared_state_.buffered_time_ranges;
+}
+
+bool PipelineImpl::RendererWrapper::DidLoadingProgress() {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+
+  base::AutoLock auto_lock(shared_state_lock_);
+  bool did_progress = shared_state_.did_loading_progress;
+  shared_state_.did_loading_progress = false;
+  return did_progress;
+}
+
+PipelineStatistics PipelineImpl::RendererWrapper::GetStatistics() const {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+
+  base::AutoLock auto_lock(shared_state_lock_);
+  return shared_state_.statistics;
+}
+
 void PipelineImpl::RendererWrapper::SetCdm(
     CdmContext* cdm_context,
     const CdmAttachedCB& cdm_attached_cb) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
-  if (!renderer_) {
+  if (!shared_state_.renderer) {
     cdm_context_ = cdm_context;
     cdm_attached_cb.Run(true);
     return;
   }
 
-  renderer_->SetCdm(cdm_context,
-                    base::Bind(&RendererWrapper::OnCdmAttached, weak_this_,
-                               cdm_attached_cb, cdm_context));
+  shared_state_.renderer->SetCdm(
+      cdm_context, base::Bind(&RendererWrapper::OnCdmAttached, weak_this_,
+                              cdm_attached_cb, cdm_context));
 }
 
 void PipelineImpl::RendererWrapper::OnCdmAttached(
     const CdmAttachedCB& cdm_attached_cb,
     CdmContext* cdm_context,
     bool success) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
   if (success)
     cdm_context_ = cdm_context;
   cdm_attached_cb.Run(success);
 }
 
 void PipelineImpl::RendererWrapper::OnTextRendererEnded() {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
   media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::TEXT_ENDED));
 
   if (state_ != kPlaying)
     return;
 
   DCHECK(!text_renderer_ended_);
   text_renderer_ended_ = true;
 
   RunEndedCallbackIfNeeded();
 }
 
 void PipelineImpl::RendererWrapper::RunEndedCallbackIfNeeded() {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
-  if (renderer_ && !renderer_ended_)
+  if (shared_state_.renderer && !renderer_ended_)
     return;
 
   if (text_renderer_ && text_renderer_->HasTracks() && !text_renderer_ended_)
     return;
 
   DCHECK_EQ(status_, PIPELINE_OK);
   main_task_runner_->PostTask(
       FROM_HERE, base::Bind(&PipelineImpl::OnEnded, weak_pipeline_));
 }
 
@@ skipping 27 matching lines @@
     const PipelineStatusCB& done_cb) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
   if (!demuxer_->GetStream(DemuxerStream::AUDIO) &&
       !demuxer_->GetStream(DemuxerStream::VIDEO)) {
     done_cb.Run(PIPELINE_ERROR_COULD_NOT_RENDER);
     return;
   }
 
   if (cdm_context_)
-    renderer_->SetCdm(cdm_context_, base::Bind(&IgnoreCdmAttached));
+    shared_state_.renderer->SetCdm(cdm_context_,
+                                   base::Bind(&IgnoreCdmAttached));
 
-  renderer_->Initialize(demuxer_, this, done_cb);
+  shared_state_.renderer->Initialize(demuxer_, this, done_cb);
+}
+
+void PipelineImpl::RendererWrapper::DestroyRenderer() {
+  DCHECK(media_task_runner_->BelongsToCurrentThread());
+
+  std::unique_ptr<Renderer> renderer;
+  {
+    base::AutoLock auto_lock(shared_state_lock_);

[DaleCurtis, 2016/06/22 15:21:15]

Weird scoping isn't necessary anymore. You can just use lock + renderer.reset().

[alokp, 2016/06/22 16:27:54]

If Renderer destructor is expensive, wouldn't we hold the lock for a long time
unnecessarily?

+    renderer.swap(shared_state_.renderer);
+  }
 }
 
 void PipelineImpl::RendererWrapper::ReportMetadata() {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
   PipelineMetadata metadata;
   metadata.timeline_offset = demuxer_->GetTimelineOffset();
   DemuxerStream* stream = demuxer_->GetStream(DemuxerStream::VIDEO);
   if (stream) {
     metadata.has_video = true;
     metadata.natural_size = stream->video_decoder_config().natural_size();
     metadata.video_rotation = stream->video_rotation();
   }
   if (demuxer_->GetStream(DemuxerStream::AUDIO)) {
     metadata.has_audio = true;
   }
 
   main_task_runner_->PostTask(FROM_HERE, base::Bind(&PipelineImpl::OnMetadata,
                                                     weak_pipeline_, metadata));
 }
 
 void PipelineImpl::OnError(PipelineStatus error) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_NE(PIPELINE_OK, error) << "PIPELINE_OK isn't an error!";
-
-  if (!IsRunning())
-    return;
+  DCHECK(IsRunning());
 
   // If the error happens during starting/seeking/suspending/resuming,
   // report the error via the completion callback for those tasks.
   // Else report error via the client interface.
   if (!seek_cb_.is_null()) {
     base::ResetAndReturn(&seek_cb_).Run(error);
   } else if (!suspend_cb_.is_null()) {
     base::ResetAndReturn(&suspend_cb_).Run(error);
   } else {
     DCHECK(client_);
     client_->OnError(error);
   }
 
   // Any kind of error stops the pipeline.
   Stop();
 }
 
 void PipelineImpl::OnEnded() {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(IsRunning());
 
-  if (IsRunning()) {
-    DCHECK(client_);
-    client_->OnEnded();
-  }
+  DCHECK(client_);
+  client_->OnEnded();
 }
 
 void PipelineImpl::OnMetadata(PipelineMetadata metadata) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(IsRunning());
 
-  if (IsRunning()) {
-    DCHECK(client_);
-    client_->OnMetadata(metadata);
-  }
+  DCHECK(client_);
+  client_->OnMetadata(metadata);
 }
 
 void PipelineImpl::OnBufferingStateChange(BufferingState state) {
   DVLOG(2) << __FUNCTION__ << "(" << state << ")";
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(IsRunning());
 
-  if (IsRunning()) {
-    DCHECK(client_);
-    client_->OnBufferingStateChange(state);
-  }
+  DCHECK(client_);
+  client_->OnBufferingStateChange(state);
 }
 
 void PipelineImpl::OnDurationChange(base::TimeDelta duration) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(IsRunning());
 
   duration_ = duration;
 
-  if (IsRunning()) {
-    DCHECK(client_);
-    client_->OnDurationChange();
-  }
+  DCHECK(client_);
+  client_->OnDurationChange();
 }
 
 void PipelineImpl::OnAddTextTrack(const TextTrackConfig& config,
                                   const AddTextTrackDoneCB& done_cb) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(IsRunning());
 
-  if (IsRunning()) {
-    DCHECK(client_);
-    client_->OnAddTextTrack(config, done_cb);
-  }
+  DCHECK(client_);
+  client_->OnAddTextTrack(config, done_cb);
 }
 
 void PipelineImpl::OnWaitingForDecryptionKey() {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(IsRunning());
 
-  if (IsRunning()) {
-    DCHECK(client_);
-    client_->OnWaitingForDecryptionKey();
-  }
+  DCHECK(client_);
+  client_->OnWaitingForDecryptionKey();
 }
 
 void PipelineImpl::OnVideoNaturalSizeChange(const gfx::Size& size) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(IsRunning());
 
-  if (IsRunning()) {
-    DCHECK(client_);
-    client_->OnVideoNaturalSizeChange(size);
-  }
+  DCHECK(client_);
+  client_->OnVideoNaturalSizeChange(size);
 }
 
 void PipelineImpl::OnVideoOpacityChange(bool opaque) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(IsRunning());
 
-  if (IsRunning()) {
-    DCHECK(client_);
-    client_->OnVideoOpacityChange(opaque);
-  }
-}
-
-void PipelineImpl::OnBufferedTimeRangesChange(
-    const Ranges<base::TimeDelta>& ranges) {
-  DVLOG(3) << __FUNCTION__;
-  DCHECK(thread_checker_.CalledOnValidThread());
-
-  buffered_time_ranges_ = ranges;
-  did_loading_progress_ = true;
-}
-
-void PipelineImpl::OnStatisticsUpdate(const PipelineStatistics& stats) {
-  DVLOG(3) << __FUNCTION__;
-  DCHECK(thread_checker_.CalledOnValidThread());
-
-  statistics_.audio_bytes_decoded += stats.audio_bytes_decoded;
-  statistics_.video_bytes_decoded += stats.video_bytes_decoded;
-  statistics_.video_frames_decoded += stats.video_frames_decoded;
-  statistics_.video_frames_dropped += stats.video_frames_dropped;
-  statistics_.audio_memory_usage += stats.audio_memory_usage;
-  statistics_.video_memory_usage += stats.video_memory_usage;
+  DCHECK(client_);
+  client_->OnVideoOpacityChange(opaque);
 }
 
 void PipelineImpl::OnSeekDone(base::TimeDelta start_time) {
   DVLOG(3) << __FUNCTION__ << "(" << start_time.InMicroseconds() << ")";
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(IsRunning());
 
-  if (IsRunning()) {
-    DCHECK(!seek_cb_.is_null());
-    base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
-  }
+  DCHECK(!seek_cb_.is_null());
+  base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
 }
 
 void PipelineImpl::OnSuspendDone(base::TimeDelta suspend_time) {
   DVLOG(3) << __FUNCTION__ << "(" << suspend_time.InMicroseconds() << ")";
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(IsRunning());
 
-  // Reset audio-video memory usage since renderer has been destroyed.
-  statistics_.audio_memory_usage = 0;
-  statistics_.video_memory_usage = 0;
-
-  if (IsRunning()) {
-    DCHECK(!suspend_cb_.is_null());
-    base::ResetAndReturn(&suspend_cb_).Run(PIPELINE_OK);
-  }
+  DCHECK(!suspend_cb_.is_null());
+  base::ResetAndReturn(&suspend_cb_).Run(PIPELINE_OK);
 }
 
 PipelineImpl::RendererWrapper::RendererWrapper(
     base::WeakPtr<PipelineImpl> weak_pipeline,
     scoped_refptr<base::SingleThreadTaskRunner> media_task_runner,
     scoped_refptr<MediaLog> media_log)
     : weak_pipeline_(weak_pipeline),
       media_task_runner_(std::move(media_task_runner)),
       main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
       media_log_(std::move(media_log)),
       demuxer_(nullptr),
       playback_rate_(kDefaultPlaybackRate),
       volume_(kDefaultVolume),
       cdm_context_(nullptr),
       state_(kCreated),
       status_(PIPELINE_OK),
-      suspend_timestamp_(kNoTimestamp()),
       renderer_ended_(false),
       text_renderer_ended_(false),
       weak_factory_(this) {
   weak_this_ = weak_factory_.GetWeakPtr();
   media_log_->AddEvent(media_log_->CreatePipelineStateChangedEvent(kCreated));
 }
 
 PipelineImpl::RendererWrapper::~RendererWrapper() {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
   DCHECK(state_ == kCreated || state_ == kStopped);
 }
 
-base::TimeDelta PipelineImpl::RendererWrapper::GetMediaTime() {
-  // This is the only member function that gets called on the main thread.
-  // TODO(alokp): Enforce that Renderer is only called on a single thread,
-  // even for accessing media time http://crbug.com/370634.
+base::TimeDelta PipelineImpl::RendererWrapper::GetMediaTime() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
-  base::AutoLock auto_lock(media_time_lock_);
-  if (suspend_timestamp_ != kNoTimestamp())
-    return suspend_timestamp_;
-  return renderer_ ? renderer_->GetMediaTime() : base::TimeDelta();
+  base::AutoLock auto_lock(shared_state_lock_);
+  if (shared_state_.suspend_timestamp != kNoTimestamp())
+    return shared_state_.suspend_timestamp;
+  return shared_state_.renderer ? shared_state_.renderer->GetMediaTime()
+                                : base::TimeDelta();
 }
 
 }  // namespace media