Revert 123635 - 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

TBR=sergeyu@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9447087

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

5 files changed, 37 insertions, 154 deletions

diff --git a/remoting/jingle_glue/iq_sender.cc b/remoting/jingle_glue/iq_sender.cc
index ca0b3f6..eb8f17a 100644
--- a/remoting/jingle_glue/iq_sender.cc
+++ b/remoting/jingle_glue/iq_sender.cc
@@ -4,10 +4,7 @@
 
 #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"
@@ -39,14 +36,13 @@ 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, addressee));
+  scoped_ptr<IqRequest> request(new IqRequest(this, callback));
   if (!callback.is_null())
     requests_[id] = request.get();
   return request.Pass();
@@ -96,72 +92,32 @@ 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;
-
-  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);
 
+  request->OnResponse(stanza);
   return true;
 }
 
-IqRequest::IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback,
-                     const std::string& addressee)
+IqRequest::IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback)
     : sender_(sender),
-      callback_(callback),
-      addressee_(addressee) {
+      callback_(callback) {
 }
 
 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) {
-  CallCallback(stanza);
+  DCHECK(!callback_.is_null());
+  IqSender::ReplyCallback callback(callback_);
+  callback_.Reset();
+  callback.Run(stanza);
 }
 
 }  // namespace remoting
diff --git a/remoting/jingle_glue/iq_sender.h b/remoting/jingle_glue/iq_sender.h
index 677bd472..5244584 100644
--- a/remoting/jingle_glue/iq_sender.h
+++ b/remoting/jingle_glue/iq_sender.h
@@ -12,7 +12,6 @@
 #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 {
@@ -28,10 +27,7 @@ class SignalStrategy;
 // those requests.
 class IqSender : public SignalStrategy::Listener {
  public:
-  // 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;
+  typedef base::Callback<void(const buzz::XmlElement*)> ReplyCallback;
 
   explicit IqSender(SignalStrategy* signal_strategy);
   virtual ~IqSender();
@@ -76,28 +72,19 @@ class IqSender : public SignalStrategy::Listener {
 };
 
 // This call must only be used on the thread it was created on.
-class IqRequest : public  base::SupportsWeakPtr<IqRequest> {
+class IqRequest {
  public:
-  IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback,
-            const std::string& addressee);
+  IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback);
   ~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 06730f5..e14cf2a 100644
--- a/remoting/jingle_glue/iq_sender_unittest.cc
+++ b/remoting/jingle_glue/iq_sender_unittest.cc
@@ -4,7 +4,6 @@
 
 #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"
@@ -15,7 +14,6 @@
 
 using ::testing::_;
 using ::testing::DeleteArg;
-using ::testing::InvokeWithoutArgs;
 using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::SaveArg;
@@ -36,7 +34,7 @@ const char kTo[] = "user@domain.com";
 
 class MockCallback {
  public:
-  MOCK_METHOD2(OnReply, void(IqRequest* request, const XmlElement* reply));
+  MOCK_METHOD1(OnReply, void(const XmlElement* reply));
 };
 
 }  // namespace
@@ -51,100 +49,43 @@ 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) {
-  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> 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;
 
   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(request_.get(), response.get()));
+  EXPECT_CALL(callback_, OnReply(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 eb5bd7b..56d93ca 100644
--- a/remoting/jingle_glue/jingle_info_request.cc
+++ b/remoting/jingle_glue/jingle_info_request.cc
@@ -32,8 +32,7 @@ void JingleInfoRequest::Send(const OnJingleInfoCallback& callback) {
       base::Bind(&JingleInfoRequest::OnResponse, base::Unretained(this)));
 }
 
-void JingleInfoRequest::OnResponse(IqRequest* request,
-                                   const buzz::XmlElement* stanza) {
+void JingleInfoRequest::OnResponse(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 a1b10de..9843586 100644
--- a/remoting/jingle_glue/jingle_info_request.h
+++ b/remoting/jingle_glue/jingle_info_request.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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(IqRequest* request, const buzz::XmlElement* stanza);
+  void OnResponse(const buzz::XmlElement* stanza);
 
   IqSender iq_sender_;
   scoped_ptr<IqRequest> request_;