[Tab Capture] Honor capture size aspect ratio

This patch changes the WCVCD preferred size calculation to honor the
aspect ratio of the capture size. If the requested capture size is
larger than the view/screen size, it is scaled to fit within the bounds
of the screen but with the same resolution.

Bug: 1194803
Change-Id: If74320f382643748983bcff9371b351922238e74
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2844952
Commit-Queue: Jordan Bayles <jophba@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#875773}

content/browser/media/capture/web_contents_frame_tracker.cc

@@ -35,7 +35,7 @@ namespace content {
 
 namespace {
 
-// Note on lifetime: this context should be deleted when the web contents
+// Note on lifetime: this context should be deleted when the WebContents
 // is destroyed.
 class WebContentsContext : public WebContentsFrameTracker::Context {
  public:
@@ -81,7 +81,7 @@ class WebContentsContext : public WebContentsFrameTracker::Context {
 
   base::ScopedClosureRunner capture_handle_;
 
-  // The backing web contents.
+  // The backing WebContents.
   WebContents* contents_;
 };
 
@@ -138,14 +138,36 @@ void WebContentsFrameTracker::DidStopCapturingWebContents() {
 // the first capturer's preferred size is set.
 gfx::Size WebContentsFrameTracker::CalculatePreferredSize(
     const gfx::Size& capture_size) {
+  if (capture_size.IsEmpty()) {
+    // NOTE: An empty preferred size will cause the WebContents to keep its
+    // previous size preference.
+    return {};
+  }
   gfx::Size preferred_size = capture_size;
 
   // If we know the available size of the screen, we don't want to exceed
   // it as it may result in strange capture behavior in some cases.
   if (context_) {
     const base::Optional<gfx::Rect> screen_bounds = context_->GetScreenBounds();
     if (screen_bounds) {
-      preferred_size.SetToMin(screen_bounds->size());
+      if (screen_bounds->size().IsEmpty()) {
+        return {};
+      }
+
+      // We want to honor the aspect ratio of the capture size request while
+      // also limiting it to the screen bounds of the view.
+      // For motivation, see https://crbug.com/1194803.
+      const double x_ratio = static_cast<double>(capture_size.width()) /
+                             static_cast<double>(screen_bounds->size().width());
+      const double y_ratio =
+          static_cast<double>(capture_size.height()) /
+          static_cast<double>(screen_bounds->size().height());
+
+      const double scale_ratio = std::max(x_ratio, y_ratio);
+      if (scale_ratio > 1.0) {
+        preferred_size = gfx::ScaleToFlooredSize(
+            preferred_size, static_cast<float>(1 / scale_ratio));
+      }
     }
   }
   return preferred_size;

content/browser/media/capture/web_contents_frame_tracker_unittest.cc

@@ -25,8 +25,11 @@ using testing::Invoke;
 using testing::NiceMock;
 using testing::StrictMock;
 
-constexpr gfx::Size kSize720p = gfx::Size(1280, 720);
-constexpr gfx::Size kSize1080p = gfx::Size(1920, 1080);
+// Standardized screen resolutions to test common scenarios.
+constexpr gfx::Size kSizeZero{0, 0};
+constexpr gfx::Size kSize720p{1280, 720};
+constexpr gfx::Size kSize1080p{1920, 1080};
+constexpr gfx::Size kSizeWsxgaPlus{1680, 1050};
 
 class SimpleContext : public WebContentsFrameTracker::Context {
  public:
@@ -103,6 +106,7 @@ class WebContentsFrameTrackerTest : public RenderViewHostTestHarness {
   void SetUpOnUIThread() {
     auto context = std::make_unique<SimpleContext>();
     raw_context_ = context.get();
+    SetScreenSize(kSize1080p);
     tracker_->SetWebContentsAndContextForTesting(web_contents_.get(),
                                                  std::move(context));
     SetFrameSinkId(viz::FrameSinkId(123, 456));
@@ -173,29 +177,124 @@ class WebContentsFrameTrackerTest : public RenderViewHostTestHarness {
 
 TEST_F(WebContentsFrameTrackerTest, CalculatesPreferredSizeClampsToView) {
   SetScreenSize(kSize720p);
-  RunAllTasksUntilIdle();
   EXPECT_EQ(kSize720p, tracker()->CalculatePreferredSize(kSize720p));
   EXPECT_EQ(kSize720p, tracker()->CalculatePreferredSize(kSize1080p));
 }
 
 TEST_F(WebContentsFrameTrackerTest,
        CalculatesPreferredSizeNoLargerThanCaptureSize) {
   SetScreenSize(kSize1080p);
-  RunAllTasksUntilIdle();
-
   EXPECT_EQ(kSize720p, tracker()->CalculatePreferredSize(kSize720p));
   EXPECT_EQ(kSize1080p, tracker()->CalculatePreferredSize(kSize1080p));
 }
 
-TEST_F(WebContentsFrameTrackerTest, UpdatesPreferredSizeOnWebContents) {
+TEST_F(WebContentsFrameTrackerTest,
+       CalculatesPreferredSizeWithCorrectAspectRatio) {
+  SetScreenSize(kSizeWsxgaPlus);
+
+  // 720P is strictly less than WSXGA+, so should be unchanged.
+  EXPECT_EQ(kSize720p, tracker()->CalculatePreferredSize(kSize720p));
+
+  // 1080P is larger, so should be scaled appropriately.
+  EXPECT_EQ((gfx::Size{1680, 945}),
+            tracker()->CalculatePreferredSize(kSize1080p));
+
+  // Wider capture size.
+  EXPECT_EQ((gfx::Size{1680, 525}),
+            tracker()->CalculatePreferredSize(gfx::Size{3360, 1050}));
+
+  // Taller capture size.
+  EXPECT_EQ((gfx::Size{500, 1050}),
+            tracker()->CalculatePreferredSize(gfx::Size{1000, 2100}));
+}
+
+TEST_F(WebContentsFrameTrackerTest,
+       CalculatesPreferredSizeAspectRatioWithNoOffByOneErrors) {
+  SetScreenSize(kSizeWsxgaPlus);
+
+  // Wider capture size.
+  EXPECT_EQ((gfx::Size{1680, 525}),
+            tracker()->CalculatePreferredSize(gfx::Size{3360, 1050}));
+  EXPECT_EQ((gfx::Size{1680, 525}),
+            tracker()->CalculatePreferredSize(gfx::Size{3360, 1051}));
+  EXPECT_EQ((gfx::Size{1680, 526}),
+            tracker()->CalculatePreferredSize(gfx::Size{3360, 1052}));
+  EXPECT_EQ((gfx::Size{1680, 525}),
+            tracker()->CalculatePreferredSize(gfx::Size{3361, 1052}));
+  EXPECT_EQ((gfx::Size{1680, 666}),
+            tracker()->CalculatePreferredSize(gfx::Size{5897, 2339}));
+
+  // Taller capture size.
+  EXPECT_EQ((gfx::Size{500, 1050}),
+            tracker()->CalculatePreferredSize(gfx::Size{1000, 2100}));
+  EXPECT_EQ((gfx::Size{499, 1050}),
+            tracker()->CalculatePreferredSize(gfx::Size{1000, 2101}));
+  EXPECT_EQ((gfx::Size{499, 1050}),
+            tracker()->CalculatePreferredSize(gfx::Size{1000, 2102}));
+  EXPECT_EQ((gfx::Size{500, 1050}),
+            tracker()->CalculatePreferredSize(gfx::Size{1001, 2102}));
+  EXPECT_EQ((gfx::Size{500, 1050}),
+            tracker()->CalculatePreferredSize(gfx::Size{1002, 2102}));
+
+  // Some larger and prime factor cases to sanity check.
+  EXPECT_EQ((gfx::Size{1680, 565}),
+            tracker()->CalculatePreferredSize(gfx::Size{21841, 7351}));
+  EXPECT_EQ((gfx::Size{1680, 565}),
+            tracker()->CalculatePreferredSize(gfx::Size{21841, 7349}));
+  EXPECT_EQ((gfx::Size{1680, 565}),
+            tracker()->CalculatePreferredSize(gfx::Size{21839, 7351}));
+  EXPECT_EQ((gfx::Size{1680, 565}),
+            tracker()->CalculatePreferredSize(gfx::Size{21839, 7349}));
+
+  EXPECT_EQ((gfx::Size{1680, 670}),
+            tracker()->CalculatePreferredSize(gfx::Size{139441, 55651}));
+  EXPECT_EQ((gfx::Size{1680, 670}),
+            tracker()->CalculatePreferredSize(gfx::Size{139439, 55651}));
+  EXPECT_EQ((gfx::Size{1680, 670}),
+            tracker()->CalculatePreferredSize(gfx::Size{139441, 55649}));
+  EXPECT_EQ((gfx::Size{1680, 670}),
+            tracker()->CalculatePreferredSize(gfx::Size{139439, 55649}));
+
+  // Finally, just check for roundoff errors.
+  SetScreenSize(gfx::Size{1000, 1000});
+  EXPECT_EQ((gfx::Size{1000, 333}),
+            tracker()->CalculatePreferredSize(gfx::Size{3000, 1000}));
+}
+
+TEST_F(WebContentsFrameTrackerTest,
+       CalculatesPreferredSizeLeavesCaptureSizeIfSmallerThanScreen) {
+  // Smaller in both directions, but different aspect ratio, should be
+  // unchanged.
   SetScreenSize(kSize1080p);
+  EXPECT_EQ(kSizeWsxgaPlus, tracker()->CalculatePreferredSize(kSizeWsxgaPlus));
+}
+
+TEST_F(WebContentsFrameTrackerTest,
+       CalculatesPreferredSizeWithZeroValueProperly) {
+  // If a capture dimension is zero, the preferred size should be (0, 0).
+  EXPECT_EQ((kSizeZero), tracker()->CalculatePreferredSize(gfx::Size{0, 1000}));
+  EXPECT_EQ((kSizeZero), tracker()->CalculatePreferredSize(kSizeZero));
+  EXPECT_EQ((kSizeZero), tracker()->CalculatePreferredSize(gfx::Size{1000, 0}));
+
+  // If a screen dimension is zero, the preferred size should be (0, 0). This
+  // probably means the tab isn't being drawn anyway.
+  SetScreenSize(gfx::Size{1920, 0});
+  EXPECT_EQ(kSizeZero, tracker()->CalculatePreferredSize(kSize720p));
+  SetScreenSize(gfx::Size{0, 1080});
+  EXPECT_EQ(kSizeZero, tracker()->CalculatePreferredSize(kSize720p));
+  SetScreenSize(kSizeZero);
+  EXPECT_EQ(kSizeZero, tracker()->CalculatePreferredSize(kSize720p));
+}
+
+TEST_F(WebContentsFrameTrackerTest, UpdatesPreferredSizeOnWebContents) {
   StartTrackerOnUIThread(kSize720p);
   RunAllTasksUntilIdle();
 
   // In this case, the capture size requested is smaller than the screen size,
   // so it should be used.
   EXPECT_EQ(kSize720p, context()->last_capture_size());
   EXPECT_EQ(context()->capturer_count(), 1);
+
   // When we stop the tracker, the web contents issues a preferred size change
   // of the "old" size--so it shouldn't change.
   StopTrackerOnUIThread();