Merge to M44: "Don't report compositor dropped frames for invisible layers."

Wires up a DidDrawFrame() signal to VideoFrameControllers so they are
always notified of DidDrawAllLayers().  The VFCs can then propagate
this signal via PutCurrentFrame() to VideoFrameProvider clients.

PutCurrentFrame() was effectively useless, so I repurposed it to
propagate the DidDrawFrame signal as necessary. As a consequence,
I removed some useless get/put symmetry checks from
WebMediaPlayerMS().

BUG=492369
TEST=Move YouTube offscreen, observe zero dropped frames. unittest TBD.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=danakj,sunnyps

Review URL: https://codereview.chromium.org/1161873008

Cr-Commit-Position: refs/heads/master@{#332046}
(cherry picked from commit e8024c4963b15aeaf03e18869a27c066ba98b0d6)

Review URL: https://codereview.chromium.org/1150583005

Cr-Commit-Position: refs/branch-heads/2403@{#155}
Cr-Branched-From: f54b8097a9c45ed4ad308133d49f05325d6c5070-refs/heads/master@{#330231}

8 files changed, 58 insertions, 10 deletions

diff --git a/cc/layers/video_frame_provider.h b/cc/layers/video_frame_provider.h
index ff632f6..9b1f96c 100644
--- a/cc/layers/video_frame_provider.h
+++ b/cc/layers/video_frame_provider.h
@@ -79,8 +79,11 @@ class CC_EXPORT VideoFrameProvider {
   // has been removed. http://crbug.com/439548
   virtual scoped_refptr<media::VideoFrame> GetCurrentFrame() = 0;
 
-  // Indicates that the last frame returned via GetCurrentFrame() is expected to
-  // be rendered. Must only occur after a previous call to GetCurrentFrame().
+  // Called in response to DidReceiveFrame() or a return value of true from
+  // UpdateCurrentFrame() if the current frame was considered for rendering; the
+  // frame may not been rendered for a variety of reasons (occlusion, etc).
+  // Providers may use the absence of this call as a signal to detect when a new
+  // frame missed its intended deadline.
   virtual void PutCurrentFrame() = 0;
 
  protected:
diff --git a/cc/layers/video_frame_provider_client_impl.cc b/cc/layers/video_frame_provider_client_impl.cc
index 5fab4e2..b854d6e 100644
--- a/cc/layers/video_frame_provider_client_impl.cc
+++ b/cc/layers/video_frame_provider_client_impl.cc
@@ -25,7 +25,8 @@ VideoFrameProviderClientImpl::VideoFrameProviderClientImpl(
       client_(client),
       active_video_layer_(nullptr),
       stopped_(false),
-      rendering_(false) {
+      rendering_(false),
+      needs_put_current_frame_(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
@@ -90,6 +91,7 @@ void VideoFrameProviderClientImpl::PutCurrentFrame() {
   DCHECK(thread_checker_.CalledOnValidThread());
   provider_lock_.AssertAcquired();
   provider_->PutCurrentFrame();
+  needs_put_current_frame_ = false;
 }
 
 void VideoFrameProviderClientImpl::ReleaseLock() {
@@ -137,6 +139,7 @@ void VideoFrameProviderClientImpl::DidReceiveFrame() {
                "active_video_layer",
                !!active_video_layer_);
   DCHECK(thread_checker_.CalledOnValidThread());
+  needs_put_current_frame_ = true;
   if (active_video_layer_)
     active_video_layer_->SetNeedsRedraw();
 }
@@ -171,4 +174,14 @@ void VideoFrameProviderClientImpl::OnBeginFrame(const BeginFrameArgs& args) {
   DidReceiveFrame();
 }
 
+void VideoFrameProviderClientImpl::DidDrawFrame() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  {
+    base::AutoLock locker(provider_lock_);
+    if (provider_ && needs_put_current_frame_)
+      provider_->PutCurrentFrame();
+  }
+  needs_put_current_frame_ = false;
+}
+
 }  // namespace cc
diff --git a/cc/layers/video_frame_provider_client_impl.h b/cc/layers/video_frame_provider_client_impl.h
index be49e15..102e227 100644
--- a/cc/layers/video_frame_provider_client_impl.h
+++ b/cc/layers/video_frame_provider_client_impl.h
@@ -47,6 +47,7 @@ class CC_EXPORT VideoFrameProviderClientImpl
 
   // VideoFrameController implementation.
   void OnBeginFrame(const BeginFrameArgs& args) override;
+  void DidDrawFrame() override;
 
   // VideoFrameProvider::Client implementation.
   // Called on the main thread.
@@ -69,6 +70,7 @@ class CC_EXPORT VideoFrameProviderClientImpl
   VideoLayerImpl* active_video_layer_;
   bool stopped_;
   bool rendering_;
+  bool needs_put_current_frame_;
 
   // Since the provider lives on another thread, it can be destroyed while the
   // frame controller are accessing its frame. Before being destroyed the
diff --git a/cc/scheduler/video_frame_controller.h b/cc/scheduler/video_frame_controller.h
index dee7df5..0307cd5 100644
--- a/cc/scheduler/video_frame_controller.h
+++ b/cc/scheduler/video_frame_controller.h
@@ -26,6 +26,11 @@ class CC_EXPORT VideoFrameController {
  public:
   virtual void OnBeginFrame(const BeginFrameArgs& args) = 0;
 
+  // Called upon completion of LayerTreeHostImpl::DidDrawAllLayers(), regardless
+  // of whether the controller issued a SetNeedsRedraw().  May be used to
+  // determine when SetNeedsRedraw() is called but the draw is aborted.
+  virtual void DidDrawFrame() = 0;
+
  protected:
   virtual ~VideoFrameController() {}
 };
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index e781640..5e02cab 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -1577,6 +1577,9 @@ void LayerTreeHostImpl::DidDrawAllLayers(const FrameData& frame) {
   for (size_t i = 0; i < frame.will_draw_layers.size(); ++i)
     frame.will_draw_layers[i]->DidDraw(resource_provider_.get());
 
+  for (auto& it : video_frame_controllers_)
+    it->DidDrawFrame();
+
   // Once all layers have been drawn, pending texture uploads should no
   // longer block future uploads.
   resource_provider_->MarkPendingUploadsAsNonBlocking();
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index 6b45ae4..ffc0b31 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -7684,12 +7684,18 @@ class FakeVideoFrameController : public VideoFrameController {
  public:
   void OnBeginFrame(const BeginFrameArgs& args) override {
     begin_frame_args_ = args;
+    did_draw_frame_ = false;
   }
 
-  const BeginFrameArgs& begin_frame_args() { return begin_frame_args_; }
+  void DidDrawFrame() override { did_draw_frame_ = true; }
+
+  const BeginFrameArgs& begin_frame_args() const { return begin_frame_args_; }
+
+  bool did_draw_frame() const { return did_draw_frame_; }
 
  private:
   BeginFrameArgs begin_frame_args_;
+  bool did_draw_frame_ = false;
 };
 
 TEST_F(LayerTreeHostImplTest, AddVideoFrameControllerInsideFrame) {
@@ -7702,6 +7708,17 @@ TEST_F(LayerTreeHostImplTest, AddVideoFrameControllerInsideFrame) {
   host_impl_->AddVideoFrameController(&controller);
   EXPECT_TRUE(controller.begin_frame_args().IsValid());
   host_impl_->DidFinishImplFrame();
+
+  EXPECT_FALSE(controller.did_draw_frame());
+  LayerTreeHostImpl::FrameData frame;
+  host_impl_->DidDrawAllLayers(frame);
+  EXPECT_TRUE(controller.did_draw_frame());
+
+  controller.OnBeginFrame(begin_frame_args);
+  EXPECT_FALSE(controller.did_draw_frame());
+  host_impl_->RemoveVideoFrameController(&controller);
+  host_impl_->DidDrawAllLayers(frame);
+  EXPECT_FALSE(controller.did_draw_frame());
 }
 
 TEST_F(LayerTreeHostImplTest, AddVideoFrameControllerOutsideFrame) {
@@ -7720,6 +7737,17 @@ TEST_F(LayerTreeHostImplTest, AddVideoFrameControllerOutsideFrame) {
   EXPECT_FALSE(controller.begin_frame_args().IsValid());
   host_impl_->WillBeginImplFrame(begin_frame_args);
   EXPECT_TRUE(controller.begin_frame_args().IsValid());
+
+  EXPECT_FALSE(controller.did_draw_frame());
+  LayerTreeHostImpl::FrameData frame;
+  host_impl_->DidDrawAllLayers(frame);
+  EXPECT_TRUE(controller.did_draw_frame());
+
+  controller.OnBeginFrame(begin_frame_args);
+  EXPECT_FALSE(controller.did_draw_frame());
+  host_impl_->RemoveVideoFrameController(&controller);
+  host_impl_->DidDrawAllLayers(frame);
+  EXPECT_FALSE(controller.did_draw_frame());
 }
 
 TEST_F(LayerTreeHostImplTest, GpuRasterizationStatusModes) {
diff --git a/content/renderer/media/webmediaplayer_ms.cc b/content/renderer/media/webmediaplayer_ms.cc
index de6f33b..a7f1e0b 100644
--- a/content/renderer/media/webmediaplayer_ms.cc
+++ b/content/renderer/media/webmediaplayer_ms.cc
@@ -108,7 +108,6 @@ WebMediaPlayerMS::WebMediaPlayerMS(
       delegate_(delegate),
       paused_(true),
       current_frame_used_(false),
-      pending_repaint_(false),
       video_frame_provider_client_(NULL),
       received_first_frame_(false),
       total_frame_count_(0),
@@ -467,18 +466,14 @@ bool WebMediaPlayerMS::UpdateCurrentFrame(base::TimeTicks deadline_min,
 scoped_refptr<media::VideoFrame> WebMediaPlayerMS::GetCurrentFrame() {
   DVLOG(3) << "WebMediaPlayerMS::GetCurrentFrame";
   base::AutoLock auto_lock(current_frame_lock_);
-  DCHECK(!pending_repaint_);
   if (!current_frame_.get())
     return NULL;
-  pending_repaint_ = true;
   current_frame_used_ = true;
   return current_frame_;
 }
 
 void WebMediaPlayerMS::PutCurrentFrame() {
   DVLOG(3) << "WebMediaPlayerMS::PutCurrentFrame";
-  DCHECK(pending_repaint_);
-  pending_repaint_ = false;
 }
 
 void WebMediaPlayerMS::OnFrameAvailable(
diff --git a/content/renderer/media/webmediaplayer_ms.h b/content/renderer/media/webmediaplayer_ms.h
index 7310fd6..d01cba6 100644
--- a/content/renderer/media/webmediaplayer_ms.h
+++ b/content/renderer/media/webmediaplayer_ms.h
@@ -188,7 +188,6 @@ class WebMediaPlayerMS
   bool current_frame_used_;
   // |current_frame_lock_| protects |current_frame_used_| and |current_frame_|.
   base::Lock current_frame_lock_;
-  bool pending_repaint_;
 
   scoped_ptr<cc_blink::WebLayerImpl> video_weblayer_;