Fix set_area sizes for RequestCopyOfOutput, and related tests.

content/browser/renderer_host/render_widget_host_view_browsertest.cc

Index: content/browser/renderer_host/render_widget_host_view_browsertest.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_browsertest.cc b/content/browser/renderer_host/render_widget_host_view_browsertest.cc
index d12edb3a10e8f863ce5409140c834666bc3cf82b..204edfa9c31d7cc95f73031ef635f6a19f219b1b 100644
--- a/content/browser/renderer_host/render_widget_host_view_browsertest.cc
+++ b/content/browser/renderer_host/render_widget_host_view_browsertest.cc
@@ -435,6 +435,11 @@ class CompositingRenderWidgetHostViewBrowserTestTabCapture
     SkAutoLockPixels bitmap_lock(bitmap);
     int fails = 0;
     for (int i = 0; i < bitmap.width() && fails < 10; ++i) {
+      // Exclude the middle 3 columns of pixels from testing since scaling could

[sky, 2014/08/13 22:49:28]

Do we really run tests at different scales? If so, wouldn't it be better to
force a particular scale so you don't have to deal with this?

[miu, 2014/08/14 00:18:33]

Removed.  Just checked this again, and it looks like it's not needed anymore. 
IIRC, an earlier iteration of the code was doing zooming (rather than
shrinking), which caused the implementation to mix pixels at the boundary
between the two color regions.

+      // cause these pixels to be a mix of the two colors.
+      if (i >= (bitmap.width() / 2 - 1) && i <= (bitmap.width() / 2 + 1))
+        continue;
+
       for (int j = 0; j < bitmap.height() && fails < 10; ++j) {
         if (!exclude_rect_.IsEmpty() && exclude_rect_.Contains(i, j))
           continue;
@@ -523,15 +528,12 @@ class CompositingRenderWidgetHostViewBrowserTestTabCapture
   // Loads a page two boxes side-by-side, each half the width of
   // |html_rect_size|, and with different background colors. The test then
   // copies from |copy_rect| region of the page into a bitmap of size
-  // |output_size|, and compares that with a bitmap of size
-  // |expected_bitmap_size|.
+  // |output_size|, and examines the resulting bitmap/VideoFrame.
   // Note that |output_size| may not have the same size as |copy_rect| (e.g.
-  // when the output is scaled). Also note that |expected_bitmap_size| may not
-  // be the same as |output_size| (e.g. when the device scale factor is not 1).
+  // when the output is scaled).
   void PerformTestWithLeftRightRects(const gfx::Size& html_rect_size,
                                      const gfx::Rect& copy_rect,
                                      const gfx::Size& output_size,
-                                     const gfx::Size& expected_bitmap_size,
                                      bool video_frame) {
     const gfx::Size box_size(html_rect_size.width() / 2,
                              html_rect_size.height());
@@ -592,7 +594,7 @@ class CompositingRenderWidgetHostViewBrowserTestTabCapture
       GiveItSomeTime();
 
     SkBitmap expected_bitmap;
-    SetupLeftRightBitmap(expected_bitmap_size, &expected_bitmap);
+    SetupLeftRightBitmap(output_size, &expected_bitmap);
     SetExpectedCopyFromCompositingSurfaceResult(true, expected_bitmap);
 
     base::RunLoop run_loop;
@@ -605,9 +607,9 @@ class CompositingRenderWidgetHostViewBrowserTestTabCapture
 
       scoped_refptr<media::VideoFrame> video_frame =
           media::VideoFrame::CreateFrame(media::VideoFrame::YV12,
-                                         expected_bitmap_size,
-                                         gfx::Rect(expected_bitmap_size),
-                                         expected_bitmap_size,
+                                         output_size,
+                                         gfx::Rect(output_size),
+                                         output_size,
                                          base::TimeDelta());
 
       base::Callback<void(bool success)> callback =
@@ -679,13 +681,11 @@ IN_PROC_BROWSER_TEST_P(CompositingRenderWidgetHostViewBrowserTestTabCapture,
                        CopyFromCompositingSurface_Origin_Unscaled) {
   gfx::Rect copy_rect(400, 300);
   gfx::Size output_size = copy_rect.size();
-  gfx::Size expected_bitmap_size = output_size;
   gfx::Size html_rect_size(400, 300);
   bool video_frame = false;
   PerformTestWithLeftRightRects(html_rect_size,
                                 copy_rect,
                                 output_size,
-                                expected_bitmap_size,
                                 video_frame);
 }
 
@@ -693,13 +693,11 @@ IN_PROC_BROWSER_TEST_P(CompositingRenderWidgetHostViewBrowserTestTabCapture,
                        CopyFromCompositingSurface_Origin_Scaled) {
   gfx::Rect copy_rect(400, 300);
   gfx::Size output_size(200, 100);
-  gfx::Size expected_bitmap_size = output_size;
   gfx::Size html_rect_size(400, 300);
   bool video_frame = false;
   PerformTestWithLeftRightRects(html_rect_size,
                                 copy_rect,
                                 output_size,
-                                expected_bitmap_size,
                                 video_frame);
 }
 
@@ -710,13 +708,11 @@ IN_PROC_BROWSER_TEST_P(CompositingRenderWidgetHostViewBrowserTestTabCapture,
   copy_rect = gfx::Rect(copy_rect.CenterPoint() - gfx::Vector2d(30, 30),
                         gfx::Size(60, 60));
   gfx::Size output_size = copy_rect.size();
-  gfx::Size expected_bitmap_size = output_size;
   gfx::Size html_rect_size(400, 300);
   bool video_frame = false;
   PerformTestWithLeftRightRects(html_rect_size,
                                 copy_rect,
                                 output_size,
-                                expected_bitmap_size,
                                 video_frame);
 }
 
@@ -727,13 +723,11 @@ IN_PROC_BROWSER_TEST_P(CompositingRenderWidgetHostViewBrowserTestTabCapture,
   copy_rect = gfx::Rect(copy_rect.CenterPoint() - gfx::Vector2d(30, 30),
                         gfx::Size(60, 60));
   gfx::Size output_size(20, 10);
-  gfx::Size expected_bitmap_size = output_size;
   gfx::Size html_rect_size(400, 300);
   bool video_frame = false;
   PerformTestWithLeftRightRects(html_rect_size,
                                 copy_rect,
                                 output_size,
-                                expected_bitmap_size,
                                 video_frame);
 }
 
@@ -744,13 +738,11 @@ IN_PROC_BROWSER_TEST_P(CompositingRenderWidgetHostViewBrowserTestTabCapture,
   copy_rect = gfx::Rect(copy_rect.CenterPoint() - gfx::Vector2d(45, 30),
                         gfx::Size(90, 60));
   gfx::Size output_size = copy_rect.size();
-  gfx::Size expected_bitmap_size = output_size;
   gfx::Size html_rect_size(400, 300);
   bool video_frame = true;
   PerformTestWithLeftRightRects(html_rect_size,
                                 copy_rect,
                                 output_size,
-                                expected_bitmap_size,
                                 video_frame);
 }
 
@@ -762,101 +754,139 @@ IN_PROC_BROWSER_TEST_P(CompositingRenderWidgetHostViewBrowserTestTabCapture,
                         gfx::Size(90, 60));
   // Scale to 30 x 20 (preserve aspect ratio).
   gfx::Size output_size(30, 20);
-  gfx::Size expected_bitmap_size = output_size;
   gfx::Size html_rect_size(400, 300);
   bool video_frame = true;
   PerformTestWithLeftRightRects(html_rect_size,
                                 copy_rect,
                                 output_size,
-                                expected_bitmap_size,
                                 video_frame);
 }
 
-class CompositingRenderWidgetHostViewTabCaptureHighDPI
+class CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI
     : public CompositingRenderWidgetHostViewBrowserTestTabCapture {
  public:
-  CompositingRenderWidgetHostViewTabCaptureHighDPI() : kScale(2.f) {}
+  CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI() {}

[danakj, 2014/08/13 14:31:30]

can remove this

[miu, 2014/08/13 18:11:10]

Tried that, and clang says:

../../content/browser/renderer_host/render_widget_host_view_browsertest.cc:859:1:
error: constructor for 'content::(anonymous
namespace)::CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI_CopyFromCompositingSurfaceToVideoFrame_ScaledResult_Test'
must explicitly initialize the base class 'content::(anonymous
namespace)::CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI' which
does not have a default constructor
IN_PROC_BROWSER_TEST_P(
^

[danakj, 2014/08/13 18:39:31]

Oh because of the DISALLOW_COPY_AND_ASSIGN I guess. I'm not used to seeing that
in *test.cc files.

 
-  virtual void SetUpOnMainThread() OVERRIDE {
-    base::CommandLine* cmd = base::CommandLine::ForCurrentProcess();
+  virtual void SetUpCommandLine(base::CommandLine* cmd) OVERRIDE {
+    CompositingRenderWidgetHostViewBrowserTestTabCapture::SetUpCommandLine(cmd);
     cmd->AppendSwitchASCII(switches::kForceDeviceScaleFactor,
                            base::StringPrintf("%f", scale()));
-#if defined(OS_WIN)
-    gfx::ForceHighDPISupportForTesting(scale());
-    gfx::EnableHighDPISupport();
-#endif
   }
 
-  float scale() const { return kScale; }
-
  private:
   virtual bool ShouldContinueAfterTestURLLoad() OVERRIDE {
     // Short-circuit a pass for platforms where setting up high-DPI fails.

[danakj, 2014/08/13 14:31:31]

Do these platforms still exist? Can you try removing this check and see if the
bots will take it?

[miu, 2014/08/13 18:11:10]

Done.  Cross your fingers!  ;-)

[miu, 2014/08/13 22:25:45]

The bots did not like this, so I put it back.

-    if (ui::GetScaleForScaleFactor(ui::GetSupportedScaleFactor(
-            GetScaleFactorForView(GetRenderWidgetHostView()))) != scale()) {
-      LOG(WARNING) << "Blindly passing this test: failed to set up "
-          "scale factor: " << scale();
+    const float actual_scale_factor =
+        GetScaleFactorForView(GetRenderWidgetHostView());
+    if (actual_scale_factor != scale()) {
+      LOG(WARNING) << "Blindly passing this test; failed to set up "
+          "scale factor: was " << actual_scale_factor
+                   << ", expected " << scale();
       return false;
     }
     return true;
   }
 
-  const float kScale;
+  static float scale() { return 2.0f; }
 
-  DISALLOW_COPY_AND_ASSIGN(CompositingRenderWidgetHostViewTabCaptureHighDPI);
+  DISALLOW_COPY_AND_ASSIGN(
+      CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI);
 };
 
-IN_PROC_BROWSER_TEST_P(CompositingRenderWidgetHostViewTabCaptureHighDPI,
-                       CopyFromCompositingSurface) {
+IN_PROC_BROWSER_TEST_P(
+    CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI,
+    CopyFromCompositingSurfaceToBitmap_EntireRegion) {
+  gfx::Size html_rect_size(200, 150);
   gfx::Rect copy_rect(200, 150);
   gfx::Size output_size = copy_rect.size();
-  gfx::Size expected_bitmap_size =
-      gfx::ToFlooredSize(gfx::ScaleSize(output_size, scale(), scale()));
+  bool video_frame = false;
+  PerformTestWithLeftRightRects(html_rect_size,
+                                copy_rect,
+                                output_size,
+                                video_frame);
+}
+
+IN_PROC_BROWSER_TEST_P(
+    CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI,
+    CopyFromCompositingSurfaceToBitmap_CenterRegion) {
   gfx::Size html_rect_size(200, 150);
+  // Grab 90x60 pixels from the center of the tab contents.
+  gfx::Rect copy_rect =
+      gfx::Rect(gfx::Rect(html_rect_size).CenterPoint() - gfx::Vector2d(45, 30),
+                gfx::Size(90, 60));
+  gfx::Size output_size = copy_rect.size();
+  bool video_frame = false;
+  PerformTestWithLeftRightRects(html_rect_size,
+                                copy_rect,
+                                output_size,
+                                video_frame);
+}
+
+IN_PROC_BROWSER_TEST_P(
+    CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI,
+    CopyFromCompositingSurfaceToBitmap_ScaledResult) {
+  gfx::Size html_rect_size(200, 100);
+  gfx::Rect copy_rect(200, 100);
+  gfx::Size output_size(100, 50);
   bool video_frame = false;
   PerformTestWithLeftRightRects(html_rect_size,
                                 copy_rect,
                                 output_size,
-                                expected_bitmap_size,
                                 video_frame);
 }
 
-IN_PROC_BROWSER_TEST_P(CompositingRenderWidgetHostViewTabCaptureHighDPI,
-                       CopyFromCompositingSurfaceVideoFrame) {
+IN_PROC_BROWSER_TEST_P(
+    CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI,
+    CopyFromCompositingSurfaceToVideoFrame_EntireRegion) {
+  gfx::Size html_rect_size(200, 150);
+  gfx::Rect copy_rect(200, 150);
+  gfx::Size output_size = copy_rect.size();
+  bool video_frame = true;
+  PerformTestWithLeftRightRects(html_rect_size,
+                                copy_rect,
+                                output_size,
+                                video_frame);
+}
+
+IN_PROC_BROWSER_TEST_P(
+    CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI,
+    CopyFromCompositingSurfaceToVideoFrame_CenterRegion) {
   gfx::Size html_rect_size(200, 150);
   // Grab 90x60 pixels from the center of the tab contents.
   gfx::Rect copy_rect =
       gfx::Rect(gfx::Rect(html_rect_size).CenterPoint() - gfx::Vector2d(45, 30),
                 gfx::Size(90, 60));
   gfx::Size output_size = copy_rect.size();

[danakj, 2014/08/13 14:31:30]

if you want something not scaled down, shouldn't the output_size be scaled by
the DSF?

[miu, 2014/08/13 18:11:10]

Done.  Good point.

-  gfx::Size expected_bitmap_size =
-      gfx::ToFlooredSize(gfx::ScaleSize(output_size, scale(), scale()));
   bool video_frame = true;
   PerformTestWithLeftRightRects(html_rect_size,
                                 copy_rect,
                                 output_size,
-                                expected_bitmap_size,
                                 video_frame);
 }
 
-#if !defined(USE_AURA) && !defined(OS_MACOSX)
-// TODO(danakj): Remove this case when GTK linux is no more and move the
-// values inline to testing::Values() below.
-static const CompositingMode kAllCompositingModes[] = {GL_COMPOSITING};
-#else
-static const CompositingMode kAllCompositingModes[] = {GL_COMPOSITING,
-                                                       SOFTWARE_COMPOSITING};
-#endif
+IN_PROC_BROWSER_TEST_P(
+    CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI,
+    CopyFromCompositingSurfaceToVideoFrame_ScaledResult) {
+  gfx::Size html_rect_size(200, 100);
+  gfx::Rect copy_rect(200, 100);
+  gfx::Size output_size(100, 50);
+  bool video_frame = true;
+  PerformTestWithLeftRightRects(html_rect_size,
+                                copy_rect,
+                                output_size,
+                                video_frame);
+}
 
 INSTANTIATE_TEST_CASE_P(GLAndSoftwareCompositing,
                         CompositingRenderWidgetHostViewBrowserTest,
-                        testing::ValuesIn(kAllCompositingModes));
+                        testing::Values(GL_COMPOSITING, SOFTWARE_COMPOSITING));
 INSTANTIATE_TEST_CASE_P(GLAndSoftwareCompositing,
                         CompositingRenderWidgetHostViewBrowserTestTabCapture,
-                        testing::ValuesIn(kAllCompositingModes));
-INSTANTIATE_TEST_CASE_P(GLAndSoftwareCompositing,
-                        CompositingRenderWidgetHostViewTabCaptureHighDPI,
-                        testing::ValuesIn(kAllCompositingModes));
+                        testing::Values(GL_COMPOSITING, SOFTWARE_COMPOSITING));
+INSTANTIATE_TEST_CASE_P(
+    GLAndSoftwareCompositing,
+    CompositingRenderWidgetHostViewBrowserTestTabCaptureHighDPI,
+    testing::Values(GL_COMPOSITING, SOFTWARE_COMPOSITING));
 
 #endif  // !defined(OS_ANDROID) && !defined(OS_IOS)