Add mechanism to signal DelegatedFrameHost that a surface is about to draw.

This allows the FrameSubscriber to request a copy of the output even when
the renderer it's subscribing to hasn't changed its contents, but a child
renderer (oopif or webview) has.

BUG=529378
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

Cr-Commit-Position: refs/heads/master@{#365653}

10 files changed, 150 insertions, 13 deletions

diff --git a/cc/surfaces/surface.cc b/cc/surfaces/surface.cc
index 563415c..29d2c47 100644
--- a/cc/surfaces/surface.cc
+++ b/cc/surfaces/surface.cc
@@ -97,11 +97,25 @@ void Surface::QueueFrame(scoped_ptr<CompositorFrame> frame,
 
 void Surface::RequestCopyOfOutput(scoped_ptr<CopyOutputRequest> copy_request) {
   if (current_frame_ &&
-      !current_frame_->delegated_frame_data->render_pass_list.empty())
-    current_frame_->delegated_frame_data->render_pass_list.back()
-        ->copy_requests.push_back(std::move(copy_request));
-  else
+      !current_frame_->delegated_frame_data->render_pass_list.empty()) {
+    std::vector<scoped_ptr<CopyOutputRequest>>& copy_requests =
+        current_frame_->delegated_frame_data->render_pass_list.back()
+            ->copy_requests;
+
+    if (void* source = copy_request->source()) {
+      // Remove existing CopyOutputRequests made on the Surface by the same
+      // source.
+      auto to_remove =
+          std::remove_if(copy_requests.begin(), copy_requests.end(),
+                         [source](const scoped_ptr<CopyOutputRequest>& x) {
+                           return x->source() == source;
+                         });
+      copy_requests.erase(to_remove, copy_requests.end());
+    }
+    copy_requests.push_back(std::move(copy_request));
+  } else {
     copy_request->SendEmptyResult();
+  }
 }
 
 void Surface::TakeCopyOutputRequests(
diff --git a/cc/surfaces/surface_aggregator.cc b/cc/surfaces/surface_aggregator.cc
index ae291cb..3c97bdc 100644
--- a/cc/surfaces/surface_aggregator.cc
+++ b/cc/surfaces/surface_aggregator.cc
@@ -573,9 +573,6 @@ gfx::Rect SurfaceAggregator::PrewalkTree(SurfaceId surface_id,
   if (provider_)
     provider_->DeclareUsedResourcesFromChild(child_id, referenced_resources);
 
-  for (const auto& render_pass : frame_data->render_pass_list)
-    result->has_copy_requests |= !render_pass->copy_requests.empty();
-
   gfx::Rect damage_rect;
   if (!frame_data->render_pass_list.empty()) {
     RenderPass* last_pass = frame_data->render_pass_list.back().get();
@@ -600,6 +597,12 @@ gfx::Rect SurfaceAggregator::PrewalkTree(SurfaceId surface_id,
     }
   }
 
+  if (surface->factory())
+    surface->factory()->WillDrawSurface(surface->surface_id(), damage_rect);
+
+  for (const auto& render_pass : frame_data->render_pass_list)
+    result->has_copy_requests |= !render_pass->copy_requests.empty();
+
   referenced_surfaces_.erase(it);
   return damage_rect;
 }
diff --git a/cc/surfaces/surface_aggregator_unittest.cc b/cc/surfaces/surface_aggregator_unittest.cc
index d4fb124..f89ae3b 100644
--- a/cc/surfaces/surface_aggregator_unittest.cc
+++ b/cc/surfaces/surface_aggregator_unittest.cc
@@ -44,8 +44,16 @@ class EmptySurfaceFactoryClient : public SurfaceFactoryClient {
  public:
   void ReturnResources(const ReturnedResourceArray& resources) override {}
 
+  void WillDrawSurface(SurfaceId id, const gfx::Rect& damage_rect) override {
+    last_surface_id_ = id;
+    last_damage_rect_ = damage_rect;
+  }
+
   void SetBeginFrameSource(SurfaceId surface_id,
                            BeginFrameSource* begin_frame_source) override {}
+
+  gfx::Rect last_damage_rect_;
+  SurfaceId last_surface_id_;
 };
 
 class FakeSurfaceAggregatorClient : public SurfaceAggregatorClient {
@@ -203,6 +211,10 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, SimpleFrame) {
   EXPECT_FALSE(surface_aggregator_client_.HasSurface(root_surface_));
   AggregateAndVerify(passes, arraysize(passes), ids, arraysize(ids));
   EXPECT_TRUE(surface_aggregator_client_.HasSurface(root_surface_));
+
+  // Check that WillDrawSurface was called.
+  EXPECT_EQ(gfx::Rect(SurfaceSize()), empty_client_.last_damage_rect_);
+  EXPECT_EQ(root_surface_id_, empty_client_.last_surface_id_);
 }
 
 TEST_F(SurfaceAggregatorValidSurfaceTest, OpacityCopied) {
diff --git a/cc/surfaces/surface_factory.cc b/cc/surfaces/surface_factory.cc
index 322d722..12afab8 100644
--- a/cc/surfaces/surface_factory.cc
+++ b/cc/surfaces/surface_factory.cc
@@ -82,6 +82,11 @@ void SurfaceFactory::RequestCopyOfSurface(
   manager_->SurfaceModified(surface_id);
 }
 
+void SurfaceFactory::WillDrawSurface(SurfaceId id,
+                                     const gfx::Rect& damage_rect) {
+  client_->WillDrawSurface(id, damage_rect);
+}
+
 void SurfaceFactory::ReceiveFromChild(
     const TransferableResourceArray& resources) {
   holder_.ReceiveFromChild(resources);
diff --git a/cc/surfaces/surface_factory.h b/cc/surfaces/surface_factory.h
index e8e4e9d..1347728 100644
--- a/cc/surfaces/surface_factory.h
+++ b/cc/surfaces/surface_factory.h
@@ -11,6 +11,7 @@
 #include "base/containers/scoped_ptr_hash_map.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/observer_list.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/surfaces/surface_id.h"
 #include "cc/surfaces/surface_resource_holder.h"
@@ -60,6 +61,8 @@ class CC_SURFACES_EXPORT SurfaceFactory
   void RequestCopyOfSurface(SurfaceId surface_id,
                             scoped_ptr<CopyOutputRequest> copy_request);
 
+  void WillDrawSurface(SurfaceId id, const gfx::Rect& damage_rect);
+
   SurfaceFactoryClient* client() { return client_; }
 
   void ReceiveFromChild(const TransferableResourceArray& resources);
diff --git a/cc/surfaces/surface_factory_client.h b/cc/surfaces/surface_factory_client.h
index 169910a..e79b185 100644
--- a/cc/surfaces/surface_factory_client.h
+++ b/cc/surfaces/surface_factory_client.h
@@ -6,7 +6,9 @@
 #define CC_SURFACES_SURFACE_FACTORY_CLIENT_H_
 
 #include "cc/resources/returned_resource.h"
+#include "cc/surfaces/surface_id.h"
 #include "cc/surfaces/surfaces_export.h"
+#include "ui/gfx/geometry/rect.h"
 
 namespace cc {
 
@@ -19,6 +21,9 @@ class CC_SURFACES_EXPORT SurfaceFactoryClient {
 
   virtual void ReturnResources(const ReturnedResourceArray& resources) = 0;
 
+  virtual void WillDrawSurface(SurfaceId surface_id,
+                               const gfx::Rect& damage_rect) {}
+
   // This allows the SurfaceFactory to tell it's client what BeginFrameSource
   // to use for a given surface_id.
   // If there are multiple active surface_ids, it is the client's
diff --git a/cc/surfaces/surface_factory_unittest.cc b/cc/surfaces/surface_factory_unittest.cc
index 749d7c5..b022327 100644
--- a/cc/surfaces/surface_factory_unittest.cc
+++ b/cc/surfaces/surface_factory_unittest.cc
@@ -4,6 +4,8 @@
 
 #include "base/bind.h"
 #include "cc/output/compositor_frame.h"
+#include "cc/output/copy_output_request.h"
+#include "cc/output/copy_output_result.h"
 #include "cc/output/delegated_frame_data.h"
 #include "cc/resources/resource_provider.h"
 #include "cc/surfaces/surface.h"
@@ -529,6 +531,62 @@ TEST_F(SurfaceFactoryTest, DestroyCycle) {
   surface_id_ = SurfaceId();
 }
 
+void CopyRequestTestCallback(bool* called,
+                             scoped_ptr<CopyOutputResult> result) {
+  *called = true;
+}
+
+TEST_F(SurfaceFactoryTest, DuplicateCopyRequest) {
+  {
+    scoped_ptr<RenderPass> render_pass(RenderPass::Create());
+    scoped_ptr<DelegatedFrameData> frame_data(new DelegatedFrameData);
+    frame_data->render_pass_list.push_back(render_pass.Pass());
+    scoped_ptr<CompositorFrame> frame(new CompositorFrame);
+    frame->metadata.referenced_surfaces.push_back(surface_id_);
+    frame->delegated_frame_data = frame_data.Pass();
+    factory_->SubmitCompositorFrame(surface_id_, frame.Pass(),
+                                    SurfaceFactory::DrawCallback());
+  }
+  void* source1 = &source1;
+  void* source2 = &source2;
+
+  bool called1 = false;
+  scoped_ptr<CopyOutputRequest> request;
+  request = CopyOutputRequest::CreateRequest(
+      base::Bind(&CopyRequestTestCallback, &called1));
+  request->set_source(source1);
+
+  factory_->RequestCopyOfSurface(surface_id_, request.Pass());
+  EXPECT_FALSE(called1);
+
+  bool called2 = false;
+  request = CopyOutputRequest::CreateRequest(
+      base::Bind(&CopyRequestTestCallback, &called2));
+  request->set_source(source2);
+
+  factory_->RequestCopyOfSurface(surface_id_, request.Pass());
+  // Callbacks have different sources so neither should be called.
+  EXPECT_FALSE(called1);
+  EXPECT_FALSE(called2);
+
+  bool called3 = false;
+  request = CopyOutputRequest::CreateRequest(
+      base::Bind(&CopyRequestTestCallback, &called3));
+  request->set_source(source1);
+
+  factory_->RequestCopyOfSurface(surface_id_, request.Pass());
+  // Two callbacks are from source1, so the first should be called.
+  EXPECT_TRUE(called1);
+  EXPECT_FALSE(called2);
+  EXPECT_FALSE(called3);
+
+  factory_->Destroy(surface_id_);
+  surface_id_ = SurfaceId();
+  EXPECT_TRUE(called1);
+  EXPECT_TRUE(called2);
+  EXPECT_TRUE(called3);
+}
+
 // Verifies BFS is forwarded to the client.
 TEST_F(SurfaceFactoryTest, SetBeginFrameSource) {
   FakeBeginFrameSource bfs1;
diff --git a/content/browser/compositor/delegated_frame_host.cc b/content/browser/compositor/delegated_frame_host.cc
index bf39969..21de012 100644
--- a/content/browser/compositor/delegated_frame_host.cc
+++ b/content/browser/compositor/delegated_frame_host.cc
@@ -276,7 +276,7 @@ void DelegatedFrameHost::CheckResizeLock() {
   resize_lock_->UnlockCompositor();
 }
 
-void DelegatedFrameHost::DidReceiveFrameFromRenderer(
+void DelegatedFrameHost::AttemptFrameSubscriberCapture(
     const gfx::Rect& damage_rect) {
   if (!frame_subscriber() || !CanCopyToVideoFrame())
     return;
@@ -320,13 +320,24 @@ void DelegatedFrameHost::DidReceiveFrameFromRenderer(
   // screenshots) since those copy requests do not specify |frame_subscriber()|
   // as a source.
   request->set_source(frame_subscriber());
-  request->set_area(gfx::Rect(current_frame_size_in_dip_));
   if (subscriber_texture.get()) {
     request->SetTextureMailbox(cc::TextureMailbox(
         subscriber_texture->mailbox(), subscriber_texture->sync_token(),
         subscriber_texture->target()));
   }
-  RequestCopyOfOutput(request.Pass());
+
+  if (surface_factory_.get()) {
+    // To avoid unnecessary composites, go directly to the Surface rather than
+    // through RequestCopyOfOutput (which goes through the browser
+    // compositor).
+    if (!request_copy_of_output_callback_for_testing_.is_null())
+      request_copy_of_output_callback_for_testing_.Run(request.Pass());
+    else
+      surface_factory_->RequestCopyOfSurface(surface_id_, request.Pass());
+  } else {
+    request->set_area(gfx::Rect(current_frame_size_in_dip_));
+    RequestCopyOfOutput(request.Pass());
+  }
 }
 
 void DelegatedFrameHost::SwapDelegatedFrame(
@@ -494,7 +505,10 @@ void DelegatedFrameHost::SwapDelegatedFrame(
   } else {
     AddOnCommitCallbackAndDisableLocks(base::Closure());
   }
-  DidReceiveFrameFromRenderer(damage_rect);
+  // With Surfaces, WillDrawSurface() will be called as the trigger to attempt
+  // a frame subscriber capture instead.
+  if (!use_surfaces_)
+    AttemptFrameSubscriberCapture(damage_rect);
   if (frame_provider_.get() || !surface_id_.is_null())
     delegated_frame_evictor_->SwappedFrame(
         client_->DelegatedFrameHostIsVisible());
@@ -555,6 +569,13 @@ void DelegatedFrameHost::ReturnResources(
     SendReturnedDelegatedResources(last_output_surface_id_);
 }
 
+void DelegatedFrameHost::WillDrawSurface(cc::SurfaceId id,
+                                         const gfx::Rect& damage_rect) {
+  if (id != surface_id_)
+    return;
+  AttemptFrameSubscriberCapture(damage_rect);
+}
+
 void DelegatedFrameHost::SetBeginFrameSource(
     cc::SurfaceId surface_id,
     cc::BeginFrameSource* begin_frame_source) {
diff --git a/content/browser/compositor/delegated_frame_host.h b/content/browser/compositor/delegated_frame_host.h
index e832cc7..de883be 100644
--- a/content/browser/compositor/delegated_frame_host.h
+++ b/content/browser/compositor/delegated_frame_host.h
@@ -119,6 +119,7 @@ class CONTENT_EXPORT DelegatedFrameHost
 
   // cc::SurfaceFactoryClient implementation.
   void ReturnResources(const cc::ReturnedResourceArray& resources) override;
+  void WillDrawSurface(cc::SurfaceId id, const gfx::Rect& damage_rect) override;
   void SetBeginFrameSource(cc::SurfaceId surface_id,
                            cc::BeginFrameSource* begin_frame_source) override;
 
@@ -254,7 +255,7 @@ class CONTENT_EXPORT DelegatedFrameHost
 
   // Called to consult the current |frame_subscriber_|, to determine and maybe
   // initiate a copy-into-video-frame request.
-  void DidReceiveFrameFromRenderer(const gfx::Rect& damage_rect);
+  void AttemptFrameSubscriberCapture(const gfx::Rect& damage_rect);
 
   DelegatedFrameHostClient* const client_;
   ui::Compositor* compositor_;
diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
index 2e32249..631dec8 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
@@ -21,6 +21,7 @@
 #include "content/browser/compositor/resize_lock.h"
 #include "content/browser/compositor/test/no_transport_image_transport_factory.h"
 #include "content/browser/frame_host/render_widget_host_view_guest.h"
+#include "content/browser/gpu/compositor_util.h"
 #include "content/browser/renderer_host/input/input_router.h"
 #include "content/browser/renderer_host/input/web_input_event_util.h"
 #include "content/browser/renderer_host/overscroll_controller.h"
@@ -192,12 +193,14 @@ class TestWindowObserver : public aura::WindowObserver {
 class FakeFrameSubscriber : public RenderWidgetHostViewFrameSubscriber {
  public:
   FakeFrameSubscriber(gfx::Size size, base::Callback<void(bool)> callback)
-      : size_(size), callback_(callback) {}
+      : size_(size), callback_(callback), should_capture_(true) {}
 
   bool ShouldCaptureFrame(const gfx::Rect& damage_rect,
                           base::TimeTicks present_time,
                           scoped_refptr<media::VideoFrame>* storage,
                           DeliverFrameCallback* callback) override {
+    if (!should_capture_)
+      return false;
     last_present_time_ = present_time;
     *storage = media::VideoFrame::CreateFrame(media::PIXEL_FORMAT_YV12, size_,
                                               gfx::Rect(size_), size_,
@@ -208,6 +211,10 @@ class FakeFrameSubscriber : public RenderWidgetHostViewFrameSubscriber {
 
   base::TimeTicks last_present_time() const { return last_present_time_; }
 
+  void set_should_capture(bool should_capture) {
+    should_capture_ = should_capture;
+  }
+
   static void CallbackMethod(base::Callback<void(bool)> callback,
                              base::TimeTicks present_time,
                              const gfx::Rect& region_in_frame,
@@ -219,6 +226,7 @@ class FakeFrameSubscriber : public RenderWidgetHostViewFrameSubscriber {
   gfx::Size size_;
   base::Callback<void(bool)> callback_;
   base::TimeTicks last_present_time_;
+  bool should_capture_;
 };
 
 class FakeWindowEventDispatcher : public aura::WindowEventDispatcher {
@@ -2239,7 +2247,10 @@ class RenderWidgetHostViewAuraCopyRequestTest
   void RunLoopUntilCallback() {
     base::RunLoop run_loop;
     quit_closure_ = run_loop.QuitClosure();
+    // Temporarily ignore real draw requests.
+    frame_subscriber_->set_should_capture(false);
     run_loop.Run();
+    frame_subscriber_->set_should_capture(true);
   }
 
   void InitializeView() {
@@ -2271,6 +2282,10 @@ class RenderWidgetHostViewAuraCopyRequestTest
   void OnSwapCompositorFrame() {
     view_->OnSwapCompositorFrame(
         1, MakeDelegatedFrame(1.f, view_rect_.size(), view_rect_));
+    cc::SurfaceId surface_id =
+        view_->GetDelegatedFrameHost()->SurfaceIdForTesting();
+    if (!surface_id.is_null())
+      view_->GetDelegatedFrameHost()->WillDrawSurface(surface_id, view_rect_);
     ASSERT_TRUE(view_->last_copy_request_);
   }