Add support for virtual displays

ui/display/chromeos/display_configurator_unittest.cc

 // Copyright 2014 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 "ui/display/chromeos/display_configurator.h"
 
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/display/chromeos/test/action_logger_util.h"
@@ skipping 94 matching lines @@
   bool SoftwareMirroringEnabled() const override {
     return software_mirroring_enabled_;
   }
 
  private:
   bool software_mirroring_enabled_;
 
   DISALLOW_COPY_AND_ASSIGN(TestMirroringController);
 };
 
+}  // namespace
+
 class DisplayConfiguratorTest : public testing::Test {
  public:
   enum CallbackResult {
     CALLBACK_FAILURE,
     CALLBACK_SUCCESS,
     CALLBACK_NOT_CALLED,
   };
 
   DisplayConfiguratorTest()
       : small_mode_(gfx::Size(1366, 768), false, 60.0f),
@@ skipping 127 matching lines @@
 
   TestDisplaySnapshot outputs_[2];
 
   CallbackResult callback_result_;
   CallbackResult display_control_result_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DisplayConfiguratorTest);
 };
 
-}  // namespace
-
 TEST_F(DisplayConfiguratorTest, FindDisplayModeMatchingSize) {
   ScopedVector<const DisplayMode> modes;
 
   // Fields are width, height, interlaced, refresh rate.
   modes.push_back(new DisplayMode(gfx::Size(1920, 1200), false, 60.0));
   DisplayMode* native_mode =
       new DisplayMode(gfx::Size(1920, 1200), false, 50.0);
   modes.push_back(native_mode);
   // Different rates.
   modes.push_back(new DisplayMode(gfx::Size(1920, 1080), false, 30.0));
@@ skipping 56 matching lines @@
   EXPECT_EQ(modes[15],
             DisplayConfigurator::FindDisplayModeMatchingSize(
                 output, gfx::Size(320, 200)));
 
   // No mode found.
   EXPECT_EQ(NULL,
             DisplayConfigurator::FindDisplayModeMatchingSize(
                 output, gfx::Size(1440, 900)));
 }
 
+TEST_F(DisplayConfiguratorTest, EnableVirtualDisplay) {
+  InitWithSingleOutput();
+
+  observer_.Reset();
+  const DisplayConfigurator::DisplayStateList* cached =
+      &configurator_.cached_displays();
+  ASSERT_EQ(static_cast<size_t>(1), cached->size());
+  EXPECT_EQ(small_mode_.size(), (*cached)[0]->current_mode()->size());
+
+  // Add virtual display.
+  int64_t virtual_display_id =
+      configurator_.AddVirtualDisplay(big_mode_.size());
+  EXPECT_FALSE(mirroring_controller_.SoftwareMirroringEnabled());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED,
+            configurator_.display_state());
+
+  // Virtual should not trigger addition of added crtc but does change FB
+  // height.
+  const int kVirtualHeight = small_mode_.size().height() +
+                             DisplayConfigurator::kVerticalGap +
+                             big_mode_.size().height();
+  EXPECT_EQ(
+      JoinActions(
+          kGrab, GetFramebufferAction(
+                     gfx::Size(big_mode_.size().width(), kVirtualHeight),
+                     &outputs_[0], NULL)
+                     .c_str(),
+          GetCrtcAction(outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
+          kUngrab, NULL),
+      log_->GetActionsAndClear());
+  EXPECT_EQ(1, observer_.num_changes());
+  cached = &configurator_.cached_displays();
+  ASSERT_EQ(static_cast<size_t>(2), cached->size());
+  EXPECT_EQ(small_mode_.size(), (*cached)[0]->current_mode()->size());
+  EXPECT_EQ(big_mode_.size(), (*cached)[1]->current_mode()->size());
+  EXPECT_EQ(virtual_display_id, (*cached)[1]->display_id());
+
+  // Remove virtual display.
+  observer_.Reset();
+  EXPECT_TRUE(configurator_.RemoveVirtualDisplay(virtual_display_id));
+  EXPECT_EQ(
+      JoinActions(
+          kGrab,
+          GetFramebufferAction(small_mode_.size(), &outputs_[0], NULL).c_str(),
+          GetCrtcAction(outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
+          kUngrab, NULL),
+      log_->GetActionsAndClear());
+  EXPECT_EQ(1, observer_.num_changes());
+  cached = &configurator_.cached_displays();
+  ASSERT_EQ(static_cast<size_t>(1), cached->size());
+  EXPECT_EQ(small_mode_.size(), (*cached)[0]->current_mode()->size());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_SINGLE, configurator_.display_state());
+}
+
+TEST_F(DisplayConfiguratorTest, EnableTwoVirtualDisplays) {
+  InitWithSingleOutput();
+
+  observer_.Reset();
+  const DisplayConfigurator::DisplayStateList* cached =

[jdufault, 2015/11/17 20:10:21]

Any reason this is a pointer? Can we just copy cached_displays, given that we're
always updating the cached list before checking it?

It looks like UpdateCachedOutputsEvenAfterFailure does the same thing you have
here - maybe change the logic there too?

[robert.bradford, 2015/11/18 12:46:10]

Good catch. As you spotted, I must have copied this from the above test, which
has the same mistake. By taking the address of the const ref to the internal
vector the goal was not to have cheap access to the internal datastructure of
the DisplayConfigurator.

I've dropped the unnecessary calls to cached_displays() in all the tests.

+      &configurator_.cached_displays();
+  ASSERT_EQ(static_cast<size_t>(1), cached->size());
+  EXPECT_EQ(small_mode_.size(), (*cached)[0]->current_mode()->size());
+
+  // Add 1st virtual display.
+  int64_t virtual_display_id_1 =
+      configurator_.AddVirtualDisplay(big_mode_.size());
+  EXPECT_FALSE(mirroring_controller_.SoftwareMirroringEnabled());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED,
+            configurator_.display_state());
+
+  // Virtual should not trigger addition of added crtc but does change FB
+  // height.
+  const int kSingleVirtualHeight = small_mode_.size().height() +
+                                   DisplayConfigurator::kVerticalGap +
+                                   big_mode_.size().height();
+  EXPECT_EQ(
+      JoinActions(
+          kGrab, GetFramebufferAction(
+                     gfx::Size(big_mode_.size().width(), kSingleVirtualHeight),
+                     &outputs_[0], NULL)
+                     .c_str(),
+          GetCrtcAction(outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
+          kUngrab, NULL),
+      log_->GetActionsAndClear());
+  EXPECT_EQ(1, observer_.num_changes());
+  cached = &configurator_.cached_displays();

[jdufault, 2015/11/17 20:10:21]

If cached remains a pointer/reference, is this needed?

[robert.bradford, 2015/11/18 12:46:10]

Acknowledged.

+  ASSERT_EQ(static_cast<size_t>(2), cached->size());
+  EXPECT_EQ(small_mode_.size(), (*cached)[0]->current_mode()->size());
+  EXPECT_EQ(big_mode_.size(), (*cached)[1]->current_mode()->size());
+  EXPECT_EQ(virtual_display_id_1, (*cached)[1]->display_id());
+
+  // Add 2nd virtual display
+  int64_t virtual_display_id_2 =
+      configurator_.AddVirtualDisplay(big_mode_.size());
+  EXPECT_FALSE(mirroring_controller_.SoftwareMirroringEnabled());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_MULTI_EXTENDED,
+            configurator_.display_state());
+
+  const int kDualVirtualHeight =
+      small_mode_.size().height() +
+      (DisplayConfigurator::kVerticalGap + big_mode_.size().height()) * 2;
+  EXPECT_EQ(
+      JoinActions(
+          kGrab, GetFramebufferAction(
+                     gfx::Size(big_mode_.size().width(), kDualVirtualHeight),
+                     &outputs_[0], NULL)
+                     .c_str(),
+          GetCrtcAction(outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
+          kUngrab, NULL),
+      log_->GetActionsAndClear());
+  EXPECT_EQ(2, observer_.num_changes());
+  cached = &configurator_.cached_displays();
+  ASSERT_EQ(static_cast<size_t>(3), cached->size());
+  EXPECT_EQ(small_mode_.size(), (*cached)[0]->current_mode()->size());
+  EXPECT_EQ(big_mode_.size(), (*cached)[1]->current_mode()->size());
+  EXPECT_EQ(big_mode_.size(), (*cached)[2]->current_mode()->size());
+  EXPECT_EQ(virtual_display_id_1, (*cached)[1]->display_id());
+  EXPECT_EQ(virtual_display_id_2, (*cached)[2]->display_id());
+
+  // Remove 1st virtual display.
+  observer_.Reset();
+  EXPECT_TRUE(configurator_.RemoveVirtualDisplay(virtual_display_id_1));
+  EXPECT_EQ(
+      JoinActions(
+          kGrab, GetFramebufferAction(
+                     gfx::Size(big_mode_.size().width(), kSingleVirtualHeight),
+                     &outputs_[0], NULL)
+                     .c_str(),
+          GetCrtcAction(outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
+          kUngrab, NULL),
+      log_->GetActionsAndClear());
+  EXPECT_EQ(1, observer_.num_changes());
+  cached = &configurator_.cached_displays();
+  ASSERT_EQ(static_cast<size_t>(2), cached->size());
+  EXPECT_EQ(small_mode_.size(), (*cached)[0]->current_mode()->size());
+  EXPECT_EQ(big_mode_.size(), (*cached)[1]->current_mode()->size());
+  EXPECT_EQ(virtual_display_id_2, (*cached)[1]->display_id());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED,
+            configurator_.display_state());
+
+  // Remove 2nd virtual display.
+  observer_.Reset();
+  EXPECT_TRUE(configurator_.RemoveVirtualDisplay(virtual_display_id_2));
+  EXPECT_EQ(
+      JoinActions(
+          kGrab,
+          GetFramebufferAction(small_mode_.size(), &outputs_[0], NULL).c_str(),
+          GetCrtcAction(outputs_[0], &small_mode_, gfx::Point(0, 0)).c_str(),
+          kUngrab, NULL),
+      log_->GetActionsAndClear());
+  EXPECT_EQ(1, observer_.num_changes());
+  cached = &configurator_.cached_displays();
+  ASSERT_EQ(static_cast<size_t>(1), cached->size());
+  EXPECT_EQ(small_mode_.size(), (*cached)[0]->current_mode()->size());
+  EXPECT_EQ(MULTIPLE_DISPLAY_STATE_SINGLE, configurator_.display_state());
+}
+
 TEST_F(DisplayConfiguratorTest, ConnectSecondOutput) {
   InitWithSingleOutput();
 
   // Connect a second output and check that the configurator enters
   // extended mode.
   observer_.Reset();
   state_controller_.set_state(MULTIPLE_DISPLAY_STATE_DUAL_EXTENDED);
   UpdateOutputs(2, true);
   const int kDualHeight = small_mode_.size().height() +
                           DisplayConfigurator::kVerticalGap +
@@ skipping 1114 matching lines @@
                         GetFramebufferAction(small_mode_.size(), &outputs_[0],
                                              nullptr).c_str(),
                         GetCrtcAction(outputs_[0], &small_mode_,
                                       gfx::Point(0, 0)).c_str(),
                         kUngrab, NULL),
             log_->GetActionsAndClear());
 }
 
 }  // namespace test
 }  // namespace ui