Remove Thread wrapper class in forwarder2.

tools/android/forwarder2/device_listener.h

Index: tools/android/forwarder2/device_listener.h
diff --git a/tools/android/forwarder2/device_listener.h b/tools/android/forwarder2/device_listener.h
index 44d37aa4ff0b9ce7087061f0228d460414df72b0..8d202425eb2c6a9ebadb8befaa2b94cd16c67c75 100644
--- a/tools/android/forwarder2/device_listener.h
+++ b/tools/android/forwarder2/device_listener.h
@@ -5,76 +5,77 @@
 #ifndef TOOLS_ANDROID_FORWARDER2_DEVICE_LISTENER_H_
 #define TOOLS_ANDROID_FORWARDER2_DEVICE_LISTENER_H_
 
-#include <pthread.h>
-
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/threading/thread.h"
 #include "tools/android/forwarder2/pipe_notifier.h"
 #include "tools/android/forwarder2/socket.h"
-#include "tools/android/forwarder2/thread.h"
 
 namespace forwarder2 {
 
 class Forwarder;
 

[digit1, 2013/07/18 20:08:39]

Can I ask you to add a comment that explains what this class does. The code is
becoming more and more complex, and I think that will help review / understand
it better :-)

Here's an example:

// A DeviceListener instance is used in the device_forwarder program to bind
// to a specific device-side |port| and wait for client connections. When a
// connection happens, it informs its creator, through |adb_control_socket|
// and waits for a call to its SetAdbDataSocket() method. When this happens,
// a new internal Forwarder instance is started.

+ explain lifecycle management.

[Philippe, 2013/07/22 15:16:14]

Thanks.

-class DeviceListener : public Thread {
+class DeviceListener {
  public:
-  DeviceListener(scoped_ptr<Socket> adb_control_socket, int port);
-  virtual ~DeviceListener();
+  // Callback that is used for self-deletion as a way to let the client perform
+  // some additional cleanup work (e.g. removing the device listener instance
+  // from a map before deleting it).

[digit1, 2013/07/18 20:08:39]

This is a bit ambiguous. Can you clarify what "client" means here. Typically it
refers to  a program connecting to the DeviceListener's bound socket :)

[Philippe, 2013/07/22 15:16:14]

Yeah :)

+  typedef base::Callback<void (int /* listener port */)> DeleteCallback;
 
-  bool WaitForAdbDataSocket();
+  static scoped_ptr<DeviceListener> Create(
+      scoped_ptr<Socket> adb_control_socket,
+      int port,
+      const DeleteCallback& delete_callback);
 
-  bool SetAdbDataSocket(scoped_ptr<Socket> adb_data_socket);
+  ~DeviceListener();
 
-  bool BindListenerSocket();
+  void Start();
 
-  // |is_alive_| is set only on BindAndListenSocket and written once when Run()
-  // terminates.  So even in case of a race condition, the worst that could
-  // happen is for the main thread to see the listener alive when it isn't. And
-  // also, this is not a problem since the main thread checks the liveliness of
-  // the listeners in a loop.
-  bool is_alive() const { return is_alive_; }
-  void ForceExit();
+  void SetAdbDataSocket(scoped_ptr<Socket> adb_data_socket);
 
   int listener_port() const { return listener_port_; }
 
- protected:
-  // Thread:
-  virtual void Run() OVERRIDE;
-
  private:
-  void RunInternal();
+  DeviceListener(scoped_ptr<PipeNotifier> pipe_notifier,
+                 scoped_ptr<Socket> listener_socket,
+                 scoped_ptr<Socket> adb_control_socket,
+                 int port,
+                 const DeleteCallback& delete_callback);
 
-  // Must be called after successfully acquired mutex.
-  void SetMustExitLocked();
+  // Pushes an AcceptClientOnInternalThread() task to the internal thread's
+  // message queue in order to wait for a new client soon.
+  void AcceptNextClientSoon();
 
-  // The listener socket for sending control commands.
-  scoped_ptr<Socket> adb_control_socket_;
+  void AcceptClientOnInternalThread();
+
+  void OnAdbDataSocketReceivedOnInternalThread(
+      scoped_ptr<Socket> adb_data_socket);
 
+  bool BindListenerSocket();
+
+  // Used for the listener thread to be notified on destruction. We have one
+  // notifier per Listener thread since each Listener thread may be requested to
+  // exit for different reasons independently from each other and independent
+  // from the main program, ex. when the host requests to forward/listen the
+  // same port again.  Both the |adb_control_socket_| and |listener_socket_|
+  // must share the same receiver file descriptor from |exit_notifier_| and it
+  // is set in the constructor.
+  scoped_ptr<PipeNotifier> exit_notifier_;
   // The local device listener socket for accepting connections from the local
   // port (listener_port_).
-  Socket listener_socket_;
-
+  scoped_ptr<Socket> listener_socket_;
+  // The listener socket for sending control commands.
+  scoped_ptr<Socket> adb_control_socket_;
+  scoped_ptr<Socket> device_data_socket_;
   // This is the adb connection to transport the actual data, used for creating
   // the forwarder. Ownership transferred to the Forwarder.
   scoped_ptr<Socket> adb_data_socket_;
-
-  int listener_port_;
-  pthread_mutex_t adb_data_socket_mutex_;
-  pthread_cond_t adb_data_socket_cond_;
-  bool is_alive_;
-  bool must_exit_;
-
-  // Used for the listener thread to be notified from ForceExit() which is
-  // called from the main thread. We have one notifier per Listener thread since
-  // each Listener thread may be requested to exit for different reasons
-  // independently from each other and independent from the main program,
-  // ex. when the host requests to forward/listen the same port again.  Both the
-  // |adb_control_socket_| and |listener_socket_| must share the same receiver
-  // file descriptor from |exit_notifier_| and it is set in the constructor.
-  PipeNotifier exit_notifier_;
+  const int listener_port_;
+  const DeleteCallback delete_callback_;
+  base::Thread thread_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceListener);
 };