Add IDs to outgoing Iq stanzas.

BUG=None
TEST=Messages sent from client to host have valid IDs.

Review URL: http://codereview.chromium.org/7276021

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

9 files changed, 40 insertions, 15 deletions

diff --git a/remoting/jingle_glue/javascript_iq_request.cc b/remoting/jingle_glue/javascript_iq_request.cc
index 4867ecf..ec83365 100644
--- a/remoting/jingle_glue/javascript_iq_request.cc
+++ b/remoting/jingle_glue/javascript_iq_request.cc
@@ -12,8 +12,7 @@
 
 namespace remoting {
 
-JavascriptIqRegistry::JavascriptIqRegistry()
-   : current_id_(0) {
+JavascriptIqRegistry::JavascriptIqRegistry() {
 }
 
 JavascriptIqRegistry::~JavascriptIqRegistry() {
@@ -67,13 +66,10 @@ void JavascriptIqRegistry::OnIncomingStanza(const buzz::XmlElement* stanza) {
   }
 }
 
-std::string JavascriptIqRegistry::RegisterRequest(
-    JavascriptIqRequest* request) {
-  ++current_id_;
-  std::string id_as_string = base::IntToString(current_id_);
-
-  requests_[id_as_string] = request;
-  return id_as_string;
+void JavascriptIqRegistry::RegisterRequest(
+    JavascriptIqRequest* request, const std::string& id) {
+  DCHECK(requests_.find(id) == requests_.end());
+  requests_[id] = request;
 }
 
 JavascriptIqRequest::JavascriptIqRequest(SignalStrategy* signal_strategy,
@@ -89,8 +85,9 @@ JavascriptIqRequest::~JavascriptIqRequest() {
 void JavascriptIqRequest::SendIq(const std::string& type,
                                  const std::string& addressee,
                                  buzz::XmlElement* iq_body) {
-  signal_strategy_->SendStanza(
-      MakeIqStanza(type, addressee, iq_body, registry_->RegisterRequest(this)));
+  std::string id = signal_strategy_->GetNextId();
+  registry_->RegisterRequest(this, id);
+  signal_strategy_->SendStanza(MakeIqStanza(type, addressee, iq_body, id));
 }
 
 void JavascriptIqRequest::set_callback(ReplyCallback* callback) {
diff --git a/remoting/jingle_glue/javascript_iq_request.h b/remoting/jingle_glue/javascript_iq_request.h
index 84bc9b2..eedc0d8 100644
--- a/remoting/jingle_glue/javascript_iq_request.h
+++ b/remoting/jingle_glue/javascript_iq_request.h
@@ -26,8 +26,8 @@ class JavascriptIqRegistry {
   // Does not take ownership of stanza.
   void DispatchResponse(buzz::XmlElement* stanza);
 
-  // Registers |request|, returning the request ID used.
-  std::string RegisterRequest(JavascriptIqRequest* request);
+  // Registers |request| with the specified |id|.
+  void RegisterRequest(JavascriptIqRequest* request, const std::string& id);
 
   // Removes all entries in the registry that refer to |request|.  Useful when
   // |request| is about to be destructed.
@@ -42,7 +42,6 @@ class JavascriptIqRegistry {
   typedef std::map<std::string, JavascriptIqRequest*> IqRequestMap;
 
   IqRequestMap requests_;
-  int current_id_;
   JavascriptIqRequest* default_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(JavascriptIqRegistry);
diff --git a/remoting/jingle_glue/javascript_signal_strategy.cc b/remoting/jingle_glue/javascript_signal_strategy.cc
index 6dcceee..a09c30a 100644
--- a/remoting/jingle_glue/javascript_signal_strategy.cc
+++ b/remoting/jingle_glue/javascript_signal_strategy.cc
@@ -7,6 +7,7 @@
 #include <algorithm>
 
 #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"
@@ -15,7 +16,8 @@ namespace remoting {
 
 JavascriptSignalStrategy::JavascriptSignalStrategy(const std::string& your_jid)
     : your_jid_(your_jid),
-      listener_(NULL) {
+      listener_(NULL),
+      last_id_(0) {
 }
 
 JavascriptSignalStrategy::~JavascriptSignalStrategy() {
@@ -67,6 +69,11 @@ void JavascriptSignalStrategy::SendStanza(buzz::XmlElement* stanza) {
   delete stanza;
 }
 
+std::string JavascriptSignalStrategy::GetNextId() {
+  ++last_id_;
+  return base::IntToString(last_id_);
+}
+
 IqRequest* JavascriptSignalStrategy::CreateIqRequest() {
   DCHECK(CalledOnValidThread());
 
diff --git a/remoting/jingle_glue/javascript_signal_strategy.h b/remoting/jingle_glue/javascript_signal_strategy.h
index d8d9768..89ef460 100644
--- a/remoting/jingle_glue/javascript_signal_strategy.h
+++ b/remoting/jingle_glue/javascript_signal_strategy.h
@@ -33,6 +33,7 @@ class JavascriptSignalStrategy : public SignalStrategy,
   virtual void Close() OVERRIDE;
   virtual void SetListener(Listener* listener) OVERRIDE;
   virtual void SendStanza(buzz::XmlElement* stanza) OVERRIDE;
+  virtual std::string GetNextId() OVERRIDE;
   virtual IqRequest* CreateIqRequest() OVERRIDE;
 
   // XmppProxy::ResponseCallback interface.
@@ -45,6 +46,8 @@ class JavascriptSignalStrategy : public SignalStrategy,
 
   Listener* listener_;
 
+  int last_id_;
+
   DISALLOW_COPY_AND_ASSIGN(JavascriptSignalStrategy);
 };
 
diff --git a/remoting/jingle_glue/jingle_signaling_connector.cc b/remoting/jingle_glue/jingle_signaling_connector.cc
index a094569..729ce6d 100644
--- a/remoting/jingle_glue/jingle_signaling_connector.cc
+++ b/remoting/jingle_glue/jingle_signaling_connector.cc
@@ -54,6 +54,13 @@ void JingleSignalingConnector::OnOutgoingMessage(
     const buzz::XmlElement* stanza) {
   DCHECK_EQ(session_manager, session_manager_);
   scoped_ptr<buzz::XmlElement> stanza_copy(new buzz::XmlElement(*stanza));
+
+  // Add ID attribute for Iq stanzas if it is not there.
+  if (stanza_copy->Name() == buzz::QN_IQ &&
+      stanza_copy->Attr(buzz::QN_ID).empty()) {
+    stanza_copy->SetAttr(buzz::QN_ID, signal_strategy_->GetNextId());
+  }
+
   signal_strategy_->SendStanza(stanza_copy.release());
 }
 
diff --git a/remoting/jingle_glue/mock_objects.h b/remoting/jingle_glue/mock_objects.h
index 56ee59f..a6a76e0 100644
--- a/remoting/jingle_glue/mock_objects.h
+++ b/remoting/jingle_glue/mock_objects.h
@@ -17,6 +17,7 @@ class MockSignalStrategy : public SignalStrategy {
   MOCK_METHOD0(Close, void());
   MOCK_METHOD1(SetListener, void(Listener* listener));
   MOCK_METHOD1(SendStanza, void(buzz::XmlElement* stanza));
+  MOCK_METHOD0(GetNextId, std::string());
   MOCK_METHOD0(CreateIqRequest, IqRequest*());
 };
 
diff --git a/remoting/jingle_glue/signal_strategy.h b/remoting/jingle_glue/signal_strategy.h
index d59341e..3d0a559 100644
--- a/remoting/jingle_glue/signal_strategy.h
+++ b/remoting/jingle_glue/signal_strategy.h
@@ -52,6 +52,12 @@ class SignalStrategy {
   // Sends a raw XMPP stanza. Takes ownership of the |stanza|.
   virtual void 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:
diff --git a/remoting/jingle_glue/xmpp_signal_strategy.cc b/remoting/jingle_glue/xmpp_signal_strategy.cc
index 0d09a1c..74260c4 100644
--- a/remoting/jingle_glue/xmpp_signal_strategy.cc
+++ b/remoting/jingle_glue/xmpp_signal_strategy.cc
@@ -79,6 +79,10 @@ void XmppSignalStrategy::SendStanza(buzz::XmlElement* stanza) {
   xmpp_client_->SendStanza(stanza);
 }
 
+std::string XmppSignalStrategy::GetNextId() {
+  return xmpp_client_->NextId();
+}
+
 IqRequest* XmppSignalStrategy::CreateIqRequest() {
   return new XmppIqRequest(thread_->message_loop(), xmpp_client_);
 }
diff --git a/remoting/jingle_glue/xmpp_signal_strategy.h b/remoting/jingle_glue/xmpp_signal_strategy.h
index ae3f825..9378489 100644
--- a/remoting/jingle_glue/xmpp_signal_strategy.h
+++ b/remoting/jingle_glue/xmpp_signal_strategy.h
@@ -35,6 +35,7 @@ class XmppSignalStrategy : public SignalStrategy,
   virtual void Close() OVERRIDE;
   virtual void SetListener(Listener* listener) OVERRIDE;
   virtual void SendStanza(buzz::XmlElement* stanza) OVERRIDE;
+  virtual std::string GetNextId() OVERRIDE;
   virtual IqRequest* CreateIqRequest() OVERRIDE;
 
   // buzz::XmppStanzaHandler interface.