Switch CandidateSession to use lists rather than vectors.

This makes it possible to add candidate configurations at either the
back (least-preferred) or front (most-preferred) of each channel's
candidate list.

This CL also disables VP9, by default, in candidate session
configurations, replaces DisableVideoCodec with 
EnableVideoCodec, and makes the Enable/Disable<foo>Codec
helpers members of CandidateSessionConfig..

Review URL: https://codereview.chromium.org/292093002

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

6 files changed, 46 insertions, 60 deletions

diff --git a/remoting/protocol/connection_to_host.cc b/remoting/protocol/connection_to_host.cc
index d09eaa1..8bb9019 100644
--- a/remoting/protocol/connection_to_host.cc
+++ b/remoting/protocol/connection_to_host.cc
@@ -142,8 +142,9 @@ void ConnectionToHost::OnSessionManagerReady() {
   // After SessionManager is initialized we can try to connect to the host.
   scoped_ptr<CandidateSessionConfig> candidate_config =
       CandidateSessionConfig::CreateDefault();
-  if (!audio_stub_)
-    CandidateSessionConfig::DisableAudioChannel(candidate_config.get());
+  if (!audio_stub_) {
+    candidate_config->DisableAudioChannel();
+  }
 
   session_ = session_manager_->Connect(
       host_jid_, authenticator_.Pass(), candidate_config.Pass());
diff --git a/remoting/protocol/content_description.cc b/remoting/protocol/content_description.cc
index 7a2db7d..6935115 100644
--- a/remoting/protocol/content_description.cc
+++ b/remoting/protocol/content_description.cc
@@ -145,7 +145,7 @@ XmlElement* ContentDescription::ToXml() const {
   XmlElement* root = new XmlElement(
       QName(kChromotingXmlNamespace, kDescriptionTag), true);
 
-  std::vector<ChannelConfig>::const_iterator it;
+  std::list<ChannelConfig>::const_iterator it;
 
   for (it = config()->control_configs().begin();
        it != config()->control_configs().end(); ++it) {
@@ -191,7 +191,7 @@ bool ContentDescription::ParseChannelConfigs(
     const char tag_name[],
     bool codec_required,
     bool optional,
-    std::vector<ChannelConfig>* const configs) {
+    std::list<ChannelConfig>* const configs) {
 
   QName tag(kChromotingXmlNamespace, tag_name);
   const XmlElement* child = element->FirstNamed(tag);
diff --git a/remoting/protocol/content_description.h b/remoting/protocol/content_description.h
index d345507..65115c2 100644
--- a/remoting/protocol/content_description.h
+++ b/remoting/protocol/content_description.h
@@ -55,7 +55,7 @@ class ContentDescription : public cricket::ContentDescription {
                                   const char tag_name[],
                                   bool codec_required,
                                   bool optional,
-                                  std::vector<ChannelConfig>* const configs);
+                                  std::list<ChannelConfig>* const configs);
 };
 
 }  // namespace protocol
diff --git a/remoting/protocol/content_description_unittest.cc b/remoting/protocol/content_description_unittest.cc
index 25aa8b6..4dfc0ee 100644
--- a/remoting/protocol/content_description_unittest.cc
+++ b/remoting/protocol/content_description_unittest.cc
@@ -51,7 +51,7 @@ TEST(ContentDescriptionTest, ParseUnknown) {
       ContentDescription::ParseXml(xml.get()));
   ASSERT_TRUE(parsed.get());
   EXPECT_EQ(1U, parsed->config()->event_configs().size());
-  EXPECT_TRUE(parsed->config()->event_configs()[0] ==
+  EXPECT_TRUE(parsed->config()->event_configs().front() ==
               ChannelConfig(ChannelConfig::TRANSPORT_STREAM,
                             kDefaultStreamVersion,
                             ChannelConfig::CODEC_UNDEFINED));
@@ -74,7 +74,7 @@ TEST(ContentDescriptionTest, NoneTransport) {
       ContentDescription::ParseXml(xml.get()));
   ASSERT_TRUE(parsed.get());
   EXPECT_EQ(1U, parsed->config()->audio_configs().size());
-  EXPECT_TRUE(parsed->config()->audio_configs()[0] == ChannelConfig());
+  EXPECT_TRUE(parsed->config()->audio_configs().front() == ChannelConfig());
 }
 
 // Verify that we can parse configs with none transport with version and
@@ -94,7 +94,7 @@ TEST(ContentDescriptionTest, NoneTransportWithCodec) {
       ContentDescription::ParseXml(xml.get()));
   ASSERT_TRUE(parsed.get());
   EXPECT_EQ(1U, parsed->config()->audio_configs().size());
-  EXPECT_TRUE(parsed->config()->audio_configs()[0] == ChannelConfig());
+  EXPECT_TRUE(parsed->config()->audio_configs().front() == ChannelConfig());
 }
 
 }  // namespace protocol
diff --git a/remoting/protocol/session_config.cc b/remoting/protocol/session_config.cc
index bee25d0..3ce7727 100644
--- a/remoting/protocol/session_config.cc
+++ b/remoting/protocol/session_config.cc
@@ -129,12 +129,12 @@ bool CandidateSessionConfig::GetFinalConfig(SessionConfig* result) const {
 
 // static
 bool CandidateSessionConfig::SelectCommonChannelConfig(
-    const std::vector<ChannelConfig>& host_configs,
-    const std::vector<ChannelConfig>& client_configs,
+    const std::list<ChannelConfig>& host_configs,
+    const std::list<ChannelConfig>& client_configs,
     ChannelConfig* config) {
   // Usually each of these vectors will contain just several elements,
   // so iterating over all of them is not a problem.
-  std::vector<ChannelConfig>::const_iterator it;
+  std::list<ChannelConfig>::const_iterator it;
   for (it = client_configs.begin(); it != client_configs.end(); ++it) {
     if (IsChannelConfigSupported(host_configs, *it)) {
       *config = *it;
@@ -146,7 +146,7 @@ bool CandidateSessionConfig::SelectCommonChannelConfig(
 
 // static
 bool CandidateSessionConfig::IsChannelConfigSupported(
-    const std::vector<ChannelConfig>& vector,
+    const std::list<ChannelConfig>& vector,
     const ChannelConfig& value) {
   return std::find(vector.begin(), vector.end(), value) != vector.end();
 }
@@ -195,10 +195,6 @@ scoped_ptr<CandidateSessionConfig> CandidateSessionConfig::CreateDefault() {
   result->mutable_video_configs()->push_back(
       ChannelConfig(ChannelConfig::TRANSPORT_STREAM,
                     kDefaultStreamVersion,
-                    ChannelConfig::CODEC_VP9));
-  result->mutable_video_configs()->push_back(
-      ChannelConfig(ChannelConfig::TRANSPORT_STREAM,
-                    kDefaultStreamVersion,
                     ChannelConfig::CODEC_VP8));
 
   // Audio channel.
@@ -211,26 +207,16 @@ scoped_ptr<CandidateSessionConfig> CandidateSessionConfig::CreateDefault() {
   return result.Pass();
 }
 
-// static
-void CandidateSessionConfig::DisableAudioChannel(
-    CandidateSessionConfig* config) {
-  config->mutable_audio_configs()->clear();
-  config->mutable_audio_configs()->push_back(ChannelConfig());
+void CandidateSessionConfig::DisableAudioChannel() {
+  mutable_audio_configs()->clear();
+  mutable_audio_configs()->push_back(ChannelConfig());
 }
 
-// static
-void CandidateSessionConfig::DisableVideoCodec(
-    CandidateSessionConfig* config,
-    ChannelConfig::Codec codec) {
-  std ::vector<ChannelConfig>::iterator i;
-  for (i = config->mutable_video_configs()->begin();
-       i != config->mutable_video_configs()->end();) {
-    if (i->codec == codec) {
-      i = config->mutable_video_configs()->erase(i);
-    } else {
-      ++i;
-    }
-  }
+void CandidateSessionConfig::EnableVideoCodec(ChannelConfig::Codec codec) {
+  mutable_video_configs()->push_front(
+      ChannelConfig(ChannelConfig::TRANSPORT_STREAM,
+                    kDefaultStreamVersion,
+                    codec));
 }
 
 }  // namespace protocol
diff --git a/remoting/protocol/session_config.h b/remoting/protocol/session_config.h
index b3fa90a..ae1ab5e 100644
--- a/remoting/protocol/session_config.h
+++ b/remoting/protocol/session_config.h
@@ -5,8 +5,8 @@
 #ifndef REMOTING_PROTOCOL_SESSION_CONFIG_H_
 #define REMOTING_PROTOCOL_SESSION_CONFIG_H_
 
+#include <list>
 #include <string>
-#include <vector>
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
@@ -48,7 +48,7 @@ struct ChannelConfig {
   ChannelConfig(TransportType transport, int version, Codec codec);
 
   // operator== is overloaded so that std::find() works with
-  // std::vector<ChannelConfig>.
+  // std::list<ChannelConfig>.
   bool operator==(const ChannelConfig& b) const;
 
   TransportType transport;
@@ -101,37 +101,42 @@ class SessionConfig {
 // because it allows one to specify multiple configurations for each channel.
 class CandidateSessionConfig {
  public:
+  static scoped_ptr<CandidateSessionConfig> CreateEmpty();
+  static scoped_ptr<CandidateSessionConfig> CreateFrom(
+      const SessionConfig& config);
+  static scoped_ptr<CandidateSessionConfig> CreateDefault();
+
   ~CandidateSessionConfig();
 
-  const std::vector<ChannelConfig>& control_configs() const {
+  const std::list<ChannelConfig>& control_configs() const {
     return control_configs_;
   }
 
-  std::vector<ChannelConfig>* mutable_control_configs() {
+  std::list<ChannelConfig>* mutable_control_configs() {
     return &control_configs_;
   }
 
-  const std::vector<ChannelConfig>& event_configs() const {
+  const std::list<ChannelConfig>& event_configs() const {
     return event_configs_;
   }
 
-  std::vector<ChannelConfig>* mutable_event_configs() {
+  std::list<ChannelConfig>* mutable_event_configs() {
     return &event_configs_;
   }
 
-  const std::vector<ChannelConfig>& video_configs() const {
+  const std::list<ChannelConfig>& video_configs() const {
     return video_configs_;
   }
 
-  std::vector<ChannelConfig>* mutable_video_configs() {
+  std::list<ChannelConfig>* mutable_video_configs() {
     return &video_configs_;
   }
 
-  const std::vector<ChannelConfig>& audio_configs() const {
+  const std::list<ChannelConfig>& audio_configs() const {
     return audio_configs_;
   }
 
-  std::vector<ChannelConfig>* mutable_audio_configs() {
+  std::list<ChannelConfig>* mutable_audio_configs() {
     return &audio_configs_;
   }
 
@@ -153,15 +158,9 @@ class CandidateSessionConfig {
 
   scoped_ptr<CandidateSessionConfig> Clone() const;
 
-  static scoped_ptr<CandidateSessionConfig> CreateEmpty();
-  static scoped_ptr<CandidateSessionConfig> CreateFrom(
-      const SessionConfig& config);
-  static scoped_ptr<CandidateSessionConfig> CreateDefault();
-
-  // Modifies |config| to disable specific features.
-  static void DisableAudioChannel(CandidateSessionConfig* config);
-  static void DisableVideoCodec(CandidateSessionConfig* config,
-                                ChannelConfig::Codec codec);
+  // Helpers for enabling/disabling specific features.
+  void DisableAudioChannel();
+  void EnableVideoCodec(ChannelConfig::Codec codec);
 
  private:
   CandidateSessionConfig();
@@ -169,16 +168,16 @@ class CandidateSessionConfig {
   CandidateSessionConfig& operator=(const CandidateSessionConfig& b);
 
   static bool SelectCommonChannelConfig(
-      const std::vector<ChannelConfig>& host_configs_,
-      const std::vector<ChannelConfig>& client_configs_,
+      const std::list<ChannelConfig>& host_configs_,
+      const std::list<ChannelConfig>& client_configs_,
       ChannelConfig* config);
-  static bool IsChannelConfigSupported(const std::vector<ChannelConfig>& vector,
+  static bool IsChannelConfigSupported(const std::list<ChannelConfig>& list,
                                        const ChannelConfig& value);
 
-  std::vector<ChannelConfig> control_configs_;
-  std::vector<ChannelConfig> event_configs_;
-  std::vector<ChannelConfig> video_configs_;
-  std::vector<ChannelConfig> audio_configs_;
+  std::list<ChannelConfig> control_configs_;
+  std::list<ChannelConfig> event_configs_;
+  std::list<ChannelConfig> video_configs_;
+  std::list<ChannelConfig> audio_configs_;
 };
 
 }  // namespace protocol