diff options
author | sergeyu@chromium.org <sergeyu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-04 01:00:49 +0000 |
---|---|---|
committer | sergeyu@chromium.org <sergeyu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-04 01:00:49 +0000 |
commit | b39e182dca78d0c165ba75bb04878f4eb71b3625 (patch) | |
tree | c8f3d4f74b05cdb50b4d5244620595a758d8a996 /remoting/jingle_glue | |
parent | dc01874febf63d62bd05178f168f5459fec05372 (diff) | |
download | chromium_src-b39e182dca78d0c165ba75bb04878f4eb71b3625.zip chromium_src-b39e182dca78d0c165ba75bb04878f4eb71b3625.tar.gz chromium_src-b39e182dca78d0c165ba75bb04878f4eb71b3625.tar.bz2 |
Refactor IqRequest.
Remove CreateIqRequest from SignalStrategy interface. Intead to send an Iq
stanza the new IqSender now need to be used. IqSender creats of IqRequest
objects and handling iq responses.
BUG=None
TEST=Unittests.
Review URL: http://codereview.chromium.org/8432009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@108606 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'remoting/jingle_glue')
-rw-r--r-- | remoting/jingle_glue/fake_signal_strategy.cc | 37 | ||||
-rw-r--r-- | remoting/jingle_glue/fake_signal_strategy.h | 11 | ||||
-rw-r--r-- | remoting/jingle_glue/iq_request.cc | 114 | ||||
-rw-r--r-- | remoting/jingle_glue/iq_request.h | 94 | ||||
-rw-r--r-- | remoting/jingle_glue/iq_request_unittest.cc | 40 | ||||
-rw-r--r-- | remoting/jingle_glue/iq_sender.cc | 109 | ||||
-rw-r--r-- | remoting/jingle_glue/iq_sender.h | 90 | ||||
-rw-r--r-- | remoting/jingle_glue/iq_sender_unittest.cc | 91 | ||||
-rw-r--r-- | remoting/jingle_glue/javascript_signal_strategy.cc | 43 | ||||
-rw-r--r-- | remoting/jingle_glue/javascript_signal_strategy.h | 12 | ||||
-rw-r--r-- | remoting/jingle_glue/jingle_info_request.cc | 17 | ||||
-rw-r--r-- | remoting/jingle_glue/jingle_info_request.h | 12 | ||||
-rw-r--r-- | remoting/jingle_glue/jingle_signaling_connector.cc | 5 | ||||
-rw-r--r-- | remoting/jingle_glue/mock_objects.cc | 3 | ||||
-rw-r--r-- | remoting/jingle_glue/mock_objects.h | 33 | ||||
-rw-r--r-- | remoting/jingle_glue/signal_strategy.h | 19 | ||||
-rw-r--r-- | remoting/jingle_glue/xmpp_signal_strategy.cc | 44 | ||||
-rw-r--r-- | remoting/jingle_glue/xmpp_signal_strategy.h | 12 |
18 files changed, 401 insertions, 385 deletions
diff --git a/remoting/jingle_glue/fake_signal_strategy.cc b/remoting/jingle_glue/fake_signal_strategy.cc index 0f7fb77..7c219a7 100644 --- a/remoting/jingle_glue/fake_signal_strategy.cc +++ b/remoting/jingle_glue/fake_signal_strategy.cc @@ -24,7 +24,6 @@ void FakeSignalStrategy::Connect(FakeSignalStrategy* peer1, FakeSignalStrategy::FakeSignalStrategy(const std::string& jid) : jid_(jid), peer_(NULL), - listener_(NULL), last_id_(0), ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)) { @@ -35,6 +34,7 @@ FakeSignalStrategy::~FakeSignalStrategy() { delete pending_messages_.front(); pending_messages_.pop(); } + DCHECK(listeners_.empty()); } void FakeSignalStrategy::Init(StatusObserver* observer) { @@ -46,27 +46,34 @@ void FakeSignalStrategy::Init(StatusObserver* observer) { void FakeSignalStrategy::Close() { DCHECK(CalledOnValidThread()); - listener_ = NULL; } -void FakeSignalStrategy::SetListener(Listener* listener) { +void FakeSignalStrategy::AddListener(Listener* listener) { DCHECK(CalledOnValidThread()); + DCHECK(std::find(listeners_.begin(), listeners_.end(), listener) == + listeners_.end()); + listeners_.push_back(listener); +} - // Don't overwrite an listener without explicitly going - // through "NULL" first. - DCHECK(listener_ == NULL || listener == NULL); - listener_ = listener; +void FakeSignalStrategy::RemoveListener(Listener* listener) { + DCHECK(CalledOnValidThread()); + std::vector<Listener*>::iterator it = + std::find(listeners_.begin(), listeners_.end(), listener); + CHECK(it != listeners_.end()); + listeners_.erase(it); } -void FakeSignalStrategy::SendStanza(buzz::XmlElement* stanza) { +bool FakeSignalStrategy::SendStanza(buzz::XmlElement* stanza) { DCHECK(CalledOnValidThread()); stanza->SetAttr(buzz::QN_FROM, jid_); if (peer_) { peer_->OnIncomingMessage(stanza); + return true; } else { delete stanza; + return false; } } @@ -75,12 +82,6 @@ std::string FakeSignalStrategy::GetNextId() { return base::IntToString(last_id_); } -IqRequest* FakeSignalStrategy::CreateIqRequest() { - DCHECK(CalledOnValidThread()); - - return new IqRequest(this, &iq_registry_); -} - void FakeSignalStrategy::OnIncomingMessage(buzz::XmlElement* stanza) { pending_messages_.push(stanza); MessageLoop::current()->PostTask( @@ -99,9 +100,11 @@ void FakeSignalStrategy::DeliverIncomingMessages() { return; } - if (listener_) - listener_->OnIncomingStanza(stanza); - iq_registry_.OnIncomingStanza(stanza); + for (std::vector<Listener*>::iterator it = listeners_.begin(); + it != listeners_.end(); ++it) { + if ((*it)->OnIncomingStanza(stanza)) + break; + } pending_messages_.pop(); delete stanza; diff --git a/remoting/jingle_glue/fake_signal_strategy.h b/remoting/jingle_glue/fake_signal_strategy.h index 37aad4a..d54276b 100644 --- a/remoting/jingle_glue/fake_signal_strategy.h +++ b/remoting/jingle_glue/fake_signal_strategy.h @@ -10,7 +10,7 @@ #include "base/task.h" #include "base/threading/non_thread_safe.h" -#include "remoting/jingle_glue/iq_request.h" +#include "remoting/jingle_glue/iq_sender.h" #include "remoting/jingle_glue/signal_strategy.h" namespace remoting { @@ -26,10 +26,10 @@ class FakeSignalStrategy : public SignalStrategy, // SignalStrategy interface. virtual void Init(StatusObserver* observer) OVERRIDE; virtual void Close() OVERRIDE; - virtual void SetListener(Listener* listener) OVERRIDE; - virtual void SendStanza(buzz::XmlElement* stanza) OVERRIDE; + virtual void AddListener(Listener* listener) OVERRIDE; + virtual void RemoveListener(Listener* listener) OVERRIDE; + virtual bool SendStanza(buzz::XmlElement* stanza) OVERRIDE; virtual std::string GetNextId() OVERRIDE; - virtual IqRequest* CreateIqRequest() OVERRIDE; private: // Called by the |peer_|. Takes ownership of |stanza|. @@ -39,8 +39,7 @@ class FakeSignalStrategy : public SignalStrategy, std::string jid_; FakeSignalStrategy* peer_; - Listener* listener_; - IqRegistry iq_registry_; + std::vector<Listener*> listeners_; int last_id_; diff --git a/remoting/jingle_glue/iq_request.cc b/remoting/jingle_glue/iq_request.cc deleted file mode 100644 index 5177ee6..0000000 --- a/remoting/jingle_glue/iq_request.cc +++ /dev/null @@ -1,114 +0,0 @@ -// 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. - -#include "remoting/jingle_glue/iq_request.h" - -#include "base/logging.h" -#include "base/string_number_conversions.h" -#include "remoting/jingle_glue/signal_strategy.h" -#include "third_party/libjingle/source/talk/xmllite/xmlelement.h" -#include "third_party/libjingle/source/talk/xmpp/constants.h" - -namespace remoting { - -IqRegistry::IqRegistry() { -} - -IqRegistry::~IqRegistry() { -} - -void IqRegistry::RemoveAllRequests(IqRequest* request) { - IqRequestMap::iterator it = requests_.begin(); - while (it != requests_.end()) { - IqRequestMap::iterator cur = it; - ++it; - if (cur->second == request) { - requests_.erase(cur); - } - } -} - -bool IqRegistry::OnIncomingStanza(const buzz::XmlElement* stanza) { - // TODO(ajwong): Can we cleanup this dispatch at all? The send is from - // IqRequest but the return is in IqRegistry. - - if (stanza->Name() != buzz::QN_IQ) { - LOG(WARNING) << "Received unexpected non-IQ packet" << stanza->Str(); - return false; - } - - if (!stanza->HasAttr(buzz::QN_ID)) { - LOG(WARNING) << "IQ packet missing id" << stanza->Str(); - return false; - } - - const std::string& id = stanza->Attr(buzz::QN_ID); - - IqRequestMap::iterator it = requests_.find(id); - if (it == requests_.end()) { - return false; - } - - // TODO(ajwong): We should look at the logic inside libjingle's - // XmppTask::MatchResponseIq() and make sure we're fully in sync. - // They check more fields and conditions than us. - - // TODO(ajwong): This logic is weird. We add to the register in - // IqRequest::SendIq(), but remove in - // IqRegistry::OnIncomingStanza(). We should try to keep the - // registration/deregistration in one spot. - if (!it->second->callback_.is_null()) { - it->second->callback_.Run(stanza); - it->second->callback_.Reset(); - } else { - VLOG(1) << "No callback, so dropping: " << stanza->Str(); - } - requests_.erase(it); - return true; -} - -void IqRegistry::RegisterRequest(IqRequest* request, const std::string& id) { - DCHECK(requests_.find(id) == requests_.end()); - requests_[id] = request; -} - -// static -buzz::XmlElement* IqRequest::MakeIqStanza(const std::string& type, - const std::string& addressee, - buzz::XmlElement* iq_body) { - buzz::XmlElement* stanza = new buzz::XmlElement(buzz::QN_IQ); - stanza->AddAttr(buzz::QN_TYPE, type); - if (!addressee.empty()) - stanza->AddAttr(buzz::QN_TO, addressee); - stanza->AddElement(iq_body); - return stanza; -} - -IqRequest::IqRequest() - : signal_strategy_(NULL), - registry_(NULL) { -} - -IqRequest::IqRequest(SignalStrategy* signal_strategy, IqRegistry* registry) - : signal_strategy_(signal_strategy), - registry_(registry) { -} - -IqRequest::~IqRequest() { - if (registry_) - registry_->RemoveAllRequests(this); -} - -void IqRequest::SendIq(buzz::XmlElement* stanza) { - std::string id = signal_strategy_->GetNextId(); - stanza->AddAttr(buzz::QN_ID, id); - registry_->RegisterRequest(this, id); - signal_strategy_->SendStanza(stanza); -} - -void IqRequest::set_callback(const ReplyCallback& callback) { - callback_ = callback; -} - -} // namespace remoting diff --git a/remoting/jingle_glue/iq_request.h b/remoting/jingle_glue/iq_request.h deleted file mode 100644 index d94e4d2..0000000 --- a/remoting/jingle_glue/iq_request.h +++ /dev/null @@ -1,94 +0,0 @@ -// 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. - -#ifndef REMOTING_JINGLE_GLUE_IQ_REQUEST_H_ -#define REMOTING_JINGLE_GLUE_IQ_REQUEST_H_ - -#include <map> -#include <string> - -#include "base/callback.h" -#include "base/compiler_specific.h" -#include "base/gtest_prod_util.h" -#include "base/memory/scoped_ptr.h" -#include "remoting/jingle_glue/iq_request.h" - -namespace buzz { -class XmlElement; -} // namespace buzz - -namespace remoting { - -class IqRequest; -class SignalStrategy; - -// IqRegistry handles routing of iq responses to corresponding -// IqRequest objects. Created in SignalStrategy and should not be used -// directly. -// -// TODO(sergeyu): Separate IqRegistry and IqRequest from -// SignalStrategy and remove CreateIqRequest() method from SignalStrategy. -class IqRegistry { - public: - IqRegistry(); - ~IqRegistry(); - - // Dispatches the response to the IqRequest callback immediately. - // - // Does not take ownership of stanza. - void DispatchResponse(buzz::XmlElement* stanza); - - // Registers |request| with the specified |id|. - void RegisterRequest(IqRequest* request, const std::string& id); - - // Removes all entries in the registry that refer to |request|. - void RemoveAllRequests(IqRequest* request); - - void SetDefaultHandler(IqRequest* default_request); - - // Called by SignalStrategy implementation. Returns true if the - // stanza was handled and should not be processed further. - bool OnIncomingStanza(const buzz::XmlElement* stanza); - - private: - typedef std::map<std::string, IqRequest*> IqRequestMap; - - IqRequestMap requests_; - - DISALLOW_COPY_AND_ASSIGN(IqRegistry); -}; - -// This call must only be used on the thread it was created on. -class IqRequest { - public: - typedef base::Callback<void(const buzz::XmlElement*)> ReplyCallback; - - static buzz::XmlElement* MakeIqStanza(const std::string& type, - const std::string& addressee, - buzz::XmlElement* iq_body); - - IqRequest(); // Should be used for tests only. - IqRequest(SignalStrategy* signal_strategy, IqRegistry* registry); - virtual ~IqRequest(); - - // Sends Iq stanza. Takes ownership of |stanza|. - virtual void SendIq(buzz::XmlElement* stanza); - - // Sets callback that is called when reply stanza is received. - virtual void set_callback(const ReplyCallback& callback); - - private: - friend class IqRegistry; - FRIEND_TEST_ALL_PREFIXES(IqRequestTest, MakeIqStanza); - - ReplyCallback callback_; - SignalStrategy* signal_strategy_; - IqRegistry* registry_; - - DISALLOW_COPY_AND_ASSIGN(IqRequest); -}; - -} // namespace remoting - -#endif // REMOTING_JINGLE_GLUE_IQ_REQUEST_H_ diff --git a/remoting/jingle_glue/iq_request_unittest.cc b/remoting/jingle_glue/iq_request_unittest.cc deleted file mode 100644 index d7eeda6..0000000 --- a/remoting/jingle_glue/iq_request_unittest.cc +++ /dev/null @@ -1,40 +0,0 @@ -// 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. - -#include "base/memory/ref_counted.h" -#include "base/stringprintf.h" -#include "remoting/jingle_glue/iq_request.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "third_party/libjingle/source/talk/xmllite/xmlelement.h" - -namespace remoting { - -TEST(IqRequestTest, MakeIqStanza) { - const char* kMessageId = "0"; - const char* kNamespace = "chromium:testns"; - const char* kNamespacePrefix = "tes"; - const char* kBodyTag = "test"; - const char* kType = "get"; - const char* kTo = "user@domain.com"; - - 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, kMessageId, kNamespacePrefix, kBodyTag, - kNamespacePrefix, kNamespace); - - buzz::XmlElement* iq_body = - new buzz::XmlElement(buzz::QName(kNamespace, kBodyTag)); - scoped_ptr<buzz::XmlElement> stanza( - IqRequest::MakeIqStanza(kType, kTo, iq_body)); - stanza->AddAttr(buzz::QName("", "id"), kMessageId); - - EXPECT_EQ(expected_xml_string, stanza->Str()); -} - -} // namespace remoting diff --git a/remoting/jingle_glue/iq_sender.cc b/remoting/jingle_glue/iq_sender.cc new file mode 100644 index 0000000..e9e6661 --- /dev/null +++ b/remoting/jingle_glue/iq_sender.cc @@ -0,0 +1,109 @@ +// 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. + +#include "remoting/jingle_glue/iq_sender.h" + +#include "base/logging.h" +#include "base/string_number_conversions.h" +#include "remoting/jingle_glue/signal_strategy.h" +#include "third_party/libjingle/source/talk/xmllite/xmlelement.h" +#include "third_party/libjingle/source/talk/xmpp/constants.h" + +namespace remoting { + +// static +buzz::XmlElement* IqSender::MakeIqStanza(const std::string& type, + const std::string& addressee, + buzz::XmlElement* iq_body) { + buzz::XmlElement* stanza = new buzz::XmlElement(buzz::QN_IQ); + stanza->AddAttr(buzz::QN_TYPE, type); + if (!addressee.empty()) + stanza->AddAttr(buzz::QN_TO, addressee); + stanza->AddElement(iq_body); + return stanza; +} + +IqSender::IqSender(SignalStrategy* signal_strategy) + : signal_strategy_(signal_strategy) { + signal_strategy_->AddListener(this); +} + +IqSender::~IqSender() { + signal_strategy_->RemoveListener(this); +} + +IqRequest* IqSender::SendIq(buzz::XmlElement* stanza, + const ReplyCallback& callback) { + std::string id = signal_strategy_->GetNextId(); + stanza->AddAttr(buzz::QN_ID, id); + if (!signal_strategy_->SendStanza(stanza)) { + return NULL; + } + DCHECK(requests_.find(id) == requests_.end()); + IqRequest* request = new IqRequest(this, callback); + if (!callback.is_null()) + requests_[id] = request; + return request; +} + +IqRequest* IqSender::SendIq(const std::string& type, + const std::string& addressee, + buzz::XmlElement* iq_body, + const ReplyCallback& callback) { + return SendIq(MakeIqStanza(type, addressee, iq_body), callback); +} + +void IqSender::RemoveRequest(IqRequest* request) { + IqRequestMap::iterator it = requests_.begin(); + while (it != requests_.end()) { + IqRequestMap::iterator cur = it; + ++it; + if (cur->second == request) { + requests_.erase(cur); + break; + } + } +} + +bool IqSender::OnIncomingStanza(const buzz::XmlElement* stanza) { + if (stanza->Name() != buzz::QN_IQ) { + LOG(WARNING) << "Received unexpected non-IQ packet" << stanza->Str(); + return false; + } + + const std::string& id = stanza->Attr(buzz::QN_ID); + if (id.empty()) { + LOG(WARNING) << "IQ packet missing id" << stanza->Str(); + return false; + } + + IqRequestMap::iterator it = requests_.find(id); + if (it == requests_.end()) { + return false; + } + + IqRequest* request = it->second; + requests_.erase(it); + + request->OnResponse(stanza); + return true; +} + +IqRequest::IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback) + : sender_(sender), + callback_(callback) { +} + +IqRequest::~IqRequest() { + sender_->RemoveRequest(this); +} + +void IqRequest::OnResponse(const buzz::XmlElement* 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 new file mode 100644 index 0000000..77b48ee --- /dev/null +++ b/remoting/jingle_glue/iq_sender.h @@ -0,0 +1,90 @@ +// 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. + +#ifndef REMOTING_JINGLE_GLUE_IQ_SENDER_H_ +#define REMOTING_JINGLE_GLUE_IQ_SENDER_H_ + +#include <map> +#include <string> + +#include "base/callback.h" +#include "base/compiler_specific.h" +#include "base/gtest_prod_util.h" +#include "base/memory/scoped_ptr.h" +#include "remoting/jingle_glue/signal_strategy.h" + +namespace buzz { +class XmlElement; +} // namespace buzz + +namespace remoting { + +class IqRequest; +class SignalStrategy; + +// IqSender handles sending iq requests and routing of responses to +// those requests. +class IqSender : public SignalStrategy::Listener { + public: + typedef base::Callback<void(const buzz::XmlElement*)> ReplyCallback; + + explicit IqSender(SignalStrategy* signal_strategy); + virtual ~IqSender(); + + // Send an iq stanza. Returns an IqRequest object that represends + // the request. |callback| is called when response to |stanza| is + // received. Destroy the returned IqRequest to cancel the callback. + // Takes ownership of |stanza|. Caller must take ownership of the + // result. Result must be destroyed before sender is destroyed. + IqRequest* SendIq(buzz::XmlElement* stanza, const ReplyCallback& callback); + + // Same as above, but also formats the message. Takes ownership of + // |iq_body|. + IqRequest* SendIq(const std::string& type, + const std::string& addressee, + buzz::XmlElement* iq_body, + const ReplyCallback& callback); + + // SignalStrategy::Listener implementation. + virtual bool OnIncomingStanza(const buzz::XmlElement* stanza) OVERRIDE; + + private: + typedef std::map<std::string, IqRequest*> IqRequestMap; + friend class IqRequest; + + // Helper function used to create iq stanzas. + static buzz::XmlElement* MakeIqStanza(const std::string& type, + const std::string& addressee, + buzz::XmlElement* iq_body); + + // Removes |request| from the list of pending requests. Called by IqRequest. + void RemoveRequest(IqRequest* request); + + SignalStrategy* signal_strategy_; + IqRequestMap requests_; + + DISALLOW_COPY_AND_ASSIGN(IqSender); +}; + +// This call must only be used on the thread it was created on. +class IqRequest { + public: + IqRequest(IqSender* sender, const IqSender::ReplyCallback& callback); + ~IqRequest(); + + private: + friend class IqSender; + + // Called by IqSender when a response is received. + void OnResponse(const buzz::XmlElement* stanza); + + IqSender* sender_; + IqSender::ReplyCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(IqRequest); +}; + +} // namespace remoting + +#endif // REMOTING_JINGLE_GLUE_IQ_SENDER_H_ diff --git a/remoting/jingle_glue/iq_sender_unittest.cc b/remoting/jingle_glue/iq_sender_unittest.cc new file mode 100644 index 0000000..733a084 --- /dev/null +++ b/remoting/jingle_glue/iq_sender_unittest.cc @@ -0,0 +1,91 @@ +// 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. + +#include "base/bind.h" +#include "base/memory/ref_counted.h" +#include "base/stringprintf.h" +#include "remoting/jingle_glue/iq_sender.h" +#include "remoting/jingle_glue/mock_objects.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/libjingle/source/talk/xmllite/xmlelement.h" +#include "third_party/libjingle/source/talk/xmpp/constants.h" + +using ::testing::_; +using ::testing::DeleteArg; +using ::testing::NotNull; +using ::testing::Return; +using ::testing::SaveArg; + +using ::buzz::QName; +using ::buzz::XmlElement; + +namespace remoting { + +namespace { + +const char kStanzaId[] = "123"; +const char kNamespace[] = "chromium:testns"; +const char kNamespacePrefix[] = "tes"; +const char kBodyTag[] = "test"; +const char kType[] = "get"; +const char kTo[] = "user@domain.com"; + +class MockCallback { + public: + MOCK_METHOD1(OnReply, void(const XmlElement* reply)); +}; + +} // namespace + +class IqSenderTest : public testing::Test { + public: + IqSenderTest() { + EXPECT_CALL(signal_strategy_, AddListener(NotNull())); + sender_.reset(new IqSender(&signal_strategy_)); + EXPECT_CALL(signal_strategy_, RemoveListener( + static_cast<SignalStrategy::Listener*>(sender_.get()))); + } + + protected: + MockSignalStrategy signal_strategy_; + scoped_ptr<IqSender> sender_; + MockCallback callback_; +}; + +TEST_F(IqSenderTest, SendIq) { + XmlElement* iq_body = + new XmlElement(QName(kNamespace, kBodyTag)); + XmlElement* sent_stanza; + EXPECT_CALL(signal_strategy_, GetNextId()) + .WillOnce(Return(kStanzaId)); + EXPECT_CALL(signal_strategy_, SendStanza(_)) + .WillOnce(DoAll(SaveArg<0>(&sent_stanza), Return(true))); + sender_->SendIq(kType, kTo, iq_body, 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); + + XmlElement* result = new XmlElement( + QName("test:namespace", "response-body")); + response->AddElement(result); + + EXPECT_CALL(callback_, OnReply(response.get())); + EXPECT_TRUE(sender_->OnIncomingStanza(response.get())); +} + +} // namespace remoting diff --git a/remoting/jingle_glue/javascript_signal_strategy.cc b/remoting/jingle_glue/javascript_signal_strategy.cc index cd14fbb..a26a385 100644 --- a/remoting/jingle_glue/javascript_signal_strategy.cc +++ b/remoting/jingle_glue/javascript_signal_strategy.cc @@ -8,7 +8,6 @@ #include "base/logging.h" #include "base/string_number_conversions.h" -#include "remoting/jingle_glue/iq_request.h" #include "remoting/jingle_glue/xmpp_proxy.h" #include "third_party/libjingle/source/talk/xmllite/xmlelement.h" @@ -16,17 +15,17 @@ namespace remoting { JavascriptSignalStrategy::JavascriptSignalStrategy(const std::string& your_jid) : your_jid_(your_jid), - listener_(NULL), last_id_(0) { } JavascriptSignalStrategy::~JavascriptSignalStrategy() { - DCHECK(listener_ == NULL); + DCHECK(listeners_.empty()); Close(); } void JavascriptSignalStrategy::AttachXmppProxy( scoped_refptr<XmppProxy> xmpp_proxy) { + DCHECK(CalledOnValidThread()); xmpp_proxy_ = xmpp_proxy; xmpp_proxy_->AttachCallback(AsWeakPtr()); } @@ -54,45 +53,47 @@ void JavascriptSignalStrategy::Close() { } } -void JavascriptSignalStrategy::SetListener(Listener* listener) { +void JavascriptSignalStrategy::AddListener(Listener* listener) { DCHECK(CalledOnValidThread()); - - // Don't overwrite an listener without explicitly going - // through "NULL" first. - DCHECK(listener_ == NULL || listener == NULL); - listener_ = listener; + DCHECK(std::find(listeners_.begin(), listeners_.end(), listener) == + listeners_.end()); + listeners_.push_back(listener); } -void JavascriptSignalStrategy::SendStanza(buzz::XmlElement* stanza) { +void JavascriptSignalStrategy::RemoveListener(Listener* listener) { DCHECK(CalledOnValidThread()); + std::vector<Listener*>::iterator it = + std::find(listeners_.begin(), listeners_.end(), listener); + CHECK(it != listeners_.end()); + listeners_.erase(it); +} +bool JavascriptSignalStrategy::SendStanza(buzz::XmlElement* stanza) { + DCHECK(CalledOnValidThread()); xmpp_proxy_->SendIq(stanza->Str()); delete stanza; + return true; } std::string JavascriptSignalStrategy::GetNextId() { + DCHECK(CalledOnValidThread()); ++last_id_; return base::IntToString(last_id_); } -IqRequest* JavascriptSignalStrategy::CreateIqRequest() { - DCHECK(CalledOnValidThread()); - - return new IqRequest(this, &iq_registry_); -} - void JavascriptSignalStrategy::OnIq(const std::string& stanza_str) { + DCHECK(CalledOnValidThread()); scoped_ptr<buzz::XmlElement> stanza(buzz::XmlElement::ForStr(stanza_str)); - if (!stanza.get()) { LOG(WARNING) << "Malformed XMPP stanza received: " << stanza_str; return; } - if (listener_ && listener_->OnIncomingStanza(stanza.get())) - return; - - iq_registry_.OnIncomingStanza(stanza.get()); + for (std::vector<Listener*>::iterator it = listeners_.begin(); + it != listeners_.end(); ++it) { + if ((*it)->OnIncomingStanza(stanza.get())) + break; + } } } // namespace remoting diff --git a/remoting/jingle_glue/javascript_signal_strategy.h b/remoting/jingle_glue/javascript_signal_strategy.h index 18cb1cb..4c8d991 100644 --- a/remoting/jingle_glue/javascript_signal_strategy.h +++ b/remoting/jingle_glue/javascript_signal_strategy.h @@ -7,11 +7,12 @@ #include "remoting/jingle_glue/signal_strategy.h" +#include <vector> + #include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/threading/non_thread_safe.h" -#include "remoting/jingle_glue/iq_request.h" #include "remoting/jingle_glue/xmpp_proxy.h" namespace remoting { @@ -30,10 +31,10 @@ class JavascriptSignalStrategy : public SignalStrategy, // SignalStrategy interface. virtual void Init(StatusObserver* observer) OVERRIDE; virtual void Close() OVERRIDE; - virtual void SetListener(Listener* listener) OVERRIDE; - virtual void SendStanza(buzz::XmlElement* stanza) OVERRIDE; + virtual void AddListener(Listener* listener) OVERRIDE; + virtual void RemoveListener(Listener* listener) OVERRIDE; + virtual bool SendStanza(buzz::XmlElement* stanza) OVERRIDE; virtual std::string GetNextId() OVERRIDE; - virtual IqRequest* CreateIqRequest() OVERRIDE; // XmppProxy::ResponseCallback interface. virtual void OnIq(const std::string& stanza); @@ -41,9 +42,8 @@ class JavascriptSignalStrategy : public SignalStrategy, private: std::string your_jid_; scoped_refptr<XmppProxy> xmpp_proxy_; - IqRegistry iq_registry_; - Listener* listener_; + std::vector<Listener*> listeners_; int last_id_; diff --git a/remoting/jingle_glue/jingle_info_request.cc b/remoting/jingle_glue/jingle_info_request.cc index 148a65f..e08dbf9 100644 --- a/remoting/jingle_glue/jingle_info_request.cc +++ b/remoting/jingle_glue/jingle_info_request.cc @@ -10,18 +10,15 @@ #include "base/stl_util.h" #include "base/string_number_conversions.h" #include "net/base/net_util.h" -#include "remoting/jingle_glue/iq_request.h" +#include "remoting/jingle_glue/iq_sender.h" #include "third_party/libjingle/source/talk/base/socketaddress.h" #include "third_party/libjingle/source/talk/xmllite/xmlelement.h" #include "third_party/libjingle/source/talk/xmpp/constants.h" namespace remoting { - -JingleInfoRequest::JingleInfoRequest(IqRequest* request) - : request_(request) { - request_->set_callback(base::Bind(&JingleInfoRequest::OnResponse, - base::Unretained(this))); +JingleInfoRequest::JingleInfoRequest(SignalStrategy* signal_strategy) + : iq_sender_(signal_strategy) { } JingleInfoRequest::~JingleInfoRequest() { @@ -29,9 +26,11 @@ JingleInfoRequest::~JingleInfoRequest() { void JingleInfoRequest::Send(const OnJingleInfoCallback& callback) { on_jingle_info_cb_ = callback; - request_->SendIq(IqRequest::MakeIqStanza( - buzz::STR_GET, buzz::STR_EMPTY, - new buzz::XmlElement(buzz::QN_JINGLE_INFO_QUERY, true))); + buzz::XmlElement* iq_body = + new buzz::XmlElement(buzz::QN_JINGLE_INFO_QUERY, true); + request_.reset(iq_sender_.SendIq( + buzz::STR_GET, buzz::STR_EMPTY, iq_body, + base::Bind(&JingleInfoRequest::OnResponse, base::Unretained(this)))); } void JingleInfoRequest::OnResponse(const buzz::XmlElement* stanza) { diff --git a/remoting/jingle_glue/jingle_info_request.h b/remoting/jingle_glue/jingle_info_request.h index 6d5bcf6..9843586 100644 --- a/remoting/jingle_glue/jingle_info_request.h +++ b/remoting/jingle_glue/jingle_info_request.h @@ -12,10 +12,9 @@ #include "base/basictypes.h" #include "base/callback.h" #include "base/memory/scoped_ptr.h" +#include "remoting/jingle_glue/iq_sender.h" #include "third_party/libjingle/source/talk/base/sigslot.h" -class Task; - namespace buzz { class XmlElement; } // namespace buzz @@ -26,9 +25,9 @@ class SocketAddress; namespace remoting { -class IqRequest; +class SignalStrategy; -// JingleInfoRequest handles requesting STUN/Relay infromation from +// JingleInfoRequest handles requesting STUN/Relay information from // the Google Talk network. The query is made when Send() is // called. The callback given to Send() is called when response to the // request is received. @@ -39,7 +38,7 @@ class IqRequest; // TODO(ajwong): Add support for a timeout. class JingleInfoRequest : public sigslot::has_slots<> { public: - // Callback to receive the Jingle configuration settings. The argumetns are + // Callback to receive the Jingle configuration settings. The arguments are // passed by pointer so the receive may call swap on them. The receiver does // NOT own the arguments, which are guaranteed only to be alive for the // duration of the callback. @@ -47,7 +46,7 @@ class JingleInfoRequest : public sigslot::has_slots<> { const std::string&, const std::vector<std::string>&, const std::vector<talk_base::SocketAddress>&)> OnJingleInfoCallback; - explicit JingleInfoRequest(IqRequest* request); + explicit JingleInfoRequest(SignalStrategy* signal_strategy); virtual ~JingleInfoRequest(); void Send(const OnJingleInfoCallback& callback); @@ -57,6 +56,7 @@ class JingleInfoRequest : public sigslot::has_slots<> { void OnResponse(const buzz::XmlElement* stanza); + IqSender iq_sender_; scoped_ptr<IqRequest> request_; OnJingleInfoCallback on_jingle_info_cb_; diff --git a/remoting/jingle_glue/jingle_signaling_connector.cc b/remoting/jingle_glue/jingle_signaling_connector.cc index 6f8a7cf..2cfe3da 100644 --- a/remoting/jingle_glue/jingle_signaling_connector.cc +++ b/remoting/jingle_glue/jingle_signaling_connector.cc @@ -6,7 +6,6 @@ #include "base/logging.h" #include "base/stl_util.h" -#include "remoting/jingle_glue/iq_request.h" #include "third_party/libjingle/source/talk/p2p/base/sessionmanager.h" #include "third_party/libjingle/source/talk/xmpp/constants.h" #include "third_party/libjingle/source/talk/xmpp/xmppclient.h" @@ -36,7 +35,7 @@ JingleSignalingConnector::JingleSignalingConnector( session_manager_->SignalOutgoingMessage.connect( this, &JingleSignalingConnector::OnOutgoingMessage); - signal_strategy_->SetListener(this); + signal_strategy_->AddListener(this); // Assume that signaling is ready from the beginning. session_manager_->SignalRequestSignaling.connect( @@ -44,7 +43,7 @@ JingleSignalingConnector::JingleSignalingConnector( } JingleSignalingConnector::~JingleSignalingConnector() { - signal_strategy_->SetListener(NULL); + signal_strategy_->RemoveListener(this); STLDeleteContainerPairSecondPointers(pending_requests_.begin(), pending_requests_.end()); } diff --git a/remoting/jingle_glue/mock_objects.cc b/remoting/jingle_glue/mock_objects.cc index a42b93b..d9178fa 100644 --- a/remoting/jingle_glue/mock_objects.cc +++ b/remoting/jingle_glue/mock_objects.cc @@ -9,7 +9,4 @@ namespace remoting { MockSignalStrategy::MockSignalStrategy() { } MockSignalStrategy::~MockSignalStrategy() { } -MockIqRequest::MockIqRequest() { } -MockIqRequest::~MockIqRequest() { } - } // namespace remoting diff --git a/remoting/jingle_glue/mock_objects.h b/remoting/jingle_glue/mock_objects.h index 5563e22..aad9697 100644 --- a/remoting/jingle_glue/mock_objects.h +++ b/remoting/jingle_glue/mock_objects.h @@ -3,7 +3,7 @@ // found in the LICENSE file. #include "base/memory/scoped_ptr.h" -#include "remoting/jingle_glue/iq_request.h" +#include "remoting/jingle_glue/iq_sender.h" #include "remoting/jingle_glue/signal_strategy.h" #include "testing/gmock/include/gmock/gmock.h" @@ -16,36 +16,11 @@ class MockSignalStrategy : public SignalStrategy { MOCK_METHOD1(Init, void(StatusObserver*)); MOCK_METHOD0(Close, void()); - MOCK_METHOD1(SetListener, void(Listener* listener)); - MOCK_METHOD1(SendStanza, void(buzz::XmlElement* stanza)); + MOCK_METHOD1(AddListener, void(Listener* listener)); + MOCK_METHOD1(RemoveListener, void(Listener* listener)); + MOCK_METHOD1(SendStanza, bool(buzz::XmlElement* stanza)); MOCK_METHOD0(GetNextId, std::string()); MOCK_METHOD0(CreateIqRequest, IqRequest*()); }; -class MockIqRequest : public IqRequest { - public: - MockIqRequest(); - virtual ~MockIqRequest(); - - MOCK_METHOD1(SendIq, void(buzz::XmlElement* stanza)); - MOCK_METHOD1(set_callback, void(const IqRequest::ReplyCallback&)); - - // Ensure this takes ownership of the pointer, as the real IqRequest object - // would, to avoid memory-leak. - void set_callback_hook(const IqRequest::ReplyCallback& callback) { - callback_ = callback; - } - - void Init() { - ON_CALL(*this, set_callback(testing::_)) - .WillByDefault(testing::Invoke( - this, &MockIqRequest::set_callback_hook)); - } - - ReplyCallback& callback() { return callback_; } - - private: - ReplyCallback callback_; -}; - } // namespace remoting diff --git a/remoting/jingle_glue/signal_strategy.h b/remoting/jingle_glue/signal_strategy.h index 1f90d03..c4bfabe 100644 --- a/remoting/jingle_glue/signal_strategy.h +++ b/remoting/jingle_glue/signal_strategy.h @@ -15,8 +15,6 @@ class XmlElement; namespace remoting { -class IqRequest; - class SignalStrategy { public: class StatusObserver { @@ -44,23 +42,20 @@ class SignalStrategy { virtual void Init(StatusObserver* observer) = 0; virtual void Close() = 0; - // Set a listener that can listen to all incoming messages. Doesn't - // take ownership of the |listener|. Can be called with |listener| - // set to NULL to unset current listener. It must be unset before - // object is destroyed. - virtual void SetListener(Listener* listener) = 0; + // Add a |listener| that can listen to all incoming + // messages. Doesn't take ownership of the |listener|. + virtual void AddListener(Listener* listener) = 0; + + // Remove a |listener| previously added with AddListener(). + virtual void RemoveListener(Listener* listener) = 0; // Sends a raw XMPP stanza. Takes ownership of the |stanza|. - virtual void SendStanza(buzz::XmlElement* stanza) = 0; + virtual bool SendStanza(buzz::XmlElement* stanza) = 0; // Returns new ID that should be used for the next outgoing IQ // request. virtual std::string GetNextId() = 0; - // TODO(sergeyu): Do we really need this method to be part of this - // interface? - virtual IqRequest* CreateIqRequest() = 0; - private: DISALLOW_COPY_AND_ASSIGN(SignalStrategy); }; diff --git a/remoting/jingle_glue/xmpp_signal_strategy.cc b/remoting/jingle_glue/xmpp_signal_strategy.cc index f68a885..f85569f 100644 --- a/remoting/jingle_glue/xmpp_signal_strategy.cc +++ b/remoting/jingle_glue/xmpp_signal_strategy.cc @@ -6,7 +6,6 @@ #include "base/logging.h" #include "jingle/notifier/base/gaia_token_pre_xmpp_auth.h" -#include "remoting/jingle_glue/iq_request.h" #include "remoting/jingle_glue/jingle_thread.h" #include "remoting/jingle_glue/xmpp_socket_adapter.h" #include "third_party/libjingle/source/talk/base/asyncsocket.h" @@ -24,12 +23,11 @@ XmppSignalStrategy::XmppSignalStrategy(JingleThread* jingle_thread, auth_token_(auth_token), auth_token_service_(auth_token_service), xmpp_client_(NULL), - observer_(NULL), - listener_(NULL) { + observer_(NULL) { } XmppSignalStrategy::~XmppSignalStrategy() { - DCHECK(listener_ == NULL); + DCHECK(listeners_.empty()); Close(); } @@ -69,20 +67,29 @@ void XmppSignalStrategy::Close() { } } -void XmppSignalStrategy::SetListener(Listener* listener) { - // Don't overwrite an listener without explicitly going - // through "NULL" first. - DCHECK(listener_ == NULL || listener == NULL); - listener_ = listener; +void XmppSignalStrategy::AddListener(Listener* listener) { + DCHECK(std::find(listeners_.begin(), listeners_.end(), listener) == + listeners_.end()); + listeners_.push_back(listener); } -void XmppSignalStrategy::SendStanza(buzz::XmlElement* stanza) { +void XmppSignalStrategy::RemoveListener(Listener* listener) { + std::vector<Listener*>::iterator it = + std::find(listeners_.begin(), listeners_.end(), listener); + CHECK(it != listeners_.end()); + listeners_.erase(it); +} + +bool XmppSignalStrategy::SendStanza(buzz::XmlElement* stanza) { if (!xmpp_client_) { LOG(INFO) << "Dropping signalling message because XMPP " "connection has been terminated."; - return; + delete stanza; + return false; } - xmpp_client_->SendStanza(stanza); + + buzz::XmppReturnStatus status = xmpp_client_->SendStanza(stanza); + return status == buzz::XMPP_RETURN_OK || status == buzz::XMPP_RETURN_PENDING; } std::string XmppSignalStrategy::GetNextId() { @@ -94,14 +101,13 @@ std::string XmppSignalStrategy::GetNextId() { return xmpp_client_->NextId(); } -IqRequest* XmppSignalStrategy::CreateIqRequest() { - return new IqRequest(this, &iq_registry_); -} - bool XmppSignalStrategy::HandleStanza(const buzz::XmlElement* stanza) { - if (listener_ && listener_->OnIncomingStanza(stanza)) - return true; - return iq_registry_.OnIncomingStanza(stanza); + for (std::vector<Listener*>::iterator it = listeners_.begin(); + it != listeners_.end(); ++it) { + if ((*it)->OnIncomingStanza(stanza)) + return true; + } + return false; } void XmppSignalStrategy::OnConnectionStateChanged( diff --git a/remoting/jingle_glue/xmpp_signal_strategy.h b/remoting/jingle_glue/xmpp_signal_strategy.h index 41f6461..81426fe 100644 --- a/remoting/jingle_glue/xmpp_signal_strategy.h +++ b/remoting/jingle_glue/xmpp_signal_strategy.h @@ -12,8 +12,9 @@ #include "remoting/jingle_glue/signal_strategy.h" +#include <vector> + #include "base/compiler_specific.h" -#include "remoting/jingle_glue/iq_request.h" #include "third_party/libjingle/source/talk/base/sigslot.h" #include "third_party/libjingle/source/talk/xmpp/xmppclient.h" @@ -34,10 +35,10 @@ class XmppSignalStrategy : public SignalStrategy, // SignalStrategy interface. virtual void Init(StatusObserver* observer) OVERRIDE; virtual void Close() OVERRIDE; - virtual void SetListener(Listener* listener) OVERRIDE; - virtual void SendStanza(buzz::XmlElement* stanza) OVERRIDE; + virtual void AddListener(Listener* listener) OVERRIDE; + virtual void RemoveListener(Listener* listener) OVERRIDE; + virtual bool SendStanza(buzz::XmlElement* stanza) OVERRIDE; virtual std::string GetNextId() OVERRIDE; - virtual IqRequest* CreateIqRequest() OVERRIDE; // buzz::XmppStanzaHandler interface. virtual bool HandleStanza(const buzz::XmlElement* stanza) OVERRIDE; @@ -53,10 +54,9 @@ class XmppSignalStrategy : public SignalStrategy, std::string auth_token_; std::string auth_token_service_; buzz::XmppClient* xmpp_client_; - IqRegistry iq_registry_; StatusObserver* observer_; - Listener* listener_; + std::vector<Listener*> listeners_; DISALLOW_COPY_AND_ASSIGN(XmppSignalStrategy); |