Remove support for GICE in CRD host and client

All current versions of host and client support standard ICE, so we no
longer need to keep GICE compatiblity.

BUG=480711

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

Cr-Commit-Position: refs/heads/master@{#340568}

8 files changed, 50 insertions, 232 deletions

diff --git a/remoting/protocol/jingle_messages.cc b/remoting/protocol/jingle_messages.cc
index 7e4c21e..f727dde 100644
--- a/remoting/protocol/jingle_messages.cc
+++ b/remoting/protocol/jingle_messages.cc
@@ -142,68 +142,6 @@ bool ParseIceTransportInfo(
   return true;
 }
 
-bool ParseGiceCandidate(const buzz::XmlElement* element,
-                        JingleMessage::NamedCandidate* candidate) {
-  DCHECK(element->Name() == QName(kGiceTransportNamespace, "candidate"));
-
-  const std::string& name = element->Attr(QName(kEmptyNamespace, "name"));
-  const std::string& address = element->Attr(QName(kEmptyNamespace, "address"));
-  const std::string& port_str = element->Attr(QName(kEmptyNamespace, "port"));
-  const std::string& type = element->Attr(QName(kEmptyNamespace, "type"));
-  const std::string& protocol =
-      element->Attr(QName(kEmptyNamespace, "protocol"));
-  const std::string& username =
-      element->Attr(QName(kEmptyNamespace, "username"));
-  const std::string& password =
-      element->Attr(QName(kEmptyNamespace, "password"));
-  const std::string& preference_str =
-      element->Attr(QName(kEmptyNamespace, "preference"));
-  const std::string& generation_str =
-      element->Attr(QName(kEmptyNamespace, "generation"));
-
-  int port;
-  double preference;
-  int generation;
-  if (name.empty() || address.empty() || !base::StringToInt(port_str, &port) ||
-      port < kPortMin || port > kPortMax || type.empty() || protocol.empty() ||
-      username.empty() || password.empty() ||
-      !base::StringToDouble(preference_str, &preference) ||
-      !base::StringToInt(generation_str, &generation)) {
-    return false;
-  }
-
-  candidate->name = name;
-
-  candidate->candidate.set_address(rtc::SocketAddress(address, port));
-  candidate->candidate.set_type(type);
-  candidate->candidate.set_protocol(protocol);
-  candidate->candidate.set_username(username);
-  candidate->candidate.set_password(password);
-  candidate->candidate.set_preference(static_cast<float>(preference));
-  candidate->candidate.set_generation(generation);
-
-  return true;
-}
-
-bool ParseGiceTransportInfo(
-    const buzz::XmlElement* element,
-    std::list<JingleMessage::NamedCandidate>* candidates) {
-  DCHECK(element->Name() == QName(kGiceTransportNamespace, "transport"));
-
-  candidates->clear();
-
-  QName qn_candidate(kGiceTransportNamespace, "candidate");
-  for (const XmlElement* candidate_tag = element->FirstNamed(qn_candidate);
-       candidate_tag; candidate_tag = candidate_tag->NextNamed(qn_candidate)) {
-    JingleMessage::NamedCandidate candidate;
-    if (!ParseGiceCandidate(candidate_tag, &candidate))
-      return false;
-    candidates->push_back(candidate);
-  }
-
-  return true;
-}
-
 XmlElement* FormatIceCredentials(
     const JingleMessage::IceCredentials& credentials) {
   XmlElement* result =
@@ -234,29 +172,6 @@ XmlElement* FormatIceCandidate(const JingleMessage::NamedCandidate& candidate) {
   return result;
 }
 
-XmlElement* FormatGiceCandidate(
-    const JingleMessage::NamedCandidate& candidate) {
-  XmlElement* result =
-      new XmlElement(QName(kGiceTransportNamespace, "candidate"));
-  result->SetAttr(QName(kEmptyNamespace, "name"), candidate.name);
-  result->SetAttr(QName(kEmptyNamespace, "address"),
-                  candidate.candidate.address().ipaddr().ToString());
-  result->SetAttr(QName(kEmptyNamespace, "port"),
-                  base::IntToString(candidate.candidate.address().port()));
-  result->SetAttr(QName(kEmptyNamespace, "type"), candidate.candidate.type());
-  result->SetAttr(QName(kEmptyNamespace, "protocol"),
-                  candidate.candidate.protocol());
-  result->SetAttr(QName(kEmptyNamespace, "username"),
-                  candidate.candidate.username());
-  result->SetAttr(QName(kEmptyNamespace, "password"),
-                  candidate.candidate.password());
-  result->SetAttr(QName(kEmptyNamespace, "preference"),
-                  base::DoubleToString(candidate.candidate.preference()));
-  result->SetAttr(QName(kEmptyNamespace, "generation"),
-                  base::IntToString(candidate.candidate.generation()));
-  return result;
-}
-
 }  // namespace
 
 JingleMessage::NamedCandidate::NamedCandidate(
@@ -284,17 +199,14 @@ std::string JingleMessage::GetActionName(ActionType action) {
   return ValueToName(kActionTypes, action);
 }
 
-JingleMessage::JingleMessage() {
-}
+JingleMessage::JingleMessage() {}
 
 JingleMessage::JingleMessage(const std::string& to,
                              ActionType action,
                              const std::string& sid)
-    : to(to), action(action), sid(sid) {
-}
+    : to(to), action(action), sid(sid) {}
 
-JingleMessage::~JingleMessage() {
-}
+JingleMessage::~JingleMessage() {}
 
 bool JingleMessage::ParseXml(const buzz::XmlElement* stanza,
                              std::string* error) {
@@ -384,27 +296,21 @@ bool JingleMessage::ParseXml(const buzz::XmlElement* stanza,
     }
   }
 
-  const XmlElement* ice_transport_tag = content_tag->FirstNamed(
-      QName(kIceTransportNamespace, "transport"));
   const XmlElement* gice_transport_tag = content_tag->FirstNamed(
       QName(kGiceTransportNamespace, "transport"));
-  if (ice_transport_tag && gice_transport_tag) {
-    *error = "ICE and GICE transport information is found in the same message";
+  if (gice_transport_tag) {
+    *error = "Legacy GICE transport information is received.";
     return false;
-  } else if (ice_transport_tag) {
-    standard_ice = true;
+  }
+
+  const XmlElement* ice_transport_tag = content_tag->FirstNamed(
+      QName(kIceTransportNamespace, "transport"));
+  if (ice_transport_tag) {
     if (!ParseIceTransportInfo(ice_transport_tag, &ice_credentials,
                                &candidates)) {
       *error = "Failed to parse transport info";
       return false;
     }
-  } else if (gice_transport_tag) {
-    standard_ice = false;
-    ice_credentials.clear();
-    if (!ParseGiceTransportInfo(gice_transport_tag, &candidates)) {
-      *error = "Failed to parse transport info";
-      return false;
-    }
   }
 
   return true;
@@ -462,26 +368,15 @@ scoped_ptr<buzz::XmlElement> JingleMessage::ToXml() const {
     if (description.get())
       content_tag->AddElement(description->ToXml());
 
-    if (standard_ice) {
+    if (!ice_credentials.empty() || !candidates.empty()) {
       XmlElement* transport_tag =
           new XmlElement(QName(kIceTransportNamespace, "transport"), true);
       content_tag->AddElement(transport_tag);
-      for (std::list<IceCredentials>::const_iterator it =
-               ice_credentials.begin();
-           it != ice_credentials.end(); ++it) {
-        transport_tag->AddElement(FormatIceCredentials(*it));
+      for (const IceCredentials& credentials : ice_credentials) {
+        transport_tag->AddElement(FormatIceCredentials(credentials));
       }
-      for (std::list<NamedCandidate>::const_iterator it = candidates.begin();
-           it != candidates.end(); ++it) {
-        transport_tag->AddElement(FormatIceCandidate(*it));
-      }
-    } else {
-      XmlElement* transport_tag =
-          new XmlElement(QName(kGiceTransportNamespace, "transport"), true);
-      content_tag->AddElement(transport_tag);
-      for (std::list<NamedCandidate>::const_iterator it = candidates.begin();
-           it != candidates.end(); ++it) {
-        transport_tag->AddElement(FormatGiceCandidate(*it));
+      for (const NamedCandidate& candidate : candidates) {
+        transport_tag->AddElement(FormatIceCandidate(candidate));
       }
     }
   }
diff --git a/remoting/protocol/jingle_messages.h b/remoting/protocol/jingle_messages.h
index c83776a..47f9afc 100644
--- a/remoting/protocol/jingle_messages.h
+++ b/remoting/protocol/jingle_messages.h
@@ -81,7 +81,6 @@ struct JingleMessage {
 
   scoped_ptr<ContentDescription> description;
 
-  bool standard_ice = true;
   std::list<IceCredentials> ice_credentials;
   std::list<NamedCandidate> candidates;
 
diff --git a/remoting/protocol/jingle_messages_unittest.cc b/remoting/protocol/jingle_messages_unittest.cc
index aa95d07..90b40ca 100644
--- a/remoting/protocol/jingle_messages_unittest.cc
+++ b/remoting/protocol/jingle_messages_unittest.cc
@@ -115,7 +115,6 @@ TEST(JingleMessageTest, SessionInitiate) {
                 "j7whCMii0Z0AAPwj7whCM/j7whCMii0Z0AAPw="
               "</auth-token></authentication>"
           "</description>"
-          "<transport xmlns='http://www.google.com/transport/p2p'/>"
           "</content>"
         "</jingle>"
       "</iq>";
@@ -156,7 +155,6 @@ TEST(JingleMessageTest, SessionAccept) {
                 "MIICpjCCAY6gW0Cert0TANBgkqhkiG9w0BAQUFA="
               "</certificate></authentication>"
             "</description>"
-            "<transport xmlns='http://www.google.com/transport/p2p'/>"
           "</content>"
         "</jingle>"
       "</cli:iq>";
@@ -197,7 +195,6 @@ TEST(JingleMessageTest, SessionAcceptNoIce) {
                 "MIICpjCCAY6gW0Cert0TANBgkqhkiG9w0BAQUFA="
               "</certificate></authentication>"
             "</description>"
-            "<transport xmlns='http://www.google.com/transport/p2p'/>"
           "</content>"
         "</jingle>"
       "</cli:iq>";
@@ -262,46 +259,6 @@ TEST(JingleMessageTest, IceTransportInfo) {
       << error;
 }
 
-TEST(JingleMessageTest, GiceTransportInfo) {
-  const char* kTestGiceTransportInfoMessage =
-      "<cli:iq to='user@gmail.com/chromoting016DBB07' type='set' "
-              "xmlns:cli='jabber:client'>"
-        "<jingle xmlns='urn:xmpp:jingle:1' action='transport-info' "
-                "sid='2227053353'>"
-          "<content name='chromoting' creator='initiator'>"
-            "<transport  xmlns='http://www.google.com/transport/p2p'>"
-              "<candidate name='event' address='172.23.164.186' port='57040' "
-                        "preference='1' username='tPUyEAmQrEw3y7hi' "
-                        "protocol='udp' generation='0' "
-                        "password='2iRdhLfawKZC5ydJ' type='local'/>"
-              "<candidate name='video' address='172.23.164.186' port='42171' "
-                         "preference='1' username='EPK3CXo5sTLJSez0' "
-                         "protocol='udp' generation='0' "
-                         "password='eM0VUfUkZ+1Pyi0M' type='local'/>"
-            "</transport>"
-          "</content>"
-        "</jingle>"
-      "</cli:iq>";
-
-  scoped_ptr<XmlElement> source_message(
-      XmlElement::ForStr(kTestGiceTransportInfoMessage));
-  ASSERT_TRUE(source_message.get());
-
-  EXPECT_TRUE(JingleMessage::IsJingleMessage(source_message.get()));
-
-  JingleMessage message;
-  std::string error;
-  EXPECT_TRUE(message.ParseXml(source_message.get(), &error)) << error;
-
-  EXPECT_EQ(message.action, JingleMessage::TRANSPORT_INFO);
-  EXPECT_EQ(message.candidates.size(), 2U);
-
-  scoped_ptr<XmlElement> formatted_message(message.ToXml());
-  ASSERT_TRUE(formatted_message.get());
-  EXPECT_TRUE(VerifyXml(source_message.get(), formatted_message.get(), &error))
-      << error;
-}
-
 TEST(JingleMessageTest, SessionTerminate) {
   const char* kTestSessionTerminateMessage =
       "<cli:iq from='user@gmail.com/chromoting016DBB07' "
@@ -441,7 +398,6 @@ TEST(JingleMessageTest, ErrorMessage) {
                 "j7whCMii0Z0AAPwj7whCM/j7whCMii0Z0AAPw="
               "</auth-token></authentication>"
             "</description>"
-            "<transport xmlns='http://www.google.com/transport/p2p'/>"
           "</content>"
         "</jingle>"
         "<error code='501' type='cancel'>"
diff --git a/remoting/protocol/jingle_session.cc b/remoting/protocol/jingle_session.cc
index 1c932d9..ba2faaa 100644
--- a/remoting/protocol/jingle_session.cc
+++ b/remoting/protocol/jingle_session.cc
@@ -264,7 +264,6 @@ void JingleSession::CreateChannel(const std::string& name,
 
   scoped_ptr<Transport> channel =
       session_manager_->transport_factory_->CreateTransport();
-  channel->SetUseStandardIce(config_->standard_ice());
   channel->Connect(name, this, callback);
   AddPendingRemoteTransportInfo(channel.get());
   channels_[name] = channel.release();
@@ -374,8 +373,6 @@ void JingleSession::EnsurePendingTransportInfoMessage() {
   if (!pending_transport_info_message_) {
     pending_transport_info_message_.reset(new JingleMessage(
         peer_jid_, JingleMessage::TRANSPORT_INFO, session_id_));
-    pending_transport_info_message_->standard_ice = config_->standard_ice();
-
     // Delay sending the new candidates in case we get more candidates
     // that we can send in one message.
     transport_info_timer_.Start(
@@ -516,14 +513,6 @@ void JingleSession::OnSessionInfo(const JingleMessage& message,
 }
 
 void JingleSession::ProcessTransportInfo(const JingleMessage& message) {
-  // Check if the transport information version matches what was negotiated.
-  if (message.standard_ice != config_->standard_ice()) {
-    LOG(ERROR) << "Received transport-info message in format different from "
-                  "negotiated.";
-    CloseInternal(INCOMPATIBLE_PROTOCOL);
-    return;
-  }
-
   for (std::list<JingleMessage::IceCredentials>::const_iterator it =
            message.ice_credentials.begin();
        it != message.ice_credentials.end(); ++it) {
diff --git a/remoting/protocol/jingle_session_unittest.cc b/remoting/protocol/jingle_session_unittest.cc
index ed5e03ba..fff9efb 100644
--- a/remoting/protocol/jingle_session_unittest.cc
+++ b/remoting/protocol/jingle_session_unittest.cc
@@ -414,25 +414,30 @@ TEST_F(JingleSessionTest, TestIncompatibleProtocol) {
   EXPECT_FALSE(host_session_);
 }
 
-// Verify that we can still connect using legacy GICE transport.
+// Verify that GICE-only client is rejected with an appropriate error code.
 TEST_F(JingleSessionTest, TestLegacyIceConnection) {
   CreateSessionManagers(1, FakeAuthenticator::ACCEPT);
 
+  EXPECT_CALL(host_server_listener_, OnIncomingSession(_, _)).Times(0);
+
+  EXPECT_CALL(client_session_event_handler_,
+              OnSessionStateChange(Session::FAILED))
+      .Times(1);
+
+  scoped_ptr<Authenticator> authenticator(new FakeAuthenticator(
+      FakeAuthenticator::CLIENT, 1, FakeAuthenticator::ACCEPT, true));
+
   scoped_ptr<CandidateSessionConfig> config =
       CandidateSessionConfig::CreateDefault();
   config->set_standard_ice(false);
-  host_server_->set_protocol_config(config.Pass());
-
-  ASSERT_NO_FATAL_FAILURE(
-      InitiateConnection(1, FakeAuthenticator::ACCEPT, false));
+  client_server_->set_protocol_config(config.Pass());
+  client_session_ = client_server_->Connect(kHostJid, authenticator.Pass());
+  client_session_->SetEventHandler(&client_session_event_handler_);
 
-  ASSERT_NO_FATAL_FAILURE(CreateChannel());
+  base::RunLoop().RunUntilIdle();
 
-  StreamConnectionTester tester(host_socket_.get(), client_socket_.get(),
-                                kMessageSize, kMessages);
-  tester.Start();
-  message_loop_->Run();
-  tester.CheckResults();
+  EXPECT_EQ(INCOMPATIBLE_PROTOCOL, client_session_->error());
+  EXPECT_FALSE(host_session_);
 }
 
 TEST_F(JingleSessionTest, DeleteSessionOnIncomingConnection) {
diff --git a/remoting/protocol/libjingle_transport_factory.cc b/remoting/protocol/libjingle_transport_factory.cc
index 50557fd..8653737 100644
--- a/remoting/protocol/libjingle_transport_factory.cc
+++ b/remoting/protocol/libjingle_transport_factory.cc
@@ -73,7 +73,6 @@ class LibjingleTransport
   void AddRemoteCandidate(const cricket::Candidate& candidate) override;
   const std::string& name() const override;
   bool is_connected() const override;
-  void SetUseStandardIce(bool use_standard_ice) override;
 
  private:
   void DoStart();
@@ -100,8 +99,6 @@ class LibjingleTransport
   NetworkSettings network_settings_;
   TransportRole role_;
 
-  bool use_standard_ice_ = true;
-
   std::string name_;
   EventHandler* event_handler_;
   Transport::ConnectedCallback callback_;
@@ -164,10 +161,6 @@ void LibjingleTransport::OnCanStart() {
   }
 
   while (!pending_candidates_.empty()) {
-    if (!use_standard_ice_) {
-      channel_->SetRemoteIceCredentials(pending_candidates_.front().username(),
-                                        pending_candidates_.front().password());
-    }
     channel_->OnCandidate(pending_candidates_.front());
     pending_candidates_.pop_front();
   }
@@ -199,16 +192,12 @@ void LibjingleTransport::DoStart() {
   channel_.reset(new cricket::P2PTransportChannel(
       std::string(), 0, nullptr, port_allocator_));
   std::string ice_password = rtc::CreateRandomString(cricket::ICE_PWD_LENGTH);
-  if (use_standard_ice_) {
-    channel_->SetIceProtocolType(cricket::ICEPROTO_RFC5245);
-    channel_->SetIceRole((role_ == TransportRole::CLIENT)
-                             ? cricket::ICEROLE_CONTROLLING
-                             : cricket::ICEROLE_CONTROLLED);
-    event_handler_->OnTransportIceCredentials(this, ice_username_fragment_,
-                                              ice_password);
-  } else {
-    channel_->SetIceProtocolType(cricket::ICEPROTO_GOOGLE);
-  }
+  channel_->SetIceProtocolType(cricket::ICEPROTO_RFC5245);
+  channel_->SetIceRole((role_ == TransportRole::CLIENT)
+                           ? cricket::ICEROLE_CONTROLLING
+                           : cricket::ICEROLE_CONTROLLED);
+  event_handler_->OnTransportIceCredentials(this, ice_username_fragment_,
+                                            ice_password);
   channel_->SetIceCredentials(ice_username_fragment_, ice_password);
   channel_->SignalRequestSignaling.connect(
       this, &LibjingleTransport::OnRequestSignaling);
@@ -267,10 +256,6 @@ void LibjingleTransport::AddRemoteCandidate(
     return;
 
   if (channel_) {
-    if (!use_standard_ice_) {
-      channel_->SetRemoteIceCredentials(candidate.username(),
-                                        candidate.password());
-    }
     channel_->OnCandidate(candidate);
   } else {
     pending_candidates_.push_back(candidate);
@@ -287,12 +272,6 @@ bool LibjingleTransport::is_connected() const {
   return callback_.is_null();
 }
 
-void LibjingleTransport::SetUseStandardIce(bool use_standard_ice) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(!channel_);
-  use_standard_ice_ = use_standard_ice;
-}
-
 void LibjingleTransport::OnRequestSignaling(
     cricket::TransportChannelImpl* channel) {
   DCHECK(CalledOnValidThread());
diff --git a/remoting/protocol/session_config.cc b/remoting/protocol/session_config.cc
index e2b0693..8994aa1 100644
--- a/remoting/protocol/session_config.cc
+++ b/remoting/protocol/session_config.cc
@@ -6,6 +6,8 @@
 
 #include <algorithm>
 
+#include "base/logging.h"
+
 namespace remoting {
 namespace protocol {
 
@@ -63,8 +65,11 @@ scoped_ptr<SessionConfig> SessionConfig::SelectCommon(
   ChannelConfig video_config;
   ChannelConfig audio_config;
 
-  result->standard_ice_ =
-      host_config->standard_ice() && client_config->standard_ice();
+  DCHECK(host_config->standard_ice());
+
+  // Reject connection if the peer doesn't support ICE.
+  if (!client_config->standard_ice())
+    return nullptr;
 
   if (!SelectCommonChannelConfig(host_config->control_configs(),
                                  client_config->control_configs(),
@@ -107,7 +112,6 @@ scoped_ptr<SessionConfig> SessionConfig::GetFinalConfig(
 // static
 scoped_ptr<SessionConfig> SessionConfig::ForTest() {
   scoped_ptr<SessionConfig> result(new SessionConfig());
-  result->standard_ice_ = true;
   result->control_config_ = ChannelConfig(ChannelConfig::TRANSPORT_MUX_STREAM,
                                           kControlStreamVersion,
                                           ChannelConfig::CODEC_UNDEFINED);
@@ -133,13 +137,12 @@ CandidateSessionConfig::CandidateSessionConfig(
 
 CandidateSessionConfig::~CandidateSessionConfig() { }
 
-bool CandidateSessionConfig::IsSupported(
-    const SessionConfig& config) const {
-  return
-      IsChannelConfigSupported(control_configs_, config.control_config()) &&
-      IsChannelConfigSupported(event_configs_, config.event_config()) &&
-      IsChannelConfigSupported(video_configs_, config.video_config()) &&
-      IsChannelConfigSupported(audio_configs_, config.audio_config());
+bool CandidateSessionConfig::IsSupported(const SessionConfig& config) const {
+  return config.standard_ice() &&
+         IsChannelConfigSupported(control_configs_, config.control_config()) &&
+         IsChannelConfigSupported(event_configs_, config.event_config()) &&
+         IsChannelConfigSupported(video_configs_, config.video_config()) &&
+         IsChannelConfigSupported(audio_configs_, config.audio_config());
 }
 
 scoped_ptr<CandidateSessionConfig> CandidateSessionConfig::Clone() const {
@@ -167,8 +170,6 @@ scoped_ptr<CandidateSessionConfig> CandidateSessionConfig::CreateFrom(
 scoped_ptr<CandidateSessionConfig> CandidateSessionConfig::CreateDefault() {
   scoped_ptr<CandidateSessionConfig> result = CreateEmpty();
 
-  result->set_standard_ice(true);
-
   // Control channel.
   result->mutable_control_configs()->push_back(
       ChannelConfig(ChannelConfig::TRANSPORT_MUX_STREAM,
diff --git a/remoting/protocol/transport.h b/remoting/protocol/transport.h
index a065e6b..14c3ec3 100644
--- a/remoting/protocol/transport.h
+++ b/remoting/protocol/transport.h
@@ -118,12 +118,6 @@ class Transport : public base::NonThreadSafe {
   // Returns true if the channel is already connected.
   virtual bool is_connected() const = 0;
 
-  // Sets ICE version for the transport.
-  //
-  // TODO(sergeyu): Remove this when support for legacy ICE is removed.
-  // crbug.com/473758
-  virtual void SetUseStandardIce(bool use_standard_ice) {}
-
  private:
   DISALLOW_COPY_AND_ASSIGN(Transport);
 };