Implement timeouts for IQ requests.

Now the IqRequest class supports setting timeouts for each request, and 
JingleSession uses it to disconnect if no response is receive within 10 
secons from a request.

BUG=107925

Review URL: http://codereview.chromium.org/9452038

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@123680 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 116 insertions, 72 deletions

diff --git a/remoting/protocol/jingle_messages.cc b/remoting/protocol/jingle_messages.cc
index 8fe749d..a1b87ff 100644
--- a/remoting/protocol/jingle_messages.cc
+++ b/remoting/protocol/jingle_messages.cc
@@ -141,6 +141,11 @@ bool JingleMessage::IsJingleMessage(const buzz::XmlElement* stanza) {
       stanza->FirstNamed(QName(kJingleNamespace, "jingle")) != NULL;
 }
 
+// static
+std::string JingleMessage::GetActionName(ActionType action) {
+  return ValueToName(kActionTypes, arraysize(kActionTypes), action);
+}
+
 JingleMessage::JingleMessage()
     : action(UNKNOWN_ACTION),
       reason(UNKNOWN_REASON) {
@@ -268,7 +273,7 @@ bool JingleMessage::ParseXml(const buzz::XmlElement* stanza,
   return true;
 }
 
-scoped_ptr<buzz::XmlElement> JingleMessage::ToXml() {
+scoped_ptr<buzz::XmlElement> JingleMessage::ToXml() const {
   scoped_ptr<XmlElement> root(
       new XmlElement(QName("jabber:client", "iq"), true));
 
diff --git a/remoting/protocol/jingle_messages.h b/remoting/protocol/jingle_messages.h
index 03e5063..3dcdd41 100644
--- a/remoting/protocol/jingle_messages.h
+++ b/remoting/protocol/jingle_messages.h
@@ -52,12 +52,13 @@ struct JingleMessage {
 
   // Caller keeps ownership of |stanza|.
   static bool IsJingleMessage(const buzz::XmlElement* stanza);
+  static std::string GetActionName(ActionType action);
 
   // Caller keeps ownership of |stanza|. |error| is set to debug error
   // message when parsing fails.
   bool ParseXml(const buzz::XmlElement* stanza, std::string* error);
 
-  scoped_ptr<buzz::XmlElement> ToXml();
+  scoped_ptr<buzz::XmlElement> ToXml() const;
 
   std::string from;
   std::string to;
diff --git a/remoting/protocol/jingle_session.cc b/remoting/protocol/jingle_session.cc
index 61d4bc9..f0edb343 100644
--- a/remoting/protocol/jingle_session.cc
+++ b/remoting/protocol/jingle_session.cc
@@ -8,6 +8,7 @@
 #include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/string_number_conversions.h"
+#include "base/time.h"
 #include "remoting/base/constants.h"
 #include "remoting/jingle_glue/iq_sender.h"
 #include "remoting/protocol/authenticator.h"
@@ -32,6 +33,11 @@ namespace {
 // process.
 const int kTransportInfoSendDelayMs = 2;
 
+// How long we should wait for a response from the other end. This
+// value is used for all requests include |session-initiate| and
+// |transport-info|.
+const int kMessageResponseTimeoutSeconds = 10;
+
 Session::Error AuthRejectionReasonToError(
     Authenticator::RejectionReason reason) {
   switch (reason) {
@@ -54,6 +60,8 @@ JingleSession::JingleSession(JingleSessionManager* session_manager)
 }
 
 JingleSession::~JingleSession() {
+  STLDeleteContainerPointers(pending_requests_.begin(),
+                             pending_requests_.end());
   STLDeleteContainerPairSecondPointers(channels_.begin(), channels_.end());
   session_manager_->SessionDestroyed(this);
 }
@@ -102,10 +110,7 @@ void JingleSession::StartConnection(
   message.description.reset(
       new ContentDescription(candidate_config_->Clone(),
                              authenticator_->GetNextMessage()));
-  initiate_request_ = session_manager_->iq_sender()->SendIq(
-      message.ToXml(),
-      base::Bind(&JingleSession::OnSessionInitiateResponse,
-                 base::Unretained(this)));
+  SendMessage(message);
 
   SetState(CONNECTING);
 }
@@ -158,10 +163,7 @@ void JingleSession::AcceptIncomingConnection(
   message.description.reset(
       new ContentDescription(CandidateSessionConfig::CreateFrom(config_),
                              auth_message.Pass()));
-  initiate_request_ = session_manager_->iq_sender()->SendIq(
-      message.ToXml(),
-      base::Bind(&JingleSession::OnSessionInitiateResponse,
-                 base::Unretained(this)));
+  SendMessage(message);
 
   // Update state.
   SetState(CONNECTED);
@@ -175,20 +177,6 @@ void JingleSession::AcceptIncomingConnection(
   return;
 }
 
-void JingleSession::OnSessionInitiateResponse(
-    const buzz::XmlElement* response) {
-  const std::string& type = response->Attr(buzz::QName("", "type"));
-  if (type != "result") {
-    LOG(ERROR) << "Received error in response to session-initiate message: \""
-               << response->Str()
-               << "\". Terminating the session.";
-
-    // TODO(sergeyu): There may be different reasons for error
-    // here. Parse the response stanza to find failure reason.
-    CloseInternal(PEER_IS_OFFLINE);
-  }
-}
-
 void JingleSession::CreateStreamChannel(
       const std::string& name,
       const StreamChannelCallback& callback) {
@@ -282,6 +270,87 @@ void JingleSession::OnTransportDeleted(Transport* transport) {
   channels_.erase(it);
 }
 
+void JingleSession::SendMessage(const JingleMessage& message) {
+  scoped_ptr<IqRequest> request = session_manager_->iq_sender()->SendIq(
+      message.ToXml(),
+      base::Bind(&JingleSession::OnMessageResponse,
+                 base::Unretained(this), message.action));
+  if (request.get()) {
+    request->SetTimeout(
+        base::TimeDelta::FromSeconds(kMessageResponseTimeoutSeconds));
+    pending_requests_.push_back(request.release());
+  } else {
+    LOG(ERROR) << "Failed to send a "
+               << JingleMessage::GetActionName(message.action) << " message";
+  }
+}
+
+void JingleSession::OnMessageResponse(
+    JingleMessage::ActionType request_type,
+    IqRequest* request,
+    const buzz::XmlElement* response) {
+  Error error = OK;
+
+  std::string type_str = JingleMessage::GetActionName(request_type);
+
+  if (!response) {
+    LOG(ERROR) << type_str << " request timed out.";
+    // Most likely the session-initiate timeout indicates a problem
+    // with the signaling.
+    error = UNKNOWN_ERROR;
+  } else {
+    const std::string& type = response->Attr(buzz::QName("", "type"));
+    if (type != "result") {
+      LOG(ERROR) << "Received error in response to " << type_str
+                 << " message: \"" << response->Str()
+                 << "\". Terminating the session.";
+
+      switch (request_type) {
+        case JingleMessage::SESSION_INFO:
+          // session-info is used for the new authentication protocol,
+          // and wasn't previously supported.
+          error = INCOMPATIBLE_PROTOCOL;
+
+        default:
+          // TODO(sergeyu): There may be different reasons for error
+          // here. Parse the response stanza to find failure reason.
+          error = PEER_IS_OFFLINE;
+      }
+    }
+  }
+
+  CleanupPendingRequests(request);
+
+  if (error != OK) {
+    CloseInternal(error);
+  }
+}
+
+void JingleSession::CleanupPendingRequests(IqRequest* request) {
+  DCHECK(!pending_requests_.empty());
+  DCHECK(request);
+
+  // This method is called whenever a response to |request| is
+  // received. Here we delete that request and all requests that were
+  // sent before it. The idea here is that if we send messages A, B
+  // and C and then suddenly receive response to C then it means that
+  // either A and B messages or the corresponding response messages
+  // were somehow lost. E.g. that may happen when the client switches
+  // from one network to another. The best way to handle that case is
+  // to ignore errors and timeouts for A and B by deleting the
+  // corresponding IqRequest objects.
+  while (!pending_requests_.empty() && pending_requests_.front() != request) {
+    delete pending_requests_.front();
+    pending_requests_.pop_front();
+  }
+
+  // Delete the |request| itself.
+  DCHECK_EQ(request, pending_requests_.front());
+  delete request;
+  if (!pending_requests_.empty())
+    pending_requests_.pop_front();
+}
+
 void JingleSession::OnIncomingMessage(const JingleMessage& message,
                                       const ReplyCallback& reply_callback) {
   DCHECK(CalledOnValidThread());
@@ -451,11 +520,7 @@ void JingleSession::ProcessAuthenticationStep() {
     JingleMessage message(peer_jid_, JingleMessage::SESSION_INFO, session_id_);
     message.info = authenticator_->GetNextMessage();
     DCHECK(message.info.get());
-
-    session_info_request_ = session_manager_->iq_sender()->SendIq(
-        message.ToXml(), base::Bind(
-            &JingleSession::OnSessionInfoResponse,
-            base::Unretained(this)));
+    SendMessage(message);
   }
   DCHECK_NE(authenticator_->state(), Authenticator::MESSAGE_READY);
 
@@ -467,42 +532,12 @@ void JingleSession::ProcessAuthenticationStep() {
   }
 }
 
-void JingleSession::OnSessionInfoResponse(const buzz::XmlElement* response) {
-  const std::string& type = response->Attr(buzz::QName("", "type"));
-  if (type != "result") {
-    LOG(ERROR) << "Received error in response to session-info message: \""
-               << response->Str()
-               << "\". Terminating the session.";
-    CloseInternal(INCOMPATIBLE_PROTOCOL);
-  }
-}
-
-void JingleSession::OnTransportInfoResponse(const buzz::XmlElement* response) {
-  const std::string& type = response->Attr(buzz::QName("", "type"));
-  if (type != "result") {
-    LOG(ERROR) << "Received error in response to session-initiate message: \""
-               << response->Str()
-               << "\". Terminating the session.";
-
-    if (state_ == CONNECTING) {
-      CloseInternal(PEER_IS_OFFLINE);
-    } else {
-      // Host has disconnected without sending session-terminate message.
-      CloseInternal(OK);
-    }
-  }
-}
-
 void JingleSession::SendTransportInfo() {
   JingleMessage message(peer_jid_, JingleMessage::TRANSPORT_INFO, session_id_);
   message.candidates.swap(pending_candidates_);
-  transport_info_request_ = session_manager_->iq_sender()->SendIq(
-      message.ToXml(), base::Bind(
-          &JingleSession::OnTransportInfoResponse,
-          base::Unretained(this)));
+  SendMessage(message);
 }
 
-
 void JingleSession::CloseInternal(Error error) {
   DCHECK(CalledOnValidThread());
 
@@ -527,8 +562,7 @@ void JingleSession::CloseInternal(Error error) {
     JingleMessage message(peer_jid_, JingleMessage::SESSION_TERMINATE,
                           session_id_);
     message.reason = reason;
-    session_manager_->iq_sender()->SendIq(
-        message.ToXml(), IqSender::ReplyCallback());
+    SendMessage(message);
   }
 
   error_ = error;
diff --git a/remoting/protocol/jingle_session.h b/remoting/protocol/jingle_session.h
index f092402..c747250 100644
--- a/remoting/protocol/jingle_session.h
+++ b/remoting/protocol/jingle_session.h
@@ -13,6 +13,7 @@
 #include "base/timer.h"
 #include "crypto/rsa_private_key.h"
 #include "net/base/completion_callback.h"
+#include "remoting/jingle_glue/iq_sender.h"
 #include "remoting/protocol/authenticator.h"
 #include "remoting/protocol/jingle_messages.h"
 #include "remoting/protocol/session.h"
@@ -25,9 +26,6 @@ class StreamSocket;
 }  // namespace net
 
 namespace remoting {
-
-class IqRequest;
-
 namespace protocol {
 
 class JingleSessionManager;
@@ -86,8 +84,15 @@ class JingleSession : public Session,
                                     scoped_ptr<Authenticator> authenticator);
   void AcceptIncomingConnection(const JingleMessage& initiate_message);
 
-  // Handler for session-initiate response.
-  void OnSessionInitiateResponse(const buzz::XmlElement* response);
+  // Helper to send IqRequests to the peer. It sets up the response
+  // callback to OnMessageResponse() which simply terminates the
+  // session whenever a request fails or times out. This method should
+  // not be used for messages that need to be handled differently.
+  void SendMessage(const JingleMessage& message);
+  void OnMessageResponse(JingleMessage::ActionType request_type,
+                         IqRequest* request,
+                         const buzz::XmlElement* response);
+  void CleanupPendingRequests(IqRequest* request);
 
   // Called by JingleSessionManager on incoming |message|. Must call
   // |reply_callback| to send reply message before sending any other
@@ -108,10 +113,8 @@ class JingleSession : public Session,
   bool InitializeConfigFromDescription(const ContentDescription* description);
 
   void ProcessAuthenticationStep();
-  void OnSessionInfoResponse(const buzz::XmlElement* response);
 
   void SendTransportInfo();
-  void OnTransportInfoResponse(const buzz::XmlElement* response);
 
   // Terminates the session and sends session-terminate if it is
   // necessary. |error| specifies the error code in case when the
@@ -136,9 +139,10 @@ class JingleSession : public Session,
 
   scoped_ptr<Authenticator> authenticator_;
 
-  scoped_ptr<IqRequest> initiate_request_;
-  scoped_ptr<IqRequest> session_info_request_;
-  scoped_ptr<IqRequest> transport_info_request_;
+  // Container for pending Iq requests. Requests are removed in
+  // CleanupPendingRequests() which is called when a response is
+  // received or one of the requests times out.
+  std::list<IqRequest*> pending_requests_;
 
   ChannelsMap channels_;