Add host-side rate-limiting to desktop resize events.

remoting/host/client_session.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/host/client_session.h"
 
 #include <algorithm>
 
 #include "base/message_loop_proxy.h"
 #include "media/video/capture/screen/screen_capturer.h"
@@ skipping 13 matching lines @@
 #include "remoting/host/screen_resolution.h"
 #include "remoting/host/video_scheduler.h"
 #include "remoting/proto/control.pb.h"
 #include "remoting/proto/event.pb.h"
 #include "remoting/protocol/client_stub.h"
 #include "remoting/protocol/clipboard_thread_proxy.h"
 
 // Default DPI to assume for old clients that use notifyClientDimensions.
 const int kDefaultDPI = 96;
 
+// All DesktopEnvironment implementations rate-limit desktop resize requests,
+// so this Capability is added automatically.
+const char kRateLimitResizeRequests[] = "rateLimitResizeRequests";
+
 namespace remoting {
 
 ClientSession::ClientSession(
     EventHandler* event_handler,
     scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> input_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> video_capture_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> video_encode_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
@@ skipping 160 matching lines @@
   // Create the desktop environment. Drop the connection if it could not be
   // created for any reason (for instance the curtain could not initialize).
   desktop_environment_ =
       desktop_environment_factory_->Create(control_factory_.GetWeakPtr());
   if (!desktop_environment_) {
     DisconnectSession();
     return;
   }
 
   host_capabilities_ = desktop_environment_->GetCapabilities();
+  host_capabilities_ = host_capabilities_ + " " + kRateLimitResizeRequests;

[Jamie, 2013/06/04 01:27:49]

Is this an acceptable way of getting the capability registered (as opposed to
doing so in every DesktopEnvironment)?

[alexeypa (please no reviews), 2013/06/04 17:07:37]

No, I don't think we should do it this way. I think each desktop environment
should explicitly specify this capability. I realize that it means more code has
to be written but there are two compelling reasons to do it:

1. It is very easy to miss that all DE instances should implement rate-limiting
unless rate limiting is implemented in ClientSession. (which is probably not a
good idea too).
2. There are at least two kinds DE that don't implement rate limiting even with
this CL applied: It2MeDesktopEnvironment and IpcDesktopEnvironment. The latter
also invokes different code for curtained (RdpDesktopSession) and not curtained
(Me2meDesktopEnvironment) sessions.

[Jamie, 2013/06/04 21:18:03]

Done. However, see my comments below.
Rate-limiting is implemented by a platform independent class, so any desktop
environment that supports resize gets it automatically. If a desktop environment
doesn't support resize, then we end up reporting that it rate-limits resize
requests, which is misleading, but not harmful afaict.
I believe that IpcDesktopEnvironment does implement rate-limiting, as long as
the underlying DE does. In any case, with the change as I've implemented it now,
it still reports that it implements rate-limiting without consulting the
underlying DE, as that would require extending the IPC, which is overkill imo.

 
   // Ignore protocol::Capabilities messages from the client if it does not
   // support any capabilities.
   if (!connection_->session()->config().SupportsCapabilities()) {
     VLOG(1) << "The client does not support any capabilities.";
 
     client_capabilities_ = make_scoped_ptr(new std::string());
     desktop_environment_->SetCapabilities(*client_capabilities_);
   }
 
@@ skipping 192 matching lines @@
     return scoped_ptr<AudioEncoder>(new AudioEncoderSpeex());
   } else if (audio_config.codec == protocol::ChannelConfig::CODEC_OPUS) {
     return scoped_ptr<AudioEncoder>(new AudioEncoderOpus());
   }
 
   NOTIMPLEMENTED();
   return scoped_ptr<AudioEncoder>(NULL);
 }
 
 }  // namespace remoting