Revert of Use standard ICE in Chromoting. (patchset Pissandshittium#7 id:160001 of https://codereview.chromium.org/1085703003/)

Reason for revert:
jingle_unittests failure on Mac ASan 64 Tests (1)

https://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Tests%20%281%29/builds/805

BUG=480107

Original issue's description:
> Use standard ICE in Chromoting.
>
> Previously we were using legacy, non-standard version of ICE. This
> change adds ICE version negotiation and enabled standard ICE by default,
> when both peers support it.
>
> BUG=473758
>
> Committed: https://crrev.com/4b35571a3085aa960dcf5533fb2277d5dcaaf11f
> Cr-Commit-Position: refs/heads/master@{#326435}

TBR=rmsousa@chromium.org,wez@chromium.org,dcaiafa@chromium.org,sergeyu@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=473758

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

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

Author: horo-t

remoting/client/jni/chromoting_jni_instance.cc

@@ -434,8 +434,7 @@ void ChromotingJniInstance::ConnectToHostOnNetworkThread() {
 
   scoped_ptr<protocol::TransportFactory> transport_factory(
       new protocol::LibjingleTransportFactory(
-          signaling_.get(), port_allocator.Pass(), network_settings,
-          protocol::TransportRole::CLIENT));
+          signaling_.get(), port_allocator.Pass(), network_settings));
 
   client_->Start(signaling_.get(), authenticator_.Pass(),
                  transport_factory.Pass(), host_jid_, capabilities_);

remoting/client/plugin/chromoting_instance.cc

@@ -712,10 +712,10 @@ void ChromotingInstance::HandleConnect(const base::DictionaryValue& data) {
   // Create TransportFactory.
   scoped_ptr<protocol::TransportFactory> transport_factory(
       new protocol::LibjingleTransportFactory(
-          signal_strategy_.get(), PepperPortAllocator::Create(this).Pass(),
+          signal_strategy_.get(),
+          PepperPortAllocator::Create(this).Pass(),
           protocol::NetworkSettings(
-              protocol::NetworkSettings::NAT_TRAVERSAL_FULL),
-          protocol::TransportRole::CLIENT));
+              protocol::NetworkSettings::NAT_TRAVERSAL_FULL)));
 
   // Create Authenticator.
   scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>

remoting/host/chromoting_host.cc

@@ -285,17 +285,15 @@ void ChromotingHost::OnIncomingSession(
     return;
   }
 
-  scoped_ptr<protocol::SessionConfig> config =
-      protocol::SessionConfig::SelectCommon(session->candidate_config(),
-                                            protocol_config_.get());
-  if (!config) {
+  protocol::SessionConfig config;
+  if (!protocol_config_->Select(session->candidate_config(), &config)) {
     LOG(WARNING) << "Rejecting connection from " << session->jid()
                  << " because no compatible configuration has been found.";
     *response = protocol::SessionManager::INCOMPATIBLE;
     return;
   }
 
-  session->set_config(config.Pass());
+  session->set_config(config);
 
   *response = protocol::SessionManager::ACCEPT;
 

remoting/host/chromoting_host_unittest.cc

@@ -108,7 +108,9 @@ class ChromotingHostTest : public testing::Test {
     session_jid1_ = "user@domain/rest-of-jid";
     session_config2_ = SessionConfig::ForTest();
     session_jid2_ = "user2@domain/rest-of-jid";
+    session_unowned_config1_ = SessionConfig::ForTest();
     session_unowned_jid1_ = "user3@doman/rest-of-jid";
+    session_unowned_config2_ = SessionConfig::ForTest();
     session_unowned_jid2_ = "user4@doman/rest-of-jid";
 
     EXPECT_CALL(*session1_, jid())
@@ -130,9 +132,9 @@ class ChromotingHostTest : public testing::Test {
         .Times(AnyNumber())
         .WillRepeatedly(SaveArg<0>(&session_unowned2_event_handler_));
     EXPECT_CALL(*session1_, config())
-        .WillRepeatedly(ReturnRef(*session_config1_));
+        .WillRepeatedly(ReturnRef(session_config1_));
     EXPECT_CALL(*session2_, config())
-        .WillRepeatedly(ReturnRef(*session_config2_));
+        .WillRepeatedly(ReturnRef(session_config2_));
 
     owned_connection1_.reset(new MockConnectionToClient(session1_,
                                                         &host_stub1_));
@@ -419,7 +421,7 @@ class ChromotingHostTest : public testing::Test {
   ClientSession* client1_;
   std::string session_jid1_;
   MockSession* session1_;  // Owned by |connection_|.
-  scoped_ptr<SessionConfig> session_config1_;
+  SessionConfig session_config1_;
   MockVideoStub video_stub1_;
   MockClientStub client_stub1_;
   MockHostStub host_stub1_;
@@ -428,13 +430,15 @@ class ChromotingHostTest : public testing::Test {
   ClientSession* client2_;
   std::string session_jid2_;
   MockSession* session2_;  // Owned by |connection2_|.
-  scoped_ptr<SessionConfig> session_config2_;
+  SessionConfig session_config2_;
   MockVideoStub video_stub2_;
   MockClientStub client_stub2_;
   MockHostStub host_stub2_;
   scoped_ptr<MockSession> session_unowned1_;  // Not owned by a connection.
+  SessionConfig session_unowned_config1_;
   std::string session_unowned_jid1_;
   scoped_ptr<MockSession> session_unowned2_;  // Not owned by a connection.
+  SessionConfig session_unowned_config2_;
   std::string session_unowned_jid2_;
   protocol::Session::EventHandler* session_unowned1_event_handler_;
   protocol::Session::EventHandler* session_unowned2_event_handler_;
@@ -595,7 +599,7 @@ TEST_F(ChromotingHostTest, IncomingSessionAccepted) {
   ExpectHostAndSessionManagerStart();
   EXPECT_CALL(*session_unowned1_, candidate_config()).WillOnce(Return(
       default_candidate_config_.get()));
-  EXPECT_CALL(*session_unowned1_, set_config_ptr(_));
+  EXPECT_CALL(*session_unowned1_, set_config(_));
   EXPECT_CALL(*session_unowned1_, Close()).WillOnce(InvokeWithoutArgs(
     this, &ChromotingHostTest::NotifyConnectionClosed1));
   EXPECT_CALL(host_status_observer_, OnAccessDenied(_));
@@ -616,7 +620,7 @@ TEST_F(ChromotingHostTest, LoginBackOffUponConnection) {
   ExpectHostAndSessionManagerStart();
   EXPECT_CALL(*session_unowned1_, candidate_config()).WillOnce(
     Return(default_candidate_config_.get()));
-  EXPECT_CALL(*session_unowned1_, set_config_ptr(_));
+  EXPECT_CALL(*session_unowned1_, set_config(_));
   EXPECT_CALL(*session_unowned1_, Close()).WillOnce(
     InvokeWithoutArgs(this, &ChromotingHostTest::NotifyConnectionClosed1));
   EXPECT_CALL(host_status_observer_, OnAccessDenied(_));
@@ -642,13 +646,13 @@ TEST_F(ChromotingHostTest, LoginBackOffUponAuthenticating) {
   Expectation start = ExpectHostAndSessionManagerStart();
   EXPECT_CALL(*session_unowned1_, candidate_config()).WillOnce(
     Return(default_candidate_config_.get()));
-  EXPECT_CALL(*session_unowned1_, set_config_ptr(_));
+  EXPECT_CALL(*session_unowned1_, set_config(_));
   EXPECT_CALL(*session_unowned1_, Close()).WillOnce(
     InvokeWithoutArgs(this, &ChromotingHostTest::NotifyConnectionClosed1));
 
   EXPECT_CALL(*session_unowned2_, candidate_config()).WillOnce(
     Return(default_candidate_config_.get()));
-  EXPECT_CALL(*session_unowned2_, set_config_ptr(_));
+  EXPECT_CALL(*session_unowned2_, set_config(_));
   EXPECT_CALL(*session_unowned2_, Close()).WillOnce(
     InvokeWithoutArgs(this, &ChromotingHostTest::NotifyConnectionClosed2));
 

remoting/host/client_session_unittest.cc

@@ -178,7 +178,7 @@ class ClientSessionTest : public testing::Test {
   MockClientSessionEventHandler session_event_handler_;
 
   // Storage for values to be returned by the protocol::Session mock.
-  scoped_ptr<SessionConfig> session_config_;
+  SessionConfig session_config_;
   const std::string client_jid_;
 
   // Stubs returned to |client_session_| components by |connection_|.
@@ -224,7 +224,7 @@ void ClientSessionTest::TearDown() {
 void ClientSessionTest::CreateClientSession() {
   // Mock protocol::Session APIs called directly by ClientSession.
   protocol::MockSession* session = new MockSession();
-  EXPECT_CALL(*session, config()).WillRepeatedly(ReturnRef(*session_config_));
+  EXPECT_CALL(*session, config()).WillRepeatedly(ReturnRef(session_config_));
   EXPECT_CALL(*session, jid()).WillRepeatedly(ReturnRef(client_jid_));
   EXPECT_CALL(*session, SetEventHandler(_));
 

remoting/host/session_manager_factory.cc

@@ -25,8 +25,7 @@ scoped_ptr<protocol::SessionManager> CreateHostSessionManager(
 
   scoped_ptr<protocol::TransportFactory> transport_factory(
       new protocol::LibjingleTransportFactory(
-          signal_strategy, port_allocator.Pass(), network_settings,
-          protocol::TransportRole::SERVER));
+          signal_strategy, port_allocator.Pass(), network_settings));
 
   scoped_ptr<protocol::JingleSessionManager> session_manager(
       new protocol::JingleSessionManager(transport_factory.Pass()));

remoting/protocol/content_description.cc

@@ -25,7 +25,6 @@ const char kDefaultNs[] = "";
 
 // Following constants are used to format session description in XML.
 const char kDescriptionTag[] = "description";
-const char kStandardIceTag[] = "standard-ice";
 const char kControlTag[] = "control";
 const char kEventTag[] = "event";
 const char kVideoTag[] = "video";
@@ -122,10 +121,17 @@ ContentDescription::ContentDescription(
 
 ContentDescription::~ContentDescription() { }
 
+ContentDescription* ContentDescription::Copy() const {
+  if (!candidate_config_.get() || !authenticator_message_.get()) {
+    return nullptr;
+  }
+  scoped_ptr<XmlElement> message(new XmlElement(*authenticator_message_));
+  return new ContentDescription(candidate_config_->Clone(), message.Pass());
+}
+
 // ToXml() creates content description for chromoting session. The
 // description looks as follows:
 //   <description xmlns="google:remoting">
-//     <standard-ice/>
 //     <control transport="stream" version="1" />
 //     <event transport="datagram" version="1" />
 //     <video transport="stream" codec="vp8" version="1" />
@@ -139,25 +145,27 @@ XmlElement* ContentDescription::ToXml() const {
   XmlElement* root = new XmlElement(
       QName(kChromotingXmlNamespace, kDescriptionTag), true);
 
-  if (config()->standard_ice()) {
-    root->AddElement(
-        new buzz::XmlElement(QName(kChromotingXmlNamespace, kStandardIceTag)));
-  }
+  std::list<ChannelConfig>::const_iterator it;
 
-  for (const ChannelConfig& channel_config : config()->control_configs()) {
-    root->AddElement(FormatChannelConfig(channel_config, kControlTag));
+  for (it = config()->control_configs().begin();
+       it != config()->control_configs().end(); ++it) {
+    root->AddElement(FormatChannelConfig(*it, kControlTag));
   }
 
-  for (const ChannelConfig& channel_config : config()->event_configs()) {
-    root->AddElement(FormatChannelConfig(channel_config, kEventTag));
+  for (it = config()->event_configs().begin();
+       it != config()->event_configs().end(); ++it) {
+    root->AddElement(FormatChannelConfig(*it, kEventTag));
   }
 
-  for (const ChannelConfig& channel_config : config()->video_configs()) {
-    root->AddElement(FormatChannelConfig(channel_config, kVideoTag));
+  for (it = config()->video_configs().begin();
+       it != config()->video_configs().end(); ++it) {
+    root->AddElement(FormatChannelConfig(*it, kVideoTag));
   }
 
-  for (const ChannelConfig& channel_config : config()->audio_configs()) {
-    root->AddElement(FormatChannelConfig(channel_config, kAudioTag));
+  for (it = config()->audio_configs().begin();
+       it != config()->audio_configs().end(); ++it) {
+    ChannelConfig config = *it;
+    root->AddElement(FormatChannelConfig(config, kAudioTag));
   }
 
   // Older endpoints require an initial-resolution tag, but otherwise ignore it.
@@ -184,6 +192,7 @@ bool ContentDescription::ParseChannelConfigs(
     bool codec_required,
     bool optional,
     std::list<ChannelConfig>* const configs) {
+
   QName tag(kChromotingXmlNamespace, tag_name);
   const XmlElement* child = element->FirstNamed(tag);
   while (child) {
@@ -209,11 +218,6 @@ scoped_ptr<ContentDescription> ContentDescription::ParseXml(
   }
   scoped_ptr<CandidateSessionConfig> config(
       CandidateSessionConfig::CreateEmpty());
-
-  config->set_standard_ice(
-      element->FirstNamed(QName(kChromotingXmlNamespace, kStandardIceTag)) !=
-      nullptr);
-
   if (!ParseChannelConfigs(element, kControlTag, false, false,
                            config->mutable_control_configs()) ||
       !ParseChannelConfigs(element, kEventTag, false, false,

remoting/protocol/content_description.h

@@ -10,6 +10,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "remoting/protocol/session_config.h"
+#include "third_party/webrtc/p2p/base/sessiondescription.h"
 
 namespace buzz {
 class XmlElement;
@@ -23,13 +24,15 @@ namespace protocol {
 //
 // This class also provides a type abstraction so that the Chromotocol Session
 // interface does not need to depend on libjingle.
-class ContentDescription {
+class ContentDescription : public cricket::ContentDescription {
  public:
   static const char kChromotingContentName[];
 
   ContentDescription(scoped_ptr<CandidateSessionConfig> config,
                      scoped_ptr<buzz::XmlElement> authenticator_message);
-  ~ContentDescription();
+  ~ContentDescription() override;
+
+  ContentDescription* Copy() const override;
 
   const CandidateSessionConfig* config() const {
     return candidate_config_.get();

remoting/protocol/fake_connection_to_host.cc

@@ -85,7 +85,7 @@ void FakeConnectionToHost::SignalConnectionReady(bool ready) {
 }
 
 const protocol::SessionConfig& FakeConnectionToHost::config() {
-  return *session_config_;
+  return session_config_;
 }
 
 protocol::ClipboardStub* FakeConnectionToHost::clipboard_forwarder() {

remoting/protocol/fake_connection_to_host.h

@@ -54,7 +54,7 @@ class FakeConnectionToHost : public protocol::ConnectionToHost {
   testing::NiceMock<protocol::MockClipboardStub> mock_clipboard_stub_;
   testing::NiceMock<protocol::MockHostStub> mock_host_stub_;
   testing::NiceMock<protocol::MockInputStub> mock_input_stub_;
-  scoped_ptr<protocol::SessionConfig> session_config_;
+  protocol::SessionConfig session_config_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeConnectionToHost);
 };

remoting/protocol/fake_session.cc

@@ -36,11 +36,11 @@ const CandidateSessionConfig* FakeSession::candidate_config() {
 }
 
 const SessionConfig& FakeSession::config() {
-  return *config_;
+  return config_;
 }
 
-void FakeSession::set_config(scoped_ptr<SessionConfig> config) {
-  config_ = config.Pass();
+void FakeSession::set_config(const SessionConfig& config) {
+  config_ = config;
 }
 
 StreamChannelFactory* FakeSession::GetTransportChannelFactory() {

remoting/protocol/fake_session.h

@@ -39,15 +39,15 @@ class FakeSession : public Session {
   const std::string& jid() override;
   const CandidateSessionConfig* candidate_config() override;
   const SessionConfig& config() override;
-  void set_config(scoped_ptr<SessionConfig> config) override;
+  void set_config(const SessionConfig& config) override;
   StreamChannelFactory* GetTransportChannelFactory() override;
   StreamChannelFactory* GetMultiplexedChannelFactory() override;
   void Close() override;
 
  public:
   EventHandler* event_handler_;
   scoped_ptr<const CandidateSessionConfig> candidate_config_;
-  scoped_ptr<SessionConfig> config_;
+  SessionConfig config_;
 
   FakeStreamChannelFactory channel_factory_;