Delete SignalThread class.

webrtc/base/nethelpers.h

Index: webrtc/base/nethelpers.h
diff --git a/webrtc/base/nethelpers.h b/webrtc/base/nethelpers.h
index b0727f861b6a2910b225b839447e21d02911bc74..e43f2d8b7499739534f75cab4beec81053453bcb 100644
--- a/webrtc/base/nethelpers.h
+++ b/webrtc/base/nethelpers.h
@@ -21,17 +21,17 @@
 #include <list>
 
 #include "webrtc/base/asyncresolverinterface.h"
-#include "webrtc/base/signalthread.h"
+#include "webrtc/base/platform_thread.h"
 #include "webrtc/base/sigslot.h"
 #include "webrtc/base/socketaddress.h"
 
 namespace rtc {
 
-class AsyncResolverTest;
+class Thread;
 
 // AsyncResolver will perform async DNS resolution, signaling the result on
 // the SignalDone from AsyncResolverInterface when the operation completes.

[Taylor Brandstetter, 2017/06/05 22:33:25]

nit: Could you add a comment saying that SignalDone will be fired from the same
thread on which the AsyncResolver was constructed?

[nisse-webrtc, 2017/06/07 07:17:31]

Done. But maybe that comment belongs with the AsyncResolverInterface class
(which doesn't mention threads at all)?

[Taylor Brandstetter, 2017/06/09 02:50:13]

The comment applies to this class's constructor specifically, so this seems like
the right place.

-class AsyncResolver : public SignalThread, public AsyncResolverInterface {
+class AsyncResolver : public AsyncResolverInterface {
  public:
   AsyncResolver();
   ~AsyncResolver() override;
@@ -44,11 +44,15 @@ class AsyncResolver : public SignalThread, public AsyncResolverInterface {
   const std::vector<IPAddress>& addresses() const { return addresses_; }
   void set_error(int error) { error_ = error; }
 
- protected:
-  void DoWork() override;
-  void OnWorkDone() override;
-
  private:
+  // Thread main function

[Taylor Brandstetter, 2017/06/05 22:33:25]

nit: Period?

+  static void ThreadEntry(void *p);

[tommi, 2017/06/08 08:02:10]

void* p
(or even |param|)

can you run git cl format?

[nisse-webrtc, 2017/06/08 09:52:33]

Done.

+  void DoWork();

[tommi, 2017/06/08 08:02:10]

can we have slightly more descriptive names for these methods?

+  void OnWorkDone();
+
+  rtc::Thread* main_;

[tommi, 2017/06/08 08:02:10]

nit: I'd prefer |thread_| to |main_|.  Actually... what thread is this? If
there's already a general name for it (e.g. the 'signaling' or 'worker' threads
from libjingle), we could name it accordingly.

+  rtc::CriticalSection lock_;

[nisse-webrtc, 2017/06/02 08:41:04]

The use of this lock is maybe overly conservative. 

I don't know if the sigslot machinery itself implies any memory barriers, but I
think the Invoke from the worker thread back to the main thread should be enough
to make stores from the worker thread visible to other threads, right?

[Taylor Brandstetter, 2017/06/05 22:33:25]

It should be enough. Can you try removing the lock? If there are TSan warnings
I'd be surprised.

[nisse-webrtc, 2017/06/07 07:17:31]

Done.

+  PlatformThread worker_;
   SocketAddress addr_;
   std::vector<IPAddress> addresses_;
   int error_;