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@123635 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 154 insertions, 37 deletions

diff --git a/remoting/jingle_glue/iq_sender.cc b/remoting/jingle_glue/iq_sender.cc
index eb8f17a..ca0b3f6 100644
--- a/remoting/jingle_glue/iq_sender.cc
+++ b/remoting/jingle_glue/iq_sender.cc
@@ -4,7 +4,10 @@
 
 #include "remoting/jingle_glue/iq_sender.h"
 
+#include "base/bind.h"
+#include "base/location.h"
 #include "base/logging.h"
+#include "base/message_loop_proxy.h"
 #include "base/string_number_conversions.h"
 #include "remoting/jingle_glue/signal_strategy.h"
 #include "third_party/libjingle/source/talk/xmllite/xmlelement.h"
@@ -36,13 +39,14 @@ IqSender::~IqSender() {
 
 scoped_ptr<IqRequest> IqSender::SendIq(scoped_ptr<buzz::XmlElement> stanza,
                                        const ReplyCallback& callback) {
+  std::string addressee = stanza->Attr(buzz::QN_TO);
   std::string id = signal_strategy_->GetNextId();
   stanza->AddAttr(buzz::QN_ID, id);
   if (!signal_strategy_->SendStanza(stanza.Pass())) {
     return scoped_ptr<IqRequest>(NULL);
   }
   DCHECK(requests_.find(id) == requests_.end());
-  scoped_ptr<IqRequest> request(new IqRequest(this, callback));
+  scoped_ptr<IqRequest> request(new IqRequest(this, callback, addressee));
   if (!callback.is_null())
     requests_[id] = request.get();
   return request.Pass();
@@ -92,32 +96,72 @@ bool IqSender::OnSignalStrategyIncomingStanza(const buzz::XmlElement* stanza) {
     return false;
   }
 
+  std::string from = stanza->Attr(buzz::QN_FROM);
+
   IqRequestMap::iterator it = requests_.find(id);
+
+  // This is a hack to workaround the issue with the WCS changing IDs
+  // of IQ responses sent from client to host. Whenever we receive a
+  // stanza with an unknown ID we try to match it with an outstanding
+  // request sent to the same peer.
+  if (it == requests_.end()) {
+    for (it = requests_.begin(); it != requests_.end(); ++it) {
+      if (it->second->addressee_ == from) {
+        break;
+      }
+    }
+  }
+
   if (it == requests_.end()) {
     return false;
   }
 
   IqRequest* request = it->second;
-  requests_.erase(it);
 
+  if (request->addressee_ != from) {
+    LOG(ERROR) << "Received IQ response from from a invalid JID. Ignoring it."
+               << " Message received from: " << from
+               << " Original JID: " << request->addressee_;
+    return false;
+  }
+
+  requests_.erase(it);
   request->OnResponse(stanza);
+
   return true;
 }
 
-IqRequest::IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback)
+IqRequest::IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback,
+                     const std::string& addressee)
     : sender_(sender),
-      callback_(callback) {
+      callback_(callback),
+      addressee_(addressee) {
 }
 
 IqRequest::~IqRequest() {
   sender_->RemoveRequest(this);
 }
 
+void IqRequest::SetTimeout(base::TimeDelta timeout) {
+  base::MessageLoopProxy::current()->PostDelayedTask(
+      FROM_HERE, base::Bind(&IqRequest::OnTimeout, AsWeakPtr()),
+      timeout.InMilliseconds());
+}
+
+void IqRequest::CallCallback(const buzz::XmlElement* stanza) {
+  if (!callback_.is_null()) {
+    IqSender::ReplyCallback callback(callback_);
+    callback_.Reset();
+    callback.Run(this, stanza);
+  }
+}
+
+void IqRequest::OnTimeout() {
+  CallCallback(NULL);
+}
+
 void IqRequest::OnResponse(const buzz::XmlElement* stanza) {
-  DCHECK(!callback_.is_null());
-  IqSender::ReplyCallback callback(callback_);
-  callback_.Reset();
-  callback.Run(stanza);
+  CallCallback(stanza);
 }
 
 }  // namespace remoting
diff --git a/remoting/jingle_glue/iq_sender.h b/remoting/jingle_glue/iq_sender.h
index 5244584..677bd472 100644
--- a/remoting/jingle_glue/iq_sender.h
+++ b/remoting/jingle_glue/iq_sender.h
@@ -12,6 +12,7 @@
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "remoting/jingle_glue/signal_strategy.h"
 
 namespace buzz {
@@ -27,7 +28,10 @@ class SignalStrategy;
 // those requests.
 class IqSender : public SignalStrategy::Listener {
  public:
-  typedef base::Callback<void(const buzz::XmlElement*)> ReplyCallback;
+  // Callback that is called when an Iq response is received. Called
+  // with the |response| set to NULL in case of a timeout.
+  typedef base::Callback<void(IqRequest* request,
+                              const buzz::XmlElement* response)> ReplyCallback;
 
   explicit IqSender(SignalStrategy* signal_strategy);
   virtual ~IqSender();
@@ -72,19 +76,28 @@ class IqSender : public SignalStrategy::Listener {
 };
 
 // This call must only be used on the thread it was created on.
-class IqRequest {
+class IqRequest : public  base::SupportsWeakPtr<IqRequest> {
  public:
-  IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback);
+  IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback,
+            const std::string& addressee);
   ~IqRequest();
 
+  // Sets timeout for the request. When the timeout expires the
+  // callback is called with the |response| set to NULL.
+  void SetTimeout(base::TimeDelta timeout);
+
  private:
   friend class IqSender;
 
+  void CallCallback(const buzz::XmlElement* stanza);
+  void OnTimeout();
+
   // Called by IqSender when a response is received.
   void OnResponse(const buzz::XmlElement* stanza);
 
   IqSender* sender_;
   IqSender::ReplyCallback callback_;
+  std::string addressee_;
 
   DISALLOW_COPY_AND_ASSIGN(IqRequest);
 };
diff --git a/remoting/jingle_glue/iq_sender_unittest.cc b/remoting/jingle_glue/iq_sender_unittest.cc
index e14cf2a..06730f5 100644
--- a/remoting/jingle_glue/iq_sender_unittest.cc
+++ b/remoting/jingle_glue/iq_sender_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "base/bind.h"
 #include "base/memory/ref_counted.h"
+#include "base/message_loop.h"
 #include "base/stringprintf.h"
 #include "remoting/jingle_glue/iq_sender.h"
 #include "remoting/jingle_glue/mock_objects.h"
@@ -14,6 +15,7 @@
 
 using ::testing::_;
 using ::testing::DeleteArg;
+using ::testing::InvokeWithoutArgs;
 using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::SaveArg;
@@ -34,7 +36,7 @@ const char kTo[] = "user@domain.com";
 
 class MockCallback {
  public:
-  MOCK_METHOD1(OnReply, void(const XmlElement* reply));
+  MOCK_METHOD2(OnReply, void(IqRequest* request, const XmlElement* reply));
 };
 
 }  // namespace
@@ -49,43 +51,100 @@ class IqSenderTest : public testing::Test {
   }
 
  protected:
+  void SendTestMessage() {
+    scoped_ptr<XmlElement> iq_body(
+        new XmlElement(QName(kNamespace, kBodyTag)));
+    XmlElement* sent_stanza;
+    EXPECT_CALL(signal_strategy_, GetNextId())
+        .WillOnce(Return(kStanzaId));
+    EXPECT_CALL(signal_strategy_, SendStanzaPtr(_))
+        .WillOnce(DoAll(SaveArg<0>(&sent_stanza), Return(true)));
+    request_ = sender_->SendIq(kType, kTo, iq_body.Pass(), base::Bind(
+        &MockCallback::OnReply, base::Unretained(&callback_)));
+
+    std::string expected_xml_string =
+        base::StringPrintf(
+            "<cli:iq type=\"%s\" to=\"%s\" id=\"%s\" "
+            "xmlns:cli=\"jabber:client\">"
+            "<%s:%s xmlns:%s=\"%s\"/>"
+            "</cli:iq>",
+            kType, kTo, kStanzaId, kNamespacePrefix, kBodyTag,
+            kNamespacePrefix, kNamespace);
+    EXPECT_EQ(expected_xml_string, sent_stanza->Str());
+    delete sent_stanza;
+  }
+
+  MessageLoop message_loop_;
   MockSignalStrategy signal_strategy_;
   scoped_ptr<IqSender> sender_;
   MockCallback callback_;
+  scoped_ptr<IqRequest> request_;
 };
 
 TEST_F(IqSenderTest, SendIq) {
-  scoped_ptr<XmlElement> iq_body(
-      new XmlElement(QName(kNamespace, kBodyTag)));
-  XmlElement* sent_stanza;
-  EXPECT_CALL(signal_strategy_, GetNextId())
-      .WillOnce(Return(kStanzaId));
-  EXPECT_CALL(signal_strategy_, SendStanzaPtr(_))
-      .WillOnce(DoAll(SaveArg<0>(&sent_stanza), Return(true)));
-  scoped_ptr<IqRequest> request =
-      sender_->SendIq(kType, kTo, iq_body.Pass(), base::Bind(
-          &MockCallback::OnReply, base::Unretained(&callback_)));
-
-  std::string expected_xml_string =
-      base::StringPrintf(
-          "<cli:iq type=\"%s\" to=\"%s\" id=\"%s\" "
-          "xmlns:cli=\"jabber:client\">"
-          "<%s:%s xmlns:%s=\"%s\"/>"
-          "</cli:iq>",
-          kType, kTo, kStanzaId, kNamespacePrefix, kBodyTag,
-          kNamespacePrefix, kNamespace);
-  EXPECT_EQ(expected_xml_string, sent_stanza->Str());
-  delete sent_stanza;
+  ASSERT_NO_FATAL_FAILURE({
+    SendTestMessage();
+  });
+
+  scoped_ptr<XmlElement> response(new XmlElement(buzz::QN_IQ));
+  response->AddAttr(QName("", "type"), "result");
+  response->AddAttr(QName("", "id"), kStanzaId);
+  response->AddAttr(QName("", "from"), kTo);
+
+  XmlElement* result = new XmlElement(
+      QName("test:namespace", "response-body"));
+  response->AddElement(result);
+
+  EXPECT_CALL(callback_, OnReply(request_.get(), response.get()));
+  EXPECT_TRUE(sender_->OnSignalStrategyIncomingStanza(response.get()));
+}
+
+TEST_F(IqSenderTest, Timeout) {
+  ASSERT_NO_FATAL_FAILURE({
+    SendTestMessage();
+  });
+
+  request_->SetTimeout(base::TimeDelta::FromMilliseconds(2));
+
+  EXPECT_CALL(callback_, OnReply(request_.get(), NULL))
+      .WillOnce(InvokeWithoutArgs(&message_loop_, &MessageLoop::Quit));
+  message_loop_.Run();
+}
+
+TEST_F(IqSenderTest, InvalidFrom) {
+  ASSERT_NO_FATAL_FAILURE({
+    SendTestMessage();
+  });
 
   scoped_ptr<XmlElement> response(new XmlElement(buzz::QN_IQ));
   response->AddAttr(QName("", "type"), "result");
   response->AddAttr(QName("", "id"), kStanzaId);
+  response->AddAttr(QName("", "from"), "different_user@domain.com");
+
+  XmlElement* result = new XmlElement(
+      QName("test:namespace", "response-body"));
+  response->AddElement(result);
+
+  EXPECT_CALL(callback_, OnReply(request_.get(), response.get()))
+      .Times(0);
+  EXPECT_FALSE(sender_->OnSignalStrategyIncomingStanza(response.get()));
+}
+
+TEST_F(IqSenderTest, IdMatchingHack) {
+  ASSERT_NO_FATAL_FAILURE({
+    SendTestMessage();
+  });
+
+  scoped_ptr<XmlElement> response(new XmlElement(buzz::QN_IQ));
+  response->AddAttr(QName("", "type"), "result");
+  response->AddAttr(QName("", "id"), "DIFFERENT_ID");
+  response->AddAttr(QName("", "from"), kTo);
 
   XmlElement* result = new XmlElement(
       QName("test:namespace", "response-body"));
   response->AddElement(result);
 
-  EXPECT_CALL(callback_, OnReply(response.get()));
+  EXPECT_CALL(callback_, OnReply(request_.get(), response.get()));
   EXPECT_TRUE(sender_->OnSignalStrategyIncomingStanza(response.get()));
 }
 
diff --git a/remoting/jingle_glue/jingle_info_request.cc b/remoting/jingle_glue/jingle_info_request.cc
index 56d93ca..eb5bd7b 100644
--- a/remoting/jingle_glue/jingle_info_request.cc
+++ b/remoting/jingle_glue/jingle_info_request.cc
@@ -32,7 +32,8 @@ void JingleInfoRequest::Send(const OnJingleInfoCallback& callback) {
       base::Bind(&JingleInfoRequest::OnResponse, base::Unretained(this)));
 }
 
-void JingleInfoRequest::OnResponse(const buzz::XmlElement* stanza) {
+void JingleInfoRequest::OnResponse(IqRequest* request,
+                                   const buzz::XmlElement* stanza) {
   const buzz::XmlElement* query =
       stanza->FirstNamed(buzz::QN_JINGLE_INFO_QUERY);
   if (query == NULL) {
diff --git a/remoting/jingle_glue/jingle_info_request.h b/remoting/jingle_glue/jingle_info_request.h
index 9843586..a1b10de 100644
--- a/remoting/jingle_glue/jingle_info_request.h
+++ b/remoting/jingle_glue/jingle_info_request.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -54,7 +54,7 @@ class JingleInfoRequest : public sigslot::has_slots<> {
  private:
   struct PendingDnsRequest;
 
-  void OnResponse(const buzz::XmlElement* stanza);
+  void OnResponse(IqRequest* request, const buzz::XmlElement* stanza);
 
   IqSender iq_sender_;
   scoped_ptr<IqRequest> request_;