Delete unused features of AsyncInvoke.

webrtc/base/thread_unittest.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 403 matching lines @@
   SetNameOnSignalQueueDestroyedTester tester2(thread2);
   delete thread2;
 }
 
 class AsyncInvokeTest : public testing::Test {
  public:
   void IntCallback(int value) {
     EXPECT_EQ(expected_thread_, Thread::Current());
     int_value_ = value;
   }
-  void AsyncInvokeIntCallback(AsyncInvoker* invoker, Thread* thread) {
-    expected_thread_ = thread;
-    invoker->AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, thread, FunctorC(),
-                         &AsyncInvokeTest::IntCallback,
-                         static_cast<AsyncInvokeTest*>(this));
-    invoke_started_.Set();
-  }
   void SetExpectedThreadForIntCallback(Thread* thread) {
     expected_thread_ = thread;
   }
 
  protected:
   enum { kWaitTimeout = 1000 };
   AsyncInvokeTest()
       : int_value_(0),
-        invoke_started_(true, false),
         expected_thread_(nullptr) {}
 
   int int_value_;
-  Event invoke_started_;
   Thread* expected_thread_;
 };
 
 TEST_F(AsyncInvokeTest, FireAndForget) {
   AsyncInvoker invoker;
   // Create and start the thread.
   Thread thread;
   thread.Start();
   // Try calling functor.
   AtomicBool called;
   invoker.AsyncInvoke<void>(RTC_FROM_HERE, &thread, FunctorB(&called));
   EXPECT_TRUE_WAIT(called.get(), kWaitTimeout);
 }
 
-TEST_F(AsyncInvokeTest, WithCallback) {
-  AsyncInvoker invoker;
-  // Create and start the thread.
-  Thread thread;
-  thread.Start();
-  // Try calling functor.
-  SetExpectedThreadForIntCallback(Thread::Current());
-  invoker.AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, &thread, FunctorA(),
-                      &AsyncInvokeTest::IntCallback,
-                      static_cast<AsyncInvokeTest*>(this));
-  EXPECT_EQ_WAIT(42, int_value_, kWaitTimeout);
-}
-
-TEST_F(AsyncInvokeTest, CancelInvoker) {
-  // Create and start the thread.
-  Thread thread;
-  thread.Start();
-  // Try destroying invoker during call.
-  {
-    AsyncInvoker invoker;
-    invoker.AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, &thread, FunctorC(),
-                        &AsyncInvokeTest::IntCallback,
-                        static_cast<AsyncInvokeTest*>(this));
-  }
-  // With invoker gone, callback should be cancelled.
-  Thread::Current()->ProcessMessages(kWaitTimeout);
-  EXPECT_EQ(0, int_value_);
-}
-
-TEST_F(AsyncInvokeTest, CancelCallingThread) {
-  AsyncInvoker invoker;
-  { // Create and start the thread.
-    Thread thread;
-    thread.Start();
-    // Try calling functor.
-    thread.Invoke<void>(
-        RTC_FROM_HERE,
-        Bind(&AsyncInvokeTest::AsyncInvokeIntCallback,
-             static_cast<AsyncInvokeTest*>(this), &invoker, Thread::Current()));
-    // Wait for the call to begin.
-    ASSERT_TRUE(invoke_started_.Wait(kWaitTimeout));
-  }
-  // Calling thread is gone. Return message shouldn't happen.
-  Thread::Current()->ProcessMessages(kWaitTimeout);
-  EXPECT_EQ(0, int_value_);
-}
-
-TEST_F(AsyncInvokeTest, KillInvokerBeforeExecute) {
-  Thread thread;
-  thread.Start();
-  {
-    AsyncInvoker invoker;
-    // Try calling functor.
-    thread.Invoke<void>(
-        RTC_FROM_HERE,
-        Bind(&AsyncInvokeTest::AsyncInvokeIntCallback,
-             static_cast<AsyncInvokeTest*>(this), &invoker, Thread::Current()));
-    // Wait for the call to begin.
-    ASSERT_TRUE(invoke_started_.Wait(kWaitTimeout));
-  }
-  // Invoker is destroyed. Function should not execute.
-  Thread::Current()->ProcessMessages(kWaitTimeout);
-  EXPECT_EQ(0, int_value_);
-}

[Taylor Brandstetter, 2017/05/17 18:25:28]

Sorry for the late feedback, but these tests need to be replaced by equivalent
tests that do the same thing (calling AsyncInvoke from an invoked task), just
without using the method being removed. This is definitely a way AsyncInvoker is
being used in practice, so we should have tests for it. One of these tests was
failing due to an actual bug I'm trying to fix
(https://codereview.webrtc.org/2876273002/).

[Taylor Brandstetter, 2017/05/17 21:15:06]

Here's a CL that adds a test that does what I'm talking about:
https://codereview.webrtc.org/2885143006/

-
 TEST_F(AsyncInvokeTest, KillInvokerDuringExecute) {
   // Use these events to get in a state where the functor is in the middle of
   // executing, and then to wait for it to finish, ensuring the "EXPECT_FALSE"
   // is run.
   Event functor_started(false, false);
   Event functor_continue(false, false);
   Event functor_finished(false, false);
 
   Thread thread;
   thread.Start();
@@ skipping 59 matching lines @@
   EXPECT_FALSE(flag1.get());
   EXPECT_TRUE(flag2.get());
 }
 
 class GuardedAsyncInvokeTest : public testing::Test {
  public:
   void IntCallback(int value) {
     EXPECT_EQ(expected_thread_, Thread::Current());
     int_value_ = value;
   }
-  void AsyncInvokeIntCallback(GuardedAsyncInvoker* invoker, Thread* thread) {
-    expected_thread_ = thread;
-    invoker->AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, FunctorC(),
-                         &GuardedAsyncInvokeTest::IntCallback,
-                         static_cast<GuardedAsyncInvokeTest*>(this));
-    invoke_started_.Set();
-  }
   void SetExpectedThreadForIntCallback(Thread* thread) {
     expected_thread_ = thread;
   }
 
  protected:
   const static int kWaitTimeout = 1000;
   GuardedAsyncInvokeTest()
       : int_value_(0),
-        invoke_started_(true, false),
         expected_thread_(nullptr) {}
 
   int int_value_;
-  Event invoke_started_;
   Thread* expected_thread_;
 };
 
 // Functor for creating an invoker.
 struct CreateInvoker {
   CreateInvoker(std::unique_ptr<GuardedAsyncInvoker>* invoker)
       : invoker_(invoker) {}
   void operator()() { invoker_->reset(new GuardedAsyncInvoker()); }
   std::unique_ptr<GuardedAsyncInvoker>* invoker_;
 };
 
 // Test that we can call AsyncInvoke<void>() after the thread died.
 TEST_F(GuardedAsyncInvokeTest, KillThreadFireAndForget) {
   // Create and start the thread.
   std::unique_ptr<Thread> thread(new Thread());
   thread->Start();
   std::unique_ptr<GuardedAsyncInvoker> invoker;
   // Create the invoker on |thread|.
   thread->Invoke<void>(RTC_FROM_HERE, CreateInvoker(&invoker));
   // Kill |thread|.
   thread = nullptr;
   // Try calling functor.
   AtomicBool called;
   EXPECT_FALSE(invoker->AsyncInvoke<void>(RTC_FROM_HERE, FunctorB(&called)));
   // With thread gone, nothing should happen.
   WAIT(called.get(), kWaitTimeout);
   EXPECT_FALSE(called.get());
 }
 
-// Test that we can call AsyncInvoke with callback after the thread died.
-TEST_F(GuardedAsyncInvokeTest, KillThreadWithCallback) {
-  // Create and start the thread.
-  std::unique_ptr<Thread> thread(new Thread());
-  thread->Start();
-  std::unique_ptr<GuardedAsyncInvoker> invoker;
-  // Create the invoker on |thread|.
-  thread->Invoke<void>(RTC_FROM_HERE, CreateInvoker(&invoker));
-  // Kill |thread|.
-  thread = nullptr;
-  // Try calling functor.
-  EXPECT_FALSE(
-      invoker->AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, FunctorC(),
-                           &GuardedAsyncInvokeTest::IntCallback,
-                           static_cast<GuardedAsyncInvokeTest*>(this)));
-  // With thread gone, callback should be cancelled.
-  Thread::Current()->ProcessMessages(kWaitTimeout);
-  EXPECT_EQ(0, int_value_);
-}
-
 // The remaining tests check that GuardedAsyncInvoker behaves as AsyncInvoker
 // when Thread is still alive.
 TEST_F(GuardedAsyncInvokeTest, FireAndForget) {
   GuardedAsyncInvoker invoker;
   // Try calling functor.
   AtomicBool called;
   EXPECT_TRUE(invoker.AsyncInvoke<void>(RTC_FROM_HERE, FunctorB(&called)));
   EXPECT_TRUE_WAIT(called.get(), kWaitTimeout);
 }
 
-TEST_F(GuardedAsyncInvokeTest, WithCallback) {
-  GuardedAsyncInvoker invoker;
-  // Try calling functor.
-  SetExpectedThreadForIntCallback(Thread::Current());
-  EXPECT_TRUE(invoker.AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, FunctorA(),
-                                  &GuardedAsyncInvokeTest::IntCallback,
-                                  static_cast<GuardedAsyncInvokeTest*>(this)));
-  EXPECT_EQ_WAIT(42, int_value_, kWaitTimeout);
-}
-
-TEST_F(GuardedAsyncInvokeTest, CancelInvoker) {
-  // Try destroying invoker during call.
-  {
-    GuardedAsyncInvoker invoker;
-    EXPECT_TRUE(
-        invoker.AsyncInvoke(RTC_FROM_HERE, RTC_FROM_HERE, FunctorC(),
-                            &GuardedAsyncInvokeTest::IntCallback,
-                            static_cast<GuardedAsyncInvokeTest*>(this)));
-  }
-  // With invoker gone, callback should be cancelled.
-  Thread::Current()->ProcessMessages(kWaitTimeout);
-  EXPECT_EQ(0, int_value_);
-}
-
-TEST_F(GuardedAsyncInvokeTest, CancelCallingThread) {
-  GuardedAsyncInvoker invoker;
-  // Try destroying calling thread during call.
-  {
-    Thread thread;
-    thread.Start();
-    // Try calling functor.
-    thread.Invoke<void>(RTC_FROM_HERE,
-                        Bind(&GuardedAsyncInvokeTest::AsyncInvokeIntCallback,
-                             static_cast<GuardedAsyncInvokeTest*>(this),
-                             &invoker, Thread::Current()));
-    // Wait for the call to begin.
-    ASSERT_TRUE(invoke_started_.Wait(kWaitTimeout));
-  }
-  // Calling thread is gone. Return message shouldn't happen.
-  Thread::Current()->ProcessMessages(kWaitTimeout);
-  EXPECT_EQ(0, int_value_);
-}
-
-TEST_F(GuardedAsyncInvokeTest, KillInvokerBeforeExecute) {
-  Thread thread;
-  thread.Start();
-  {
-    GuardedAsyncInvoker invoker;
-    // Try calling functor.
-    thread.Invoke<void>(RTC_FROM_HERE,
-                        Bind(&GuardedAsyncInvokeTest::AsyncInvokeIntCallback,
-                             static_cast<GuardedAsyncInvokeTest*>(this),
-                             &invoker, Thread::Current()));
-    // Wait for the call to begin.
-    ASSERT_TRUE(invoke_started_.Wait(kWaitTimeout));
-  }
-  // Invoker is destroyed. Function should not execute.
-  Thread::Current()->ProcessMessages(kWaitTimeout);
-  EXPECT_EQ(0, int_value_);
-}
-
 TEST_F(GuardedAsyncInvokeTest, Flush) {
   GuardedAsyncInvoker invoker;
   AtomicBool flag1;
   AtomicBool flag2;
   // Queue two async calls to the current thread.
   EXPECT_TRUE(invoker.AsyncInvoke<void>(RTC_FROM_HERE, FunctorB(&flag1)));
   EXPECT_TRUE(invoker.AsyncInvoke<void>(RTC_FROM_HERE, FunctorB(&flag2)));
   // Because we haven't pumped messages, these should not have run yet.
   EXPECT_FALSE(flag1.get());
   EXPECT_FALSE(flag2.get());
@@ skipping 16 matching lines @@
   // Execute pending calls with id == 5.
   EXPECT_TRUE(invoker.Flush(5));
   EXPECT_TRUE(flag1.get());
   EXPECT_FALSE(flag2.get());
   flag1 = false;
   // Execute all pending calls. The id == 5 call should not execute again.
   EXPECT_TRUE(invoker.Flush());
   EXPECT_FALSE(flag1.get());
   EXPECT_TRUE(flag2.get());
 }