cc: Various code safety improvements in video compositing code.

cc/layers/video_frame_provider_client_impl.cc

Index: cc/layers/video_frame_provider_client_impl.cc
diff --git a/cc/layers/video_frame_provider_client_impl.cc b/cc/layers/video_frame_provider_client_impl.cc
index 4bbd92f284b74584f3e6c37ddc0b30f21179fe7f..76b77ca9773f4277d23496e7b89d1e16237fe037 100644
--- a/cc/layers/video_frame_provider_client_impl.cc
+++ b/cc/layers/video_frame_provider_client_impl.cc
@@ -13,17 +13,13 @@ namespace cc {
 
 // static
 scoped_refptr<VideoFrameProviderClientImpl>
-    VideoFrameProviderClientImpl::Create(
-        VideoFrameProvider* provider) {
-  return make_scoped_refptr(
-      new VideoFrameProviderClientImpl(provider));
+VideoFrameProviderClientImpl::Create(VideoFrameProvider* provider) {
+  return make_scoped_refptr(new VideoFrameProviderClientImpl(provider));
 }
 
-VideoFrameProviderClientImpl::~VideoFrameProviderClientImpl() {}
-
 VideoFrameProviderClientImpl::VideoFrameProviderClientImpl(
     VideoFrameProvider* provider)
-    : active_video_layer_(nullptr), provider_(provider) {
+    : provider_(provider), active_video_layer_(nullptr), stopped_(false) {
   // This only happens during a commit on the compositor thread while the main
   // thread is blocked. That makes this a thread-safe call to set the video
   // frame provider client that does not require a lock. The same is true of
@@ -39,27 +35,35 @@ VideoFrameProviderClientImpl::VideoFrameProviderClientImpl(
       0.0, 0.0, 0.0, 1.0);
 }
 
-void VideoFrameProviderClientImpl::SetActiveVideoLayer(
-    VideoLayerImpl* video_layer) {
+VideoFrameProviderClientImpl::~VideoFrameProviderClientImpl() {
+  DCHECK(Stopped());
+}
+
+void VideoFrameProviderClientImpl::Start(VideoLayerImpl* active_video_layer) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(active_video_layer);
+  active_video_layer_ = active_video_layer;
+}
+
+bool VideoFrameProviderClientImpl::Started() const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(video_layer);
-  active_video_layer_ = video_layer;
+  return !!active_video_layer_;
 }
 
 void VideoFrameProviderClientImpl::Stop() {
-  // It's called when the main thread is blocked, so lock isn't needed.
-  if (!provider_)
-    return;
   DCHECK(thread_checker_.CalledOnValidThread());
-  provider_->SetVideoFrameProviderClient(nullptr);
-  provider_ = nullptr;
+  // It's called when the main thread is blocked, so lock isn't needed.
+  if (provider_) {
+    provider_->SetVideoFrameProviderClient(nullptr);
+    provider_ = nullptr;
+  }
+  active_video_layer_ = nullptr;
+  stopped_ = true;
 }
 
-bool VideoFrameProviderClientImpl::Stopped() {
+bool VideoFrameProviderClientImpl::Stopped() const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  // |provider_| is changed while the main thread is blocked, and not changed
-  // thereafter, so lock isn't needed.
-  return !provider_;
+  return stopped_;
 }
 
 scoped_refptr<media::VideoFrame>
@@ -86,18 +90,16 @@ void VideoFrameProviderClientImpl::ReleaseLock() {
 }
 
 void VideoFrameProviderClientImpl::StopUsingProvider() {
-  // Block the provider from shutting down until this client is done
-  // using the frame.
+  // Block the provider from shutting down until the frame controller and it's

[danakj, 2015/03/24 17:38:01]

this is the only mention of a frame controller, i think this comment shouldn't
change?

[sunnyps, 2015/03/24 19:43:38]

Done.

+  // layer is done using the frame.
   base::AutoLock locker(provider_lock_);
   provider_ = nullptr;
 }
 
 void VideoFrameProviderClientImpl::DidReceiveFrame() {
-  TRACE_EVENT1("cc",
-               "VideoFrameProviderClientImpl::DidReceiveFrame",
-               "active_video_layer",
-               !!active_video_layer_);
   DCHECK(thread_checker_.CalledOnValidThread());
+  TRACE_EVENT1("cc", "VideoFrameProviderClientImpl::DidReceiveFrame", "Started",
+               Started());
   if (active_video_layer_)
     active_video_layer_->SetNeedsRedraw();
 }