Improve VideoCaptureImpl unittest.

content/renderer/media/video_capture_impl_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/memory/shared_memory.h"
 #include "base/message_loop/message_loop.h"
 #include "content/child/child_process.h"
 #include "content/common/media/video_capture_messages.h"
 #include "content/renderer/media/video_capture_impl.h"
 #include "media/base/bind_to_current_loop.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::_;
 using ::testing::AtLeast;
@@ skipping 12 matching lines @@
   // Filter implementation.
   MOCK_METHOD1(Send, bool(IPC::Message* message));
 
  protected:
   virtual ~MockVideoCaptureMessageFilter() {}
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockVideoCaptureMessageFilter);
 };
 
-class VideoCaptureImplTest : public ::testing::Test {
+struct BufferReceivedTestArg {
+  BufferReceivedTestArg(int buffer_id,
+                        media::VideoPixelFormat pixel_format,
+                        media::VideoCapturePixelFormat capture_pixel_format,
+                        const gfx::Size& coded_size,
+                        const gpu::MailboxHolder& mailbox_holder)
+      : buffer_id(buffer_id), pixel_format(pixel_format),
+        capture_pixel_format(capture_pixel_format), coded_size(coded_size),
+        mailbox_holder(mailbox_holder) {}
+
+  int buffer_id;
+  media::VideoPixelFormat pixel_format;
+  media::VideoCapturePixelFormat capture_pixel_format;
+  const gfx::Size coded_size;
+  const gpu::MailboxHolder mailbox_holder;
+};

[mcasas, 2015/08/31 17:33:26]

Prefer 
struct Bla {
  // ...
} kBlas[] = {
  // ...
};

for one-off struts, so is easier to see that it's only
used in one place.

[msu.koo, 2015/09/01 05:25:01]

Moved |kBufferFormats| to INSTANTIATE_TEST_CASE_P as you commented below.

+
+static const BufferReceivedTestArg kBufferFormats[] = {
+    BufferReceivedTestArg(0, media::PIXEL_FORMAT_I420,

[mcasas, 2015/08/31 17:33:26]

|buffer_id| and |coded_size| are common among
the entries, so set them explicitly in the 
test itself.

And I'd call the struct BufferFormat, simpler :)

[msu.koo, 2015/09/01 05:25:01]

I basically agree with your opinion, but these variables may be possibly reused
when expending this unittest.
I think it's not bad to leave them as it is, WDYT?

On 2015/08/31 17:33:26, mcasas wrote:

[mcasas, 2015/09/01 18:09:53]

Ah, we'll extend it when the moment comes,
no dead code and no duplicated code in Chromium!

[msu.koo, 2015/09/02 08:38:10]

Done.

+                          media::VIDEO_CAPTURE_PIXEL_FORMAT_I420,
+                          gfx::Size(1280, 720),
+                          gpu::MailboxHolder()),
+    BufferReceivedTestArg(0, media::PIXEL_FORMAT_ARGB,
+                          media::VIDEO_CAPTURE_PIXEL_FORMAT_ARGB,
+                          gfx::Size(1280, 720),
+                          gpu::MailboxHolder(gpu::Mailbox::Generate(), 0, 0))};
+
+class VideoCaptureImplTest :
+    public ::testing::TestWithParam<BufferReceivedTestArg> {
  public:
   class MockVideoCaptureImpl : public VideoCaptureImpl {
    public:
     MockVideoCaptureImpl(const media::VideoCaptureSessionId id,
                          VideoCaptureMessageFilter* filter)
-        : VideoCaptureImpl(id, filter) {
+        : VideoCaptureImpl(id, filter), received_buffer_counts_(0) {
     }
     ~MockVideoCaptureImpl() override {}
 
     // Override Send() to mimic device to send events.
     void Send(IPC::Message* message) override {
       CHECK(message);
 
       // In this method, messages are sent to the according handlers as if
       // we are the device.
       bool handled = true;
@@ skipping 23 matching lines @@
 
     void DevicePauseCapture(int device_id) {}
 
     void DeviceStopCapture(int device_id) {
       OnStateChanged(VIDEO_CAPTURE_STATE_STOPPED);
     }
 
     void DeviceReceiveEmptyBuffer(int device_id,
                                   int buffer_id,
                                   uint32 sync_point,
-                                  double consumer_resource_utilization) {}
+                                  double consumer_resource_utilization) {
+      received_buffer_counts_++;
+    }
 
     void DeviceGetSupportedFormats(int device_id,
                                    media::VideoCaptureSessionId session_id) {
       // When the mock message filter receives a request for the device
       // supported formats, replies immediately with an empty format list.
       OnDeviceSupportedFormatsEnumerated(media::VideoCaptureFormats());
     }
 
     void DeviceGetFormatsInUse(int device_id,
                                media::VideoCaptureSessionId session_id) {
       OnDeviceFormatsInUseReceived(media::VideoCaptureFormats());
     }
 
     void ReceiveStateChangeMessage(VideoCaptureState state) {
       OnStateChanged(state);
     }
 
     const media::VideoCaptureParams& capture_params() const {

[mcasas, 2015/08/31 17:33:26]

unused now? (both method and member variable).

[msu.koo, 2015/09/01 05:25:01]

Done.

       return capture_params_;
     }
 
+    int GetReceivedBufferCounts() const { return received_buffer_counts_; }

[mcasas, 2015/08/31 17:33:26]

s/GetReceivedBufferCounts()/received_buffer_count()/

[1]
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Names

[msu.koo, 2015/09/01 05:25:01]

Done.

+
    private:
     media::VideoCaptureParams capture_params_;
+    int received_buffer_counts_;

[mcasas, 2015/08/31 17:33:26]

s/received_buffer_counts_/received_buffer_count_/

[msu.koo, 2015/09/01 05:25:01]

Done.

   };
 
   VideoCaptureImplTest() {
     params_small_.requested_format = media::VideoCaptureFormat(
         gfx::Size(176, 144), 30, media::VIDEO_CAPTURE_PIXEL_FORMAT_I420);
 
     params_large_.requested_format = media::VideoCaptureFormat(
         gfx::Size(320, 240), 30, media::VIDEO_CAPTURE_PIXEL_FORMAT_I420);
 
     child_process_.reset(new ChildProcess());
@@ skipping 26 matching lines @@
   void StartCapture(const media::VideoCaptureParams& params) {
     video_capture_impl_->StartCapture(
         params, base::Bind(&VideoCaptureImplTest::OnStateUpdate,
                            base::Unretained(this)),
         base::Bind(&VideoCaptureImplTest::OnFrameReady,
                    base::Unretained(this)));
   }
 
   void StopCapture() { video_capture_impl_->StopCapture(); }
 
+  void NewBuffer(int buffer_id, const base::SharedMemory& shm) {
+    video_capture_impl_->OnBufferCreated(
+        base::SharedMemory::DuplicateHandle(shm.handle()),
+        shm.mapped_size(), buffer_id);
+  }
+
+  void BufferReceived(int buffer_id, const gfx::Size& size,
+                      media::VideoPixelFormat pixel_format,
+                      const gpu::MailboxHolder& mailbox_holder) {
+    video_capture_impl_->OnBufferReceived(
+        buffer_id, base::TimeTicks::Now(), base::DictionaryValue(),
+        pixel_format, media::VideoFrame::STORAGE_SHMEM, size,
+        gfx::Rect(size), mailbox_holder);
+  }
+
+  void BufferDestroyed(int buffer_id) {
+    video_capture_impl_->OnBufferDestroyed(buffer_id);
+  }
+
   void DeInit() {
     video_capture_impl_->DeInit();
   }
 
   void GetDeviceSupportedFormats() {
     const base::Callback<void(const media::VideoCaptureFormats&)>
         callback = base::Bind(
             &VideoCaptureImplTest::OnDeviceSupportedFormats,
             base::Unretained(this));
     video_capture_impl_->GetDeviceSupportedFormats(callback);
@@ skipping 54 matching lines @@
 // Check that a request to GetDeviceFormatsInUse() ends up eventually in the
 // provided callback.
 TEST_F(VideoCaptureImplTest, GetDeviceFormatsInUse) {
   EXPECT_CALL(*this, OnDeviceFormatsInUse(_));
 
   Init();
   GetDeviceFormatsInUse();
   DeInit();
 }
 
+TEST_P(VideoCaptureImplTest, BufferReceivedWithFormat) {
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STARTED)).Times(1);
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STOPPED)).Times(1);
+  EXPECT_CALL(*this, OnFrameReady(_, _)).Times(1);
+
+  const BufferReceivedTestArg& buffer_arg = GetParam();
+
+  // Create a fake shared memory for buffer.
+  base::SharedMemory shm;
+  const size_t frame_size = media::VideoFrame::AllocationSize(
+      buffer_arg.pixel_format, buffer_arg.coded_size);
+  ASSERT_TRUE(shm.CreateAndMapAnonymous(frame_size));
+
+  media::VideoCaptureParams params;
+  params.requested_format = media::VideoCaptureFormat(
+      buffer_arg.coded_size, 30, buffer_arg.capture_pixel_format);
+
+  Init();
+  StartCapture(params);
+  NewBuffer(buffer_arg.buffer_id, shm);
+  BufferReceived(buffer_arg.buffer_id, buffer_arg.coded_size,
+                 buffer_arg.pixel_format, buffer_arg.mailbox_holder);
+  StopCapture();
+  BufferDestroyed(buffer_arg.buffer_id);
+  DeInit();
+}
+
+INSTANTIATE_TEST_CASE_P(I420AndARGB,
+                        VideoCaptureImplTest,
+                        testing::ValuesIn(kBufferFormats));

[mcasas, 2015/08/31 17:33:26]

You can also bring |kBufferFormats| here .

[msu.koo, 2015/09/01 05:25:01]

Done.

+
+TEST_F(VideoCaptureImplTest, BufferReceivedAfterStop) {
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STARTED)).Times(1);
+  EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STOPPED)).Times(1);
+  EXPECT_CALL(*this, OnFrameReady(_, _)).Times(0);
+
+  // Create a fake shared memory for buffer.
+  base::SharedMemory shm;
+  size_t i420_frame_size = media::VideoFrame::AllocationSize(

[mcasas, 2015/08/31 17:33:26]

const

[msu.koo, 2015/09/01 05:25:01]

Done.

+      media::PIXEL_FORMAT_I420, params_large_.requested_format.frame_size);
+  ASSERT_TRUE(shm.CreateAndMapAnonymous(i420_frame_size));
+
+  Init();
+  StartCapture(params_large_);
+  NewBuffer(0, shm);
+  StopCapture();
+  BufferReceived(0, params_large_.requested_format.frame_size,
+                 media::PIXEL_FORMAT_I420, gpu::MailboxHolder());
+  BufferDestroyed(0);
+  DeInit();
+
+  EXPECT_EQ(this->video_capture_impl_->GetReceivedBufferCounts(), 1);
+}
+
 TEST_F(VideoCaptureImplTest, EndedBeforeStop) {
    EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STARTED));
    EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STOPPED));
 
    Init();
    StartCapture(params_small_);
 
    // Receive state change message from browser.
    video_capture_impl_->ReceiveStateChangeMessage(VIDEO_CAPTURE_STATE_ENDED);
 
    StopCapture();
    DeInit();
 }
 
 TEST_F(VideoCaptureImplTest, ErrorBeforeStop) {
    EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_STARTED));
    EXPECT_CALL(*this, OnStateUpdate(VIDEO_CAPTURE_STATE_ERROR));
 
    Init();
    StartCapture(params_small_);
 
    // Receive state change message from browser.
    video_capture_impl_->ReceiveStateChangeMessage(VIDEO_CAPTURE_STATE_ERROR);
 
    StopCapture();
    DeInit();
 }
 
 }  // namespace content