Schedule a CreatePermissionRequest after the success of a previous request

webrtc/p2p/base/turnport.cc

Index: webrtc/p2p/base/turnport.cc
diff --git a/webrtc/p2p/base/turnport.cc b/webrtc/p2p/base/turnport.cc
index 1cc885e27d06fb086252366606da637273aacee6..ab0a611a74c3f9e8df315107668704df1f54aada 100644
--- a/webrtc/p2p/base/turnport.cc
+++ b/webrtc/p2p/base/turnport.cc
@@ -141,7 +141,7 @@ class TurnEntry : public sigslot::has_slots<> {
   BindState state() const { return state_; }
 
   // Helper methods to send permission and channel bind requests.
-  void SendCreatePermissionRequest();
+  void SendCreatePermissionRequest(int delay);
   void SendChannelBindRequest(int delay);
   // Sends a packet to the given destination address.
   // This will wrap the packet in STUN if necessary.
@@ -1289,12 +1289,12 @@ TurnEntry::TurnEntry(TurnPort* port, int channel_id,
       ext_addr_(ext_addr),
       state_(STATE_UNBOUND) {
   // Creating permission for |ext_addr_|.
-  SendCreatePermissionRequest();
+  SendCreatePermissionRequest(0);
 }
 
-void TurnEntry::SendCreatePermissionRequest() {
-  port_->SendRequest(new TurnCreatePermissionRequest(
-      port_, this, ext_addr_), 0);
+void TurnEntry::SendCreatePermissionRequest(int delay) {
+  port_->SendRequest(new TurnCreatePermissionRequest(port_, this, ext_addr_),
+                     delay);
 }

[pthatcher1, 2015/11/11 20:22:09]

To make it a little more readable, can you make two methods?

SendCreatePermissionRequest()
SendCreatePermissionRequestAfter(int delay)

Obviously, the first can call the second.

[honghaiz3, 2015/11/11 23:51:58]

Of course I can, but using a delay parameter for both delayed and non-delayed
sending is used in multiple occasions in this file (as well as request manager).
For example, The next method SendChannelBindRequest, TurnPort::SendRequest. So
perhaps, using a single method with a delay parameter is more consistent with
the rest of the implementation. 

[pthatcher1, 2015/11/12 00:21:08]

Good point.  Feel free to leave it.

 
 void TurnEntry::SendChannelBindRequest(int delay) {
@@ -1337,12 +1337,18 @@ void TurnEntry::OnCreatePermissionSuccess() {
                         << " succeeded";
   // For success result code will be 0.
   port_->SignalCreatePermissionResult(port_, ext_addr_, 0);
+
+  // If |state_| is STATE_BINDING or STATE_BOUND, the permission is refreshed
+  // by ChannelBindRequest.

[pthatcher1, 2015/11/11 20:22:09]

If we are STATE_BINDING and the ChannelBindRequest fails, wouldn't we still need
the CreatePermissionRequest refresh?  Should we just check for != STATE_BOUND?

[honghaiz3, 2015/11/11 23:51:58]

Done

+  if (state_ == STATE_UNBOUND) {
+    SendCreatePermissionRequest(TURN_PERMISSION_TIMEOUT - 60000);

[pthatcher1, 2015/11/11 20:22:09]

Can you explain the 60000, or make it a named constant?

[honghaiz3, 2015/11/11 23:51:58]

Added comments. 

+  }
 }
 
 void TurnEntry::OnCreatePermissionError(StunMessage* response, int code) {
   if (code == STUN_ERROR_STALE_NONCE) {
     if (port_->UpdateNonce(response)) {
-      SendCreatePermissionRequest();
+      SendCreatePermissionRequest(0);
     }
   } else {
     // Send signal with error code.