Fix cropped snapshots in layout tests

Layout tests modify their device scale factor without notifying the
browser. This results in the browser making mistakes when calculating
the pixel size of the renderer. As a workaround, don't set
CopyOutputRequest's area if neither src_subrect or output_size is
provided to CopyFromCompositingSurface in order to avoid mismatch
between the area and the actual surface size.

Bug: 895556
Change-Id: I6096c1d2ef05919e3c7ea8c7a43f922c9d96cc7a
Reviewed-on: https://chromium-review.googlesource.com/c/1298112Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602862}

components/viz/service/frame_sinks/compositor_frame_sink_support.h

@@ -168,6 +168,12 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
   // string.
   static const char* GetSubmitResultAsString(SubmitResult result);
 
+  const std::vector<
+      std::pair<LocalSurfaceId, std::unique_ptr<CopyOutputRequest>>>&
+  copy_output_requests_for_testing() const {
+    return copy_output_requests_;
+  }
+
  private:
   friend class FrameSinkManagerTest;
 

content/browser/renderer_host/delegated_frame_host.cc

@@ -109,29 +109,21 @@ void DelegatedFrameHost::CopyFromCompositingSurface(
               },
               std::move(callback)));
 
-  if (!src_subrect.IsEmpty())
-    request->set_area(src_subrect);
-  if (!output_size.IsEmpty())
+  if (!src_subrect.IsEmpty()) {
+    request->set_area(
+        gfx::ScaleToRoundedRect(src_subrect, client_->GetDeviceScaleFactor()));
+  }
+  if (!output_size.IsEmpty()) {
+    // The CopyOutputRequest API does not allow fixing the output size. Instead
+    // we have the set area and scale in such a way that it would result in the
+    // desired output size.
+    if (!request->has_area())
+      request->set_area(gfx::Rect(surface_dip_size_));
     request->set_result_selection(gfx::Rect(output_size));
-
-  if (!request->has_area())
-    request->set_area(gfx::Rect(surface_dip_size_));
-
-  request->set_area(gfx::ScaleToRoundedRect(request->area(),
-                                            client_->GetDeviceScaleFactor()));
-
-  if (request->has_result_selection()) {
     const gfx::Rect& area = request->area();
-    const gfx::Rect& result_selection = request->result_selection();
-    if (area.IsEmpty() || result_selection.IsEmpty()) {
-      // Viz would normally return an empty result for an empty selection.
-      // However, this guard here is still necessary to protect against setting
-      // an illegal scaling ratio.
-      return;
-    }
     request->SetScaleRatio(
         gfx::Vector2d(area.width(), area.height()),
-        gfx::Vector2d(result_selection.width(), result_selection.height()));
+        gfx::Vector2d(output_size.width(), output_size.height()));
   }
   DCHECK(host_frame_sink_manager_);
   host_frame_sink_manager_->RequestCopyOfOutput(

ui/android/delegated_frame_host_android.cc

@@ -162,24 +162,17 @@ void DelegatedFrameHostAndroid::CopyFromCompositingSurface(
 
   if (!src_subrect.IsEmpty())
     request->set_area(src_subrect);
-  if (!output_size.IsEmpty())
+  if (!output_size.IsEmpty()) {
+    // The CopyOutputRequest API does not allow fixing the output size. Instead
+    // we have the set area and scale in such a way that it would result in the
+    // desired output size.
+    if (!request->has_area())
+      request->set_area(gfx::Rect(surface_size_in_pixels_));
     request->set_result_selection(gfx::Rect(output_size));
-
-  if (!request->has_area())
-    request->set_area(gfx::Rect(surface_size_in_pixels_));
-
-  if (request->has_result_selection()) {
     const gfx::Rect& area = request->area();
-    const gfx::Rect& result_selection = request->result_selection();
-    if (area.IsEmpty() || result_selection.IsEmpty()) {
-      // Viz would normally return an empty result for an empty selection.
-      // However, this guard here is still necessary to protect against setting
-      // an illegal scaling ratio.
-      return;
-    }
     request->SetScaleRatio(
         gfx::Vector2d(area.width(), area.height()),
-        gfx::Vector2d(result_selection.width(), result_selection.height()));
+        gfx::Vector2d(output_size.width(), output_size.height()));
   }
 
   host_frame_sink_manager_->RequestCopyOfOutput(
@@ -187,8 +180,7 @@ void DelegatedFrameHostAndroid::CopyFromCompositingSurface(
 }
 
 bool DelegatedFrameHostAndroid::CanCopyFromCompositingSurface() const {
-  return local_surface_id_.is_valid() && view_->GetWindowAndroid() &&
-         view_->GetWindowAndroid()->GetCompositor();
+  return local_surface_id_.is_valid();
 }
 
 void DelegatedFrameHostAndroid::EvictDelegatedFrame() {

ui/android/delegated_frame_host_android_unittest.cc

@@ -321,5 +321,31 @@ TEST_F(DelegatedFrameHostAndroidSurfaceSynchronizationTest, EmbedWhileHidden) {
   EXPECT_TRUE(frame_host_->HasSavedFrame());
 }
 
+// Verify that when a source rect or output size is not provided to
+// CopyFromCompositingSurface, the corresponding values in CopyOutputRequest
+// are also not initialized.
+TEST_F(DelegatedFrameHostAndroidSurfaceSynchronizationTest,
+       FullSurfaceCapture) {
+  // First embed a surface to make sure we have something to copy from.
+  viz::LocalSurfaceId id = allocator_.GenerateId();
+  gfx::Size size(100, 100);
+  frame_host_->EmbedSurface(id, size, cc::DeadlinePolicy::UseDefaultDeadline());
+
+  // Request readback without source rect or output size specified.
+  frame_host_->CopyFromCompositingSurface(gfx::Rect(), gfx::Size(),
+                                          base::DoNothing());
+
+  // Make sure the resulting CopyOutputRequest does not have its area or result
+  // selection set.
+  const std::vector<
+      std::pair<viz::LocalSurfaceId, std::unique_ptr<viz::CopyOutputRequest>>>&
+      requests = frame_sink_manager_impl_.GetFrameSinkForId(frame_sink_id_)
+                     ->copy_output_requests_for_testing();
+  ASSERT_EQ(1u, requests.size());
+  viz::CopyOutputRequest* request = requests[0].second.get();
+  EXPECT_FALSE(request->has_area());
+  EXPECT_FALSE(request->has_result_selection());
+}
+
 }  // namespace
 }  // namespace ui