Parse termination reason and propagate the error to the Session interface.

remoting/protocol/jingle_messages.cc

Index: remoting/protocol/jingle_messages.cc
diff --git a/remoting/protocol/jingle_messages.cc b/remoting/protocol/jingle_messages.cc
index a7c71c3f3b1e7e8a614bdba3e88477229b654550..3e1cd17485013248a4a03e2056e12f0de060d648 100644
--- a/remoting/protocol/jingle_messages.cc
+++ b/remoting/protocol/jingle_messages.cc
@@ -26,14 +26,48 @@ namespace {
 const char kEmptyNamespace[] = "";
 const char kXmlNamespace[] = "http://www.w3.org/XML/1998/namespace";
 
-const char kSessionInitiateAction[] = "session-initiate";
-const char kSessionAcceptAction[] = "session-accept";
-const char kSessionTerminateAction[] = "session-terminate";
-const char kTransportInfoAction[] = "transport-info";
-
 const int kPortMin = 1000;
 const int kPortMax = 65535;
 
+template <typename T>
+struct NameMapElement {
+  const T value;
+  const char* const name;
+};
+
+template <typename T>
+const char* const ValueToName(const NameMapElement<T> map[], size_t map_size,
+                              T value) {
+  for (size_t i = 0; i < map_size; ++i) {
+    if (map[i].value == value)
+      return map[i].name;
+  }
+  return NULL;
+}
+
+template <typename T>
+T const NameToValue(const NameMapElement<T> map[], size_t map_size,
+                    const std::string& name, T default_value) {
+  for (size_t i = 0; i < map_size; ++i) {
+    if (map[i].name == name)

[simonmorris, 2011/09/27 01:26:10]

Is this doing a pointer comparison instead of a string
comparison? A unit test would make sure.

[Sergey Ulanov, 2011/09/27 03:16:18]

No, |name| is a std::string, so it compares string content. There are unittests
for this file and they would fail if this function wasn't working correctly.

+      return map[i].value;
+  }
+  return default_value;
+}
+
+static const NameMapElement<JingleMessage::ActionType> kActionTypes[] = {
+  { JingleMessage::SESSION_INITIATE, "session-initiate" },
+  { JingleMessage::SESSION_ACCEPT, "session-accept" },
+  { JingleMessage::SESSION_TERMINATE, "session-terminate" },
+  { JingleMessage::TRANSPORT_INFO, "transport-info" },
+};
+
+static const NameMapElement<JingleMessage::Reason> kReasons[] = {
+  { JingleMessage::SUCCESS, "success" },
+  { JingleMessage::DECLINE, "decline" },
+  { JingleMessage::INCOMPATIBLE_PARAMETERS, "incompatible-parameters" },
+};
+
 bool ParseCandidate(const buzz::XmlElement* element,
                     cricket::Candidate* candidate) {
   DCHECK(element->Name() == QName(kP2PTransportNamespace, "candidate"));
@@ -104,7 +138,7 @@ bool JingleMessage::IsJingleMessage(const buzz::XmlElement* stanza) {
 
 JingleMessage::JingleMessage()
     : action(UNKNOWN_ACTION),
-      termination_reason(kJingleNamespace, "success") {
+      reason(UNKNOWN_REASON) {
 }
 
 JingleMessage::JingleMessage(
@@ -135,15 +169,10 @@ bool JingleMessage::ParseXml(const buzz::XmlElement* stanza,
   if (action_str.empty()) {
     *error = "action attribute is missing";
     return false;
-  } else if (action_str == kSessionInitiateAction) {
-    action = SESSION_INITIATE;
-  } else if (action_str == kSessionAcceptAction) {
-    action = SESSION_ACCEPT;
-  } else if (action_str == kSessionTerminateAction) {
-    action = SESSION_TERMINATE;
-  } else if (action_str == kTransportInfoAction) {
-    action = TRANSPORT_INFO;
-  } else {
+  }
+  action = NameToValue(
+      kActionTypes, arraysize(kActionTypes), action_str, UNKNOWN_ACTION);
+  if (action == UNKNOWN_ACTION) {
     *error = "Unknown action " + action_str;
     return false;
   }
@@ -154,14 +183,17 @@ bool JingleMessage::ParseXml(const buzz::XmlElement* stanza,
     return false;
   }
 
-  if (action == SESSION_TERMINATE) {
-    const XmlElement* reason_tag =
-        jingle_tag->FirstNamed(QName(kJingleNamespace, "reason"));
-    if (reason_tag && reason_tag->FirstElement())
-      termination_reason = reason_tag->FirstElement()->Name();
-    return true;
+  const XmlElement* reason_tag =
+      jingle_tag->FirstNamed(QName(kJingleNamespace, "reason"));
+  if (reason_tag && reason_tag->FirstElement()) {
+    reason = NameToValue(
+        kReasons, arraysize(kReasons),
+        reason_tag->FirstElement()->Name().LocalPart(), UNKNOWN_REASON);
   }
 
+  if (action == SESSION_TERMINATE)
+    return true;
+
   const XmlElement* content_tag =
       jingle_tag->FirstNamed(QName(kJingleNamespace, "content"));
   if (!content_tag) {
@@ -227,35 +259,27 @@ buzz::XmlElement* JingleMessage::ToXml() {
   root->AddElement(jingle_tag);
   jingle_tag->AddAttr(QName(kEmptyNamespace, "sid"), sid);
 
-  std::string action_attr;
-  switch (action) {
-    case SESSION_INITIATE:
-      action_attr = kSessionInitiateAction;
-      break;
-    case SESSION_ACCEPT:
-      action_attr = kSessionAcceptAction;
-      break;
-    case SESSION_TERMINATE:
-      action_attr = kSessionTerminateAction;
-      break;
-    case TRANSPORT_INFO:
-      action_attr = kTransportInfoAction;
-      break;
-    default:
-      NOTREACHED();
-      break;
-  }
+  const char* action_attr = ValueToName(
+      kActionTypes, arraysize(kActionTypes), action);
+  if (!action_attr)
+    LOG(FATAL) << "Invalid action value " << action;
   jingle_tag->AddAttr(QName(kEmptyNamespace, "action"), action_attr);
 
   if (action == SESSION_INITIATE)
     jingle_tag->AddAttr(QName(kEmptyNamespace, "initiator"), from);
 
-  if (action == SESSION_TERMINATE) {
+  if (reason != UNKNOWN_REASON) {
     XmlElement* reason_tag = new XmlElement(QName(kJingleNamespace, "reason"));
     jingle_tag->AddElement(reason_tag);
+    const char* reason_string =
+        ValueToName(kReasons, arraysize(kReasons), reason);
+    if (reason_string == NULL)
+      LOG(FATAL) << "Invalid reason: " << reason;
+    reason_tag->AddElement(new XmlElement(
+        QName(kJingleNamespace, reason_string)));
+  }
 
-    reason_tag->AddElement(new XmlElement(termination_reason));
-  } else {
+  if (action != SESSION_TERMINATE) {
     XmlElement* content_tag =
         new XmlElement(QName(kJingleNamespace, "content"));
     jingle_tag->AddElement(content_tag);