HWComposer: Unset mPrimaryHwcDisplayId after disconnectDisplay
We should unset mPrimaryHwcDisplayId when disconnectDisplay()
is called for the primary display. This can happen during
display change and it will cause isHeadless() to be false.
This way SurfaceFlinger will call getPrimaryDisplayId()
which will crash.
Test: HWComposerTest
Test: libsurfaceflinger_unittest
Bug: 195900759
Change-Id: I3c00e3b864fef6f1b088ffae0539c1ad16b7492b
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 256bca9..35dd2a8 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -669,6 +669,13 @@
mPhysicalDisplayIdMap.erase(hwcDisplayId);
mDisplayData.erase(displayId);
+
+ // Reset the primary display ID if we're disconnecting it.
+ // This way isHeadless() will return false, which is necessary
+ // because getPrimaryDisplayId() will crash.
+ if (mPrimaryHwcDisplayId == hwcDisplayId) {
+ mPrimaryHwcDisplayId.reset();
+ }
}
status_t HWComposer::setOutputBuffer(HalVirtualDisplayId displayId, const sp<Fence>& acquireFence,
diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index 0a090da..66edd65 100644
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -231,9 +231,12 @@
virtual Hwc2::Composer* getComposer() const = 0;
- // Returns the first display connected at boot. It cannot be disconnected, which implies an
- // internal connection type. Its connection via HWComposer::onHotplug, which in practice is
- // immediately after HWComposer construction, must occur before any call to this function.
+ // Returns the first display connected at boot. Its connection via HWComposer::onHotplug,
+ // which in practice is immediately after HWComposer construction, must occur before any
+ // call to this function.
+ // The primary display can be temporarily disconnected from the perspective
+ // of this class. Callers must not call getPrimaryHwcDisplayId() or getPrimaryDisplayId()
+ // if isHeadless().
//
// TODO(b/182939859): Remove special cases for primary display.
virtual hal::HWDisplayId getPrimaryHwcDisplayId() const = 0;
diff --git a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
index 655baf8..7fda8b3 100644
--- a/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/HWComposerTest.cpp
@@ -36,6 +36,7 @@
#include "DisplayHardware/DisplayMode.h"
#include "DisplayHardware/HWComposer.h"
#include "DisplayHardware/Hal.h"
+#include "DisplayIdentificationTest.h"
#include "mock/DisplayHardware/MockComposer.h"
#include "mock/DisplayHardware/MockHWC2.h"
@@ -57,6 +58,29 @@
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());
+
+ const hal::HWDisplayId hwcId = 1;
+
+ EXPECT_CALL(*mHal, getDisplayIdentificationData(_, _, _))
+ .WillOnce(DoAll(SetArgPointee<2>(getExternalEdid()),
+ Return(hardware::graphics::composer::V2_1::Error::NONE)));
+
+ EXPECT_CALL(*mHal, setVsyncEnabled(_, _));
+ EXPECT_CALL(*mHal, setClientTargetSlotCount(_));
+
+ auto info = hwc.onHotplug(hwcId, hal::Connection::CONNECTED);
+ ASSERT_TRUE(info);
+ auto displayId = info->id;
+ ASSERT_FALSE(hwc.isHeadless());
+
+ hwc.disconnectDisplay(displayId);
+ ASSERT_TRUE(hwc.isHeadless());
+}
+
struct MockHWC2ComposerCallback final : StrictMock<HWC2::ComposerCallback> {
MOCK_METHOD2(onComposerHalHotplug, void(hal::HWDisplayId, hal::Connection));
MOCK_METHOD1(onComposerHalRefresh, void(hal::HWDisplayId));