Always enabled audio in remoting host, disable it for It2Me, minor cleanups.

It's not desired to have audio to be enabled by default in It2Me 
case. Specifically because in most cases helper and helpee will have separate 
audio channel (e.g. Hangout or Skype) that's connected before the session 
is started. Don't enable it until we have UI to toggle audio support.
This change also includes some minor code cleanups.


Review URL: https://chromiumcodereview.appspot.com/10928159

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

10 files changed, 53 insertions, 57 deletions

diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index 4c1503d..487ac35 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -76,11 +76,8 @@ ChromotingHost::ChromotingHost(
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
 
   if (!desktop_environment_factory_->SupportsAudioCapture()) {
-    // Disable audio by replacing our list of supported audio configurations
-    // with the NONE config.
-    protocol_config_->mutable_audio_configs()->clear();
-    protocol_config_->mutable_audio_configs()->push_back(
-        protocol::ChannelConfig());
+    protocol::CandidateSessionConfig::DisableAudioChannel(
+        protocol_config_.get());
   }
 }
 
@@ -320,11 +317,11 @@ void ChromotingHost::OnIncomingSession(
 }
 
 void ChromotingHost::set_protocol_config(
-    protocol::CandidateSessionConfig* config) {
+    scoped_ptr<protocol::CandidateSessionConfig> config) {
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
-  DCHECK(config);
+  DCHECK(config.get());
   DCHECK_EQ(state_, kInitial);
-  protocol_config_.reset(config);
+  protocol_config_ = config.Pass();
 }
 
 void ChromotingHost::OnLocalMouseMoved(const SkIPoint& new_pos) {
diff --git a/remoting/host/chromoting_host.h b/remoting/host/chromoting_host.h
index bd731ca..a700c24 100644
--- a/remoting/host/chromoting_host.h
+++ b/remoting/host/chromoting_host.h
@@ -125,9 +125,8 @@ class ChromotingHost : public base::RefCountedThreadSafe<ChromotingHost>,
   // MouseMoveObserver interface.
   virtual void OnLocalMouseMoved(const SkIPoint& new_pos) OVERRIDE;
 
-  // Sets desired configuration for the protocol. Ownership of the
-  // |config| is transferred to the object. Must be called before Start().
-  void set_protocol_config(protocol::CandidateSessionConfig* config);
+  // Sets desired configuration for the protocol. Must be called before Start().
+  void set_protocol_config(scoped_ptr<protocol::CandidateSessionConfig> config);
 
   // Pause or unpause the session. While the session is paused, remote input
   // is ignored. Can be called from any thread.
diff --git a/remoting/host/chromoting_host_unittest.cc b/remoting/host/chromoting_host_unittest.cc
index e75a1cb..a380d8d 100644
--- a/remoting/host/chromoting_host_unittest.cc
+++ b/remoting/host/chromoting_host_unittest.cc
@@ -581,8 +581,6 @@ TEST_F(ChromotingHostTest, IncomingSessionIncompatible) {
       empty_candidate_config_.get()));
   EXPECT_CALL(host_status_observer_, OnShutdown());
 
-  host_->set_protocol_config(
-      protocol::CandidateSessionConfig::CreateDefault().release());
   host_->Start(xmpp_login_);
 
   protocol::SessionManager::IncomingSessionResponse response =
@@ -604,8 +602,6 @@ TEST_F(ChromotingHostTest, IncomingSessionAccepted) {
   EXPECT_CALL(host_status_observer_, OnAccessDenied(_));
   EXPECT_CALL(host_status_observer_, OnShutdown());
 
-  host_->set_protocol_config(
-      protocol::CandidateSessionConfig::CreateDefault().release());
   host_->Start(xmpp_login_);
 
   protocol::SessionManager::IncomingSessionResponse response =
@@ -627,8 +623,6 @@ TEST_F(ChromotingHostTest, IncomingSessionOverload) {
   EXPECT_CALL(host_status_observer_, OnAccessDenied(_));
   EXPECT_CALL(host_status_observer_, OnShutdown());
 
-  host_->set_protocol_config(
-      protocol::CandidateSessionConfig::CreateDefault().release());
   host_->Start(xmpp_login_);
 
   protocol::SessionManager::IncomingSessionResponse response =
diff --git a/remoting/host/plugin/host_script_object.cc b/remoting/host/plugin/host_script_object.cc
index 7dd58f6b..c4989c6 100644
--- a/remoting/host/plugin/host_script_object.cc
+++ b/remoting/host/plugin/host_script_object.cc
@@ -580,7 +580,7 @@ void HostNPScriptObject::FinishConnect(
     network_settings.max_port = NetworkSettings::kDefaultMaxPort;
   }
 
-  // Create the Host.
+  // Create the host.
   host_ = new ChromotingHost(
       host_context_.get(), signal_strategy_.get(),
       desktop_environment_factory_.get(),
@@ -589,17 +589,30 @@ void HostNPScriptObject::FinishConnect(
   host_->AddStatusObserver(this);
   log_to_server_.reset(
       new LogToServer(host_, ServerLogEntry::IT2ME, signal_strategy_.get()));
-  base::Closure disconnect_callback = base::Bind(
-      &ChromotingHost::Shutdown, base::Unretained(host_.get()),
-      base::Closure());
-  it2me_host_user_interface_->Start(host_.get(), disconnect_callback);
-  host_event_logger_ = HostEventLogger::Create(host_, kApplicationName);
 
+  // Disable audio by default.
+  // TODO(sergeyu): Add UI to enable it.
+  scoped_ptr<protocol::CandidateSessionConfig> protocol_config =
+      protocol::CandidateSessionConfig::CreateDefault();
+  protocol::CandidateSessionConfig::DisableAudioChannel(protocol_config.get());
+  host_->set_protocol_config(protocol_config.Pass());
+
+  // Provide localization strings to the host.
   {
     base::AutoLock auto_lock(ui_strings_lock_);
     host_->SetUiStrings(ui_strings_);
   }
 
+  // Create user interface.
+  base::Closure disconnect_callback = base::Bind(
+      &ChromotingHost::Shutdown, base::Unretained(host_.get()),
+      base::Closure());
+  it2me_host_user_interface_->Start(host_.get(), disconnect_callback);
+
+  // Create event logger.
+  host_event_logger_ = HostEventLogger::Create(host_, kApplicationName);
+
+  // Connect signaling and start the host.
   signal_strategy_->Connect();
   host_->Start(uid);
 
diff --git a/remoting/host/simple_host_process.cc b/remoting/host/simple_host_process.cc
index 38902f9..8d11db0 100644
--- a/remoting/host/simple_host_process.cc
+++ b/remoting/host/simple_host_process.cc
@@ -283,7 +283,7 @@ class SimpleHost : public HeartbeatSender::Listener {
     }
 
     if (protocol_config_.get()) {
-      host_->set_protocol_config(protocol_config_.release());
+      host_->set_protocol_config(protocol_config_.Pass());
     }
 
     if (is_it2me_) {
diff --git a/remoting/protocol/connection_to_host.cc b/remoting/protocol/connection_to_host.cc
index 138c37b..dd21220 100644
--- a/remoting/protocol/connection_to_host.cc
+++ b/remoting/protocol/connection_to_host.cc
@@ -139,7 +139,6 @@ void ConnectionToHost::OnSessionManagerReady() {
   // After SessionManager is initialized we can try to connect to the host.
   scoped_ptr<CandidateSessionConfig> candidate_config =
       CandidateSessionConfig::CreateDefault();
-  CandidateSessionConfig::EnableAudioChannel(candidate_config.get());
   session_ = session_manager_->Connect(
       host_jid_, authenticator_.Pass(), candidate_config.Pass());
   session_->SetEventHandler(this);
diff --git a/remoting/protocol/content_description.cc b/remoting/protocol/content_description.cc
index 7a75ae1..1ea74ee 100644
--- a/remoting/protocol/content_description.cc
+++ b/remoting/protocol/content_description.cc
@@ -214,9 +214,8 @@ bool ContentDescription::ParseChannelConfigs(
     child = child->NextNamed(tag);
   }
   if (optional && configs->empty()) {
-      // If there's no mention of the tag, implicitly assume
-      // TRANSPORT_NONE for the channel.
-      configs->push_back(ChannelConfig());
+      // If there's no mention of the tag, implicitly assume disabled channel.
+      configs->push_back(ChannelConfig::None());
   }
   return true;
 }
diff --git a/remoting/protocol/session_config.cc b/remoting/protocol/session_config.cc
index b4e1da0..28ffc19 100644
--- a/remoting/protocol/session_config.cc
+++ b/remoting/protocol/session_config.cc
@@ -11,6 +11,10 @@ namespace protocol {
 
 const int kDefaultStreamVersion = 2;
 
+ChannelConfig ChannelConfig::None() {
+  return ChannelConfig();
+}
+
 ChannelConfig::ChannelConfig()
     : transport(TRANSPORT_NONE),
       version(0),
@@ -31,6 +35,7 @@ bool ChannelConfig::operator==(const ChannelConfig& b) const {
 }
 
 SessionConfig::SessionConfig() {
+
 }
 
 // static
@@ -190,34 +195,27 @@ scoped_ptr<CandidateSessionConfig> CandidateSessionConfig::CreateDefault() {
                     ChannelConfig::CODEC_VP8));
 
   // Audio channel.
-  result->mutable_audio_configs()->push_back(ChannelConfig());
-#if defined(ENABLE_REMOTING_AUDIO)
-  EnableAudioChannel(result.get());
-#endif  // defined(ENABLE_REMOTING_AUDIO)
-
-  return result.Pass();
-}
-
-// static
-void CandidateSessionConfig::EnableAudioChannel(
-  CandidateSessionConfig* config) {
-  config->mutable_audio_configs()->clear();
-  config->mutable_audio_configs()->push_back(
+  result->mutable_audio_configs()->push_back(
       ChannelConfig(ChannelConfig::TRANSPORT_MUX_STREAM,
                     kDefaultStreamVersion,
                     ChannelConfig::CODEC_SPEEX));
-  config->mutable_audio_configs()->push_back(
+  result->mutable_audio_configs()->push_back(
       ChannelConfig(ChannelConfig::TRANSPORT_STREAM,
                     kDefaultStreamVersion,
                     ChannelConfig::CODEC_SPEEX));
-  config->mutable_audio_configs()->push_back(
+  result->mutable_audio_configs()->push_back(
       ChannelConfig(ChannelConfig::TRANSPORT_MUX_STREAM,
                     kDefaultStreamVersion,
                     ChannelConfig::CODEC_VERBATIM));
-  config->mutable_audio_configs()->push_back(
-      ChannelConfig(ChannelConfig::TRANSPORT_STREAM,
-                    kDefaultStreamVersion,
-                    ChannelConfig::CODEC_VERBATIM));
+  result->mutable_audio_configs()->push_back(ChannelConfig::None());
+
+  return result.Pass();
+}
+
+// static
+void CandidateSessionConfig::DisableAudioChannel(
+  CandidateSessionConfig* config) {
+  config->mutable_audio_configs()->clear();
   config->mutable_audio_configs()->push_back(ChannelConfig());
 }
 
diff --git a/remoting/protocol/session_config.h b/remoting/protocol/session_config.h
index d5a5b5f..e4e0574 100644
--- a/remoting/protocol/session_config.h
+++ b/remoting/protocol/session_config.h
@@ -36,8 +36,11 @@ struct ChannelConfig {
     CODEC_SPEEX,
   };
 
-  // The constructor that creates a config with transport field set to
-  // TRANSPORT_NONE which indicates that corresponding channel is disabled.
+  // Creates a config with transport field set to TRANSPORT_NONE which indicates
+  // that corresponding channel is disabled.
+  static ChannelConfig None();
+
+  // Default constructor. Equivalent to None().
   ChannelConfig();
 
   // Creates a channel config with the specified parameters.
@@ -150,8 +153,8 @@ class CandidateSessionConfig {
       const SessionConfig& config);
   static scoped_ptr<CandidateSessionConfig> CreateDefault();
 
-  // Helper method that modifies |config| to enable audio support.
-  static void EnableAudioChannel(CandidateSessionConfig* config);
+  // Helper method that modifies |config| to disable audio support.
+  static void DisableAudioChannel(CandidateSessionConfig* config);
 
  private:
   CandidateSessionConfig();
diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp
index fe7ad36..0172c32 100644
--- a/remoting/remoting.gyp
+++ b/remoting/remoting.gyp
@@ -8,7 +8,6 @@
     # duplicated from chrome.gyp
     'chromium_code': 1,
 
-    'remoting_audio%': 1,
     'remoting_multi_process%': 0,
 
     # The version is composed from major & minor versions specific to remoting
@@ -199,11 +198,6 @@
       '..',  # Root of Chrome checkout
     ],
     'conditions': [
-      ['remoting_audio == 1', {
-        'defines': [
-          'ENABLE_REMOTING_AUDIO',
-        ],
-      }],
       ['remoting_multi_process != 0', {
         'defines': [
           'REMOTING_MULTI_PROCESS',