Make setActiveConfig() private, remove plumbing
This is to address leftover code review feedback from ag/9714241. We no
longer use setActiveConfig() except for a call from an internal debug
command, so make it private, and remove the plumbing to support calling
it from other processes.
Bug: 142507213
Test: atest libsurfaceflinger_unittest
Test: atest libgui_test
Test: atest SurfaceFlinger_test
Test: atest sffakehwc_test
Change-Id: I84c55f238b93b40b87219e0d584f1ab270bc3eda
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index ab4d51e..62579e5 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -397,32 +397,6 @@
return reply.readInt32();
}
- virtual status_t setActiveConfig(const sp<IBinder>& display, int id)
- {
- Parcel data, reply;
- status_t result = data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
- if (result != NO_ERROR) {
- ALOGE("setActiveConfig failed to writeInterfaceToken: %d", result);
- return result;
- }
- result = data.writeStrongBinder(display);
- if (result != NO_ERROR) {
- ALOGE("setActiveConfig failed to writeStrongBinder: %d", result);
- return result;
- }
- result = data.writeInt32(id);
- if (result != NO_ERROR) {
- ALOGE("setActiveConfig failed to writeInt32: %d", result);
- return result;
- }
- result = remote()->transact(BnSurfaceComposer::SET_ACTIVE_CONFIG, data, &reply);
- if (result != NO_ERROR) {
- ALOGE("setActiveConfig failed to transact: %d", result);
- return result;
- }
- return reply.readInt32();
- }
-
virtual status_t getDisplayColorModes(const sp<IBinder>& display,
Vector<ColorMode>* outColorModes) {
Parcel data, reply;
@@ -1358,14 +1332,6 @@
reply->writeInt32(id);
return NO_ERROR;
}
- case SET_ACTIVE_CONFIG: {
- CHECK_INTERFACE(ISurfaceComposer, data, reply);
- sp<IBinder> display = data.readStrongBinder();
- int id = data.readInt32();
- status_t result = setActiveConfig(display, id);
- reply->writeInt32(result);
- return NO_ERROR;
- }
case GET_DISPLAY_COLOR_MODES: {
CHECK_INTERFACE(ISurfaceComposer, data, reply);
Vector<ColorMode> colorModes;
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 1d887ea..5531676 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1601,10 +1601,6 @@
return ComposerService::getComposerService()->getActiveConfig(display);
}
-status_t SurfaceComposerClient::setActiveConfig(const sp<IBinder>& display, int id) {
- return ComposerService::getComposerService()->setActiveConfig(display, id);
-}
-
status_t SurfaceComposerClient::setDesiredDisplayConfigSpecs(const sp<IBinder>& displayToken,
int32_t defaultConfig,
float minRefreshRate,
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index 998973c..3f9ccc5 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -182,10 +182,6 @@
* currently active */
virtual int getActiveConfig(const sp<IBinder>& display) = 0;
- /* specifies which configuration (of those returned by getDisplayInfo)
- * should be used */
- virtual status_t setActiveConfig(const sp<IBinder>& display, int id) = 0;
-
virtual status_t getDisplayColorModes(const sp<IBinder>& display,
Vector<ui::ColorMode>* outColorModes) = 0;
virtual status_t getDisplayNativePrimaries(const sp<IBinder>& display,
@@ -513,7 +509,6 @@
GET_SUPPORTED_FRAME_TIMESTAMPS,
GET_DISPLAY_CONFIGS,
GET_ACTIVE_CONFIG,
- SET_ACTIVE_CONFIG,
CONNECT_DISPLAY_UNUSED, // unused, fails permissions check
CAPTURE_SCREEN,
CAPTURE_LAYERS,
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 86468a4..0c830c9 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -113,10 +113,6 @@
// returned by getDisplayInfo)
static int getActiveConfig(const sp<IBinder>& display);
- // Set a new active configuration using an index relative to the list
- // returned by getDisplayInfo
- static status_t setActiveConfig(const sp<IBinder>& display, int id);
-
// Sets the refresh rate boundaries for display configuration.
// For all other parameters, default configuration is used. The index for the default is
// corresponting to the configs returned from getDisplayConfigs().
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 0445755..8c56724 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -723,10 +723,6 @@
status_t getDisplayStats(const sp<IBinder>& /*display*/,
DisplayStatInfo* /*stats*/) override { return NO_ERROR; }
int getActiveConfig(const sp<IBinder>& /*display*/) override { return 0; }
- status_t setActiveConfig(const sp<IBinder>& /*display*/, int /*id*/)
- override {
- return NO_ERROR;
- }
status_t getDisplayColorModes(const sp<IBinder>& /*display*/,
Vector<ColorMode>* /*outColorModes*/) override {
return NO_ERROR;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index d02d783..821e15c2 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4522,7 +4522,6 @@
case ENABLE_VSYNC_INJECTIONS:
case GET_ANIMATION_FRAME_STATS:
case GET_HDR_CAPABILITIES:
- case SET_ACTIVE_CONFIG:
case SET_DESIRED_DISPLAY_CONFIG_SPECS:
case GET_DESIRED_DISPLAY_CONFIG_SPECS:
case SET_ACTIVE_COLOR_MODE:
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 2f84b13..4683cef 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -445,7 +445,6 @@
bool* outSupported) const override;
void setGameContentType(const sp<IBinder>& displayToken, bool on) override;
void setPowerMode(const sp<IBinder>& displayToken, int mode) override;
- status_t setActiveConfig(const sp<IBinder>& displayToken, int id) override;
status_t clearAnimationFrameStats() override;
status_t getAnimationFrameStats(FrameStats* outStats) const override;
status_t getHdrCapabilities(const sp<IBinder>& displayToken,
@@ -537,6 +536,7 @@
void onInitializeDisplays() REQUIRES(mStateLock);
// Sets the desired active config bit. It obtains the lock, and sets mDesiredActiveConfig.
void setDesiredActiveConfig(const ActiveConfigInfo& info) REQUIRES(mStateLock);
+ status_t setActiveConfig(const sp<IBinder>& displayToken, int id);
// Once HWC has returned the present fence, this sets the active config and a new refresh
// rate in SF.
void setActiveConfigInternal() REQUIRES(mStateLock);
diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp
index b1bb7fd..11eecba 100644
--- a/services/surfaceflinger/tests/Credentials_test.cpp
+++ b/services/surfaceflinger/tests/Credentials_test.cpp
@@ -215,10 +215,17 @@
ASSERT_NO_FATAL_FAILURE(checkWithPrivileges<status_t>(condition, NO_ERROR, NO_ERROR));
}
-TEST_F(CredentialsTest, SetActiveConfigTest) {
+TEST_F(CredentialsTest, SetDesiredDisplayConfigsTest) {
const auto display = SurfaceComposerClient::getInternalDisplayToken();
+ int32_t defaultConfig;
+ float minFps;
+ float maxFps;
+ status_t res = SurfaceComposerClient::getDesiredDisplayConfigSpecs(display, &defaultConfig,
+ &minFps, &maxFps);
+ ASSERT_EQ(res, NO_ERROR);
std::function<status_t()> condition = [=]() {
- return SurfaceComposerClient::setActiveConfig(display, 0);
+ return SurfaceComposerClient::setDesiredDisplayConfigSpecs(display, defaultConfig, minFps,
+ maxFps);
};
ASSERT_NO_FATAL_FAILURE(checkWithPrivileges<status_t>(condition, NO_ERROR, PERMISSION_DENIED));
}
diff --git a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
index 09fdbdf..e778b85 100644
--- a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
+++ b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
@@ -433,7 +433,10 @@
for (int i = 0; i < configs.size(); i++) {
if (configs[i].w == 800u) {
- EXPECT_EQ(NO_ERROR, SurfaceComposerClient::setActiveConfig(display, i));
+ EXPECT_EQ(NO_ERROR,
+ SurfaceComposerClient::setDesiredDisplayConfigSpecs(display, i,
+ configs[i].fps,
+ configs[i].fps));
waitForDisplayTransaction();
EXPECT_TRUE(waitForConfigChangedEvent(EXTERNAL_DISPLAY, i));
break;
@@ -531,7 +534,10 @@
for (int i = 0; i < configs.size(); i++) {
if (configs[i].fps == 1e9f / 11'111'111) {
- EXPECT_EQ(NO_ERROR, SurfaceComposerClient::setActiveConfig(display, i));
+ EXPECT_EQ(NO_ERROR,
+ SurfaceComposerClient::setDesiredDisplayConfigSpecs(display, i,
+ configs[i].fps,
+ configs[i].fps));
waitForDisplayTransaction();
EXPECT_TRUE(waitForConfigChangedEvent(EXTERNAL_DISPLAY, i));
break;
@@ -639,7 +645,10 @@
for (int i = 0; i < configs.size(); i++) {
if (configs[i].w == 800u && configs[i].fps == 1e9f / 11'111'111) {
- EXPECT_EQ(NO_ERROR, SurfaceComposerClient::setActiveConfig(display, i));
+ EXPECT_EQ(NO_ERROR,
+ SurfaceComposerClient::setDesiredDisplayConfigSpecs(display, i,
+ configs[i].fps,
+ configs[i].fps));
waitForDisplayTransaction();
EXPECT_TRUE(waitForConfigChangedEvent(EXTERNAL_DISPLAY, i));
break;
@@ -679,7 +688,10 @@
for (int i = 0; i < configs.size(); i++) {
if (configs[i].fps == 1e9f / 8'333'333) {
- EXPECT_EQ(NO_ERROR, SurfaceComposerClient::setActiveConfig(display, i));
+ EXPECT_EQ(NO_ERROR,
+ SurfaceComposerClient::setDesiredDisplayConfigSpecs(display, i,
+ configs[i].fps,
+ configs[i].fps));
waitForDisplayTransaction();
EXPECT_TRUE(waitForConfigChangedEvent(EXTERNAL_DISPLAY, i));
break;
@@ -719,7 +731,10 @@
for (int i = 0; i < configs.size(); i++) {
if (configs[i].w == 1600 && configs[i].fps == 1e9f / 11'111'111) {
- EXPECT_EQ(NO_ERROR, SurfaceComposerClient::setActiveConfig(display, i));
+ EXPECT_EQ(NO_ERROR,
+ SurfaceComposerClient::setDesiredDisplayConfigSpecs(display, i,
+ configs[i].fps,
+ configs[i].fps));
waitForDisplayTransaction();
EXPECT_TRUE(waitForConfigChangedEvent(EXTERNAL_DISPLAY, i));
break;