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

5 files changed, 40 insertions, 165 deletions

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<MockCallback>* callback = new StrictMock<MockCallback>();
+  StrictMock<MockClosure>* callback = new StrictMock<MockClosure>();
   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<MockCallback>* callback = new StrictMock<MockCallback>();
-  composite_->Play(base::Bind(&MockCallback::Run, callback));
+  StrictMock<MockClosure>* callback = new StrictMock<MockClosure>();
+  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<MockCallback>* callback = new StrictMock<MockCallback>();
-  composite_->Pause(base::Bind(&MockCallback::Run, callback));
+  StrictMock<MockClosure>* callback = new StrictMock<MockClosure>();
+  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<MockCallback>* callback = new StrictMock<MockCallback>();
-  composite_->Flush(base::Bind(&MockCallback::Run, callback));
+  StrictMock<MockClosure>* callback = new StrictMock<MockClosure>();
+  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<MockCallback>* callback = new StrictMock<MockCallback>();
-  composite_->Stop(base::Bind(&MockCallback::Run, callback));
+  StrictMock<MockClosure>* callback = new StrictMock<MockClosure>();
+  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<MockCallback>* callback = new StrictMock<MockCallback>();
-  composite_->Play(base::Bind(&MockCallback::Run, callback));
+  StrictMock<MockClosure>* callback = new StrictMock<MockClosure>();
+  composite_->Play(base::Bind(&MockClosure::Run, callback));
 
   // Note: Play() is pending on |filter_1_| right now.
 
-  callback = new StrictMock<MockCallback>();
-  composite_->Stop(base::Bind(&MockCallback::Run, callback));
+  callback = new StrictMock<MockClosure>();
+  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<MockCallback>* callback = new StrictMock<MockCallback>();
-  composite_->Flush(base::Bind(&MockCallback::Run, callback));
+  StrictMock<MockClosure>* callback = new StrictMock<MockClosure>();
+  composite_->Flush(base::Bind(&MockClosure::Run, callback));
 
   // Note: |filter_1_| and |filter_2_| have pending Flush() calls at this point.
 
-  callback = new StrictMock<MockCallback>();
-  composite_->Stop(base::Bind(&MockCallback::Run, callback));
+  callback = new StrictMock<MockClosure>();
+  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<MockCallback>* callback = new StrictMock<MockCallback>();
+base::Closure NewExpectedClosure() {
+  StrictMock<MockClosure>* callback = new StrictMock<MockClosure>();
   EXPECT_CALL(*callback, Run());
-  return base::Bind(&MockCallback::Run, callback);
+  return base::Bind(&MockClosure::Run, callback);
 }
 
 class MockStatusCB : public base::RefCountedThreadSafe<MockStatusCB> {
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<MockCallback> {
+// 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<MockClosure> {
  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<void(PipelineStatus)> 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_