Using CompositorFrameSinkSupport to handle the copy request

In the current implementation, the copy request is handled by
DelegatedFrameHost::RequestCopyOfOutput, which add copy request to
the compositor frame of the browser. After this patch, the copy
request will be handled by the
CompositorFrameSinkSupport:RequestCopyOfSurface which adds the
copy request to the surface where the copy request needs to be make.

TEST: exiting test cases on copy requests should cover all the usage

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I5c0b4572a7b8ed5fce0f5ee10f5d4dd982912e0a
Reviewed-on: https://chromium-review.googlesource.com/881548Reviewed-by: Yuri Wiitala <miu@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: Yi Xu <yiyix@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534838}

content/browser/renderer_host/browser_compositor_view_mac.h

@@ -89,17 +89,6 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient {
   // hidden (e.g, because it is occluded by another window).
   void SetNSViewAttachedToWindow(bool attached);
 
-  // These functions will track the number of outstanding copy requests, and
-  // will not allow the ui::Compositor to be detached until all outstanding
-  // copies have returned.
-  void CopyFromCompositingSurface(const gfx::Rect& src_subrect,
-                                  const gfx::Size& dst_size,
-                                  const ReadbackRequestCallback& callback,
-                                  SkColorType preferred_color_type);
-  void CopyFromCompositingSurfaceToVideoFrame(
-      const gfx::Rect& src_subrect,
-      scoped_refptr<media::VideoFrame> target,
-      const base::Callback<void(const gfx::Rect&, bool)>& callback);
   viz::FrameSinkId GetRootFrameSinkId();
 
   // Indicate that the recyclable compositor should be destroyed, and no future
@@ -167,19 +156,6 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient {
   void GetViewProperties(gfx::Size* bounds_in_dip,
                          float* scale_factor,
                          gfx::ColorSpace* color_space) const;
-
-  static void CopyCompleted(
-      base::WeakPtr<BrowserCompositorMac> browser_compositor,
-      const ReadbackRequestCallback& callback,
-      const SkBitmap& bitmap,
-      ReadbackResponse response);
-  static void CopyToVideoFrameCompleted(
-      base::WeakPtr<BrowserCompositorMac> browser_compositor,
-      const base::Callback<void(const gfx::Rect&, bool)>& callback,
-      const gfx::Rect& rect,
-      bool result);
-  uint64_t outstanding_copy_count_ = 0;
-
   bool render_widget_host_is_hidden_ = true;
   bool ns_view_attached_to_window_ = false;
 

content/browser/renderer_host/browser_compositor_view_mac.mm

@@ -236,62 +236,6 @@ void BrowserCompositorMac::ClearCompositorFrame() {
     delegated_frame_host_->ClearDelegatedFrame();
 }
 
-void BrowserCompositorMac::CopyCompleted(
-    base::WeakPtr<BrowserCompositorMac> browser_compositor,
-    const ReadbackRequestCallback& callback,
-    const SkBitmap& bitmap,
-    ReadbackResponse response) {
-  callback.Run(bitmap, response);
-  if (browser_compositor) {
-    browser_compositor->outstanding_copy_count_ -= 1;
-    browser_compositor->UpdateState();
-  }
-}
-
-void BrowserCompositorMac::CopyFromCompositingSurface(
-    const gfx::Rect& src_subrect,
-    const gfx::Size& dst_size,
-    const ReadbackRequestCallback& callback,
-    SkColorType preferred_color_type) {
-  DCHECK(delegated_frame_host_);
-  DCHECK(state_ == HasAttachedCompositor);
-  outstanding_copy_count_ += 1;
-
-  auto callback_with_decrement =
-      base::Bind(&BrowserCompositorMac::CopyCompleted,
-                 weak_factory_.GetWeakPtr(), callback);
-  delegated_frame_host_->CopyFromCompositingSurface(
-      src_subrect, dst_size, callback_with_decrement, preferred_color_type);
-}
-
-void BrowserCompositorMac::CopyToVideoFrameCompleted(
-    base::WeakPtr<BrowserCompositorMac> browser_compositor,
-    const base::Callback<void(const gfx::Rect&, bool)>& callback,
-    const gfx::Rect& rect,
-    bool result) {
-  callback.Run(rect, result);
-  if (browser_compositor) {
-    browser_compositor->outstanding_copy_count_ -= 1;
-    browser_compositor->UpdateState();
-  }
-}
-
-void BrowserCompositorMac::CopyFromCompositingSurfaceToVideoFrame(
-    const gfx::Rect& src_subrect,
-    scoped_refptr<media::VideoFrame> target,
-    const base::Callback<void(const gfx::Rect&, bool)>& callback) {
-  DCHECK(delegated_frame_host_);
-  DCHECK(state_ == HasAttachedCompositor);
-  outstanding_copy_count_ += 1;
-
-  auto callback_with_decrement =
-      base::Bind(&BrowserCompositorMac::CopyToVideoFrameCompleted,
-                 weak_factory_.GetWeakPtr(), callback);
-
-  delegated_frame_host_->CopyFromCompositingSurfaceToVideoFrame(
-      src_subrect, std::move(target), callback_with_decrement);
-}
-
 viz::FrameSinkId BrowserCompositorMac::GetRootFrameSinkId() {
   if (recyclable_compositor_->compositor())
     return recyclable_compositor_->compositor()->frame_sink_id();
@@ -372,7 +316,7 @@ void BrowserCompositorMac::SetNSViewAttachedToWindow(bool attached) {
 }
 
 void BrowserCompositorMac::UpdateState() {
-  if (!render_widget_host_is_hidden_ || outstanding_copy_count_ > 0)
+  if (!render_widget_host_is_hidden_)
     TransitionToState(HasAttachedCompositor);
   else if (ns_view_attached_to_window_)
     TransitionToState(HasDetachedCompositor);

content/browser/renderer_host/delegated_frame_host.cc

@@ -156,9 +156,15 @@ void DelegatedFrameHost::CopyFromCompositingSurface(
           viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE,
           base::BindOnce(&CopyFromCompositingSurfaceHasResult, output_size,
                          preferred_color_type, callback));
-  if (!src_subrect.IsEmpty())
-    request->set_area(src_subrect);
-  RequestCopyOfOutput(std::move(request));
+  if (!src_subrect.IsEmpty()) {
+    request->set_area(
+        gfx::ScaleToRoundedRect(src_subrect, active_device_scale_factor_));
+  }
+
+  if (request_copy_of_output_callback_for_testing_.is_null())
+    support_->RequestCopyOfSurface(std::move(request));
+  else
+    request_copy_of_output_callback_for_testing_.Run(std::move(request));
 }
 
 void DelegatedFrameHost::CopyFromCompositingSurfaceToVideoFrame(
@@ -177,13 +183,19 @@ void DelegatedFrameHost::CopyFromCompositingSurfaceToVideoFrame(
           &DelegatedFrameHost::CopyFromCompositingSurfaceHasResultForVideo,
           AsWeakPtr(),  // For caching the ReadbackYUVInterface on this class.
           nullptr, std::move(target), callback));
-  if (!src_subrect.IsEmpty())
-    request->set_area(src_subrect);
-  RequestCopyOfOutput(std::move(request));
+  if (!src_subrect.IsEmpty()) {
+    request->set_area(
+        gfx::ScaleToRoundedRect(src_subrect, active_device_scale_factor_));
+  }
+
+  if (request_copy_of_output_callback_for_testing_.is_null())
+    support_->RequestCopyOfSurface(std::move(request));
+  else
+    request_copy_of_output_callback_for_testing_.Run(std::move(request));
 }
 
 bool DelegatedFrameHost::CanCopyFromCompositingSurface() const {
-  return compositor_ && HasFallbackSurface();
+  return support_ && HasFallbackSurface() && active_device_scale_factor_ != 0.f;
 }
 
 void DelegatedFrameHost::BeginFrameSubscription(
@@ -438,12 +450,10 @@ void DelegatedFrameHost::AttemptFrameSubscriberCapture(
 
   // To avoid unnecessary browser composites, try to go directly to the Surface
   // rather than through the Layer (which goes through the browser compositor).
-  if (HasFallbackSurface() &&
-      request_copy_of_output_callback_for_testing_.is_null()) {
+  if (request_copy_of_output_callback_for_testing_.is_null())
     support_->RequestCopyOfSurface(std::move(request));
-  } else {
-    RequestCopyOfOutput(std::move(request));
-  }
+  else
+    request_copy_of_output_callback_for_testing_.Run(std::move(request));
 }
 
 void DelegatedFrameHost::DidCreateNewRendererCompositorFrameSink(
@@ -601,6 +611,7 @@ void DelegatedFrameHost::OnFirstSurfaceActivation(
   client_->DelegatedFrameHostGetLayer()->SetFallbackSurfaceId(
       surface_info.id());
   local_surface_id_ = surface_info.id().local_surface_id();
+  active_device_scale_factor_ = surface_info.device_scale_factor();
 
   // Surface synchronization deals with resizes in WasResized().
   if (!enable_surface_synchronization_) {
@@ -923,21 +934,6 @@ void DelegatedFrameHost::LockResources() {
   frame_evictor_->LockFrame();
 }
 
-void DelegatedFrameHost::RequestCopyOfOutput(
-    std::unique_ptr<viz::CopyOutputRequest> request) {
-  // If a specific area has not been requested, set one to ensure correct
-  // clipping occurs.
-  if (!request->has_area())
-    request->set_area(gfx::Rect(current_frame_size_in_dip_));
-
-  if (request_copy_of_output_callback_for_testing_.is_null()) {
-    client_->DelegatedFrameHostGetLayer()->RequestCopyOfOutput(
-        std::move(request));
-  } else {
-    request_copy_of_output_callback_for_testing_.Run(std::move(request));
-  }
-}
-
 void DelegatedFrameHost::UnlockResources() {
   DCHECK(local_surface_id_.is_valid());
   frame_evictor_->UnlockFrame();

content/browser/renderer_host/delegated_frame_host.h

@@ -231,7 +231,6 @@ class CONTENT_EXPORT DelegatedFrameHost
   }
   void LockResources();
   void UnlockResources();
-  void RequestCopyOfOutput(std::unique_ptr<viz::CopyOutputRequest> request);
 
   bool ShouldSkipFrame(const gfx::Size& size_in_dip);
 
@@ -287,6 +286,10 @@ class CONTENT_EXPORT DelegatedFrameHost
   const bool enable_viz_;
   ui::Compositor* compositor_ = nullptr;
 
+  // It reflects the scale factor of the surface most recently activated via
+  // OnFirstSurfaceActivation.
+  float active_device_scale_factor_ = 0.f;
+
   // The vsync manager we are observing for changes, if any.
   scoped_refptr<ui::CompositorVSyncManager> vsync_manager_;
 

content/browser/renderer_host/render_widget_host_view_mac.mm

@@ -1181,7 +1181,7 @@ void RenderWidgetHostViewMac::CopyFromSurface(
     const gfx::Size& dst_size,
     const ReadbackRequestCallback& callback,
     const SkColorType preferred_color_type) {
-  browser_compositor_->CopyFromCompositingSurface(
+  browser_compositor_->GetDelegatedFrameHost()->CopyFromCompositingSurface(
       src_subrect, dst_size, callback, preferred_color_type);
 }
 
@@ -1189,8 +1189,9 @@ void RenderWidgetHostViewMac::CopyFromSurfaceToVideoFrame(
     const gfx::Rect& src_subrect,
     scoped_refptr<media::VideoFrame> target,
     const base::Callback<void(const gfx::Rect&, bool)>& callback) {
-  browser_compositor_->CopyFromCompositingSurfaceToVideoFrame(
-      src_subrect, std::move(target), callback);
+  browser_compositor_->GetDelegatedFrameHost()
+      ->CopyFromCompositingSurfaceToVideoFrame(src_subrect, std::move(target),
+                                               callback);
 }
 
 void RenderWidgetHostViewMac::BeginFrameSubscription(