SF: Remove obsolete HWComposer members
Remove mExternalHwcDisplayId, an obsolete special case for legacy multi-
display mode. Assert primary display existence instead of propagating to
callers, which is closer to how headless mode would work, i.e. injecting
a placeholder primary display. Remove isVirtual checks now that they are
enforced at compile time. Prevent primary display disconnection for now.
Bug: 182939859
Test: libsurfaceflinger_unittest
Change-Id: I507db6a3ac0bda93005a86b38cca6ebbcd5c0155
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp
index b713334..313ab03 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp
@@ -282,7 +282,8 @@
}
TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectPrimaryDisplay) {
- processesHotplugDisconnectCommon<SimplePrimaryDisplayCase>();
+ EXPECT_EXIT(processesHotplugDisconnectCommon<SimplePrimaryDisplayCase>(),
+ testing::KilledBySignal(SIGABRT), "Primary display cannot be disconnected.");
}
TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectExternalDisplay) {
@@ -290,96 +291,106 @@
}
TEST_F(HandleTransactionLockedTest, processesHotplugConnectThenDisconnectPrimary) {
- using Case = SimplePrimaryDisplayCase;
+ EXPECT_EXIT(
+ [this] {
+ using Case = SimplePrimaryDisplayCase;
- // --------------------------------------------------------------------
- // Preconditions
+ // --------------------------------------------------------------------
+ // Preconditions
- setupCommonPreconditions<Case>();
+ setupCommonPreconditions<Case>();
- // A hotplug connect event is enqueued for a display
- Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
- // A hotplug disconnect event is also enqueued for the same display
- Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
+ // A hotplug connect event is enqueued for a display
+ Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
+ // A hotplug disconnect event is also enqueued for the same display
+ Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
- // --------------------------------------------------------------------
- // Call Expectations
+ // --------------------------------------------------------------------
+ // Call Expectations
- setupCommonCallExpectationsForConnectProcessing<Case>();
- setupCommonCallExpectationsForDisconnectProcessing<Case>();
+ setupCommonCallExpectationsForConnectProcessing<Case>();
+ setupCommonCallExpectationsForDisconnectProcessing<Case>();
- EXPECT_CALL(*mComposer,
- setVsyncEnabled(Case::Display::HWC_DISPLAY_ID, IComposerClient::Vsync::DISABLE))
- .WillOnce(Return(Error::NONE));
- EXPECT_CALL(*mConsumer, consumerDisconnect()).WillOnce(Return(NO_ERROR));
+ EXPECT_CALL(*mComposer,
+ setVsyncEnabled(Case::Display::HWC_DISPLAY_ID,
+ IComposerClient::Vsync::DISABLE))
+ .WillOnce(Return(Error::NONE));
+ EXPECT_CALL(*mConsumer, consumerDisconnect()).WillOnce(Return(NO_ERROR));
- // --------------------------------------------------------------------
- // Invocation
+ // --------------------------------------------------------------------
+ // Invocation
- mFlinger.handleTransactionLocked(eDisplayTransactionNeeded);
+ mFlinger.handleTransactionLocked(eDisplayTransactionNeeded);
- // --------------------------------------------------------------------
- // Postconditions
+ // --------------------------------------------------------------------
+ // Postconditions
- // HWComposer should not have an entry for the display
- EXPECT_FALSE(hasPhysicalHwcDisplay(Case::Display::HWC_DISPLAY_ID));
+ // HWComposer should not have an entry for the display
+ EXPECT_FALSE(hasPhysicalHwcDisplay(Case::Display::HWC_DISPLAY_ID));
- // SF should not have a display token.
- const auto displayId = Case::Display::DISPLAY_ID::get();
- ASSERT_TRUE(PhysicalDisplayId::tryCast(displayId));
- ASSERT_TRUE(mFlinger.mutablePhysicalDisplayTokens().count(displayId) == 0);
+ // SF should not have a display token.
+ const auto displayId = Case::Display::DISPLAY_ID::get();
+ ASSERT_TRUE(PhysicalDisplayId::tryCast(displayId));
+ ASSERT_TRUE(mFlinger.mutablePhysicalDisplayTokens().count(displayId) == 0);
+ }(),
+ testing::KilledBySignal(SIGABRT), "Primary display cannot be disconnected.");
}
TEST_F(HandleTransactionLockedTest, processesHotplugDisconnectThenConnectPrimary) {
- using Case = SimplePrimaryDisplayCase;
+ EXPECT_EXIT(
+ [this] {
+ using Case = SimplePrimaryDisplayCase;
- // --------------------------------------------------------------------
- // Preconditions
+ // --------------------------------------------------------------------
+ // Preconditions
- setupCommonPreconditions<Case>();
+ setupCommonPreconditions<Case>();
- // The display is already completely set up.
- Case::Display::injectHwcDisplay(this);
- auto existing = Case::Display::makeFakeExistingDisplayInjector(this);
- existing.inject();
+ // The display is already completely set up.
+ Case::Display::injectHwcDisplay(this);
+ auto existing = Case::Display::makeFakeExistingDisplayInjector(this);
+ existing.inject();
- // A hotplug disconnect event is enqueued for a display
- Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
- // A hotplug connect event is also enqueued for the same display
- Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
+ // A hotplug disconnect event is enqueued for a display
+ Case::Display::injectPendingHotplugEvent(this, Connection::DISCONNECTED);
+ // A hotplug connect event is also enqueued for the same display
+ Case::Display::injectPendingHotplugEvent(this, Connection::CONNECTED);
- // --------------------------------------------------------------------
- // Call Expectations
+ // --------------------------------------------------------------------
+ // Call Expectations
- setupCommonCallExpectationsForConnectProcessing<Case>();
- setupCommonCallExpectationsForDisconnectProcessing<Case>();
+ setupCommonCallExpectationsForConnectProcessing<Case>();
+ setupCommonCallExpectationsForDisconnectProcessing<Case>();
- // --------------------------------------------------------------------
- // Invocation
+ // --------------------------------------------------------------------
+ // Invocation
- mFlinger.handleTransactionLocked(eDisplayTransactionNeeded);
+ mFlinger.handleTransactionLocked(eDisplayTransactionNeeded);
- // --------------------------------------------------------------------
- // Postconditions
+ // --------------------------------------------------------------------
+ // Postconditions
- // The existing token should have been removed
- verifyDisplayIsNotConnected(existing.token());
- const auto displayId = Case::Display::DISPLAY_ID::get();
- ASSERT_TRUE(PhysicalDisplayId::tryCast(displayId));
- ASSERT_TRUE(mFlinger.mutablePhysicalDisplayTokens().count(displayId) == 1);
- EXPECT_NE(existing.token(), mFlinger.mutablePhysicalDisplayTokens()[displayId]);
+ // The existing token should have been removed
+ verifyDisplayIsNotConnected(existing.token());
+ const auto displayId = Case::Display::DISPLAY_ID::get();
+ ASSERT_TRUE(PhysicalDisplayId::tryCast(displayId));
+ ASSERT_TRUE(mFlinger.mutablePhysicalDisplayTokens().count(displayId) == 1);
+ EXPECT_NE(existing.token(), mFlinger.mutablePhysicalDisplayTokens()[displayId]);
- // A new display should be connected in its place
+ // A new display should be connected in its place
- verifyPhysicalDisplayIsConnected<Case>();
+ verifyPhysicalDisplayIsConnected<Case>();
- // --------------------------------------------------------------------
- // Cleanup conditions
+ // --------------------------------------------------------------------
+ // Cleanup conditions
- EXPECT_CALL(*mComposer,
- setVsyncEnabled(Case::Display::HWC_DISPLAY_ID, IComposerClient::Vsync::DISABLE))
- .WillOnce(Return(Error::NONE));
- EXPECT_CALL(*mConsumer, consumerDisconnect()).WillOnce(Return(NO_ERROR));
+ EXPECT_CALL(*mComposer,
+ setVsyncEnabled(Case::Display::HWC_DISPLAY_ID,
+ IComposerClient::Vsync::DISABLE))
+ .WillOnce(Return(Error::NONE));
+ EXPECT_CALL(*mConsumer, consumerDisconnect()).WillOnce(Return(NO_ERROR));
+ }(),
+ testing::KilledBySignal(SIGABRT), "Primary display cannot be disconnected.");
}
TEST_F(HandleTransactionLockedTest, processesVirtualDisplayAdded) {
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
index e32c4bf..7ead0af 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetupNewDisplayDeviceInternalTest.cpp
@@ -292,7 +292,9 @@
}
TEST_F(SetupNewDisplayDeviceInternalTest, createSimpleExternalDisplay) {
- setupNewDisplayDeviceInternalTest<SimpleExternalDisplayCase>();
+ // External displays must be secondary, as the primary display cannot be disconnected.
+ EXPECT_EXIT(setupNewDisplayDeviceInternalTest<SimpleExternalDisplayCase>(),
+ testing::KilledBySignal(SIGABRT), "Missing primary display");
}
TEST_F(SetupNewDisplayDeviceInternalTest, createNonHwcVirtualDisplay) {
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 56ae414..e036f4d 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -439,8 +439,7 @@
auto& mutableHwcDisplayData() { return getHwComposer().mDisplayData; }
auto& mutableHwcPhysicalDisplayIdMap() { return getHwComposer().mPhysicalDisplayIdMap; }
- auto& mutableInternalHwcDisplayId() { return getHwComposer().mInternalHwcDisplayId; }
- auto& mutableExternalHwcDisplayId() { return getHwComposer().mExternalHwcDisplayId; }
+ auto& mutablePrimaryHwcDisplayId() { return getHwComposer().mPrimaryHwcDisplayId; }
auto& mutableUseFrameRateApi() { return mFlinger->useFrameRateApi; }
auto fromHandle(const sp<IBinder>& handle) {
@@ -600,13 +599,12 @@
LOG_ALWAYS_FATAL_IF(!physicalId);
flinger->mutableHwcPhysicalDisplayIdMap().emplace(mHwcDisplayId, *physicalId);
if (mIsPrimary) {
- flinger->mutableInternalHwcDisplayId() = mHwcDisplayId;
+ flinger->mutablePrimaryHwcDisplayId() = mHwcDisplayId;
} else {
- // If there is an external HWC display there should always be an internal ID
+ // If there is an external HWC display, there should always be a primary ID
// as well. Set it to some arbitrary value.
- auto& internalId = flinger->mutableInternalHwcDisplayId();
- if (!internalId) internalId = mHwcDisplayId - 1;
- flinger->mutableExternalHwcDisplayId() = mHwcDisplayId;
+ auto& primaryId = flinger->mutablePrimaryHwcDisplayId();
+ if (!primaryId) primaryId = mHwcDisplayId - 1;
}
}
}