Separate StunProber::Start into Prepare and Run so we could create multiple of them and send out ST…

webrtc/p2p/stunprober/stunprober.h

 /*
  *  Copyright 2015 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 14 matching lines @@
 #include "webrtc/base/socketaddress.h"
 #include "webrtc/base/thread.h"
 #include "webrtc/base/thread_checker.h"
 #include "webrtc/typedefs.h"
 
 namespace rtc {
 class AsyncPacketSocket;
 class PacketSocketFactory;
 class Thread;
 class NetworkManager;
+class AsyncResolverInterface;
 }  // namespace rtc
 
 namespace stunprober {
 
 class StunProber;
 
 static const int kMaxUdpBufferSize = 1200;
 
 typedef rtc::Callback2<void, StunProber*, int> AsyncCallback;
 
 enum NatType {
   NATTYPE_INVALID,
   NATTYPE_NONE,          // Not behind a NAT.
   NATTYPE_UNKNOWN,       // Behind a NAT but type can't be determine.
   NATTYPE_SYMMETRIC,     // Behind a symmetric NAT.
   NATTYPE_NON_SYMMETRIC  // Behind a non-symmetric NAT.
 };
 
 class StunProber : public sigslot::has_slots<> {
  public:
   enum Status {       // Used in UMA_HISTOGRAM_ENUMERATION.
     SUCCESS,          // Successfully received bytes from the server.
     GENERIC_FAILURE,  // Generic failure.
     RESOLVE_FAILED,   // Host resolution failed.
     WRITE_FAILED,     // Sending a message to the server failed.
     READ_FAILED,      // Reading the reply from the server failed.
   };
 
+  class Observer {
+   public:
+    virtual ~Observer() = default;
+    virtual void OnPrepared(StunProber* prober, StunProber::Status status) = 0;
+    virtual void OnFinished(StunProber* prober, StunProber::Status status) = 0;
+  };
+
   struct Stats {
     Stats() {}
 
     int num_request_sent = 0;
     int num_response_received = 0;
     NatType nat_type = NATTYPE_INVALID;
     int average_rtt_ms = -1;
     int success_percent = 0;
     int target_request_interval_ns = 0;
     int actual_request_interval_ns = 0;
@@ skipping 18 matching lines @@
   // |shared_socket_mode| is false, each request will be done with a new socket.
   // Otherwise, a unique socket will be used for a single round of requests
   // against all resolved IPs. No single socket will be used against a given IP
   // more than once.  The interval of requests will be as close to the requested
   // inter-probe interval |stun_ta_interval_ms| as possible. After sending out
   // the last scheduled request, the probe will wait |timeout_ms| for request
   // responses and then call |finish_callback|.  |requests_per_ip| indicates how
   // many requests should be tried for each resolved IP address. In shared mode,
   // (the number of sockets to be created) equals to |requests_per_ip|. In
   // non-shared mode, (the number of sockets) equals to requests_per_ip * (the
-  // number of resolved IP addresses).
+  // number of resolved IP addresses). TODO(guoweis): Remove this once
+  // everything moved to Prepare() and Run().
   bool Start(const std::vector<rtc::SocketAddress>& servers,
              bool shared_socket_mode,
              int stun_ta_interval_ms,
              int requests_per_ip,
              int timeout_ms,
              const AsyncCallback finish_callback);
 
+  // TODO(guoweis): The combination of Prepare() and Run() are equivalent to the
+  // Start() above. Remove Start() once everything is migrated.
+  bool Prepare(const std::vector<rtc::SocketAddress>& servers,
+               bool shared_socket_mode,
+               int stun_ta_interval_ms,
+               int requests_per_ip,
+               int timeout_ms,
+               StunProber::Observer* observer);
+
+  // Start to send out the STUN probes.
+  bool Run(StunProber::Observer* observer);

[pthatcher1, 2015/10/22 22:26:22]

Run makes it sound synchronous.  

Shouldn't we call this Start() also? 

+
   // Method to retrieve the Stats once |finish_callback| is invoked. Returning
   // false when the result is inconclusive, for example, whether it's behind a
   // NAT or not.
   bool GetStats(Stats* stats) const;
 
+  // Return the estimated execution time, i.e. |requests_per_ip_| *
+  // |all_servers_addrs_|.size() * |interval_ms_|.

[pthatcher1, 2015/10/22 22:26:22]

This comment is just a duplication of the method name and code.  You could
probably remove it.

+  int estimated_execution_time() {
+    return requests_per_ip_ * all_servers_addrs_.size() * interval_ms_;
+  }
+
  private:
   // A requester tracks the requests and responses from a single socket to many
   // STUN servers.
   class Requester;
 
+  // TODO(guoweis): Remove this once all dependencies move away from
+  // AsyncCallback.
+  class ObserverAdapter : public Observer {
+   public:
+    void set_callback(AsyncCallback callback) { callback_ = callback; }
+    void OnPrepared(StunProber* stunprober, Status status) {
+      if (status == SUCCESS) {
+        stunprober->Run(this);
+      } else {
+        callback_(stunprober, status);
+      }
+    }
+    void OnFinished(StunProber* stunprober, Status status) {
+      callback_(stunprober, status);
+    }
+
+   private:
+    AsyncCallback callback_;
+  };
+
   bool ResolveServerName(const rtc::SocketAddress& addr);
   void OnServerResolved(rtc::AsyncResolverInterface* resolver);
 
   void OnSocketReady(rtc::AsyncPacketSocket* socket,
                      const rtc::SocketAddress& addr);
 
   bool Done() {
     return num_request_sent_ >= requests_per_ip_ * all_servers_addrs_.size();
   }
 
   size_t total_socket_required() {
     return (shared_socket_mode_ ? 1 : all_servers_addrs_.size()) *
            requests_per_ip_;
   }
 
+  bool should_send_next_request(uint32_t now);
+  int get_wake_up_interval_ms();
+
   bool SendNextRequest();
 
   // Will be invoked in 1ms intervals and schedule the next request from the
   // |current_requester_| if the time has passed for another request.
   void MaybeScheduleStunRequests();
 
-  // End the probe with the given |status|.  Invokes |fininsh_callback|, which
-  // may destroy the class.
-  void End(StunProber::Status status);
+  void ReportOnPrepared(StunProber::Status status);
+  void ReportOnFinished(StunProber::Status status);
 
   Requester* CreateRequester();
 
   Requester* current_requester_ = nullptr;
 
   // The time when the next request should go out.
   uint64_t next_request_time_ms_ = 0;
 
   // Total requests sent so far.
   uint32_t num_request_sent_ = 0;
@@ skipping 12 matching lines @@
   // STUN server name to be resolved.
   std::vector<rtc::SocketAddress> servers_;
 
   // Weak references.
   rtc::PacketSocketFactory* socket_factory_;
   rtc::Thread* thread_;
 
   // Accumulate all resolved addresses.
   std::vector<rtc::SocketAddress> all_servers_addrs_;
 
-  // Caller-supplied callback executed when testing is completed, called by
-  // End().
-  AsyncCallback finished_callback_;
-
   // The set of STUN probe sockets and their state.
   std::vector<Requester*> requesters_;
 
   rtc::ThreadChecker thread_checker_;
 
   // Temporary storage for created sockets.
   std::vector<rtc::AsyncPacketSocket*> sockets_;
   // This tracks how many of the sockets are ready.
   size_t total_ready_sockets_ = 0;
 
   rtc::AsyncInvoker invoker_;
 
+  Observer* observer_ = nullptr;
+  // TODO(guoweis): Remove this once all dependencies move away from
+  // AsyncCallback.
+  ObserverAdapter observer_adapter_;
+
   rtc::NetworkManager::NetworkList networks_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(StunProber);
 };
 
 }  // namespace stunprober
 
 #endif  // WEBRTC_P2P_STUNPROBER_STUNPROBER_H_