Skip unknown channel configurations when parsing session config.

remoting/protocol/content_description.cc

Index: remoting/protocol/content_description.cc
diff --git a/remoting/protocol/content_description.cc b/remoting/protocol/content_description.cc
index 0cd7ce441d56bf781ebc87a72f63f090341e3da9..cd136a5efbda2887e17948ec14cff0ba71780db2 100644
--- a/remoting/protocol/content_description.cc
+++ b/remoting/protocol/content_description.cc
@@ -235,9 +235,9 @@ bool ContentDescription::ParseChannelConfigs(
   const XmlElement* child = element->FirstNamed(tag);
   while (child) {
     ChannelConfig channel_config;
-    if (!ParseChannelConfig(child, codec_required, &channel_config))
-      return false;
-    configs->push_back(channel_config);
+    if (ParseChannelConfig(child, codec_required, &channel_config)) {
+      configs->push_back(channel_config);
+    }
     child = child->NextNamed(tag);
   }
   if (optional && configs->empty()) {
@@ -251,39 +251,34 @@ bool ContentDescription::ParseChannelConfigs(
 }
 
 // static
-ContentDescription* ContentDescription::ParseXml(
+scoped_ptr<ContentDescription> ContentDescription::ParseXml(
     const XmlElement* element) {
-  if (element->Name() == QName(kChromotingXmlNamespace, kDescriptionTag)) {
-    scoped_ptr<CandidateSessionConfig> config(
-        CandidateSessionConfig::CreateEmpty());
-    const XmlElement* child = NULL;
-
-    if (!ParseChannelConfigs(element, kControlTag, false, false,
-                             config->mutable_control_configs())) {
-      return NULL;
-    }
-    if (!ParseChannelConfigs(element, kEventTag, false, false,
-                             config->mutable_event_configs())) {
-      return NULL;
-    }
-    if (!ParseChannelConfigs(element, kVideoTag, true, false,
-                             config->mutable_video_configs())) {
-      return NULL;
-    }
-    if (!ParseChannelConfigs(element, kAudioTag, true, true,
-                             config->mutable_audio_configs())) {
-      return NULL;
-    }
+  if (element->Name() != QName(kChromotingXmlNamespace, kDescriptionTag)) {
+    LOG(ERROR) << "Invalid description: " << element->Str();
+    return scoped_ptr<ContentDescription>();
+  }
+  scoped_ptr<CandidateSessionConfig> config(
+      CandidateSessionConfig::CreateEmpty());
+  const XmlElement* child = NULL;

[simonmorris, 2012/07/26 23:08:51]

You could move the declaration of child down to where it's initialized.

[Sergey Ulanov, 2012/07/26 23:34:13]

Done.

+
+  if (!ParseChannelConfigs(element, kControlTag, false, false,
+                           config->mutable_control_configs()) ||
+      !ParseChannelConfigs(element, kEventTag, false, false,
+                           config->mutable_event_configs()) ||
+      !ParseChannelConfigs(element, kVideoTag, true, false,
+                           config->mutable_video_configs()) ||
+      !ParseChannelConfigs(element, kAudioTag, true, true,
+                           config->mutable_audio_configs())) {
+    return scoped_ptr<ContentDescription>(NULL);

[simonmorris, 2012/07/26 23:08:51]

Remove NULL here (or add NULL to the initialization of the value you return in
the earlier error case), just for consistency.

[Sergey Ulanov, 2012/07/26 23:34:13]

Done.

+  }
 
-    scoped_ptr<XmlElement> authenticator_message;
-    child = Authenticator::FindAuthenticatorMessage(element);
-    if (child)
-      authenticator_message.reset(new XmlElement(*child));
+  scoped_ptr<XmlElement> authenticator_message;
+  child = Authenticator::FindAuthenticatorMessage(element);
+  if (child)
+    authenticator_message.reset(new XmlElement(*child));
 
-    return new ContentDescription(config.Pass(), authenticator_message.Pass());
-  }
-  LOG(ERROR) << "Invalid description: " << element->Str();
-  return NULL;
+  return scoped_ptr<ContentDescription>(
+      new ContentDescription(config.Pass(), authenticator_message.Pass()));
 }
 
 }  // namespace protocol