commit e2c5b0ae04dc9965d67b442170d2ac1e6f862085
author Dominik Laskowski <domlaskowski@google.com> Wed Aug 10 14:53:53 2022 -0700
committer Dominik Laskowski <domlaskowski@google.com> Fri Aug 12 10:05:36 2022 -0700
tree e05d73fa5207be5b59e347fa743776e6b7d06594
parent 0c903e14788ce5fbbe9083d4df48ca0b95247a91

SF: Handle all HWComposer::getActiveMode errors

A garbage mode was returned for errors other than BAD_CONFIG.

Bug: 241286153
Bug: 235488695
Test: HWComposerTest.getActiveMode
Change-Id: I48b055363330b784f3d54a7d6ccef8bf4bcb2ead

services/surfaceflinger/tests/unittests/HWComposerTest.cpp

diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index 5241604..9d8e0a2 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -59,27 +59,62 @@
 using ::testing::SetArgPointee;
 using ::testing::StrictMock;
 
-TEST(HWComposerTest, isHeadless) {
-    Hwc2::mock::Composer* mHal = new StrictMock<Hwc2::mock::Composer>();
-    impl::HWComposer hwc{std::unique_ptr<Hwc2::Composer>(mHal)};
-    ASSERT_TRUE(hwc.isHeadless());
+struct HWComposerTest : testing::Test {
+    using HalError = hardware::graphics::composer::V2_1::Error;
 
-    const hal::HWDisplayId hwcId = 1;
+    Hwc2::mock::Composer* const mHal = new StrictMock<Hwc2::mock::Composer>();
+    impl::HWComposer mHwc{std::unique_ptr<Hwc2::Composer>(mHal)};
 
-    EXPECT_CALL(*mHal, getDisplayIdentificationData(_, _, _))
-            .WillOnce(DoAll(SetArgPointee<2>(getExternalEdid()),
-                            Return(hardware::graphics::composer::V2_1::Error::NONE)));
+    void expectHotplugConnect(hal::HWDisplayId hwcDisplayId) {
+        constexpr uint8_t kPort = 255;
+        EXPECT_CALL(*mHal, getDisplayIdentificationData(hwcDisplayId, _, _))
+                .WillOnce(DoAll(SetArgPointee<1>(kPort),
+                                SetArgPointee<2>(getExternalEdid()), Return(HalError::NONE)));
 
-    EXPECT_CALL(*mHal, setVsyncEnabled(_, _));
-    EXPECT_CALL(*mHal, setClientTargetSlotCount(_));
+        EXPECT_CALL(*mHal, setClientTargetSlotCount(_));
+        EXPECT_CALL(*mHal, setVsyncEnabled(hwcDisplayId, Hwc2::IComposerClient::Vsync::DISABLE));
+    }
+};
 
-    auto info = hwc.onHotplug(hwcId, hal::Connection::CONNECTED);
+TEST_F(HWComposerTest, isHeadless) {
+    ASSERT_TRUE(mHwc.isHeadless());
+
+    constexpr hal::HWDisplayId kHwcDisplayId = 1;
+    expectHotplugConnect(kHwcDisplayId);
+
+    const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
     ASSERT_TRUE(info);
-    auto displayId = info->id;
-    ASSERT_FALSE(hwc.isHeadless());
 
-    hwc.disconnectDisplay(displayId);
-    ASSERT_TRUE(hwc.isHeadless());
+    ASSERT_FALSE(mHwc.isHeadless());
+
+    mHwc.disconnectDisplay(info->id);
+    ASSERT_TRUE(mHwc.isHeadless());
+}
+
+TEST_F(HWComposerTest, getActiveMode) {
+    // Unknown display.
+    EXPECT_EQ(mHwc.getActiveMode(PhysicalDisplayId::fromPort(0)), std::nullopt);
+
+    constexpr hal::HWDisplayId kHwcDisplayId = 2;
+    expectHotplugConnect(kHwcDisplayId);
+
+    const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);
+    ASSERT_TRUE(info);
+
+    {
+        // Display is known to SF but not HWC, e.g. the hotplug disconnect is pending.
+        EXPECT_CALL(*mHal, getActiveConfig(kHwcDisplayId, _))
+                .WillOnce(Return(HalError::BAD_DISPLAY));
+
+        EXPECT_EQ(mHwc.getActiveMode(info->id), std::nullopt);
+    }
+    {
+        constexpr hal::HWConfigId kConfigId = 42;
+        EXPECT_CALL(*mHal, getActiveConfig(kHwcDisplayId, _))
+                .WillOnce(DoAll(SetArgPointee<1>(kConfigId), Return(HalError::NONE)));
+
+        EXPECT_EQ(mHwc.getActiveMode(info->id), kConfigId);
+    }
 }
 
 struct MockHWC2ComposerCallback final : StrictMock<HWC2::ComposerCallback> {
@@ -93,8 +128,7 @@
     MOCK_METHOD1(onComposerHalVsyncIdle, void(hal::HWDisplayId));
 };
 
-struct HWComposerSetCallbackTest : testing::Test {
-    Hwc2::mock::Composer* mHal = new StrictMock<Hwc2::mock::Composer>();
+struct HWComposerSetCallbackTest : HWComposerTest {
     MockHWC2ComposerCallback mCallback;
 };
 
@@ -113,10 +147,9 @@
                             Return(hardware::graphics::composer::V2_4::Error::NONE)));
     EXPECT_CALL(*mHal, registerCallback(_));
 
-    impl::HWComposer hwc{std::unique_ptr<Hwc2::Composer>(mHal)};
-    hwc.setCallback(mCallback);
+    mHwc.setCallback(mCallback);
 
-    const auto& supported = hwc.getSupportedLayerGenericMetadata();
+    const auto& supported = mHwc.getSupportedLayerGenericMetadata();
     EXPECT_EQ(2u, supported.size());
     EXPECT_EQ(1u, supported.count(kMetadata1Name));
     EXPECT_EQ(kMetadata1Mandatory, supported.find(kMetadata1Name)->second);
@@ -130,11 +163,10 @@
             .WillOnce(Return(hardware::graphics::composer::V2_4::Error::UNSUPPORTED));
     EXPECT_CALL(*mHal, registerCallback(_));
 
-    impl::HWComposer hwc{std::unique_ptr<Hwc2::Composer>(mHal)};
-    hwc.setCallback(mCallback);
+    mHwc.setCallback(mCallback);
 
-    const auto& supported = hwc.getSupportedLayerGenericMetadata();
-    EXPECT_EQ(0u, supported.size());
+    const auto& supported = mHwc.getSupportedLayerGenericMetadata();
+    EXPECT_TRUE(supported.empty());
 }
 
 struct HWComposerLayerTest : public testing::Test {