[Chromoting] Use SessionOptions instead of SetPreferredVideoCodec

After cl 776197, SessionOptions can be referred by protocol/ and codec/, so we
can use it to forward the session options instead of using pure string for
video codec selection.

This change should have no behavior impact.

Bug: chromium:781432
Change-Id: Id7b87d460a4789dead75806f6f33d731cf8ddada
Reviewed-on: https://chromium-review.googlesource.com/777776
Commit-Queue: Zijie He <zijiehe@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517908}

remoting/base/session_options.cc

@@ -33,12 +33,19 @@ bool KeyIsValid(const std::string& key) {
 }  // namespace
 
 SessionOptions::SessionOptions() = default;
-SessionOptions::~SessionOptions() = default;
+SessionOptions::SessionOptions(const SessionOptions& other) = default;
+SessionOptions::SessionOptions(SessionOptions&& other) = default;
 
 SessionOptions::SessionOptions(const std::string& parameter) {
   Import(parameter);
 }
 
+SessionOptions::~SessionOptions() = default;
+
+SessionOptions& SessionOptions::operator=(
+    const SessionOptions& other) = default;
+SessionOptions& SessionOptions::operator=(SessionOptions&& other) = default;
+
 void SessionOptions::Append(const std::string& key,
                                 const std::string& value) {
   DCHECK(KeyIsValid(key));

remoting/base/session_options.h

@@ -8,7 +8,6 @@
 #include <map>
 #include <string>
 
-#include "base/macros.h"
 #include "base/optional.h"
 
 namespace remoting {
@@ -19,9 +18,14 @@ namespace remoting {
 class SessionOptions final {
  public:
   SessionOptions();
+  SessionOptions(const SessionOptions& other);
+  SessionOptions(SessionOptions&& other);
+  explicit SessionOptions(const std::string& parameter);
+
   ~SessionOptions();
 
-  SessionOptions(const std::string& parameter);
+  SessionOptions& operator=(const SessionOptions& other);
+  SessionOptions& operator=(SessionOptions&& other);
 
   // Appends one key-value pair into current instance.
   void Append(const std::string& key, const std::string& value);
@@ -51,10 +55,6 @@ class SessionOptions final {
 
  private:
   std::map<std::string, std::string> options_;
-
-  SessionOptions(SessionOptions&&) = delete;
-  SessionOptions& operator=(SessionOptions&&) = delete;
-  DISALLOW_COPY_AND_ASSIGN(SessionOptions);
 };
 
 }  // namespace remoting

remoting/host/client_session.cc

@@ -247,11 +247,7 @@ void ClientSession::OnConnectionAuthenticated() {
   const SessionOptions session_options(
       host_experiment_session_plugin_.configuration());
 
-  base::Optional<std::string> video_codec =
-      session_options.Get("Video-Codec");
-  if (video_codec) {
-    connection_->SetPreferredVideoCodec(*video_codec);
-  }
+  connection_->ApplySessionOptions(session_options);
 
   DesktopEnvironmentOptions options = desktop_environment_options_;
   options.ApplySessionOptions(session_options);

remoting/protocol/connection_to_client.h

@@ -9,6 +9,7 @@
 
 #include <string>
 
+#include "remoting/base/session_options.h"
 #include "remoting/protocol/message_pipe.h"
 #include "remoting/protocol/transport.h"
 
@@ -100,9 +101,10 @@ class ConnectionToClient {
   virtual void set_host_stub(HostStub* host_stub) = 0;
   virtual void set_input_stub(InputStub* input_stub) = 0;
 
-  // Set the preferred video codec for the connection. Implementations can
-  // ignore this function if no extra codec can be chosen from.
-  virtual void SetPreferredVideoCodec(const std::string& codec) {}
+  // Applies the |options| to current session. SessionOptions usually controls
+  // experimental behaviors, implementations can ignore this function if no
+  // control logic can be applied.
+  virtual void ApplySessionOptions(const SessionOptions& options) {}
 };
 
 }  // namespace protocol

remoting/protocol/webrtc_connection_to_client.cc

@@ -82,7 +82,8 @@ std::unique_ptr<VideoStream> WebrtcConnectionToClient::StartVideoStream(
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(transport_);
 
-  std::unique_ptr<WebrtcVideoStream> stream(new WebrtcVideoStream());
+  std::unique_ptr<WebrtcVideoStream> stream(
+      new WebrtcVideoStream(session_options_));
   stream->Start(std::move(desktop_capturer), transport_.get(),
                 video_encode_task_runner_);
   stream->SetEventTimestampsSource(
@@ -122,10 +123,11 @@ void WebrtcConnectionToClient::set_input_stub(protocol::InputStub* input_stub) {
   event_dispatcher_->set_input_stub(input_stub);
 }
 
-void WebrtcConnectionToClient::SetPreferredVideoCodec(
-    const std::string& codec) {
+void WebrtcConnectionToClient::ApplySessionOptions(
+    const SessionOptions& options) {
+  session_options_ = options;
   DCHECK(transport_);
-  transport_->SetPreferredVideoCodec(codec);
+  transport_->ApplySessionOptions(options);
 }
 
 void WebrtcConnectionToClient::OnSessionStateChange(Session::State state) {

remoting/protocol/webrtc_connection_to_client.h

@@ -50,7 +50,7 @@ class WebrtcConnectionToClient : public ConnectionToClient,
   void set_clipboard_stub(ClipboardStub* clipboard_stub) override;
   void set_host_stub(HostStub* host_stub) override;
   void set_input_stub(InputStub* input_stub) override;
-  void SetPreferredVideoCodec(const std::string& codec) override;
+  void ApplySessionOptions(const SessionOptions& options) override;
 
   // Session::EventHandler interface.
   void OnSessionStateChange(Session::State state) override;
@@ -84,6 +84,8 @@ class WebrtcConnectionToClient : public ConnectionToClient,
   scoped_refptr<base::SingleThreadTaskRunner> video_encode_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner_;
 
+  SessionOptions session_options_;
+
   std::unique_ptr<HostControlDispatcher> control_dispatcher_;
   std::unique_ptr<HostEventDispatcher> event_dispatcher_;
   base::WeakPtrFactory<WebrtcConnectionToClient> weak_factory_;

remoting/protocol/webrtc_frame_scheduler_simple.cc

@@ -55,8 +55,9 @@ int64_t GetRegionArea(const webrtc::DesktopRegion& region) {
 
 }  // namespace
 
-// TODO(zijiehe): Make the bandwidth estimator configurable.
-WebrtcFrameSchedulerSimple::WebrtcFrameSchedulerSimple()
+// TODO(zijiehe): Use |options| to select bandwidth estimator.
+WebrtcFrameSchedulerSimple::WebrtcFrameSchedulerSimple(
+    const SessionOptions& options)
     : pacing_bucket_(LeakyBucket::kUnlimitedDepth, 0),
       updated_region_area_(kStatsWindow),
       bandwidth_estimator_(new WebrtcBandwidthEstimator()),

remoting/protocol/webrtc_frame_scheduler_simple.h

@@ -14,6 +14,7 @@
 #include "base/timer/timer.h"
 #include "remoting/base/leaky_bucket.h"
 #include "remoting/base/running_samples.h"
+#include "remoting/base/session_options.h"
 #include "remoting/codec/frame_processing_time_estimator.h"
 #include "remoting/protocol/video_channel_state_observer.h"
 
@@ -29,7 +30,7 @@ class BandwidthEstimator;
 class WebrtcFrameSchedulerSimple : public VideoChannelStateObserver,
                                    public WebrtcFrameScheduler {
  public:
-  WebrtcFrameSchedulerSimple();
+  explicit WebrtcFrameSchedulerSimple(const SessionOptions& options);
   ~WebrtcFrameSchedulerSimple() override;
 
   // VideoChannelStateObserver implementation.

remoting/protocol/webrtc_frame_scheduler_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/test/test_simple_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "remoting/base/session_options.h"
 #include "remoting/protocol/webrtc_dummy_video_encoder.h"
 #include "remoting/protocol/webrtc_frame_scheduler_simple.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -25,7 +26,7 @@ class WebrtcFrameSchedulerTest : public ::testing::Test {
         task_runner_handle_(task_runner_.get()),
         now_(base::TimeTicks::Now()) {
     video_encoder_factory_.reset(new WebrtcDummyVideoEncoderFactory());
-    scheduler_.reset(new WebrtcFrameSchedulerSimple());
+    scheduler_.reset(new WebrtcFrameSchedulerSimple(SessionOptions()));
     scheduler_->SetCurrentTimeForTest(now_);
     scheduler_->Start(video_encoder_factory_.get(),
                       base::Bind(&WebrtcFrameSchedulerTest::CaptureCallback,

remoting/protocol/webrtc_transport.cc

@@ -13,6 +13,7 @@
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/optional.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
@@ -489,7 +490,9 @@ void WebrtcTransport::OnLocalSessionDescriptionCreated(
 
   SdpMessage sdp_message(description_sdp);
   UpdateCodecParameters(&sdp_message, /*incoming=*/false);
-  if (!preferred_video_codec_.empty()) {
+  if (preferred_video_codec_.empty()) {
+    sdp_message.PreferVideoCodec("VP8");
+  } else {
     sdp_message.PreferVideoCodec(preferred_video_codec_);
   }
   description_sdp = sdp_message.ToString();
@@ -737,8 +740,11 @@ void WebrtcTransport::Close(ErrorCode error) {
     event_handler_->OnWebrtcTransportError(error);
 }
 
-void WebrtcTransport::SetPreferredVideoCodec(const std::string& codec) {
-  preferred_video_codec_ = codec;
+void WebrtcTransport::ApplySessionOptions(const SessionOptions& options) {
+  base::Optional<std::string> video_codec = options.Get("Video-Codec");
+  if (video_codec) {
+    preferred_video_codec_ = *video_codec;
+  }
 }
 
 }  // namespace protocol

remoting/protocol/webrtc_transport.h

@@ -15,6 +15,7 @@
 #include "base/threading/thread_checker.h"
 #include "base/timer/timer.h"
 #include "crypto/hmac.h"
+#include "remoting/base/session_options.h"
 #include "remoting/protocol/transport.h"
 #include "remoting/protocol/webrtc_data_stream_adapter.h"
 #include "remoting/protocol/webrtc_dummy_video_encoder.h"
@@ -84,7 +85,7 @@ class WebrtcTransport : public Transport {
   bool ProcessTransportInfo(buzz::XmlElement* transport_info) override;
   void Close(ErrorCode error);
 
-  void SetPreferredVideoCodec(const std::string& codec);
+  void ApplySessionOptions(const SessionOptions& options);
 
  private:
   // PeerConnectionWrapper is responsible for PeerConnection creation,

remoting/protocol/webrtc_video_stream.cc

@@ -66,8 +66,10 @@ struct WebrtcVideoStream::FrameStats {
   uint32_t capturer_id = 0;
 };
 
-WebrtcVideoStream::WebrtcVideoStream()
-    : video_stats_dispatcher_(kStreamLabel), weak_factory_(this) {
+WebrtcVideoStream::WebrtcVideoStream(const SessionOptions& session_options)
+    : video_stats_dispatcher_(kStreamLabel),
+      session_options_(session_options),
+      weak_factory_(this) {
   encoder_selector_.RegisterEncoder(
       base::Bind(&WebrtcVideoEncoderVpx::IsSupportedByVP8),
       base::Bind(&WebrtcVideoEncoderVpx::CreateForVP8));
@@ -139,7 +141,7 @@ void WebrtcVideoStream::Start(
   result = peer_connection_->AddStream(stream_.get());
   DCHECK(result);
 
-  scheduler_.reset(new WebrtcFrameSchedulerSimple());
+  scheduler_.reset(new WebrtcFrameSchedulerSimple(session_options_));
   scheduler_->Start(
       webrtc_transport_->video_encoder_factory(),
       base::Bind(&WebrtcVideoStream::CaptureNextFrame, base::Unretained(this)));

remoting/protocol/webrtc_video_stream.h

@@ -15,6 +15,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_checker.h"
+#include "remoting/base/session_options.h"
 #include "remoting/codec/webrtc_video_encoder.h"
 #include "remoting/codec/webrtc_video_encoder_selector.h"
 #include "remoting/protocol/host_video_stats_dispatcher.h"
@@ -38,7 +39,7 @@ class WebrtcVideoStream : public VideoStream,
                           public webrtc::DesktopCapturer::Callback,
                           public HostVideoStatsDispatcher::EventHandler {
  public:
-  WebrtcVideoStream();
+  explicit WebrtcVideoStream(const SessionOptions& options);
   ~WebrtcVideoStream() override;
 
   void Start(std::unique_ptr<webrtc::DesktopCapturer> desktop_capturer,
@@ -101,6 +102,8 @@ class WebrtcVideoStream : public VideoStream,
 
   base::ThreadChecker thread_checker_;
 
+  const SessionOptions session_options_;
+
   base::WeakPtrFactory<WebrtcVideoStream> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(WebrtcVideoStream);