content/browser/renderer_host/media/video_capture_host_unittest.cc - Issue 2403533002: VideoCaptureHostTest: simplify testing and remove unused code

Unified Diff: content/browser/renderer_host/media/video_capture_host_unittest.cc

Issue 2403533002: VideoCaptureHostTest: simplify testing and remove unused code (Closed)

Patch Set: Created 4 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: content/browser/renderer_host/media/video_capture_host_unittest.cc

diff --git a/content/browser/renderer_host/media/video_capture_host_unittest.cc b/content/browser/renderer_host/media/video_capture_host_unittest.cc

index 58ba4ceb8734176f21e1af7abc38eade4ec9044a..c586f88f284e7d5499453e7851985044d44f2b9b 100644

--- a/content/browser/renderer_host/media/video_capture_host_unittest.cc

+++ b/content/browser/renderer_host/media/video_capture_host_unittest.cc

@@ -12,20 +12,16 @@

#include "base/bind.h"

#include "base/command_line.h"

-#include "base/files/file_util.h"

-#include "base/files/scoped_file.h"

#include "base/location.h"

#include "base/macros.h"

#include "base/memory/ptr_util.h"

#include "base/run_loop.h"

#include "base/single_thread_task_runner.h"

-#include "base/strings/stringprintf.h"

#include "base/threading/thread_task_runner_handle.h"

#include "build/build_config.h"

#include "content/browser/browser_thread_impl.h"

#include "content/browser/renderer_host/media/media_stream_manager.h"

#include "content/browser/renderer_host/media/media_stream_requester.h"

-#include "content/browser/renderer_host/media/media_stream_ui_proxy.h"

#include "content/browser/renderer_host/media/video_capture_manager.h"

#include "content/common/media/video_capture_messages.h"

#include "content/public/common/content_switches.h"

@@ -36,7 +32,6 @@

#include "media/audio/audio_manager.h"

#include "media/base/media_switches.h"

#include "media/base/video_capture_types.h"

-#include "media/base/video_frame.h"

#include "net/url_request/url_request_context.h"

#include "testing/gmock/include/gmock/gmock.h"

#include "testing/gtest/include/gtest/gtest.h"

@@ -60,39 +55,6 @@ namespace content {

// video_capture_host. This is an arbitrary value.

static const int kDeviceId = 555;

-// Define to enable test where video is dumped to file.

-// #define DUMP_VIDEO

-// Define to use a real video capture device.

-// #define TEST_REAL_CAPTURE_DEVICE

-// Simple class used for dumping video to a file. This can be used for verifying

-// the output.

-class DumpVideo {

- public:

- DumpVideo() {}

- const gfx::Size& coded_size() const { return coded_size_; }

- void StartDump(const gfx::Size& coded_size) {

- base::FilePath file_name = base::FilePath(base::StringPrintf(

- FILE_PATH_LITERAL("dump_w%d_h%d.yuv"),

- coded_size.width(),

- coded_size.height()));

- file_.reset(base::OpenFile(file_name, "wb"));

- coded_size_ = coded_size;

- }

- void NewVideoFrame(const void* buffer) {

- if (file_.get() != NULL) {

- const int size = media::VideoFrame::AllocationSize(

- media::PIXEL_FORMAT_I420, coded_size_);

- ASSERT_EQ(1U, fwrite(buffer, size, 1, file_.get()));

- }

- private:

- base::ScopedFILE file_;

- gfx::Size coded_size_;

-};

class MockMediaStreamRequester : public MediaStreamRequester {

public:

MockMediaStreamRequester() {}

@@ -129,11 +91,8 @@ class MockMediaStreamRequester : public MediaStreamRequester {

class MockVideoCaptureHost : public VideoCaptureHost {

public:

MockVideoCaptureHost(MediaStreamManager* manager)

- : VideoCaptureHost(manager),

- return_buffers_(false),

- dump_video_(false) {}

+ : VideoCaptureHost(manager), return_buffers_immediately_(false) {}

- // A list of mock methods.

MOCK_METHOD4(OnNewBufferCreated,

void(int device_id,

base::SharedMemoryHandle handle,

@@ -143,42 +102,24 @@ class MockVideoCaptureHost : public VideoCaptureHost {

MOCK_METHOD1(OnBufferFilled, void(int device_id));

MOCK_METHOD2(OnStateChanged, void(int device_id, VideoCaptureState state));

- // Use class DumpVideo to write I420 video to file.

- void SetDumpVideo(bool enable) {

- dump_video_ = enable;

+ void SetReturnReceivedBuffersImmediately(bool enable) {

+ return_buffers_immediately_ = enable;

}

- void SetReturnReceivedDibs(bool enable) {

- return_buffers_ = enable;

- }

- // Return Dibs we currently have received.

- void ReturnReceivedDibs(int device_id) {

- int handle = GetReceivedDib();

- while (handle) {

- this->OnRendererFinishedWithBuffer(device_id, handle, gpu::SyncToken(),

+ void ReturnReceivedBuffers(int device_id) {

+ while (!buffer_ids_.empty()) {

+ int buffer_id = *buffer_ids_.begin();

xianglu 2016/10/07 19:35:08 Might be better to use buffer_ids_.front() so no n

mcasas 2016/10/07 21:41:51 Done.

+ buffer_ids_.pop_front();

+ this->OnRendererFinishedWithBuffer(device_id, buffer_id, gpu::SyncToken(),

-1.0);

- handle = GetReceivedDib();

}

- int GetReceivedDib() {

- if (filled_dib_.empty())

- return 0;

- auto it = filled_dib_.begin();

- int h = it->first;

- filled_dib_.erase(it);

- return h;

- }

private:

- ~MockVideoCaptureHost() override {

- }

+ ~MockVideoCaptureHost() override {}

// This method is used to dispatch IPC messages to the renderer. We intercept

- // these messages here and dispatch to our mock methods to verify the

- // conversation between this object and the renderer.

+ // some of these messages here to MOCK them and to map/unmap buffers.

bool Send(IPC::Message* message) override {

CHECK(message);

@@ -189,7 +130,7 @@ class MockVideoCaptureHost : public VideoCaptureHost {

IPC_MESSAGE_HANDLER(VideoCaptureMsg_NewBuffer, OnNewBufferCreatedDispatch)

IPC_MESSAGE_HANDLER(VideoCaptureMsg_FreeBuffer, OnBufferFreedDispatch)

IPC_MESSAGE_HANDLER(VideoCaptureMsg_BufferReady, OnBufferFilledDispatch)

- IPC_MESSAGE_HANDLER(VideoCaptureMsg_StateChanged, OnStateChangedDispatch)

+ IPC_MESSAGE_HANDLER(VideoCaptureMsg_StateChanged, OnStateChanged)

IPC_MESSAGE_UNHANDLED(handled = false)

IPC_END_MESSAGE_MAP()

EXPECT_TRUE(handled);

@@ -204,48 +145,29 @@ class MockVideoCaptureHost : public VideoCaptureHost {

uint32_t length,

int buffer_id) {

OnNewBufferCreated(device_id, handle, length, buffer_id);

- std::unique_ptr<base::SharedMemory> dib =

- base::MakeUnique<base::SharedMemory>(handle, false);

- dib->Map(length);

- filled_dib_[buffer_id] = std::move(dib);

+ buffer_ids_.push_back(buffer_id);

}

void OnBufferFreedDispatch(int device_id, int buffer_id) {

OnBufferFreed(device_id, buffer_id);

- auto it = filled_dib_.find(buffer_id);

- ASSERT_TRUE(it != filled_dib_.end());

- filled_dib_.erase(it);

+ auto buffer = std::find(buffer_ids_.begin(), buffer_ids_.end(), buffer_id);

+ ASSERT_TRUE(buffer != buffer_ids_.end());

+ buffer_ids_.erase(buffer);

}

void OnBufferFilledDispatch(

const VideoCaptureMsg_BufferReady_Params& params) {

- base::SharedMemory* dib = filled_dib_[params.buffer_id].get();

- ASSERT_TRUE(dib != NULL);

- if (dump_video_) {

- if (dumper_.coded_size().IsEmpty())

- dumper_.StartDump(params.coded_size);

- ASSERT_TRUE(dumper_.coded_size() == params.coded_size)

- << "Dump format does not handle variable resolution.";

- dumper_.NewVideoFrame(dib->memory());

- }

OnBufferFilled(params.device_id);

- if (return_buffers_) {

- VideoCaptureHost::OnRendererFinishedWithBuffer(

- params.device_id, params.buffer_id, gpu::SyncToken(), -1.0);

- }

+ if (!return_buffers_immediately_)

+ return;

- void OnStateChangedDispatch(int device_id, VideoCaptureState state) {

- OnStateChanged(device_id, state);

+ VideoCaptureHost::OnRendererFinishedWithBuffer(

+ params.device_id, params.buffer_id, gpu::SyncToken(), -1.0);

}

- std::map<int, std::unique_ptr<base::SharedMemory>> filled_dib_;

- bool return_buffers_;

- bool dump_video_;

- media::VideoCaptureFormat format_;

- DumpVideo dumper_;

+ std::list<int> buffer_ids_;

+ bool return_buffers_immediately_;

};

ACTION_P2(ExitMessageLoop, task_runner, quit_closure) {

@@ -269,15 +191,12 @@ class VideoCaptureHostTest : public testing::Test {

chromeos::CrasAudioHandler::InitializeForTesting();

#endif

- // Create our own MediaStreamManager.

audio_manager_ = media::AudioManager::CreateForTesting(task_runner_);

-#ifndef TEST_REAL_CAPTURE_DEVICE

base::CommandLine::ForCurrentProcess()->AppendSwitch(

switches::kUseFakeDeviceForMediaStream);

-#endif

+ base::CommandLine::ForCurrentProcess()->AppendSwitch(

+ switches::kUseFakeUIForMediaStream);

media_stream_manager_.reset(new MediaStreamManager(audio_manager_.get()));

- media_stream_manager_->UseFakeUIForTests(

- std::unique_ptr<FakeMediaStreamUIProxy>());

// Create a Host and connect it to a simulated IPC channel.

host_ = new MockVideoCaptureHost(media_stream_manager_.get());

@@ -287,8 +206,6 @@ class VideoCaptureHostTest : public testing::Test {

}

void TearDown() override {

- // Verifies and removes the expectations on host_ and

- // returns true iff successful.

Mock::VerifyAndClearExpectations(host_.get());

EXPECT_TRUE(host_->controllers_.empty());

@@ -299,7 +216,7 @@ class VideoCaptureHostTest : public testing::Test {

// Release the reference to the mock object. The object will be destructed

// on the current message loop.

- host_ = NULL;

+ host_ = nullptr;

#if defined(OS_CHROMEOS)

chromeos::CrasAudioHandler::Shutdown();

@@ -423,26 +340,6 @@ class VideoCaptureHostTest : public testing::Test {

WaitForVideoDeviceThread();

}

-#ifdef DUMP_VIDEO

- void CaptureAndDumpVideo(int width, int height, int frame_rate) {

- InSequence s;

- EXPECT_CALL(*host_.get(), OnNewBufferCreated(kDeviceId, _, _, _))

- .Times(AnyNumber()).WillRepeatedly(Return());

- base::RunLoop run_loop;

- EXPECT_CALL(*host_, OnBufferFilled(kDeviceId, _, _, _, _, _, _, _, _, _))

- .Times(AnyNumber())

- .WillOnce(ExitMessageLoop(message_loop_, run_loop.QuitClosure()));

- media::VideoCaptureParams params;

- params.requested_format =

- media::VideoCaptureFormat(gfx::Size(width, height), frame_rate);

- host_->SetDumpVideo(true);

- host_->Start(kDeviceId, opened_session_id_, params);

- run_loop.Run();

- }

-#endif

void StopCapture() {

base::RunLoop run_loop;

EXPECT_CALL(*host_.get(),

@@ -450,12 +347,11 @@ class VideoCaptureHostTest : public testing::Test {

.WillOnce(ExitMessageLoop(task_runner_, run_loop.QuitClosure()));

host_->Stop(kDeviceId);

- host_->SetReturnReceivedDibs(true);

- host_->ReturnReceivedDibs(kDeviceId);

+ host_->SetReturnReceivedBuffersImmediately(true);

+ host_->ReturnReceivedBuffers(kDeviceId);

run_loop.Run();

- host_->SetReturnReceivedDibs(false);

// Expect the VideoCaptureDevice has been stopped

EXPECT_TRUE(host_->controllers_.empty());

}

@@ -470,7 +366,7 @@ class VideoCaptureHostTest : public testing::Test {

}

void ReturnReceivedPackets() {

- host_->ReturnReceivedDibs(kDeviceId);

+ host_->ReturnReceivedBuffers(kDeviceId);

}

void SimulateError() {

@@ -497,11 +393,12 @@ class VideoCaptureHostTest : public testing::Test {

private:

// |media_stream_manager_| needs to outlive |thread_bundle_| because it is a

- // MessageLoop::DestructionObserver. |audio_manager_| needs to outlive

- // |thread_bundle_| because it uses the underlying message loop.

+ // MessageLoop::DestructionObserver.

StrictMock<MockMediaStreamRequester> stream_requester_;

std::unique_ptr<MediaStreamManager> media_stream_manager_;

const content::TestBrowserThreadBundle thread_bundle_;

+ //|audio_manager_| needs to outlive |thread_bundle_| because it uses the

xianglu 2016/10/07 19:35:08 nit: missing one space

mcasas 2016/10/07 21:41:51 Done.

+ // underlying message loop.

media::ScopedAudioManagerPtr audio_manager_;

content::TestBrowserContext browser_context_;

content::TestContentBrowserClient browser_client_;

@@ -550,15 +447,6 @@ TEST_F(VideoCaptureHostTest, StartCaptureError) {

base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(200));

}

-#ifdef DUMP_VIDEO

-TEST_F(VideoCaptureHostTest, CaptureAndDumpVideoVga) {

- CaptureAndDumpVideo(640, 480, 30);

-TEST_F(VideoCaptureHostTest, CaptureAndDump720P) {

- CaptureAndDumpVideo(1280, 720, 30);

-#endif

TEST_F(VideoCaptureHostTest, PauseResumeCapture) {

StartCapture();

PauseResumeCapture();

« no previous file with comments | « content/browser/renderer_host/media/video_capture_host.cc ('k') | no next file » | no next file with comments »