Refactor ResizingHostObserver tests

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 08713f1b4aa33072b89331f9fa4a41c80e59d81c..5c43fd51a22331b010a28e1dc357bde189e3d29f 100644
--- a/remoting/host/resizing_host_observer_unittest.cc
+++ b/remoting/host/resizing_host_observer_unittest.cc
@@ -40,64 +40,60 @@ ScreenResolution MakeResolution(int width, int height) {
 
 class FakeDesktopResizer : public DesktopResizer {
  public:
-  FakeDesktopResizer(const ScreenResolution& initial_resolution,
-                     bool exact_size_supported,
-                     const ScreenResolution* supported_resolutions,
-                     int num_supported_resolutions,
-                     int* restore_resolution_call_count)
-      : initial_resolution_(initial_resolution),
-        current_resolution_(initial_resolution),
-        exact_size_supported_(exact_size_supported),
-        set_resolution_call_count_(0),
-        restore_resolution_call_count_(restore_resolution_call_count) {
-    for (int i = 0; i < num_supported_resolutions; ++i) {
-      supported_resolutions_.push_back(supported_resolutions[i]);
-    }
+  struct CallCounts {
+    int set_resolution = 0;
+    int restore_resolution = 0;
+  };
+
+  FakeDesktopResizer(bool exact_size_supported,
+                     std::vector<ScreenResolution> supported_resolutions,
+                     ScreenResolution* current_resolution,
+                     CallCounts* call_counts)
+      : exact_size_supported_(exact_size_supported),
+        initial_resolution_(*current_resolution),
+        current_resolution_(current_resolution),
+        supported_resolutions_(std::move(supported_resolutions)),
+        call_counts_(call_counts) {
   }
 
   ~FakeDesktopResizer() override {
     EXPECT_EQ(initial_resolution_, GetCurrentResolution());
   }
 
-  int set_resolution_call_count() { return set_resolution_call_count_; }
-
   // remoting::DesktopResizer interface
   ScreenResolution GetCurrentResolution() override {
-    return current_resolution_;
+    return *current_resolution_;
   }
   std::list<ScreenResolution> GetSupportedResolutions(
       const ScreenResolution& preferred) override {
-    std::list<ScreenResolution> result = supported_resolutions_;
+    std::list<ScreenResolution> result(supported_resolutions_.begin(),
+                                       supported_resolutions_.end());
     if (exact_size_supported_) {
       result.push_back(preferred);
     }
     return result;
   }
   void SetResolution(const ScreenResolution& resolution) override {
-    current_resolution_ = resolution;
-    ++set_resolution_call_count_;
+    *current_resolution_ = resolution;
+    ++call_counts_->set_resolution;
   }
   void RestoreResolution(const ScreenResolution& resolution) override {
-    current_resolution_ = resolution;
-    if (restore_resolution_call_count_)
-      ++(*restore_resolution_call_count_);
+    *current_resolution_ = resolution;
+    ++call_counts_->restore_resolution;
   }
 
  private:
-  ScreenResolution initial_resolution_;
-  ScreenResolution current_resolution_;
   bool exact_size_supported_;
-  std::list<ScreenResolution> supported_resolutions_;
-
-  int set_resolution_call_count_;
-  int* restore_resolution_call_count_;
+  ScreenResolution initial_resolution_;
+  ScreenResolution *current_resolution_;
+  std::vector<ScreenResolution> supported_resolutions_;
+  CallCounts* call_counts_;
 };
 
 class ResizingHostObserverTest : public testing::Test {
  public:
   ResizingHostObserverTest()
-      : desktop_resizer_(nullptr),
-        now_(base::Time::Now()) {
+      : now_(base::Time::Now()) {
   }
 
   // This needs to be public because the derived test-case class needs to
@@ -107,12 +103,16 @@ class ResizingHostObserverTest : public testing::Test {
   }
 
  protected:
-  void SetDesktopResizer(std::unique_ptr<FakeDesktopResizer> desktop_resizer) {
-    CHECK(!desktop_resizer_) << "Call SetDeskopResizer once per test";
-    desktop_resizer_ = desktop_resizer.get();
-
-    resizing_host_observer_.reset(
-        new ResizingHostObserver(std::move(desktop_resizer)));
+  void InitDesktopResizer(const ScreenResolution& initial_resolution,
+                         bool exact_size_supported,
+                         std::vector<ScreenResolution> supported_resolutions) {
+    current_resolution_ = initial_resolution;
+    call_counts_ = FakeDesktopResizer::CallCounts();
+    resizing_host_observer_ = base::MakeUnique<ResizingHostObserver>(
+        base::MakeUnique<FakeDesktopResizer>(exact_size_supported,
+                                             std::move(supported_resolutions),
+                                             &current_resolution_,
+                                             &call_counts_));
     resizing_host_observer_->SetNowFunctionForTesting(
         base::Bind(&ResizingHostObserverTest::GetTimeAndIncrement,
                    base::Unretained(this)));
@@ -120,16 +120,18 @@ class ResizingHostObserverTest : public testing::Test {
 
   ScreenResolution GetBestResolution(const ScreenResolution& client_size) {
     resizing_host_observer_->SetScreenResolution(client_size);
-    return desktop_resizer_->GetCurrentResolution();
+    return current_resolution_;
   }
 
-  void VerifySizes(const ScreenResolution* client_sizes,
-                   const ScreenResolution* expected_sizes,
-                   int number_of_sizes) {
-    for (int i = 0; i < number_of_sizes; ++i) {
-      ScreenResolution best_size = GetBestResolution(client_sizes[i]);
-      EXPECT_EQ(expected_sizes[i], best_size)
-          << "Input resolution = " << client_sizes[i];
+  void VerifySizes(const std::vector<ScreenResolution>& client_sizes,
+                   const std::vector<ScreenResolution>& expected_sizes) {
+    CHECK_EQ(client_sizes.length(), expected_sizes.length())
+        << "Client and expected vectors must have the same length";

[rkjnsn, 2016/06/06 18:26:47]

Is CHECK the appropriate thing to use here?

[Jamie, 2016/06/06 19:12:42]

See my other comment. Since a failure here indicates a bug in the test code,
either ASSERT or CHECK would be appropriate. You could grep our test code to see
which is more common if you're interested.

+    for (auto client = client_sizes.begin(), expected = expected_sizes.begin();
+         client != client_sizes.end() && expected != expected_sizes.end();
+         ++client, ++expected) {
+      ScreenResolution best_size = GetBestResolution(*client);
+      EXPECT_EQ(*expected, best_size) << "Input resolution = " << *client;
     }
   }
 
@@ -139,141 +141,111 @@ class ResizingHostObserverTest : public testing::Test {
     return result;
   }
 
+  ScreenResolution current_resolution_;
+  FakeDesktopResizer::CallCounts call_counts_;
   std::unique_ptr<ResizingHostObserver> resizing_host_observer_;
-  FakeDesktopResizer* desktop_resizer_;
   base::Time now_;
 };
 
 // Check that the resolution isn't restored if it wasn't changed by this class.
 TEST_F(ResizingHostObserverTest, NoRestoreResolution) {
-  int restore_resolution_call_count = 0;
-  ScreenResolution initial = MakeResolution(640, 480);
-  std::unique_ptr<FakeDesktopResizer> desktop_resizer(new FakeDesktopResizer(
-      initial, false, nullptr, 0, &restore_resolution_call_count));
-  SetDesktopResizer(std::move(desktop_resizer));
-  VerifySizes(nullptr, nullptr, 0);
+  InitDesktopResizer(MakeResolution(640, 480), false, {});
   resizing_host_observer_.reset();
-  EXPECT_EQ(0, restore_resolution_call_count);
+  EXPECT_EQ(0, call_counts_.restore_resolution);
 }
 
 // Check that the host is not resized if GetSupportedSizes returns an empty
 // list (even if GetCurrentSize is supported).
 TEST_F(ResizingHostObserverTest, EmptyGetSupportedSizes) {
-  int restore_resolution_call_count = 0;
   ScreenResolution initial = MakeResolution(640, 480);
-  std::unique_ptr<FakeDesktopResizer> desktop_resizer(new FakeDesktopResizer(
-      initial, false, nullptr, 0, &restore_resolution_call_count));
-  SetDesktopResizer(std::move(desktop_resizer));
-
-  ScreenResolution client_sizes[] = { MakeResolution(200, 100),
-                                      MakeResolution(100, 200) };
-  ScreenResolution expected_sizes[] = { initial, initial };
-  VerifySizes(client_sizes, expected_sizes, arraysize(client_sizes));
-
+  InitDesktopResizer(initial, false, {});
+  VerifySizes({MakeResolution(200, 100), MakeResolution(100, 200)},
+              {initial, initial});
   resizing_host_observer_.reset();
-  EXPECT_EQ(0, restore_resolution_call_count);
+  EXPECT_EQ(0, call_counts_.set_resolution);
+  EXPECT_EQ(0, call_counts_.restore_resolution);
 }
 
 // Check that if the implementation supports exact size matching, it is used.
 TEST_F(ResizingHostObserverTest, SelectExactSize) {
-  int restore_resolution_call_count = 0;
-  std::unique_ptr<FakeDesktopResizer> desktop_resizer(
-      new FakeDesktopResizer(MakeResolution(640, 480), true, nullptr, 0,
-                             &restore_resolution_call_count));
-  SetDesktopResizer(std::move(desktop_resizer));
-
-  ScreenResolution client_sizes[] = { MakeResolution(200, 100),
-                                      MakeResolution(100, 200),
-                                      MakeResolution(640, 480),
-                                      MakeResolution(480, 640),
-                                      MakeResolution(1280, 1024) };
-  VerifySizes(client_sizes, client_sizes, arraysize(client_sizes));
+  InitDesktopResizer(MakeResolution(640, 480), true, {});
+  std::vector<ScreenResolution> client_sizes = {MakeResolution(200, 100),
+                                                MakeResolution(100, 200),
+                                                MakeResolution(640, 480),
+                                                MakeResolution(480, 640),
+                                                MakeResolution(1280, 1024)};
+  VerifySizes(client_sizes, client_sizes);
   resizing_host_observer_.reset();
-  EXPECT_EQ(1, restore_resolution_call_count);
+  EXPECT_EQ(1, call_counts_.restore_resolution);
 }
 
 // Check that if the implementation supports a size that is no larger than
 // the requested size, then the largest such size is used.
 TEST_F(ResizingHostObserverTest, SelectBestSmallerSize) {
-  ScreenResolution supported_sizes[] = { MakeResolution(639, 479),
-                                         MakeResolution(640, 480) };
-  std::unique_ptr<FakeDesktopResizer> desktop_resizer(
-      new FakeDesktopResizer(MakeResolution(640, 480), false, supported_sizes,
-                             arraysize(supported_sizes), nullptr));
-  SetDesktopResizer(std::move(desktop_resizer));
-
-  ScreenResolution client_sizes[] = { MakeResolution(639, 479),
-                                      MakeResolution(640, 480),
-                                      MakeResolution(641, 481),
-                                      MakeResolution(999, 999) };
-  ScreenResolution expected_sizes[] = { supported_sizes[0], supported_sizes[1],
-                               supported_sizes[1], supported_sizes[1] };
-  VerifySizes(client_sizes, expected_sizes, arraysize(client_sizes));
+  std::vector<ScreenResolution> supported_sizes = {MakeResolution(639, 479),
+                                                   MakeResolution(640, 480)};
+  InitDesktopResizer(MakeResolution(640, 480), false, supported_sizes);
+  VerifySizes({MakeResolution(639, 479),
+               MakeResolution(640, 480),
+               MakeResolution(641, 481),
+               MakeResolution(999, 999)},
+              {supported_sizes[0],
+               supported_sizes[1],
+               supported_sizes[1],
+               supported_sizes[1]});
 }
 
 // Check that if the implementation supports only sizes that are larger than
 // the requested size, then the one that requires the least down-scaling.
 TEST_F(ResizingHostObserverTest, SelectBestScaleFactor) {
-  ScreenResolution supported_sizes[] = { MakeResolution(100, 100),
-                                         MakeResolution(200, 100) };
-  std::unique_ptr<FakeDesktopResizer> desktop_resizer(
-      new FakeDesktopResizer(MakeResolution(200, 100), false, supported_sizes,
-                             arraysize(supported_sizes), nullptr));
-  SetDesktopResizer(std::move(desktop_resizer));
-
-  ScreenResolution client_sizes[] = { MakeResolution(1, 1),
-                                      MakeResolution(99, 99),
-                                      MakeResolution(199, 99) };
-  ScreenResolution expected_sizes[] = { supported_sizes[0], supported_sizes[0],
-                               supported_sizes[1] };
-  VerifySizes(client_sizes, expected_sizes, arraysize(client_sizes));
+  std::vector<ScreenResolution> supported_sizes = {MakeResolution(100, 100),
+                                                   MakeResolution(200, 100)};
+  InitDesktopResizer(MakeResolution(200, 100), false, supported_sizes);
+  VerifySizes({MakeResolution(1, 1),
+               MakeResolution(99, 99),
+               MakeResolution(199, 99)},
+              {supported_sizes[0],
+               supported_sizes[0],
+               supported_sizes[1]});
 }
 
 // Check that if the implementation supports two sizes that have the same
 // resultant scale factor, then the widest one is selected.
 TEST_F(ResizingHostObserverTest, SelectWidest) {
-  ScreenResolution supported_sizes[] = { MakeResolution(640, 480),
-                                         MakeResolution(480, 640) };
-  std::unique_ptr<FakeDesktopResizer> desktop_resizer(
-      new FakeDesktopResizer(MakeResolution(480, 640), false, supported_sizes,
-                             arraysize(supported_sizes), nullptr));
-  SetDesktopResizer(std::move(desktop_resizer));
-
-  ScreenResolution client_sizes[] = { MakeResolution(100, 100),
-                                      MakeResolution(480, 480),
-                                      MakeResolution(500, 500),
-                                      MakeResolution(640, 640),
-                                      MakeResolution(1000, 1000) };
-  ScreenResolution expected_sizes[] = { supported_sizes[0], supported_sizes[0],
-                               supported_sizes[0], supported_sizes[0],
-                               supported_sizes[0] };
-  VerifySizes(client_sizes, expected_sizes, arraysize(client_sizes));
+  std::vector<ScreenResolution> supported_sizes = {MakeResolution(640, 480),
+                                                   MakeResolution(480, 640)};
+  InitDesktopResizer(MakeResolution(480, 640), false, supported_sizes);
+  VerifySizes({MakeResolution(100, 100),
+               MakeResolution(480, 480),
+               MakeResolution(500, 500),
+               MakeResolution(640, 640),
+               MakeResolution(1000, 1000)},
+              {supported_sizes[0],
+               supported_sizes[0],
+               supported_sizes[0],
+               supported_sizes[0],
+               supported_sizes[0]});
 }
 
 // Check that if the best match for the client size doesn't change, then we
 // don't call SetSize.
 TEST_F(ResizingHostObserverTest, NoSetSizeForSameSize) {
-  ScreenResolution supported_sizes[] = { MakeResolution(640, 480),
-                                         MakeResolution(480, 640) };
-  SetDesktopResizer(base::WrapUnique(
-      new FakeDesktopResizer(MakeResolution(480, 640), false, supported_sizes,
-                             arraysize(supported_sizes), nullptr)));
-
-  ScreenResolution client_sizes[] = { MakeResolution(640, 640),
-                                      MakeResolution(1024, 768),
-                                      MakeResolution(640, 480) };
-  ScreenResolution expected_sizes[] = { MakeResolution(640, 480),
-                                        MakeResolution(640, 480),
-                                        MakeResolution(640, 480) };
-  VerifySizes(client_sizes, expected_sizes, arraysize(client_sizes));
-  EXPECT_EQ(desktop_resizer_->set_resolution_call_count(), 1);
+  std::vector<ScreenResolution> supported_sizes = {MakeResolution(640, 480),
+                                                   MakeResolution(480, 640)};
+  InitDesktopResizer(MakeResolution(480, 640), false, supported_sizes);
+  VerifySizes({MakeResolution(640, 640),
+               MakeResolution(1024, 768),
+               MakeResolution(640, 480)},
+              {supported_sizes[0],
+               supported_sizes[0],
+               supported_sizes[0]});
+  EXPECT_EQ(1, call_counts_.set_resolution);
 }
 
 // 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) {
-  SetDesktopResizer(base::WrapUnique(new FakeDesktopResizer(
-      MakeResolution(640, 480), true, nullptr, 0, nullptr)));
+  InitDesktopResizer(MakeResolution(640, 480), true, {});
   resizing_host_observer_->SetNowFunctionForTesting(
       base::Bind(&ResizingHostObserverTest::GetTime, base::Unretained(this)));
 
@@ -302,7 +274,7 @@ TEST_F(ResizingHostObserverTest, RateLimited) {
   run_loop.Run();
 
   // If the QuitClosure fired before the final resize, it's a test failure.
-  EXPECT_EQ(desktop_resizer_->GetCurrentResolution(), MakeResolution(300, 300));
+  EXPECT_EQ(current_resolution_, MakeResolution(300, 300));
 }
 
 }  // namespace remoting