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

remoting/host/resizing_host_observer.cc

Index: remoting/host/resizing_host_observer.cc
diff --git a/remoting/host/resizing_host_observer.cc b/remoting/host/resizing_host_observer.cc
index ccb141feb205caa6f588268b42701235c0976053..ed25a593611bd4674d31ea30e04818b8fcaeda26 100644
--- a/remoting/host/resizing_host_observer.cc
+++ b/remoting/host/resizing_host_observer.cc
@@ -6,12 +6,16 @@
 
 #include <list>
 
+#include "base/bind.h"
 #include "base/logging.h"
+#include "base/message_loop.h"
 #include "remoting/host/desktop_resizer.h"
 #include "remoting/host/screen_resolution.h"
 
 namespace {
 
+const int kMinimumResizeIntervalMs = 1000;

[Jamie, 2013/06/03 23:26:28]

This value is taken directly from the previous implementation. The bug report
suggests making this configurable per-platform. I don't think that's worth doing
unless we have some data on what suitable resize times might be on each
platform.

[alexeypa (please no reviews), 2013/06/04 00:05:26]

nit #1: Add a comment explaining what this timeout is for.

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

Done.

+
 class CandidateSize {
  public:
   CandidateSize(const SkISize& candidate, const SkISize& preferred)
@@ -101,7 +105,9 @@ namespace remoting {
 ResizingHostObserver::ResizingHostObserver(
     scoped_ptr<DesktopResizer> desktop_resizer)
     : desktop_resizer_(desktop_resizer.Pass()),
-      original_size_(desktop_resizer_->GetCurrentSize()) {
+      original_size_(desktop_resizer_->GetCurrentSize()),
+      resize_pending_(false),
+      weak_factory_(this) {
 }
 
 ResizingHostObserver::~ResizingHostObserver() {
@@ -114,17 +120,37 @@ void ResizingHostObserver::SetScreenResolution(
   if (resolution.IsEmpty())
     return;
 
+  // Resizing the desktop too often is probably not a good idea, so apply a
+  // simple rate-limiting scheme.
+  base::TimeDelta time_since_previous_resize =
+      base::Time::Now() - previous_resize_time_;

[alexeypa (please no reviews), 2013/06/04 00:05:26]

nit: This looks like potential integer overflow.

if (previous_resize_time_ +
base::TimeDelta::FromMilliseconds(kMinimumResizeIntervalMs) <=
base::Time::Now()) {
  ...
}

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

I've rewritten it to something I think is a bit clearer than both my original
and your suggested replacement. LMKWYT.

+  base::TimeDelta time_until_resize_permitted =
+      base::TimeDelta::FromMilliseconds(kMinimumResizeIntervalMs) -
+      time_since_previous_resize;
+  if (time_until_resize_permitted > base::TimeDelta::FromMilliseconds(0)) {
+    pending_resolution_ = resolution;
+    if (!resize_pending_) {
+      resize_pending_ = true;
+      base::MessageLoop::current()->PostDelayedTask(

[alexeypa (please no reviews), 2013/06/04 00:05:26]

nit: Actually base::OneShotTimer expresses the intent much better. 

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

Nice, I was wasn't familiar with that.

+          FROM_HERE,
+          base::Bind(&ResizingHostObserver::SetPendingScreenResolution,
+                     weak_factory_.GetWeakPtr()),
+          time_until_resize_permitted);
+    }
+    return;
+  }
+
   // If the implementation returns any sizes, pick the best one according to
   // the algorithm described in CandidateSize::IsBetterThen.
-  SkISize dimentions = SkISize::Make(
+  SkISize dimensions = SkISize::Make(
       resolution.dimensions().width(), resolution.dimensions().height());
-  std::list<SkISize> sizes = desktop_resizer_->GetSupportedSizes(dimentions);
+  std::list<SkISize> sizes = desktop_resizer_->GetSupportedSizes(dimensions);
   if (sizes.empty())
     return;
-  CandidateSize best_size(sizes.front(), dimentions);
+  CandidateSize best_size(sizes.front(), dimensions);
   for (std::list<SkISize>::const_iterator i = ++sizes.begin();
        i != sizes.end(); ++i) {
-    CandidateSize candidate_size(*i, dimentions);
+    CandidateSize candidate_size(*i, dimensions);
     if (candidate_size.IsBetterThan(best_size)) {
       best_size = candidate_size;
     }
@@ -132,6 +158,14 @@ void ResizingHostObserver::SetScreenResolution(
   SkISize current_size = desktop_resizer_->GetCurrentSize();
   if (best_size.size() != current_size)
     desktop_resizer_->SetSize(best_size.size());
+
+  // Update the time of last resize to allow it to be rate-limited.
+  previous_resize_time_ = base::Time::Now();
+}
+
+void ResizingHostObserver::SetPendingScreenResolution() {
+  resize_pending_ = false;

[alexeypa (please no reviews), 2013/06/04 00:05:26]

nit: This flag and the whole function is not needed when base::OneShotTimer is
used. SetScreenResolution() will simply rearm the timer every time a resize
message arrives too early.

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

Done.

+  SetScreenResolution(pending_resolution_);
 }
 
 }  // namespace remoting