Refactor IqRequest.

remoting/jingle_glue/iq_sender.cc

Index: remoting/jingle_glue/iq_sender.cc
diff --git a/remoting/jingle_glue/iq_sender.cc b/remoting/jingle_glue/iq_sender.cc
new file mode 100644
index 0000000000000000000000000000000000000000..242869274dd2871bcb865b70db95a3fe30bcf8d1
--- /dev/null
+++ b/remoting/jingle_glue/iq_sender.cc
@@ -0,0 +1,101 @@
+// 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);
+  requests_[id] = request;
+  return request;
+}
+
+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;
+  }
+
+  it->second->OnResponse(stanza);

[Wez, 2011/11/03 02:09:30]

OnResponse() runs an arbitrary callback that was associated with the IqRequest;
what guarantee do we have that that doesn't tear down the IqSender?

[Sergey Ulanov, 2011/11/03 02:41:31]

Good catch. Fixed.

+  requests_.erase(it);
+  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) {
+  if (!callback_.is_null()) {
+    callback_.Run(stanza);
+    callback_.Reset();
+  } else {
+    VLOG(1) << "No callback, so dropping: " << stanza->Str();

[Wez, 2011/11/03 02:09:30]

Rather than checking here, why even enter the request into the map if it has no
callback?

[Sergey Ulanov, 2011/11/03 02:41:31]

Done.

+  }
+}
+
+}  // namespace remoting