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/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5181fb8..9a575ca 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -56,6 +56,7 @@
#include <configstore/Utils.h>
#include <cutils/compiler.h>
#include <cutils/properties.h>
+#include <fmt/format.h>
#include <ftl/algorithm.h>
#include <ftl/concat.h>
#include <ftl/fake_guard.h>
@@ -573,8 +574,9 @@
mScheduler->run();
}
-sp<IBinder> SurfaceFlinger::createDisplay(const String8& displayName, bool isSecure,
- const std::string& uniqueId, float requestedRefreshRate) {
+sp<IBinder> SurfaceFlinger::createVirtualDisplay(const std::string& displayName, bool isSecure,
+ const std::string& uniqueId,
+ float requestedRefreshRate) {
// SurfaceComposerAIDL checks for some permissions, but adding an additional check here.
// This is to ensure that only root, system, and graphics can request to create a secure
// display. Secure displays can show secure content so we add an additional restriction on it.
@@ -614,22 +616,23 @@
return token;
}
-void SurfaceFlinger::destroyDisplay(const sp<IBinder>& displayToken) {
+status_t SurfaceFlinger::destroyVirtualDisplay(const sp<IBinder>& displayToken) {
Mutex::Autolock lock(mStateLock);
const ssize_t index = mCurrentState.displays.indexOfKey(displayToken);
if (index < 0) {
ALOGE("%s: Invalid display token %p", __func__, displayToken.get());
- return;
+ return NAME_NOT_FOUND;
}
const DisplayDeviceState& state = mCurrentState.displays.valueAt(index);
if (state.physical) {
ALOGE("%s: Invalid operation on physical display", __func__);
- return;
+ return INVALID_OPERATION;
}
mCurrentState.displays.removeItemsAt(index);
setTransactionFlags(eDisplayTransactionNeeded);
+ return NO_ERROR;
}
void SurfaceFlinger::enableHalVirtualDisplays(bool enable) {
@@ -6531,18 +6534,19 @@
StringAppendF(&result, "DisplayColorSetting: %s\n",
decodeDisplayColorSetting(mDisplayColorSetting).c_str());
- // TODO: print out if wide-color mode is active or not
+ // TODO: print out if wide-color mode is active or not.
for (const auto& [id, display] : mPhysicalDisplays) {
StringAppendF(&result, "Display %s color modes:\n", to_string(id).c_str());
for (const auto mode : display.snapshot().colorModes()) {
- StringAppendF(&result, " %s (%d)\n", decodeColorMode(mode).c_str(), mode);
+ StringAppendF(&result, " %s (%d)\n", decodeColorMode(mode).c_str(),
+ fmt::underlying(mode));
}
if (const auto display = getDisplayDeviceLocked(id)) {
ui::ColorMode currentMode = display->getCompositionDisplay()->getState().colorMode;
StringAppendF(&result, " Current color mode: %s (%d)\n",
- decodeColorMode(currentMode).c_str(), currentMode);
+ decodeColorMode(currentMode).c_str(), fmt::underlying(currentMode));
}
}
result.append("\n");
@@ -6999,8 +7003,8 @@
// Used by apps to hook Choreographer to SurfaceFlinger.
case CREATE_DISPLAY_EVENT_CONNECTION:
case CREATE_CONNECTION:
- case CREATE_DISPLAY:
- case DESTROY_DISPLAY:
+ case CREATE_VIRTUAL_DISPLAY:
+ case DESTROY_VIRTUAL_DISPLAY:
case GET_PRIMARY_PHYSICAL_DISPLAY_ID:
case GET_PHYSICAL_DISPLAY_IDS:
case GET_PHYSICAL_DISPLAY_TOKEN:
@@ -9553,26 +9557,25 @@
}
}
-binder::Status SurfaceComposerAIDL::createDisplay(const std::string& displayName, bool isSecure,
- const std::string& uniqueId,
- float requestedRefreshRate,
- sp<IBinder>* outDisplay) {
+binder::Status SurfaceComposerAIDL::createVirtualDisplay(const std::string& displayName,
+ bool isSecure, const std::string& uniqueId,
+ float requestedRefreshRate,
+ sp<IBinder>* outDisplay) {
status_t status = checkAccessPermission();
if (status != OK) {
return binderStatusFromStatusT(status);
}
- String8 displayName8 = String8::format("%s", displayName.c_str());
- *outDisplay = mFlinger->createDisplay(displayName8, isSecure, uniqueId, requestedRefreshRate);
+ *outDisplay =
+ mFlinger->createVirtualDisplay(displayName, isSecure, uniqueId, requestedRefreshRate);
return binder::Status::ok();
}
-binder::Status SurfaceComposerAIDL::destroyDisplay(const sp<IBinder>& display) {
+binder::Status SurfaceComposerAIDL::destroyVirtualDisplay(const sp<IBinder>& displayToken) {
status_t status = checkAccessPermission();
if (status != OK) {
return binderStatusFromStatusT(status);
}
- mFlinger->destroyDisplay(display);
- return binder::Status::ok();
+ return binder::Status::fromStatusT(mFlinger->destroyVirtualDisplay(displayToken));
}
binder::Status SurfaceComposerAIDL::getPhysicalDisplayIds(std::vector<int64_t>* outDisplayIds) {