Ozone: HDC should only keep track of planes owned by it’s crtc controllers

ui/ozone/platform/drm/gpu/hardware_display_controller_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 "base/bind.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "ui/ozone/platform/drm/gpu/crtc_controller.h"
 #include "ui/ozone/platform/drm/gpu/drm_buffer.h"
 #include "ui/ozone/platform/drm/gpu/drm_device.h"
@@ skipping 255 matching lines @@
       base::Bind(&HardwareDisplayControllerTest::PageFlipCallback,
                  base::Unretained(this))));
   drm_->RunCallbacks();
 
   const ui::HardwareDisplayPlane* owned_plane = nullptr;
   for (const auto& plane : drm_->plane_manager()->planes())
     if (plane->in_use())
       owned_plane = plane;
   ASSERT_TRUE(owned_plane != nullptr);
   EXPECT_EQ(kPrimaryCrtc, owned_plane->owning_crtc());
-  // Removing the crtc should free the plane.
+  // Removing the crtc should not free the plane or change ownership.
   scoped_ptr<ui::CrtcController> crtc =
       controller_->RemoveCrtc(drm_, kPrimaryCrtc);
+  EXPECT_TRUE(owned_plane->in_use());
+  EXPECT_EQ(kPrimaryCrtc, owned_plane->owning_crtc());
+}
+
+TEST_F(HardwareDisplayControllerTest, PlaneStateAfterDestroyingCrtc) {
+  ui::OverlayPlane plane1(scoped_refptr<ui::ScanoutBuffer>(
+      new MockScanoutBuffer(kDefaultModeSize)));
+  EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
+  std::vector<ui::OverlayPlane> planes =
+      std::vector<ui::OverlayPlane>(1, plane1);
+  EXPECT_TRUE(controller_->SchedulePageFlip(
+      planes, false, false,
+      base::Bind(&HardwareDisplayControllerTest::PageFlipCallback,
+                 base::Unretained(this))));
+  drm_->RunCallbacks();
+
+  const ui::HardwareDisplayPlane* owned_plane = nullptr;
+  for (const auto& plane : drm_->plane_manager()->planes())
+    if (plane->in_use())
+      owned_plane = plane;
+  ASSERT_TRUE(owned_plane != nullptr);
+  EXPECT_EQ(kPrimaryCrtc, owned_plane->owning_crtc());
+  scoped_ptr<ui::CrtcController> crtc =
+      controller_->RemoveCrtc(drm_, kPrimaryCrtc);
+  // Destroying crtc should free the plane.
+  crtc.reset();
+  uint32_t crtc_nullid = 0;
   EXPECT_FALSE(owned_plane->in_use());
+  EXPECT_EQ(crtc_nullid, owned_plane->owning_crtc());
+}
+
+TEST_F(HardwareDisplayControllerTest, OwnedPlanesAfterRemoveCrtc) {
+  ui::OverlayPlane plane1(scoped_refptr<ui::ScanoutBuffer>(
+      new MockScanoutBuffer(kDefaultModeSize)));
+  EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
+  std::vector<ui::OverlayPlane> planes =
+      std::vector<ui::OverlayPlane>(1, plane1);
+  EXPECT_TRUE(controller_->SchedulePageFlip(
+      planes, false, false,
+      base::Bind(&HardwareDisplayControllerTest::PageFlipCallback,
+                 base::Unretained(this))));
+  drm_->RunCallbacks();
+
+  const ui::HardwareDisplayPlane* primary_crtc_plane = nullptr;
+  for (const auto& plane : drm_->plane_manager()->planes()) {
+    if (plane->in_use() && kPrimaryCrtc == plane->owning_crtc())
+      primary_crtc_plane = plane;
+  }
+
+  ASSERT_TRUE(primary_crtc_plane != nullptr);
+  scoped_ptr<ui::CrtcController> crtc =
+      controller_->RemoveCrtc(drm_, kPrimaryCrtc);
+  EXPECT_FALSE(controller_->HasPlane(drm_.get(), primary_crtc_plane->plane_id(),

[dnicoara, 2015/08/19 14:09:44]

I'm not a fan of test only functions in non-test objects. Could you just test
for behavior? (ie: make sure that when the HDC schedules another page flip, the
planes for the removed CRTC aren't affected)

[kalyank, 2015/08/19 15:38:55]

Done.

+                                     kPrimaryCrtc));
+}
+
+TEST_F(HardwareDisplayControllerTest, OwnedPlanesAfterAddCrtc) {
+  ui::OverlayPlane plane1(scoped_refptr<ui::ScanoutBuffer>(
+      new MockScanoutBuffer(kDefaultModeSize)));
+  EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
+  std::vector<ui::OverlayPlane> planes =
+      std::vector<ui::OverlayPlane>(1, plane1);
+  EXPECT_TRUE(controller_->SchedulePageFlip(
+      planes, false, false,
+      base::Bind(&HardwareDisplayControllerTest::PageFlipCallback,
+                 base::Unretained(this))));
+  drm_->RunCallbacks();
+
+  const ui::HardwareDisplayPlane* primary_crtc_plane = nullptr;
+  for (const auto& plane : drm_->plane_manager()->planes()) {
+    if (plane->in_use() && kPrimaryCrtc == plane->owning_crtc())
+      primary_crtc_plane = plane;
+  }
+
+  ASSERT_TRUE(primary_crtc_plane != nullptr);
+
+  scoped_ptr<ui::HardwareDisplayController> hdc_controller;
+  hdc_controller.reset(new ui::HardwareDisplayController(
+      controller_->RemoveCrtc(drm_, kPrimaryCrtc), controller_->origin()));
+  EXPECT_FALSE(controller_->HasPlane(drm_.get(), primary_crtc_plane->plane_id(),
+                                     kPrimaryCrtc));
+  EXPECT_TRUE(hdc_controller->HasPlane(
+      drm_.get(), primary_crtc_plane->plane_id(), kPrimaryCrtc));
 }
 
 TEST_F(HardwareDisplayControllerTest, ModesetWhilePageFlipping) {
   ui::OverlayPlane plane1(scoped_refptr<ui::ScanoutBuffer>(
       new MockScanoutBuffer(kDefaultModeSize)));
   EXPECT_TRUE(controller_->Modeset(plane1, kDefaultMode));
   std::vector<ui::OverlayPlane> planes =
       std::vector<ui::OverlayPlane>(1, plane1);
   EXPECT_TRUE(controller_->SchedulePageFlip(
       planes, false, false,
@@ skipping 33 matching lines @@
       planes, false, false,
       base::Bind(&HardwareDisplayControllerTest::PageFlipCallback,
                  base::Unretained(this))));
 
   controller_->RemoveCrtc(drm_, kPrimaryCrtc);
 
   EXPECT_EQ(1, page_flips_);
   drm_->RunCallbacks();
   EXPECT_EQ(1, page_flips_);
 }