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

remoting/host/resizing_host_observer_unittest.cc

Index: remoting/host/resizing_host_observer_unittest.cc
diff --git a/remoting/host/resizing_host_observer_unittest.cc b/remoting/host/resizing_host_observer_unittest.cc
index 2d61e5e4e8dd312d0f1508e028d303c2bf2cc31e..4b665cbe83c519a26abc7aeb39c8656b292eafe9 100644
--- a/remoting/host/resizing_host_observer_unittest.cc
+++ b/remoting/host/resizing_host_observer_unittest.cc
@@ -6,6 +6,8 @@
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/message_loop.h"
+#include "base/run_loop.h"
 #include "remoting/host/desktop_resizer.h"
 #include "remoting/host/resizing_host_observer.h"
 #include "remoting/host/screen_resolution.h"
@@ -80,6 +82,8 @@ class ResizingHostObserverTest : public testing::Test {
 
     resizing_host_observer_.reset(
         new ResizingHostObserver(desktop_resizer.PassAs<DesktopResizer>()));
+    resizing_host_observer_->SetMinimumResizeIntervalForTesting(
+        base::TimeDelta::FromMilliseconds(0));
   }
 
   SkISize GetBestSize(const SkISize& client_size) {
@@ -98,7 +102,7 @@ class ResizingHostObserverTest : public testing::Test {
     }
   }
 
- private:
+ protected:
   scoped_ptr<ResizingHostObserver> resizing_host_observer_;
   FakeDesktopResizer* desktop_resizer_;
 };
@@ -191,4 +195,30 @@ TEST_F(ResizingHostObserverTest, NoSetSizeForSameSize) {
   EXPECT_EQ(desktop_resizer->set_size_call_count(), 0);
 }
 
+// Check that desktop resizes are rate-limited, and that if multiple resize
+// requests are received in the time-out period, the most recent is respected.
+TEST_F(ResizingHostObserverTest, RateLimited) {
+  int interval_ms = 10;

[alexeypa (please no reviews), 2013/06/06 21:45:11]

1ms should be good enough. All we need is to cause a task to be posted to the
message loop. Any non zero interval will work.

Also, there is no need to make it a variable. Remove |interval_ms|.

[Jamie, 2013/06/06 22:08:04]

There is a small chance that it might take > 1ms between the first two calls to
GetBestSize, which would make the test flaky. It's very unlikely, but I think an
extra 9ms is worth it in this case. LMK if you still disagree and I'll change it
as you suggest.
It's also used as a parameter to PostDelayedTask, so I'd prefer to keep it.

[alexeypa (please no reviews), 2013/06/07 16:23:10]

Oh, I see. So the first SetScreenResoultion() records base::Time::Now() but by
the time of the seconds call base::Time::Now() can already return an updated
value. In this case 10ms is not enough. The precision of base::Time::Now() is
18ms or so on Windows. Also if the machine running the test is under heavy load
the thread could easily be suspended for longer time. I.e. this is flaky.

Another approach we can try is instead of setting the timeout via
SetMinimumResizeIntervalForTesting() the test should control whether the next
resize attempt should be allowed or result in a PostTask (Arming the timer for
0ms seems to be doing PostTask).

+  FakeDesktopResizer* desktop_resizer =
+      new FakeDesktopResizer(SkISize::Make(640, 480), true, NULL, 0);
+  SetDesktopResizer(scoped_ptr<FakeDesktopResizer>(desktop_resizer));
+  resizing_host_observer_->SetMinimumResizeIntervalForTesting(
+      base::TimeDelta::FromMilliseconds(interval_ms));
+
+  base::MessageLoop message_loop;
+  base::RunLoop run_loop;
+
+  EXPECT_EQ(GetBestSize(SkISize::Make(100, 100)), SkISize::Make(100, 100));

[alexeypa (please no reviews), 2013/06/06 21:45:11]

If I read this correctly, it does not check whether the size was eventually set.
Can you add an expectation that size will be changed twice? You will be able to
bind posting of QuitClosure() to this expectation.

[Jamie, 2013/06/06 22:08:04]

That expectation is at the end of the test, after the RunLoop exits.

+  EXPECT_EQ(GetBestSize(SkISize::Make(200, 200)), SkISize::Make(100, 100));
+  EXPECT_EQ(GetBestSize(SkISize::Make(300, 300)), SkISize::Make(100, 100));
+
+  message_loop.PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&base::RunLoop::Quit, base::Unretained(&run_loop)),

[alexeypa (please no reviews), 2013/06/06 21:45:11]

Use |run_loop.QuitClosure()| instead of base::Bind.

[Jamie, 2013/06/06 22:08:04]

Done.

+      base::TimeDelta::FromMilliseconds(interval_ms + 1));
+  run_loop.Run();
+
+  EXPECT_EQ(desktop_resizer_->GetCurrentSize(), SkISize::Make(300, 300));

[alexeypa (please no reviews), 2013/06/07 16:23:10]

This expectation is flaky. Posting QuitClosure() before the timeout elapses will
exit the loop before the delayed tasks have a chance to be executed.

+}
+
 }  // namespace remoting