Delete SignalThread class.

webrtc/base/nethelpers.cc

Index: webrtc/base/nethelpers.cc
diff --git a/webrtc/base/nethelpers.cc b/webrtc/base/nethelpers.cc
index 6c11ef8c38fa04800df834bc8fa0f0f8939b3b89..487969cffc79e85fc31e85677a9af2c6d39cc0d8 100644
--- a/webrtc/base/nethelpers.cc
+++ b/webrtc/base/nethelpers.cc
@@ -25,13 +25,17 @@
 #endif
 #endif  // defined(WEBRTC_POSIX) && !defined(__native_client__)
 
+#include "webrtc/base/bind.h"
 #include "webrtc/base/byteorder.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
-#include "webrtc/base/signalthread.h"
+#include "webrtc/base/ptr_util.h"
+#include "webrtc/base/task_queue.h"
+#include "webrtc/base/thread.h"
 
 namespace rtc {
 
+namespace {
 int ResolveHostname(const std::string& hostname, int family,
                     std::vector<IPAddress>* addresses) {
 #ifdef __native_client__
@@ -81,17 +85,37 @@ int ResolveHostname(const std::string& hostname, int family,
   return 0;
 #endif  // !__native_client__
 }
+}  // namespace
 
 // AsyncResolver
 AsyncResolver::AsyncResolver()
-    : SignalThread(false /* use_socket_server */), error_(-1) {}
+    : state_(State::NONE), construction_thread_(Thread::Current()) {
+  RTC_DCHECK(construction_thread_);
+}
 
-AsyncResolver::~AsyncResolver() = default;
+AsyncResolver::~AsyncResolver() {
+  RTC_DCHECK(construction_thread_->IsCurrent());
+}
 
 void AsyncResolver::Start(const SocketAddress& addr) {
+  RTC_DCHECK_RUN_ON(construction_thread_);
+  RTC_DCHECK(!resolver_queue_);
+  RTC_DCHECK(state_ == State::NONE);
+  // TODO(nisse): Support injection of task queue at construction?
+  resolver_queue_ = rtc::MakeUnique<TaskQueue>("AsyncResolverQueue");
   addr_ = addr;
-  // SignalThred Start will kickoff the resolve process.
-  SignalThread::Start();
+  resolver_queue_->PostTask([this]() {
+    std::vector<IPAddress> addresses;
+    int error =
+        ResolveHostname(addr_.hostname().c_str(), addr_.family(), &addresses);
+    // Ensure SignalDone is called on the main thread.
+    // TODO(nisse): Despite std::move below, we likely get a copy, due to the
+    // way rtc::Bind is implemented.
+    construction_thread_->Post(
+        RTC_FROM_HERE, rtc::Bind(&AsyncResolver::ResolveDone, this, error,
+                                 std::move(addresses)));
+  });
+  state_ = State::PENDING;
 }
 
 bool AsyncResolver::GetResolvedAddress(int family, SocketAddress* addr) const {
@@ -113,16 +137,34 @@ int AsyncResolver::GetError() const {
 }
 
 void AsyncResolver::Destroy(bool wait) {
-  SignalThread::Destroy(wait);
-}
-
-void AsyncResolver::DoWork() {
-  error_ = ResolveHostname(addr_.hostname().c_str(), addr_.family(),
-                           &addresses_);
+  RTC_DCHECK_RUN_ON(construction_thread_);
+  RTC_DCHECK(state_ != State::DESTROYED);
+  // If we don't wait here, we will nevertheless wait in the destructor.
+  if (wait || state_ != State::PENDING) {
+    // Destroy task queue, blocks on any currently running task. If we have a
+    // pending task, it will post a call to ResolveDone before finishing, which
+    // we will never handle.

[tommi, 2017/06/27 12:01:14]

this comment doesn't really explain why it's OK to sometimes delete |this| and
sometimes not. Especially considering that we might do that depending on
internal state, I'm wondering how the external code can know what to do with the
pointer to the AsyncResolver object

[nisse-webrtc, 2017/06/28 11:47:54]

The AsyncResolverInterface is poorly documented, but the way I understand it,
after Destroy() returns, the user should consider the object deleted and not
access it in any way.

Maybe users of the class also shouldn't call the destructor explicitly, perhaps
~AsyncResolverInterface() should be demoted from public to protected. But in any
case, it's invalid to explicitly destruct the object after calling the Destroy
method.

+    delete this;
+  } else {
+    state_ = State::DESTROYED;
+  }
 }
 
-void AsyncResolver::OnWorkDone() {
-  SignalDone(this);
+void AsyncResolver::ResolveDone(int error, std::vector<IPAddress> addresses) {
+  RTC_DCHECK_RUN_ON(construction_thread_);
+  error_ = error;
+  addresses_ = std::move(addresses);
+  switch (state_) {
+    case State::PENDING:
+      SignalDone(this);
+      state_ = State::NONE;
+      break;
+    case State::DESTROYED:
+      delete this;
+      break;
+    default:
+      RTC_NOTREACHED();
+  }
 }
 
 const char* inet_ntop(int af, const void *src, char* dst, socklen_t size) {