Encoder experiment cleanup

This CL cleans up some code that was used in the past to select VP9
and H.264 at connection time.  AFAIK neither of the mobile clients are
set up to select a codec other than VP8.  The website uses WebRTC which
negotiates the codec using SDPs.

While I was working on getting the h.264 codec back in working order, I
needed to read through this code to figure out if it was useful.  Since
it appears that it isn't (and likely never will be) I think we should
clean it up.

Change-Id: Ibac53315d9a8daa71353551011654b94430982d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2568291
Commit-Queue: Jamie Walch <jamiewalch@chromium.org>
Auto-Submit: Joe Downing <joedow@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835739}

remoting/codec/video_encoder.cc

@@ -4,7 +4,6 @@
 
 #include "remoting/codec/video_encoder.h"
 
-#include "base/memory/ptr_util.h"
 #include "base/notreached.h"
 #include "remoting/codec/video_encoder_verbatim.h"
 #include "remoting/codec/video_encoder_vpx.h"
@@ -21,7 +20,7 @@ std::unique_ptr<VideoEncoder> VideoEncoder::Create(
   } else if (video_config.codec == protocol::ChannelConfig::CODEC_VP9) {
     return VideoEncoderVpx::CreateForVP9();
   } else if (video_config.codec == protocol::ChannelConfig::CODEC_VERBATIM) {
-    return base::WrapUnique(new VideoEncoderVerbatim());
+    return std::make_unique<VideoEncoderVerbatim>();
   }
 
   NOTREACHED() << "Unknown codec type: " << video_config.codec;

remoting/host/daemon_process_win.cc

@@ -79,13 +79,7 @@ namespace remoting {
 
 class WtsTerminalMonitor;
 
-// The command line parameters that should be copied from the service's command
-// line to the host process.
-const char kEnableVp9SwitchName[] = "enable-vp9";
-const char kEnableH264SwitchName[] = "enable-h264";
-const char* kCopiedSwitchNames[] = {switches::kV, switches::kVModule,
-                                    kEnableVp9SwitchName,
-                                    kEnableH264SwitchName};
+const char* kCopiedSwitchNames[] = {switches::kV, switches::kVModule};
 
 class DaemonProcessWin : public DaemonProcess {
  public:

remoting/host/remoting_me2me_host.cc

@@ -176,12 +176,6 @@ const char kAuthSocknameSwitchName[] = "ssh-auth-sockname";
 // when it is successfully started.
 const char kSignalParentSwitchName[] = "signal-parent";
 
-// Command line switch used to enable VP9 encoding.
-const char kEnableVp9SwitchName[] = "enable-vp9";
-
-// Command line switch used to enable hardware H264 encoding.
-const char kEnableH264SwitchName[] = "enable-h264";
-
 // Command line switch used to send a custom offline reason and exit.
 const char kReportOfflineReasonSwitchName[] = "report-offline-reason";
 
@@ -387,8 +381,6 @@ class HostProcess : public ConfigWatcher::Delegate,
   std::string serialized_config_;
   std::string host_owner_;
   bool is_googler_ = false;
-  bool enable_vp9_ = false;
-  bool enable_h264_ = false;
 
   std::unique_ptr<PolicyWatcher> policy_watcher_;
   PolicyState policy_state_ = POLICY_INITIALIZING;
@@ -1041,22 +1033,6 @@ bool HostProcess::ApplyConfig(const base::DictionaryValue& config) {
   is_googler_ = base::EndsWith(host_owner_, kGooglerEmailDomain,
                                base::CompareCase::INSENSITIVE_ASCII);
 
-  // Allow offering of VP9 encoding to be overridden by the command-line.
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(kEnableVp9SwitchName)) {
-    enable_vp9_ = true;
-  } else {
-    config.GetBoolean(kEnableVp9ConfigPath, &enable_vp9_);
-  }
-
-  // Allow offering of hardware H264 encoding to be overridden by the command
-  // line.
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          kEnableH264SwitchName)) {
-    enable_h264_ = true;
-  } else {
-    config.GetBoolean(kEnableH264ConfigPath, &enable_h264_);
-  }
-
   return true;
 }
 
@@ -1552,10 +1528,6 @@ void HostProcess::StartHost() {
       protocol::CandidateSessionConfig::CreateDefault();
   if (!desktop_environment_factory_->SupportsAudioCapture())
     protocol_config->DisableAudioChannel();
-  if (enable_vp9_)
-    protocol_config->set_vp9_experiment_enabled(true);
-  if (enable_h264_)
-    protocol_config->set_h264_experiment_enabled(true);
   protocol_config->set_webrtc_supported(true);
   session_manager->set_protocol_config(std::move(protocol_config));
 

remoting/protocol/content_description.cc

@@ -34,8 +34,6 @@ const char kControlTag[] = "control";
 const char kEventTag[] = "event";
 const char kVideoTag[] = "video";
 const char kAudioTag[] = "audio";
-const char kVp9ExperimentTag[] = "vp9-experiment";
-const char kH264ExperimentTag[] = "h264-experiment";
 
 const char kTransportAttr[] = "transport";
 const char kVersionAttr[] = "version";
@@ -168,11 +166,6 @@ XmlElement* ContentDescription::ToXml() const {
     root->AddElement(new XmlElement(*authenticator_message_));
   }
 
-  if (config()->vp9_experiment_enabled()) {
-    root->AddElement(
-        new XmlElement(QName(kChromotingXmlNamespace, kVp9ExperimentTag)));
-  }
-
   return root;
 }
 
@@ -229,16 +222,6 @@ std::unique_ptr<ContentDescription> ContentDescription::ParseXml(
     }
   }
 
-  // Check if VP9 experiment is enabled.
-  if (element->FirstNamed(QName(kChromotingXmlNamespace, kVp9ExperimentTag))) {
-    config->set_vp9_experiment_enabled(true);
-  }
-
-  // Check if H264 experiment is enabled.
-  if (element->FirstNamed(QName(kChromotingXmlNamespace, kH264ExperimentTag))) {
-    config->set_h264_experiment_enabled(true);
-  }
-
   std::unique_ptr<XmlElement> authenticator_message;
   const XmlElement* child = Authenticator::FindAuthenticatorMessage(element);
   if (child)

remoting/protocol/session_config.cc

@@ -86,24 +86,12 @@ std::unique_ptr<SessionConfig> SessionConfig::SelectCommon(
 
   std::unique_ptr<SessionConfig> result(new SessionConfig(Protocol::ICE));
 
-  // If neither host nor the client have VP9 experiment enabled then remove it
-  // from the list of host video configs.
   std::list<ChannelConfig> host_video_configs = host_config->video_configs();
-  if (!client_config->vp9_experiment_enabled() &&
-      !host_config->vp9_experiment_enabled()) {
-    host_video_configs.remove_if([](const ChannelConfig& config) {
-      return config.codec == ChannelConfig::CODEC_VP9;
-    });
-  }
-
-  // If neither host nor the client have H264 experiment enabled then remove it
-  // from the list of host video configs.
-  if (!client_config->h264_experiment_enabled() &&
-      !host_config->h264_experiment_enabled()) {
-    host_video_configs.remove_if([](const ChannelConfig& config) {
-      return config.codec == ChannelConfig::CODEC_H264;
-    });
-  }
+  host_video_configs.remove_if([](const ChannelConfig& config) {
+    // Older ICE-based clients do not support VP9 or H.264 so remove them.
+    return config.codec == ChannelConfig::CODEC_H264 ||
+           config.codec == ChannelConfig::CODEC_VP9;
+  });
 
   if (!SelectCommonChannelConfig(host_config->control_configs(),
                                  client_config->control_configs(),

remoting/protocol/session_config.h

@@ -65,10 +65,10 @@ class CandidateSessionConfig;
 class SessionConfig {
  public:
   enum class Protocol {
-    // Current ICE-based protocol.
+    // ICE-based protocol (aka Chromotocol).
     ICE,
 
-    // New WebRTC-based protocol.
+    // WebRTC-based protocol.
     WEBRTC,
   };
 
@@ -82,7 +82,7 @@ class SessionConfig {
 
   // Extracts final protocol configuration. Must be used for the description
   // received in the session-accept stanza. If the selection is ambiguous
-  // (e.g. there is more than one configuration for one of the channel)
+  // (e.g. there is more than one configuration for one of the channels)
   // or undefined (e.g. no configurations for a channel) then nullptr is
   // returned.
   static std::unique_ptr<SessionConfig> GetFinalConfig(
@@ -169,23 +169,6 @@ class CandidateSessionConfig {
     return &audio_configs_;
   }
 
-  // Old clients always list VP9 as supported and preferred even though they
-  // shouldn't have it enabled yet. I.e. the host cannot trust VP9 codec
-  // preference received from the client. To workaround this issue the client
-  // adds a hint in the session-initiate message to indicate that it actually
-  // wants VP9 to be enabled.
-  //
-  // TODO(sergeyu): Remove this kludge as soon as VP9 is enabled by default.
-  bool vp9_experiment_enabled() const { return vp9_experiment_enabled_; }
-  void set_vp9_experiment_enabled(bool value) {
-    vp9_experiment_enabled_ = value;
-  }
-
-  bool h264_experiment_enabled() const { return h264_experiment_enabled_; }
-  void set_h264_experiment_enabled(bool value) {
-    h264_experiment_enabled_ = value;
-  }
-
   // Returns true if |config| is supported.
   bool IsSupported(const SessionConfig& config) const;