Implement all resolution change policies for desktop and tab capture.

content/browser/media/capture/web_contents_video_capture_device_unittest.cc

Index: content/browser/media/capture/web_contents_video_capture_device_unittest.cc
diff --git a/content/browser/media/capture/web_contents_video_capture_device_unittest.cc b/content/browser/media/capture/web_contents_video_capture_device_unittest.cc
index ab430a7625f31ecb52a1b94d90e1d34c17e63164..911feae0a4da9f82921de898f4056b374a20808f 100644
--- a/content/browser/media/capture/web_contents_video_capture_device_unittest.cc
+++ b/content/browser/media/capture/web_contents_video_capture_device_unittest.cc
@@ -162,13 +162,25 @@ class CaptureTestView : public TestRenderWidgetHostView {
   explicit CaptureTestView(RenderWidgetHostImpl* rwh,
                            CaptureTestSourceController* controller)
       : TestRenderWidgetHostView(rwh),
-        controller_(controller) {}
+        rwh_(rwh),
+        controller_(controller),
+        fake_bounds_(100, 100, 100 + kTestWidth, 100 + kTestHeight) {}
 
   ~CaptureTestView() override {}
 
   // TestRenderWidgetHostView overrides.
   gfx::Rect GetViewBounds() const override {
-    return gfx::Rect(100, 100, 100 + kTestWidth, 100 + kTestHeight);
+    return fake_bounds_;
+  }
+
+  void SetSize(const gfx::Size& size) override {
+    fake_bounds_.set_size(size);
+    rwh_->WasResized();

[Wez, 2015/05/14 01:44:56]

nit: Reimpl this in terms of SetBounds() via gfx::Rect(Size)?

[miu, 2015/05/14 21:12:27]

Done.

+  }
+
+  void SetBounds(const gfx::Rect& rect) override {
+    fake_bounds_ = rect;
+    rwh_->WasResized();

[miu, 2015/05/14 21:12:27]

FYI--I found a bug, where WasResized() wasn't notifying the WebContents as
expected because our test context doesn't spawn a render process.  Therefore, we
have to notify the WebContentsImpl ourselves.  I added this to the
SimulateSourceSizeChange() method below.

   }
 
   bool CanCopyToVideoFrame() const override {
@@ -212,7 +224,12 @@ class CaptureTestView : public TestRenderWidgetHostView {
 
  private:
   scoped_ptr<RenderWidgetHostViewFrameSubscriber> subscriber_;
+  // TODO(miu): Investigate why TestRenderWidgetHostView::GetRenderWidgetHost()
+  // always returns null, and perhaps fix it in order to remove this |rwh_|
+  // member.

[Wez, 2015/05/14 01:44:56]

Bug # for that change?

Looking at the code, doesn't seem there's a sound reason; the rwh parameter is
cast to a RWHImpl and stored in |rwh_|, which seems perfectly safe to cast back
to plain old RWH...

[miu, 2015/05/14 21:12:27]

Done.  http://crbug.com/487939

FYI--I reworked a few things around the frame size checking and didn't need to
access the RWH from this class anymore.
I suspect it was just an oversight.  However, I'd want to first make sure there
isn't code anywhere that purposely interprets a nullptr return value as "Do some
other behavior."

+  RenderWidgetHostImpl* const rwh_;
   CaptureTestSourceController* const controller_;
+  gfx::Rect fake_bounds_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(CaptureTestView);
 };
@@ -311,7 +328,9 @@ class StubClient : public media::VideoCaptureDevice::Client {
   StubClient(const base::Callback<void(SkColor)>& color_callback,
              const base::Closure& error_callback)
       : color_callback_(color_callback),
-        error_callback_(error_callback) {
+        error_callback_(error_callback),
+        expected_frame_size_(kTestWidth, kTestHeight),
+        resolution_change_policy_(media::RESOLUTION_POLICY_FIXED_RESOLUTION) {
     buffer_pool_ = new VideoCaptureBufferPool(2);
   }
   ~StubClient() override {}
@@ -335,6 +354,12 @@ class StubClient : public media::VideoCaptureDevice::Client {
 
   MOCK_METHOD0(DoOnIncomingCapturedBuffer, void(void));
 
+  void SetExpectedFrameSizeProperties(const gfx::Size& max_size,

[miu, 2015/05/14 21:12:27]

FYI--This was moved into StubClientObserver, since there are threading issues
and also delays in frame delivery after the source color or size are changed.

+                                      media::ResolutionChangePolicy policy) {
+    expected_frame_size_ = max_size;
+    resolution_change_policy_ = policy;
+  }
+
   scoped_ptr<media::VideoCaptureDevice::Client::Buffer> ReserveOutputBuffer(
       media::VideoPixelFormat format,
       const gfx::Size& dimensions) override {
@@ -360,7 +385,31 @@ class StubClient : public media::VideoCaptureDevice::Client {
       scoped_ptr<Buffer> buffer,
       const scoped_refptr<media::VideoFrame>& frame,
       const base::TimeTicks& timestamp) override {
-    EXPECT_EQ(gfx::Size(kTestWidth, kTestHeight), frame->visible_rect().size());
+    switch (resolution_change_policy_) {
+      case media::RESOLUTION_POLICY_FIXED_RESOLUTION:
+        EXPECT_EQ(expected_frame_size_, frame->visible_rect().size());
+        break;
+      case media::RESOLUTION_POLICY_FIXED_ASPECT_RATIO:
+        EXPECT_FALSE(frame->visible_rect().IsEmpty());
+        EXPECT_GE(expected_frame_size_.width(), frame->visible_rect().width());
+        EXPECT_GE(expected_frame_size_.height(),
+                  frame->visible_rect().height());
+        EXPECT_NEAR(static_cast<double>(expected_frame_size_.width()) /
+                        expected_frame_size_.height(),
+                    static_cast<double>(frame->visible_rect().width()) /
+                        frame->visible_rect().height(),
+                    0.01);
+        break;
+      case media::RESOLUTION_POLICY_ANY_WITHIN_LIMIT:
+        EXPECT_FALSE(frame->visible_rect().IsEmpty());
+        EXPECT_GE(expected_frame_size_.width(), frame->visible_rect().width());
+        EXPECT_GE(expected_frame_size_.height(),
+                  frame->visible_rect().height());
+        break;
+      default:
+        FAIL();
+        break;
+    }
     EXPECT_EQ(media::VideoFrame::I420, frame->format());
     double frame_rate = 0;
     EXPECT_TRUE(
@@ -410,6 +459,9 @@ class StubClient : public media::VideoCaptureDevice::Client {
   base::Callback<void(SkColor)> color_callback_;
   base::Closure error_callback_;
 
+  gfx::Size expected_frame_size_;

[Wez, 2015/05/14 01:44:56]

This is a misleading name, since it's not the actual expected frame size. :P

Generally speaking, it's preferable to have tests explicitly list out the values
that they expect rather than replicating production size-calculation logic,
which is basically what you're doing above in OnIncomingCapturedVideoFrame,
IIUC.

[miu, 2015/05/14 21:12:27]

Done.

+  media::ResolutionChangePolicy resolution_change_policy_;
+
   DISALLOW_COPY_AND_ASSIGN(StubClient);
 };
 
@@ -622,6 +674,13 @@ class WebContentsVideoCaptureDeviceTest : public testing::Test {
     }
   }
 
+  void SimulateSourceSizeChange(const gfx::Size& size) {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    CaptureTestView* test_view = static_cast<CaptureTestView*>(
+        web_contents_->GetRenderViewHost()->GetView());
+    test_view->SetSize(size);
+  }
+
   void DestroyVideoCaptureDevice() { device_.reset(); }
 
   StubClientObserver* client_observer() {
@@ -879,5 +938,157 @@ TEST_F(WebContentsVideoCaptureDeviceTest, BadFramesGoodFrames) {
   device()->StopAndDeAllocate();
 }
 
+// Tests that, when configured with the FIXED_ASPECT_RATIO resolution change
+// policy, the source size changes result in video frames of possibly varying
+// resolutions, but all with the same aspect ratio.
+TEST_F(WebContentsVideoCaptureDeviceTest, VariableResolution_FixedAspectRatio) {
+  media::VideoCaptureParams capture_params;
+  capture_params.requested_format.frame_size.SetSize(kTestWidth, kTestHeight);
+  capture_params.requested_format.frame_rate = kTestFramesPerSecond;
+  capture_params.requested_format.pixel_format = media::PIXEL_FORMAT_I420;
+  capture_params.resolution_change_policy =
+      media::RESOLUTION_POLICY_FIXED_ASPECT_RATIO;
+
+  scoped_ptr<media::VideoCaptureDevice::Client> client =
+      client_observer()->PassClient();
+  StubClient* const stub_client = static_cast<StubClient*>(client.get());
+  stub_client->SetExpectedFrameSizeProperties(
+      gfx::Size(kTestWidth, kTestHeight),
+      media::RESOLUTION_POLICY_FIXED_ASPECT_RATIO);
+  device()->AllocateAndStart(capture_params, client.Pass());
+
+  for (int i = 0; i < 6; i++) {
+    const char* name = NULL;
+    switch (i % 3) {
+      case 0:
+        source()->SetCanCopyToVideoFrame(true);
+        source()->SetUseFrameSubscriber(false);
+        name = "VideoFrame";
+        break;
+      case 1:
+        source()->SetCanCopyToVideoFrame(false);
+        source()->SetUseFrameSubscriber(true);
+        name = "Subscriber";
+        break;
+      case 2:
+        source()->SetCanCopyToVideoFrame(false);
+        source()->SetUseFrameSubscriber(false);
+        name = "SkBitmap";
+        break;
+      default:
+        FAIL();
+    }
+
+    SCOPED_TRACE(base::StringPrintf("Using %s path, iteration #%d", name, i));
+
+    // Source size equals maximum size.  StubClient will expect frames to be
+    // kTestWidth by kTestHeight.
+    SimulateSourceSizeChange(gfx::Size(kTestWidth, kTestHeight));
+    source()->SetSolidColor(SK_ColorRED);
+    SimulateDrawEvent();
+    ASSERT_NO_FATAL_FAILURE(client_observer()->WaitForNextColor(SK_ColorRED));
+
+    // Source size is half in both dimensions.  StubClient will confirm the
+    // aspect ratio of the resulting frames is correct.
+    SimulateSourceSizeChange(gfx::Size(kTestWidth / 2, kTestHeight / 2));
+    source()->SetSolidColor(SK_ColorGREEN);
+    SimulateDrawEvent();
+    ASSERT_NO_FATAL_FAILURE(client_observer()->WaitForNextColor(SK_ColorGREEN));
+
+    // Source size changes aspect ratio.  StubClient will confirm the aspect
+    // ratio of the resulting frames is correct.
+    SimulateSourceSizeChange(gfx::Size(kTestWidth / 2, kTestHeight));
+    source()->SetSolidColor(SK_ColorBLUE);
+    SimulateDrawEvent();
+    ASSERT_NO_FATAL_FAILURE(client_observer()->WaitForNextColor(SK_ColorBLUE));
+
+    // Source size changes aspect ratio again.  StubClient will confirm the
+    // aspect ratio of the resulting frames is correct.
+    SimulateSourceSizeChange(gfx::Size(kTestWidth, kTestHeight / 2));
+    source()->SetSolidColor(SK_ColorBLACK);
+    SimulateDrawEvent();
+    ASSERT_NO_FATAL_FAILURE(client_observer()->WaitForNextColor(SK_ColorBLACK));
+  }
+
+  device()->StopAndDeAllocate();
+}
+
+// Tests that, when configured with the ANY_WITHIN_LIMIT resolution change
+// policy, the source size changes result in video frames of possibly varying
+// resolutions.
+TEST_F(WebContentsVideoCaptureDeviceTest, VariableResolution_AnyWithinLimits) {
+  media::VideoCaptureParams capture_params;
+  capture_params.requested_format.frame_size.SetSize(kTestWidth, kTestHeight);
+  capture_params.requested_format.frame_rate = kTestFramesPerSecond;
+  capture_params.requested_format.pixel_format = media::PIXEL_FORMAT_I420;
+  capture_params.resolution_change_policy =
+      media::RESOLUTION_POLICY_ANY_WITHIN_LIMIT;
+
+  scoped_ptr<media::VideoCaptureDevice::Client> client =
+      client_observer()->PassClient();
+  StubClient* const stub_client = static_cast<StubClient*>(client.get());
+  stub_client->SetExpectedFrameSizeProperties(
+      gfx::Size(kTestWidth, kTestHeight),
+      media::RESOLUTION_POLICY_ANY_WITHIN_LIMIT);
+  device()->AllocateAndStart(capture_params, client.Pass());
+
+  for (int i = 0; i < 6; i++) {
+    const char* name = NULL;
+    switch (i % 3) {
+      case 0:
+        source()->SetCanCopyToVideoFrame(true);
+        source()->SetUseFrameSubscriber(false);
+        name = "VideoFrame";
+        break;
+      case 1:
+        source()->SetCanCopyToVideoFrame(false);
+        source()->SetUseFrameSubscriber(true);
+        name = "Subscriber";
+        break;
+      case 2:
+        source()->SetCanCopyToVideoFrame(false);
+        source()->SetUseFrameSubscriber(false);
+        name = "SkBitmap";
+        break;
+      default:
+        FAIL();
+    }
+
+    SCOPED_TRACE(base::StringPrintf("Using %s path, iteration #%d", name, i));
+
+    // Source size equals maximum size.  StubClient will expect frames to be
+    // kTestWidth by kTestHeight.
+    SimulateSourceSizeChange(gfx::Size(kTestWidth, kTestHeight));
+    source()->SetSolidColor(SK_ColorRED);
+    SimulateDrawEvent();
+    ASSERT_NO_FATAL_FAILURE(client_observer()->WaitForNextColor(SK_ColorRED));
+
+    // Source size is half in both dimensions.  StubClient will confirm the
+    // resulting frame is also half in both dimensions.
+    SimulateSourceSizeChange(gfx::Size(kTestWidth / 2, kTestHeight / 2));
+    source()->SetSolidColor(SK_ColorGREEN);
+    SimulateDrawEvent();
+    ASSERT_NO_FATAL_FAILURE(client_observer()->WaitForNextColor(SK_ColorGREEN));
+
+    // Source size changes to something arbitrary.  StubClient will confirm the
+    // resulting frame has the same dimensions.
+    SimulateSourceSizeChange(gfx::Size(kTestWidth / 2 + 42,
+                                       kTestHeight * 2 - 10));
+    source()->SetSolidColor(SK_ColorBLUE);
+    SimulateDrawEvent();
+    ASSERT_NO_FATAL_FAILURE(client_observer()->WaitForNextColor(SK_ColorBLUE));
+
+    // Source size changes to something arbitrary that exceeds the maximum frame
+    // size.  StubClient will confirm the resulting frame has dimensions within
+    // the maximum limit.
+    SimulateSourceSizeChange(gfx::Size(kTestWidth * 2 + 99, kTestHeight / 2));
+    source()->SetSolidColor(SK_ColorBLACK);
+    SimulateDrawEvent();
+    ASSERT_NO_FATAL_FAILURE(client_observer()->WaitForNextColor(SK_ColorBLACK));
+  }
+
+  device()->StopAndDeAllocate();
+}
+
 }  // namespace
 }  // namespace content