Reland: Image Capture Linux/CrOs: wire some capabilities set/get

media/capture/video/video_capture_device_unittest.cc

Index: media/capture/video/video_capture_device_unittest.cc
diff --git a/media/capture/video/video_capture_device_unittest.cc b/media/capture/video/video_capture_device_unittest.cc
index 4d97f924dafbd09d6ed23d8f0a1f3b573d1f2ce2..d3a557c7abe33ac0a41352f26d2e3485c21312fe 100644
--- a/media/capture/video/video_capture_device_unittest.cc
+++ b/media/capture/video/video_capture_device_unittest.cc
@@ -49,14 +49,17 @@
 // We will always get YUYV from the Mac AVFoundation implementations.
 #define MAYBE_CaptureMjpeg DISABLED_CaptureMjpeg
 #define MAYBE_TakePhoto TakePhoto
+#define MAYBE_GetPhotoCapabilities DISABLED_GetPhotoCapabilities
 #elif defined(OS_WIN)
 #define MAYBE_AllocateBadSize AllocateBadSize
 #define MAYBE_CaptureMjpeg CaptureMjpeg
 #define MAYBE_TakePhoto TakePhoto
+#define MAYBE_GetPhotoCapabilities DISABLED_GetPhotoCapabilities
 #elif defined(OS_ANDROID)
 #define MAYBE_AllocateBadSize AllocateBadSize
 #define MAYBE_CaptureMjpeg CaptureMjpeg
 #define MAYBE_TakePhoto TakePhoto
+#define MAYBE_GetPhotoCapabilities GetPhotoCapabilities
 #elif defined(OS_LINUX)
 // AllocateBadSize will hang when a real camera is attached and if more than one
 // test is trying to use the camera (even across processes). Do NOT renable
@@ -65,10 +68,12 @@
 #define MAYBE_AllocateBadSize DISABLED_AllocateBadSize
 #define MAYBE_CaptureMjpeg CaptureMjpeg
 #define MAYBE_TakePhoto TakePhoto
+#define MAYBE_GetPhotoCapabilities GetPhotoCapabilities
 #else
 #define MAYBE_AllocateBadSize AllocateBadSize
 #define MAYBE_CaptureMjpeg CaptureMjpeg
 #define MAYBE_TakePhoto DISABLED_TakePhoto
+#define MAYBE_GetPhotoCapabilities DISABLED_GetPhotoCapabilities
 #endif
 
 using ::testing::_;
@@ -78,6 +83,10 @@ using ::testing::SaveArg;
 namespace media {
 namespace {
 
+ACTION_P(RunClosure, closure) {
+  closure.Run();
+}
+
 void DumpError(const tracked_objects::Location& location,
                const std::string& message) {
   DPLOG(ERROR) << location.ToString() << " " << message;
@@ -172,9 +181,22 @@ class MockImageCaptureClient : public base::RefCounted<MockImageCaptureClient> {
   MOCK_METHOD1(OnTakePhotoFailure,
                void(const base::Callback<void(mojom::BlobPtr)>&));
 
+  // GMock doesn't support move-only arguments, so we use this forward method.
+  void DoOnGetPhotoCapabilities(mojom::PhotoCapabilitiesPtr capabilities) {
+    capabilities_ = std::move(capabilities);
+    OnCorrectGetPhotoCapabilities();
+  }
+  MOCK_METHOD0(OnCorrectGetPhotoCapabilities, void(void));
+  MOCK_METHOD1(OnGetPhotoCapabilitiesFailure,
+               void(const base::Callback<void(mojom::PhotoCapabilitiesPtr)>&));
+
+  const mojom::PhotoCapabilities* capabilities() { return capabilities_.get(); }
+
  private:
   friend class base::RefCounted<MockImageCaptureClient>;
   virtual ~MockImageCaptureClient() {}
+
+  mojom::PhotoCapabilitiesPtr capabilities_;
 };
 
 class DeviceEnumerationListener
@@ -521,7 +543,7 @@ TEST_F(VideoCaptureDeviceTest, NoCameraSupportsPixelFormatMax) {
   ASSERT_FALSE(device_descriptor);
 }
 
-// Starts the camera and take a photo.
+// Starts the camera and takes a photo.

[chfremer, 2016/09/23 21:18:30]

This description is not really an answer to the question "What does this test?".
It is a description of the exercise, but does not say what is verified.

I would think something along the lines of "Tests that TakePhotoCallback is
invoked with valid data when calling TakePhoto() on an active capture device."

[mcasas, 2016/09/23 22:10:57]

Done.

 TEST_F(VideoCaptureDeviceTest, MAYBE_TakePhoto) {
   if (!EnumerateAndFindUsableDevices())
     return;
@@ -554,10 +576,63 @@ TEST_F(VideoCaptureDeviceTest, MAYBE_TakePhoto) {
       media::BindToCurrentLoop(base::Bind(
           &MockImageCaptureClient::OnTakePhotoFailure, image_capture_client_)));
 
-  EXPECT_CALL(*image_capture_client_.get(), OnCorrectPhotoTaken()).Times(1);
+  base::RunLoop run_loop;
+  base::Closure quit_closure = media::BindToCurrentLoop(run_loop.QuitClosure());
+  EXPECT_CALL(*image_capture_client_.get(), OnCorrectPhotoTaken())
+      .Times(1)
+      .WillOnce(RunClosure(quit_closure));
+
   device->TakePhoto(std::move(scoped_callback));
+  run_loop.Run();
+
+  device->StopAndDeAllocate();
+}
+
+// Starts the camera and tries to get the Photo capabilities.
+TEST_F(VideoCaptureDeviceTest, MAYBE_GetPhotoCapabilities) {
+  if (!EnumerateAndFindUsableDevices())
+    return;

[chfremer, 2016/09/23 21:18:29]

I assume we do this to not fail on machines that do not have devices attached.
But this way it will report the test as passed even though it really was
skipped. Is there a way in gtest to bail out of a test and have it marked as
skipped?

[mcasas, 2016/09/23 22:10:57]

Hmm that's a good suggestion. This behaviour is
pervasive among WebRtc tests, because many bots
don't have any camera. I would love to drop off
in the SetUp() version of the test, in that case
not logging the test's name on the console, but
I'm not sure how to do it.  I'll investigate.

Update: investigated and found out that there
is no runtime test-skipping features [1]. The
only way to skip it is to mark the test case 
name with a DISABLED_ prefix.


[1] https://github.com/google/googletest/issues/490

+
+#if defined(OS_ANDROID)
+  // TODO(mcasas): fails on Lollipop devices, reconnect https://crbug.com/646840
+  if (base::android::BuildInfo::GetInstance()->sdk_int() <
+      base::android::SDK_VERSION_MARSHMALLOW) {
+    return;
+  }
+#endif
+
+  std::unique_ptr<VideoCaptureDevice> device(
+      video_capture_device_factory_->CreateDevice(
+          device_descriptors_->front()));
+  ASSERT_TRUE(device);
+
+  EXPECT_CALL(*video_capture_client_, OnError(_, _)).Times(0);
+
+  VideoCaptureParams capture_params;
+  capture_params.requested_format.frame_size.SetSize(320, 240);
+  capture_params.requested_format.frame_rate = 30;
+  capture_params.requested_format.pixel_format = PIXEL_FORMAT_I420;
+  device->AllocateAndStart(capture_params, std::move(video_capture_client_));
   WaitForCapturedFrame();
 
+  VideoCaptureDevice::GetPhotoCapabilitiesCallback scoped_get_callback(
+      base::Bind(&MockImageCaptureClient::DoOnGetPhotoCapabilities,
+                 image_capture_client_),
+      media::BindToCurrentLoop(
+          base::Bind(&MockImageCaptureClient::OnGetPhotoCapabilitiesFailure,
+                     image_capture_client_)));
+
+  base::RunLoop run_loop;
+  base::Closure quit_closure = media::BindToCurrentLoop(run_loop.QuitClosure());
+  EXPECT_CALL(*image_capture_client_.get(), OnCorrectGetPhotoCapabilities())
+      .Times(1)
+      .WillOnce(RunClosure(quit_closure));
+
+  device->GetPhotoCapabilities(std::move(scoped_get_callback));
+  run_loop.Run();
+
+  ASSERT_TRUE(image_capture_client_->capabilities());
+
   device->StopAndDeAllocate();
 }