Add support for virtual displays

ui/display/chromeos/display_configurator_unittest.cc

Index: ui/display/chromeos/display_configurator_unittest.cc
diff --git a/ui/display/chromeos/display_configurator_unittest.cc b/ui/display/chromeos/display_configurator_unittest.cc
index bd5f7e9d8cead04bf485626034edbe751b54953d..700c8a244dcece9a08bed22867eac2e665782849 100644
--- a/ui/display/chromeos/display_configurator_unittest.cc
+++ b/ui/display/chromeos/display_configurator_unittest.cc
@@ -112,6 +112,8 @@ class TestMirroringController
   DISALLOW_COPY_AND_ASSIGN(TestMirroringController);
 };
 
+}  // namespace
+
 class DisplayConfiguratorTest : public testing::Test {
  public:
   enum CallbackResult {
@@ -259,8 +261,6 @@ class DisplayConfiguratorTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(DisplayConfiguratorTest);
 };
 
-}  // namespace
-
 TEST_F(DisplayConfiguratorTest, FindDisplayModeMatchingSize) {
   ScopedVector<const DisplayMode> modes;
 
@@ -337,6 +337,163 @@ TEST_F(DisplayConfiguratorTest, 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();