SF: Cleanup creating / destroying virtual display APIs
Follow-up CL for cleanup requested by SF owner in http://ag/27202517.
Misc:
- Fix type conversion warnings for print format.
- Return status destroying virtual display in SF and add appropriate
checks in UT.
- Use static constexpr / const for compile-time effciciency.
Bug: 339525838
Bug: 137375833
Bug: 194863377
Test: atest libsurfaceflinger_unittest
Test: atest SurfaceFlinger_test
Flag: EXEMPT refactor
Change-Id: I7859720989c9244b26f8764c3323a8495064c888
diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp
index c1ef48e..ebe11fb 100644
--- a/services/surfaceflinger/tests/Credentials_test.cpp
+++ b/services/surfaceflinger/tests/Credentials_test.cpp
@@ -39,8 +39,8 @@
using ui::ColorMode;
namespace {
-const String8 DISPLAY_NAME("Credentials Display Test");
-const String8 SURFACE_NAME("Test Surface Name");
+const std::string kDisplayName("Credentials Display Test");
+const String8 kSurfaceName("Test Surface Name");
} // namespace
/**
@@ -99,7 +99,7 @@
ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getActiveDisplayMode(mDisplay, &mode));
// Background surface
- mBGSurfaceControl = mComposerClient->createSurface(SURFACE_NAME, mode.resolution.getWidth(),
+ mBGSurfaceControl = mComposerClient->createSurface(kSurfaceName, mode.resolution.getWidth(),
mode.resolution.getHeight(),
PIXEL_FORMAT_RGBA_8888, 0);
ASSERT_TRUE(mBGSurfaceControl != nullptr);
@@ -232,7 +232,7 @@
TEST_F(CredentialsTest, CreateDisplayTest) {
// Only graphics and system processes can create a secure display.
std::function<bool()> condition = [=]() {
- sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, true);
+ sp<IBinder> testDisplay = SurfaceComposerClient::createVirtualDisplay(kDisplayName, true);
return testDisplay.get() != nullptr;
};
@@ -267,7 +267,7 @@
}
condition = [=]() {
- sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, false);
+ sp<IBinder> testDisplay = SurfaceComposerClient::createVirtualDisplay(kDisplayName, false);
return testDisplay.get() != nullptr;
};
ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, false));
diff --git a/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp b/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp
index 7fce7e9..56cf13d 100644
--- a/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp
+++ b/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp
@@ -53,14 +53,15 @@
}
virtual void TearDown() {
- SurfaceComposerClient::destroyDisplay(mVirtualDisplay);
+ EXPECT_EQ(NO_ERROR, SurfaceComposerClient::destroyVirtualDisplay(mVirtualDisplay));
LayerTransactionTest::TearDown();
mColorLayer = 0;
}
void createDisplay(const ui::Size& layerStackSize, ui::LayerStack layerStack) {
+ static const std::string kDisplayName("VirtualDisplay");
mVirtualDisplay =
- SurfaceComposerClient::createDisplay(String8("VirtualDisplay"), false /*secure*/);
+ SurfaceComposerClient::createVirtualDisplay(kDisplayName, false /*isSecure*/);
asTransaction([&](Transaction& t) {
t.setDisplaySurface(mVirtualDisplay, mProducer);
t.setDisplayLayerStack(mVirtualDisplay, layerStack);
diff --git a/services/surfaceflinger/tests/TransactionTestHarnesses.h b/services/surfaceflinger/tests/TransactionTestHarnesses.h
index 87e6d3e..af3cb9a 100644
--- a/services/surfaceflinger/tests/TransactionTestHarnesses.h
+++ b/services/surfaceflinger/tests/TransactionTestHarnesses.h
@@ -66,8 +66,9 @@
sp<BufferListener> listener = sp<BufferListener>::make(this);
itemConsumer->setFrameAvailableListener(listener);
- vDisplay = SurfaceComposerClient::createDisplay(String8("VirtualDisplay"),
- false /*secure*/);
+ static const std::string kDisplayName("VirtualDisplay");
+ vDisplay = SurfaceComposerClient::createVirtualDisplay(kDisplayName,
+ false /*isSecure*/);
constexpr ui::LayerStack layerStack{
848472}; // ASCII for TTH (TransactionTestHarnesses)
@@ -107,7 +108,7 @@
t.setLayerStack(mirrorSc, ui::INVALID_LAYER_STACK);
t.apply(true);
}
- SurfaceComposerClient::destroyDisplay(vDisplay);
+ SurfaceComposerClient::destroyVirtualDisplay(vDisplay);
return sc;
}
}
diff --git a/services/surfaceflinger/tests/VirtualDisplay_test.cpp b/services/surfaceflinger/tests/VirtualDisplay_test.cpp
index f31f582..cd66dd2 100644
--- a/services/surfaceflinger/tests/VirtualDisplay_test.cpp
+++ b/services/surfaceflinger/tests/VirtualDisplay_test.cpp
@@ -41,14 +41,15 @@
};
TEST_F(VirtualDisplayTest, VirtualDisplayDestroyedSurfaceReuse) {
+ static const std::string kDisplayName("VirtualDisplay");
sp<IBinder> virtualDisplay =
- SurfaceComposerClient::createDisplay(String8("VirtualDisplay"), false /*secure*/);
+ SurfaceComposerClient::createVirtualDisplay(kDisplayName, false /*isSecure*/);
SurfaceComposerClient::Transaction t;
t.setDisplaySurface(virtualDisplay, mProducer);
t.apply(true);
- SurfaceComposerClient::destroyDisplay(virtualDisplay);
+ EXPECT_EQ(NO_ERROR, SurfaceComposerClient::destroyVirtualDisplay(virtualDisplay));
virtualDisplay.clear();
// Sync here to ensure the display was completely destroyed in SF
t.apply(true);
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp
index 5852b1c..ff7612e 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp
@@ -53,7 +53,8 @@
mFlinger.commitAndComposite();
EXPECT_COLOR_MATRIX_CHANGED(false, false);
- mFlinger.createDisplay(String8("Test Display"), false);
+ static const std::string kDisplayName("Test Display");
+ mFlinger.createVirtualDisplay(kDisplayName, false /*isSecure=*/);
mFlinger.commit();
EXPECT_COLOR_MATRIX_CHANGED(false, true);
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
index bf5ae21..e5f2a91 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
@@ -27,7 +27,7 @@
class CreateDisplayTest : public DisplayTransactionTest {
public:
- void createDisplayWithRequestedRefreshRate(const String8& name, uint64_t displayId,
+ void createDisplayWithRequestedRefreshRate(const std::string& name, uint64_t displayId,
float pacesetterDisplayRefreshRate,
float requestedRefreshRate,
float expectedAdjustedRefreshRate) {
@@ -37,7 +37,7 @@
// --------------------------------------------------------------------
// Invocation
- sp<IBinder> displayToken = mFlinger.createDisplay(name, false, requestedRefreshRate);
+ sp<IBinder> displayToken = mFlinger.createVirtualDisplay(name, false, requestedRefreshRate);
// --------------------------------------------------------------------
// Postconditions
@@ -73,7 +73,7 @@
};
TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) {
- const String8 name("virtual.test");
+ static const std::string name("virtual.test");
// --------------------------------------------------------------------
// Call Expectations
@@ -81,7 +81,7 @@
// --------------------------------------------------------------------
// Invocation
- sp<IBinder> displayToken = mFlinger.createDisplay(name, false);
+ sp<IBinder> displayToken = mFlinger.createVirtualDisplay(name, false);
// --------------------------------------------------------------------
// Postconditions
@@ -101,7 +101,7 @@
}
TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) {
- const String8 name("virtual.test");
+ static const std::string kDisplayName("virtual.test");
// --------------------------------------------------------------------
// Call Expectations
@@ -112,7 +112,7 @@
// Set the calling identity to graphics so captureDisplay with secure is allowed.
IPCThreadState::self()->restoreCallingIdentity(static_cast<int64_t>(AID_GRAPHICS) << 32 |
AID_GRAPHICS);
- sp<IBinder> displayToken = mFlinger.createDisplay(name, true);
+ sp<IBinder> displayToken = mFlinger.createVirtualDisplay(kDisplayName, true);
IPCThreadState::self()->restoreCallingIdentity(oldId);
// --------------------------------------------------------------------
@@ -123,7 +123,7 @@
const auto& display = getCurrentDisplayState(displayToken);
EXPECT_TRUE(display.isVirtual());
EXPECT_TRUE(display.isSecure);
- EXPECT_EQ(name.c_str(), display.displayName);
+ EXPECT_EQ(kDisplayName.c_str(), display.displayName);
// --------------------------------------------------------------------
// Cleanup conditions
@@ -133,8 +133,8 @@
}
TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) {
- const String8 name("virtual.test");
- const std::string uniqueId = "virtual:package:id";
+ static const std::string kDisplayName("virtual.test");
+ static const std::string kUniqueId = "virtual:package:id";
// --------------------------------------------------------------------
// Call Expectations
@@ -142,7 +142,7 @@
// --------------------------------------------------------------------
// Invocation
- sp<IBinder> displayToken = mFlinger.createDisplay(name, false, uniqueId);
+ sp<IBinder> displayToken = mFlinger.createVirtualDisplay(kDisplayName, false, kUniqueId);
// --------------------------------------------------------------------
// Postconditions
@@ -153,7 +153,7 @@
EXPECT_TRUE(display.isVirtual());
EXPECT_FALSE(display.isSecure);
EXPECT_EQ(display.uniqueId, "virtual:package:id");
- EXPECT_EQ(name.c_str(), display.displayName);
+ EXPECT_EQ(kDisplayName.c_str(), display.displayName);
// --------------------------------------------------------------------
// Cleanup conditions
@@ -164,78 +164,78 @@
// Requesting 0 tells SF not to do anything, i.e., default to refresh as physical displays
TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRate0) {
- const String8 displayName("virtual.test");
- const uint64_t displayId = 123ull;
- const float kPacesetterDisplayRefreshRate = 60.f;
- const float kRequestedRefreshRate = 0.f;
- const float kExpectedAdjustedRefreshRate = 0.f;
- createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+ static const std::string kDisplayName("virtual.test");
+ constexpr uint64_t kDisplayId = 123ull;
+ constexpr float kPacesetterDisplayRefreshRate = 60.f;
+ constexpr float kRequestedRefreshRate = 0.f;
+ constexpr float kExpectedAdjustedRefreshRate = 0.f;
+ createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
}
// Requesting negative refresh rate, will be ignored, same as requesting 0
TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateNegative) {
- const String8 displayName("virtual.test");
- const uint64_t displayId = 123ull;
- const float kPacesetterDisplayRefreshRate = 60.f;
- const float kRequestedRefreshRate = -60.f;
- const float kExpectedAdjustedRefreshRate = 0.f;
- createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+ static const std::string kDisplayName("virtual.test");
+ constexpr uint64_t kDisplayId = 123ull;
+ constexpr float kPacesetterDisplayRefreshRate = 60.f;
+ constexpr float kRequestedRefreshRate = -60.f;
+ constexpr float kExpectedAdjustedRefreshRate = 0.f;
+ createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
}
// Requesting a higher refresh rate than the pacesetter
TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateHigh) {
- const String8 displayName("virtual.test");
- const uint64_t displayId = 123ull;
- const float kPacesetterDisplayRefreshRate = 60.f;
- const float kRequestedRefreshRate = 90.f;
- const float kExpectedAdjustedRefreshRate = 60.f;
- createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+ static const std::string kDisplayName("virtual.test");
+ constexpr uint64_t kDisplayId = 123ull;
+ constexpr float kPacesetterDisplayRefreshRate = 60.f;
+ constexpr float kRequestedRefreshRate = 90.f;
+ constexpr float kExpectedAdjustedRefreshRate = 60.f;
+ createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
}
// Requesting the same refresh rate as the pacesetter
TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateSame) {
- const String8 displayName("virtual.test");
- const uint64_t displayId = 123ull;
- const float kPacesetterDisplayRefreshRate = 60.f;
- const float kRequestedRefreshRate = 60.f;
- const float kExpectedAdjustedRefreshRate = 60.f;
- createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+ static const std::string kDisplayName("virtual.test");
+ constexpr uint64_t kDisplayId = 123ull;
+ constexpr float kPacesetterDisplayRefreshRate = 60.f;
+ constexpr float kRequestedRefreshRate = 60.f;
+ constexpr float kExpectedAdjustedRefreshRate = 60.f;
+ createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
}
// Requesting a divisor (30) of the pacesetter (60) should be honored
TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateDivisor) {
- const String8 displayName("virtual.test");
- const uint64_t displayId = 123ull;
- const float kPacesetterDisplayRefreshRate = 60.f;
- const float kRequestedRefreshRate = 30.f;
- const float kExpectedAdjustedRefreshRate = 30.f;
- createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+ static const std::string kDisplayName("virtual.test");
+ constexpr uint64_t kDisplayId = 123ull;
+ constexpr float kPacesetterDisplayRefreshRate = 60.f;
+ constexpr float kRequestedRefreshRate = 30.f;
+ constexpr float kExpectedAdjustedRefreshRate = 30.f;
+ createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
}
// Requesting a non divisor (45) of the pacesetter (120) should round up to a divisor (60)
TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateNoneDivisor) {
- const String8 displayName("virtual.test");
- const uint64_t displayId = 123ull;
- const float kPacesetterDisplayRefreshRate = 120.f;
- const float kRequestedRefreshRate = 45.f;
- const float kExpectedAdjustedRefreshRate = 60.f;
- createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+ static const std::string kDisplayName("virtual.test");
+ constexpr uint64_t kDisplayId = 123ull;
+ constexpr float kPacesetterDisplayRefreshRate = 120.f;
+ constexpr float kRequestedRefreshRate = 45.f;
+ constexpr float kExpectedAdjustedRefreshRate = 60.f;
+ createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
}
// Requesting a non divisor (75) of the pacesetter (120) should round up to pacesetter (120)
TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateNoneDivisorMax) {
- const String8 displayName("virtual.test");
- const uint64_t displayId = 123ull;
- const float kPacesetterDisplayRefreshRate = 120.f;
- const float kRequestedRefreshRate = 75.f;
- const float kExpectedAdjustedRefreshRate = 120.f;
- createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+ static const std::string kDisplayName("virtual.test");
+ constexpr uint64_t kDisplayId = 123ull;
+ constexpr float kPacesetterDisplayRefreshRate = 120.f;
+ constexpr float kRequestedRefreshRate = 75.f;
+ constexpr float kExpectedAdjustedRefreshRate = 120.f;
+ createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
}
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
index 93a3811..f8ad8e1 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
@@ -43,18 +43,18 @@
// --------------------------------------------------------------------
// Invocation
- mFlinger.destroyDisplay(existing.token());
+ EXPECT_EQ(NO_ERROR, mFlinger.destroyVirtualDisplay(existing.token()));
// --------------------------------------------------------------------
// Postconditions
- // The display should have been removed from the current state
+ // The display should have been removed from the current state.
EXPECT_FALSE(hasCurrentDisplayState(existing.token()));
- // Ths display should still exist in the drawing state
+ // Ths display should still exist in the drawing state.
EXPECT_TRUE(hasDrawingDisplayState(existing.token()));
- // The display transaction needed flasg should be set
+ // The display transaction needed flags should be set.
EXPECT_TRUE(hasTransactionFlagSet(eDisplayTransactionNeeded));
}
@@ -67,7 +67,7 @@
// --------------------------------------------------------------------
// Invocation
- mFlinger.destroyDisplay(displayToken);
+ EXPECT_EQ(NAME_NOT_FOUND, mFlinger.destroyVirtualDisplay(displayToken));
}
} // namespace
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp
index 4e9fba7..f424133 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp
@@ -42,8 +42,8 @@
}
TEST_F(SurfaceFlingerGetDisplayStatsTest, invalidToken) {
- const String8 displayName("fakeDisplay");
- sp<IBinder> displayToken = mFlinger.createDisplay(displayName, false);
+ static const std::string kDisplayName("fakeDisplay");
+ sp<IBinder> displayToken = mFlinger.createVirtualDisplay(kDisplayName, false /*isSecure*/);
DisplayStatInfo info;
status_t status = mFlinger.getDisplayStats(displayToken, &info);
EXPECT_EQ(status, NAME_NOT_FOUND);
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index b251e2c..265f804 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -420,19 +420,21 @@
commit(kComposite);
}
- auto createDisplay(const String8& displayName, bool isSecure,
- float requestedRefreshRate = 0.0f) {
- const std::string testId = "virtual:libsurfaceflinger_unittest:TestableSurfaceFlinger";
- return mFlinger->createDisplay(displayName, isSecure, testId, requestedRefreshRate);
+ auto createVirtualDisplay(const std::string& displayName, bool isSecure,
+ float requestedRefreshRate = 0.0f) {
+ static const std::string kTestId =
+ "virtual:libsurfaceflinger_unittest:TestableSurfaceFlinger";
+ return mFlinger->createVirtualDisplay(displayName, isSecure, kTestId, requestedRefreshRate);
}
- auto createDisplay(const String8& displayName, bool isSecure, const std::string& uniqueId,
- float requestedRefreshRate = 0.0f) {
- return mFlinger->createDisplay(displayName, isSecure, uniqueId, requestedRefreshRate);
+ auto createVirtualDisplay(const std::string& displayName, bool isSecure,
+ const std::string& uniqueId, float requestedRefreshRate = 0.0f) {
+ return mFlinger->createVirtualDisplay(displayName, isSecure, uniqueId,
+ requestedRefreshRate);
}
- auto destroyDisplay(const sp<IBinder>& displayToken) {
- return mFlinger->destroyDisplay(displayToken);
+ auto destroyVirtualDisplay(const sp<IBinder>& displayToken) {
+ return mFlinger->destroyVirtualDisplay(displayToken);
}
auto getDisplay(const sp<IBinder>& displayToken) {