Schedule a CreatePermissionRequest after the success of a previous request

webrtc/p2p/base/turnport.cc

 /*
  *  Copyright 2012 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 123 matching lines @@
   TurnEntry(TurnPort* port, int channel_id,
             const rtc::SocketAddress& ext_addr);
 
   TurnPort* port() { return port_; }
 
   int channel_id() const { return channel_id_; }
   const rtc::SocketAddress& address() const { return ext_addr_; }
   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.
   int Send(const void* data, size_t size, bool payload,
            const rtc::PacketOptions& options);
 
   void OnCreatePermissionSuccess();
   void OnCreatePermissionError(StunMessage* response, int code);
   void OnChannelBindSuccess();
   void OnChannelBindError(StunMessage* response, int code);
@@ skipping 1127 matching lines @@
   entry_ = NULL;
 }
 
 TurnEntry::TurnEntry(TurnPort* port, int channel_id,
                      const rtc::SocketAddress& ext_addr)
     : port_(port),
       channel_id_(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) {
   port_->SendRequest(new TurnChannelBindRequest(
       port_, this, channel_id_, ext_addr_), delay);
 }
 
 int TurnEntry::Send(const void* data, size_t size, bool payload,
                     const rtc::PacketOptions& options) {
   rtc::ByteBuffer buf;
   if (state_ != STATE_BOUND) {
@@ skipping 21 matching lines @@
   }
   return port_->Send(buf.Data(), buf.Length(), options);
 }
 
 void TurnEntry::OnCreatePermissionSuccess() {
   LOG_J(LS_INFO, port_) << "Create permission for "
                         << ext_addr_.ToSensitiveString()
                         << " 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.
     port_->SignalCreatePermissionResult(port_, ext_addr_, code);
   }
 }
 
 void TurnEntry::OnChannelBindSuccess() {
   LOG_J(LS_INFO, port_) << "Channel bind for " << ext_addr_.ToSensitiveString()
                         << " succeeded";
   ASSERT(state_ == STATE_BINDING || state_ == STATE_BOUND);
   state_ = STATE_BOUND;
 }
 
 void TurnEntry::OnChannelBindError(StunMessage* response, int code) {
   // TODO(mallinath) - Implement handling of error response for channel
   // bind request as per http://tools.ietf.org/html/rfc5766#section-11.3
   if (code == STUN_ERROR_STALE_NONCE) {
     if (port_->UpdateNonce(response)) {
       // Send channel bind request with fresh nonce.
       SendChannelBindRequest(0);
     }
   }
 }
 
 }  // namespace cricket