Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(225)

Unified Diff: remoting/protocol/jingle_session.cc

Issue 9452038: Implement timeouts for IQ requests. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: fix release build Created 8 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « remoting/protocol/jingle_session.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: remoting/protocol/jingle_session.cc
diff --git a/remoting/protocol/jingle_session.cc b/remoting/protocol/jingle_session.cc
index 61d4bc975c72a17ea60b9f01c837808c2d0cf608..f0edb343fcb34c6deb73d28cc385c58653b649d7 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;
« no previous file with comments | « remoting/protocol/jingle_session.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698