From 08a77cd3d169e297cf1c13670821600867aa5043 Mon Sep 17 00:00:00 2001 From: "fischman@chromium.org" Date: Fri, 30 Sep 2011 15:09:52 +0000 Subject: Follow-up cleanup promised during r103376's CR. - mock_task.{h,cc} is gone. - MockCallback is now MockClosure, and its commentary brought up to date (the commentary checked in was a mix of old and attempt-at-new that never materialized) - NewExpectedCallback is NewExpectedClosure. A bit of background on FooCallback vs. FooCB: when acolwell@ & I did the first conversions to the new world, everything was named FooCallback. I proposed using FooCB for the migrated ones as a way to both easily visually differentiate as well as save characters (!). Now that we have an additional "don't typedef Closures" guideline I like having FooCB for non-closure new-style callbacks, and FooClosure for new-style closures. BUG=none TEST=trybots Review URL: http://codereview.chromium.org/8085017 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@103464 0039d316-1c4b-4281-b951-d872f2087c98 --- media/base/composite_filter_unittest.cc | 54 ++++++++--------- media/base/mock_callback.cc | 10 ++-- media/base/mock_callback.h | 16 ++--- media/base/mock_task.cc | 22 ------- media/base/mock_task.h | 103 -------------------------------- 5 files changed, 40 insertions(+), 165 deletions(-) delete mode 100644 media/base/mock_task.cc delete mode 100644 media/base/mock_task.h (limited to 'media/base') diff --git a/media/base/composite_filter_unittest.cc b/media/base/composite_filter_unittest.cc index 52b80db..23b7f56 100644 --- a/media/base/composite_filter_unittest.cc +++ b/media/base/composite_filter_unittest.cc @@ -260,9 +260,9 @@ void CompositeFilterTest::ExpectSuccess(MethodToCall method_to_call, } // Make method call on the composite. - StrictMock* callback = new StrictMock(); + StrictMock* callback = new StrictMock(); DoFilterCall(method_to_call, composite_.get(), seek_time, - base::Bind(&MockCallback::Run, callback), + base::Bind(&MockClosure::Run, callback), PIPELINE_OK); if (is_parallel_call) { @@ -318,7 +318,7 @@ void CompositeFilterTest::ExpectInvalidStateFail(MethodToCall method_to_call, .WillOnce(Return()); } - DoFilterCall(method_to_call, composite_, seek_time, NewExpectedCallback(), + DoFilterCall(method_to_call, composite_, seek_time, NewExpectedClosure(), PIPELINE_ERROR_INVALID_STATE); // Make sure that neither of the filters were called by @@ -405,7 +405,7 @@ TEST_F(CompositeFilterTest, TestPlay) { // Try calling Play() again to make sure that we simply get a callback. // We are already in the Play() state so there is no point calling the // filters. - composite_->Play(NewExpectedCallback()); + composite_->Play(NewExpectedClosure()); // Verify that neither of the filter callbacks were set. EXPECT_FALSE(HasFilter1Callback()); @@ -417,7 +417,7 @@ TEST_F(CompositeFilterTest, TestPlay) { // At this point we should be in the kStopped state. // Try calling Stop() again to make sure neither filter is called. - composite_->Stop(NewExpectedCallback()); + composite_->Stop(NewExpectedClosure()); // Verify that neither of the filter callbacks were set. EXPECT_FALSE(HasFilter1Callback()); @@ -436,8 +436,8 @@ TEST_F(CompositeFilterTest, TestPlayErrors) { EXPECT_CALL(*filter_1_, Play(_)); // Call Play() on the composite. - StrictMock* callback = new StrictMock(); - composite_->Play(base::Bind(&MockCallback::Run, callback)); + StrictMock* callback = new StrictMock(); + composite_->Play(base::Bind(&MockClosure::Run, callback)); EXPECT_CALL(*filter_2_, Play(_)); @@ -484,7 +484,7 @@ TEST_F(CompositeFilterTest, TestPause) { // Try calling Pause() again to make sure that the filters aren't called // because we are already in the paused state. - composite_->Pause(NewExpectedCallback()); + composite_->Pause(NewExpectedClosure()); // Verify that neither of the filter callbacks were set. EXPECT_FALSE(HasFilter1Callback()); @@ -515,8 +515,8 @@ TEST_F(CompositeFilterTest, TestPauseErrors) { EXPECT_CALL(*filter_1_, Pause(_)); // Call Pause() on the composite. - StrictMock* callback = new StrictMock(); - composite_->Pause(base::Bind(&MockCallback::Run, callback)); + StrictMock* callback = new StrictMock(); + composite_->Pause(base::Bind(&MockClosure::Run, callback)); // Simulate an error by calling SetError() on |filter_1_|'s FilterHost // interface. @@ -582,8 +582,8 @@ TEST_F(CompositeFilterTest, TestFlushErrors) { EXPECT_CALL(*filter_2_, Flush(_)); // Call Flush() on the composite. - StrictMock* callback = new StrictMock(); - composite_->Flush(base::Bind(&MockCallback::Run, callback)); + StrictMock* callback = new StrictMock(); + composite_->Flush(base::Bind(&MockClosure::Run, callback)); // Simulate an error by calling SetError() on |filter_1_|'s FilterHost // interface. @@ -649,8 +649,8 @@ TEST_F(CompositeFilterTest, TestStop) { EXPECT_CALL(*filter_1_, Stop(_)); - StrictMock* callback = new StrictMock(); - composite_->Stop(base::Bind(&MockCallback::Run, callback)); + StrictMock* callback = new StrictMock(); + composite_->Stop(base::Bind(&MockClosure::Run, callback)); // Have |filter_1_| signal an error. filter_1_->host()->SetError(PIPELINE_ERROR_READ); @@ -672,13 +672,13 @@ TEST_F(CompositeFilterTest, TestStopWhilePlayPending) { EXPECT_CALL(*filter_1_, Play(_)); - StrictMock* callback = new StrictMock(); - composite_->Play(base::Bind(&MockCallback::Run, callback)); + StrictMock* callback = new StrictMock(); + composite_->Play(base::Bind(&MockClosure::Run, callback)); // Note: Play() is pending on |filter_1_| right now. - callback = new StrictMock(); - composite_->Stop(base::Bind(&MockCallback::Run, callback)); + callback = new StrictMock(); + composite_->Stop(base::Bind(&MockClosure::Run, callback)); EXPECT_CALL(*filter_1_, Stop(_)); @@ -705,13 +705,13 @@ TEST_F(CompositeFilterTest, TestStopWhileFlushPending) { EXPECT_CALL(*filter_1_, Flush(_)); EXPECT_CALL(*filter_2_, Flush(_)); - StrictMock* callback = new StrictMock(); - composite_->Flush(base::Bind(&MockCallback::Run, callback)); + StrictMock* callback = new StrictMock(); + composite_->Flush(base::Bind(&MockClosure::Run, callback)); // Note: |filter_1_| and |filter_2_| have pending Flush() calls at this point. - callback = new StrictMock(); - composite_->Stop(base::Bind(&MockCallback::Run, callback)); + callback = new StrictMock(); + composite_->Stop(base::Bind(&MockClosure::Run, callback)); // Run callback to indicate that |filter_1_|'s Flush() has completed. RunFilter1Callback(); @@ -769,23 +769,23 @@ TEST_F(CompositeFilterTest, TestEmptyComposite) { composite_->set_host(mock_filter_host_.get()); // Issue a Play() and expect no errors. - composite_->Play(NewExpectedCallback()); + composite_->Play(NewExpectedClosure()); // Issue a Pause() and expect no errors. - composite_->Pause(NewExpectedCallback()); + composite_->Pause(NewExpectedClosure()); // Issue a Flush() and expect no errors. - composite_->Flush(NewExpectedCallback()); + composite_->Flush(NewExpectedClosure()); // Issue a Seek() and expect no errors. composite_->Seek(base::TimeDelta::FromSeconds(5), NewExpectedStatusCB(PIPELINE_OK)); // Issue a Play() and expect no errors. - composite_->Play(NewExpectedCallback()); + composite_->Play(NewExpectedClosure()); // Issue a Stop() and expect no errors. - composite_->Stop(NewExpectedCallback()); + composite_->Stop(NewExpectedClosure()); } } // namespace media diff --git a/media/base/mock_callback.cc b/media/base/mock_callback.cc index fe991f8..58cc98e 100644 --- a/media/base/mock_callback.cc +++ b/media/base/mock_callback.cc @@ -11,13 +11,13 @@ using ::testing::StrictMock; namespace media { -MockCallback::MockCallback() {} -MockCallback::~MockCallback() {} +MockClosure::MockClosure() {} +MockClosure::~MockClosure() {} -base::Closure NewExpectedCallback() { - StrictMock* callback = new StrictMock(); +base::Closure NewExpectedClosure() { + StrictMock* callback = new StrictMock(); EXPECT_CALL(*callback, Run()); - return base::Bind(&MockCallback::Run, callback); + return base::Bind(&MockClosure::Run, callback); } class MockStatusCB : public base::RefCountedThreadSafe { diff --git a/media/base/mock_callback.h b/media/base/mock_callback.h index adee5c6..6d50844 100644 --- a/media/base/mock_callback.h +++ b/media/base/mock_callback.h @@ -11,20 +11,20 @@ namespace media { -// Utility class that presents a base::Closure interface (through as_closure()) -// and the ability to set a gMock expectation of being called (through -// ExpectCall). -class MockCallback : public base::RefCountedThreadSafe { +// Utility mock for testing methods expecting Closures. See +// NewExpectedClosure() below for a helper suitable when expectation order is +// not checked (or when the expectation can be set at mock construction time). +class MockClosure : public base::RefCountedThreadSafe { public: - MockCallback(); - virtual ~MockCallback(); + MockClosure(); + virtual ~MockClosure(); MOCK_METHOD0(Run, void()); private: - DISALLOW_COPY_AND_ASSIGN(MockCallback); + DISALLOW_COPY_AND_ASSIGN(MockClosure); }; // Return a callback that expects to be run once. -base::Closure NewExpectedCallback(); +base::Closure NewExpectedClosure(); base::Callback NewExpectedStatusCB(PipelineStatus status); } // namespace media diff --git a/media/base/mock_task.cc b/media/base/mock_task.cc deleted file mode 100644 index f9f8823..0000000 --- a/media/base/mock_task.cc +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (c) 2011 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 "media/base/mock_task.h" - -namespace media { - -TaskMocker::TaskMocker() - : outstanding_tasks_(0) { -} - -TaskMocker::~TaskMocker() { - CHECK(outstanding_tasks_ == 0) - << "If outstanding_tasks_ is not zero, tasks have been leaked."; -} - -Task* TaskMocker::CreateTask() { - return new CountingTask(this); -} - -} // namespace media diff --git a/media/base/mock_task.h b/media/base/mock_task.h deleted file mode 100644 index 53b66b7..0000000 --- a/media/base/mock_task.h +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright (c) 2011 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. - -// This file provides some utility classes that help with testing APIs which use -// callbacks. -// -// -- InvokeRunnable -- -// The InvokeRunnable is an action that can be used a gMock mock object to -// invoke the Run() method on mock argument. Example: -// -// class MockFoo : public Foo { -// public: -// MOCK_METHOD0(DoSomething, void(Task* done_cb)); -// }; -// -// EXPECT_CALL(foo, DoSomething(_)).WillOnce(WithArg<0>(InvokeRunnable())); -// -// Then you pass "foo" to something that will eventually call DoSomething(). -// The mock action will ensure that passed in done_cb is invoked. -// -// -// -- TaskMocker -- -// The TaskMocker class lets you create mock callbacks. Callbacks are -// difficult to mock because ownership of the callback object is often passed -// to the funciton being invoked. TaskMocker solves this by providing a -// GetTask() function that creates a new, single-use task that delegates to -// the originating TaskMocker object. Expectations are placed on the -// originating TaskMocker object. Each callback retrieved by GetTask() is -// tracked to ensure that it is properly deleted. The TaskMocker expects to -// outlive all the callbacks retrieved by GetTask(). -// -// Example: -// -// TaskMocker done_cb; -// EXPECT_CALL(done_cb, Run()).Times(3); -// -// func1(done_cb.GetTask()); -// func2(done_cb.GetTask()); -// func3(done_cb.GetTask()); -// -// // All 3 callbacks from GetTask() should be deleted before done_cb goes out -// // of scope. -// -// This class is not threadsafe. -// -// TODO(ajwong): Is it even worth bothering with gmock here? -// TODO(ajwong): Move MockCallback here and merge the implementation -// differences. - -#ifndef MEDIA_BASE_MOCK_TASK_H_ -#define MEDIA_BASE_MOCK_TASK_H_ - -#include "base/task.h" -#include "testing/gmock/include/gmock/gmock.h" - -namespace media { - -ACTION(InvokeRunnable) { - arg0->Run(); - delete arg0; -} - -class TaskMocker { - public: - TaskMocker(); - ~TaskMocker(); - - Task* CreateTask(); - - MOCK_METHOD0(Run, void()); - - private: - friend class CountingTask; - class CountingTask : public Task { - public: - CountingTask(TaskMocker* origin) - : origin_(origin) { - origin_->outstanding_tasks_++; - } - - virtual void Run() { - origin_->Run(); - } - - virtual ~CountingTask() { - origin_->outstanding_tasks_--; - } - - private: - TaskMocker* origin_; - - DISALLOW_COPY_AND_ASSIGN(CountingTask); - }; - - int outstanding_tasks_; - - DISALLOW_COPY_AND_ASSIGN(TaskMocker); -}; - -} // namespace media - -#endif //MEDIA_BASE_MOCK_TASK_H_ -- cgit v1.1