SF: Fix two display identification bugs
1) getDisplayIdentificationData() was called on hotplug disconnect and
not just on hotplug connect. The DisplayTransactionTest had a
warning, and I've fixed the issue and made the test explicitly expect
zero calls so it will fail instead.
2) One of the tests in DisplayIdentificationTest had a use after free,
detected by enabling the address sanitizer on the unit test executable.
Bug: None
Test: Test passes with no errors or warnings
Change-Id: Iceda0d499d692991adfcbf5c772da736bd4a843f
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 2c94a0e..d963ab6 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -160,11 +160,13 @@
std::optional<DisplayId> displayId;
- uint8_t port;
- DisplayIdentificationData data;
- if (getDisplayIdentificationData(hwcDisplayId, &port, &data)) {
- displayId = generateDisplayId(port, data);
- ALOGE_IF(!displayId, "Failed to generate stable ID for display %" PRIu64, hwcDisplayId);
+ if (connection == HWC2::Connection::Connected) {
+ uint8_t port;
+ DisplayIdentificationData data;
+ if (getDisplayIdentificationData(hwcDisplayId, &port, &data)) {
+ displayId = generateDisplayId(port, data);
+ ALOGE_IF(!displayId, "Failed to generate stable ID for display %" PRIu64, hwcDisplayId);
+ }
}
// Disconnect is handled through HWComposer::disconnectDisplay via
diff --git a/services/surfaceflinger/tests/unittests/DisplayIdentificationTest.cpp b/services/surfaceflinger/tests/unittests/DisplayIdentificationTest.cpp
index 9113171..4f1c99e 100644
--- a/services/surfaceflinger/tests/unittests/DisplayIdentificationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayIdentificationTest.cpp
@@ -77,20 +77,23 @@
}
TEST(DisplayIdentificationTest, parseEdid) {
- auto edid = parseEdid(asDisplayIdentificationData(kInternalEdid));
+ auto data = asDisplayIdentificationData(kInternalEdid);
+ auto edid = parseEdid(data);
ASSERT_TRUE(edid);
EXPECT_EQ(0x4ca3u, edid->manufacturerId);
EXPECT_STREQ("SEC", edid->pnpId.data());
// ASCII text should be used as fallback if display name and serial number are missing.
EXPECT_EQ("121AT11-801", edid->displayName);
- edid = parseEdid(asDisplayIdentificationData(kExternalEdid));
+ data = asDisplayIdentificationData(kExternalEdid);
+ edid = parseEdid(data);
ASSERT_TRUE(edid);
EXPECT_EQ(0x22f0u, edid->manufacturerId);
EXPECT_STREQ("HWP", edid->pnpId.data());
EXPECT_EQ("HP ZR30w", edid->displayName);
- edid = parseEdid(asDisplayIdentificationData(kExternalEedid));
+ data = asDisplayIdentificationData(kExternalEedid);
+ edid = parseEdid(data);
ASSERT_TRUE(edid);
EXPECT_EQ(0x4c2du, edid->manufacturerId);
EXPECT_STREQ("SAM", edid->pnpId.data());
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index 32712c7..aa377af 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -662,7 +662,7 @@
using SimpleHwcVirtualDisplayVariant = HwcVirtualDisplayVariant<1024, 768, Secure::TRUE>;
using HwcVirtualDisplayCase =
Case<SimpleHwcVirtualDisplayVariant, WideColorSupportNotConfiguredVariant,
- HdrNotSupportedVariant<SimpleHwcVirtualDisplayVariant>,
+ HdrNotSupportedVariant<SimpleHwcVirtualDisplayVariant>,
NoPerFrameMetadataSupportVariant<SimpleHwcVirtualDisplayVariant>>;
using WideColorP3ColorimetricDisplayCase =
Case<PrimaryDisplayVariant, WideColorP3ColorimetricSupportedVariant<PrimaryDisplayVariant>,
@@ -1304,6 +1304,8 @@
// Call Expectations
EXPECT_CALL(*mComposer, isUsingVrComposer()).WillRepeatedly(Return(false));
+ EXPECT_CALL(*mComposer, getDisplayIdentificationData(Case::Display::HWC_DISPLAY_ID, _, _))
+ .Times(0);
setupCommonCallExpectationsForDisconnectProcessing<Case>();