Use continual gathering to restore backup connections

webrtc/p2p/base/p2ptransportchannel.h

Index: webrtc/p2p/base/p2ptransportchannel.h
diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h
index ada3ecfb8bb2deeb6bc5d9120eb1647d7cd1a1da..c4bc0180b6b5a91d84fa9a224de75aa0490c9564 100644
--- a/webrtc/p2p/base/p2ptransportchannel.h
+++ b/webrtc/p2p/base/p2ptransportchannel.h
@@ -40,6 +40,12 @@ namespace cricket {
 
 extern const int WEAK_PING_INTERVAL;
 static const int MIN_PINGS_AT_WEAK_PING_INTERVAL = 3;
+// When the selected connection is suddenly deleted and no other connection
+// is available, we will try to recover the ICE connectivity via continual
+// gathering on the local client. If no writable connection is established
+// after this amount of time, will declare the transport channel to be
+// unwritable.
+static const int MAX_RECOVERING_TIME = 5000;  // 5 seconds.

[pthatcher1, 2016/06/07 18:54:33]

I think this needs a more clear name.  This isn't a time limit on recovering,
it's a time limit on how long we hide the fact that we're not connected from the
application.  So perhaps something like
CONTINUAL_GATHERING_DISCONNECTED_STATE_DELAY.

 
 struct IceParameters {
   std::string ufrag;
@@ -197,10 +203,16 @@ class P2PTransportChannel : public TransportChannelImpl,
   const std::vector<RemoteCandidate>& remote_candidates() const {
     return remote_candidates_;
   }
+  // For testing.
+  // TODO(honghaiz): Remove this method and use the fake clock for the test.
+  void set_check_restore_backup_connection_interval(int interval);

[pthatcher1, 2016/06/07 18:54:34]

Can we just use the fake clock now?  This looks kind of ugly.

 
  private:
   rtc::Thread* thread() { return worker_thread_; }
   bool IsGettingPorts() { return allocator_session()->IsGettingPorts(); }
+  // Start regathering on the given list of networks. If |networks| is nullptr,
+  // start regathering on all active networks.
+  void StartRegathering(std::vector<rtc::Network*>* networks);

[pthatcher1, 2016/06/07 18:54:34]

It think it would be more clear to have two methods:

RegatherAllNetworks()
RegatherNetworks(const std::vector<rtc::Network*>& networks)

 
   // A transport channel is weak if the current best connection is either
   // not receiving or not writable, or if there is no best connection at all.
@@ -257,6 +269,7 @@ class P2PTransportChannel : public TransportChannelImpl,
   void OnMessage(rtc::Message* pmsg) override;
   void OnSort();
   void OnCheckAndPing();
+  void OnCheckAndRestoreBackupConnection();
 
   void PruneConnections();
   Connection* best_nominated_connection() const;
@@ -292,6 +305,11 @@ class P2PTransportChannel : public TransportChannelImpl,
                : static_cast<uint32_t>(remote_ice_parameters_.size() - 1);
   }
 
+  bool IsRecoveringConnectivity() const {
+    return recovering_start_time_ > 0 &&
+           rtc::TimeMillis() - recovering_start_time_ <= MAX_RECOVERING_TIME;
+  }

[pthatcher1, 2016/06/07 18:54:34]

***

+
   PortAllocator* allocator_;
   rtc::Thread* worker_thread_;
   bool incoming_only_;
@@ -327,11 +345,14 @@ class P2PTransportChannel : public TransportChannelImpl,
   IceGatheringState gathering_state_;
 
   int check_receiving_interval_;
+  int check_restore_backup_connection_interval_;
   int64_t last_ping_sent_ms_ = 0;
   int weak_ping_interval_ = WEAK_PING_INTERVAL;
   TransportChannelState state_ = TransportChannelState::STATE_INIT;
   IceConfig config_;
   int last_sent_packet_id_ = -1;  // -1 indicates no packet was sent before.
+  int64_t recovering_start_time_ = 0;
+  std::vector<Candidate> candidates_removed_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(P2PTransportChannel);
 };