Skip to content

Commit 42a97cf

Browse files
committed
SF: Allow debug.sf.hwc_service_name to be fully-qualified
The checks in ComposerHal and AidlComposerHal for whether or not to use the AIDL service interface or fall back to the HIDL service interface just checked if the name could qualified using the AIDL service name prefix, and matched to a name in the VINTF manifest. If it was not found, the name was assumed to be a HIDL interface. This check meant that a test that registers its own AIDL service and sets debug.sf.hwc_service_name appropriately cannot get SurfaceFlinger to load it. This change allows the string to be set to a fully qualified name, like "android.hardware.graphics.composer3.IComposer/fake", and AidlComposerHal will now recognize from the prefix that it is meant to be an AIDL implementation, and so it should be used. Importantly this also skips the check that the name is registered, which is problematic for a test. The old functionality is kept. Setting the string to "default" will cause a check to see if "android.hardware.graphics.composer3.IComposer/default" is registered, and use it if found, or otherwise fall back to the HIDL "default" interface if not. Flag: EXEMPT for use by tests only Bug: 372735083 Test: m and run cf target Change-Id: I26a0d92d1600a48facb75f956400433d03efcf5e
1 parent 9211819 commit 42a97cf

3 files changed

Lines changed: 23 additions & 12 deletions

File tree

services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,15 @@
2626
#include <android/binder_manager.h>
2727
#include <common/FlagManager.h>
2828
#include <common/trace.h>
29+
#include <fmt/core.h>
2930
#include <log/log.h>
3031

3132
#include <aidl/android/hardware/graphics/composer3/BnComposerCallback.h>
3233

3334
#include <algorithm>
3435
#include <cinttypes>
36+
#include <string>
37+
#include <string_view>
3538

3639
#include "HWC2.h"
3740

@@ -229,25 +232,32 @@ class AidlIComposerCallbackWrapper : public BnComposerCallback {
229232
HWC2::ComposerCallback& mCallback;
230233
};
231234

232-
std::string AidlComposer::instance(const std::string& serviceName) {
233-
return std::string(AidlIComposer::descriptor) + "/" + serviceName;
235+
std::string AidlComposer::ensureFullyQualifiedName(std::string_view serviceName) {
236+
if (!serviceName.starts_with(AidlIComposer::descriptor)) {
237+
return fmt::format("{}/{}", AidlIComposer::descriptor, serviceName);
238+
} else {
239+
return std::string{serviceName};
240+
}
234241
}
235242

236-
bool AidlComposer::isDeclared(const std::string& serviceName) {
237-
return AServiceManager_isDeclared(instance(serviceName).c_str());
243+
bool AidlComposer::namesAnAidlComposerService(std::string_view serviceName) {
244+
if (!serviceName.starts_with(AidlIComposer::descriptor)) {
245+
return AServiceManager_isDeclared(ensureFullyQualifiedName(serviceName).c_str());
246+
}
247+
return true;
238248
}
239249

240250
AidlComposer::AidlComposer(const std::string& serviceName) {
241251
// This only waits if the service is actually declared
242-
mAidlComposer = AidlIComposer::fromBinder(
243-
ndk::SpAIBinder(AServiceManager_waitForService(instance(serviceName).c_str())));
252+
mAidlComposer = AidlIComposer::fromBinder(ndk::SpAIBinder(
253+
AServiceManager_waitForService(ensureFullyQualifiedName(serviceName).c_str())));
244254
if (!mAidlComposer) {
245255
LOG_ALWAYS_FATAL("Failed to get AIDL composer service");
246256
return;
247257
}
248258

249259
if (!mAidlComposer->createClient(&mAidlComposerClient).isOk()) {
250-
LOG_ALWAYS_FATAL("Can't create AidlComposerClient, fallback to HIDL");
260+
LOG_ALWAYS_FATAL("Can't create AidlComposerClient");
251261
return;
252262
}
253263

services/surfaceflinger/DisplayHardware/AidlComposerHal.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
#include <functional>
2525
#include <optional>
2626
#include <string>
27-
#include <utility>
27+
#include <string_view>
2828
#include <vector>
2929

3030
#include <android/hardware/graphics/composer/2.4/IComposer.h>
@@ -53,7 +53,8 @@ class AidlIComposerCallbackWrapper;
5353
// Composer is a wrapper to IComposer, a proxy to server-side composer.
5454
class AidlComposer final : public Hwc2::Composer {
5555
public:
56-
static bool isDeclared(const std::string& serviceName);
56+
// Returns true if serviceName appears to be something that is meant to be used by AidlComposer.
57+
static bool namesAnAidlComposerService(std::string_view serviceName);
5758

5859
explicit AidlComposer(const std::string& serviceName);
5960
~AidlComposer() override;
@@ -258,8 +259,8 @@ class AidlComposer final : public Hwc2::Composer {
258259
// this function to execute the command queue.
259260
Error execute(Display) REQUIRES_SHARED(mMutex);
260261

261-
// returns the default instance name for the given service
262-
static std::string instance(const std::string& serviceName);
262+
// Ensures serviceName is fully qualified.
263+
static std::string ensureFullyQualifiedName(std::string_view serviceName);
263264

264265
ftl::Optional<std::reference_wrapper<ComposerClientWriter>> getWriter(Display)
265266
REQUIRES_SHARED(mMutex);

services/surfaceflinger/DisplayHardware/ComposerHal.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ namespace android::Hwc2 {
2626
Composer::~Composer() = default;
2727

2828
std::unique_ptr<Composer> Composer::create(const std::string& serviceName) {
29-
if (AidlComposer::isDeclared(serviceName)) {
29+
if (AidlComposer::namesAnAidlComposerService(serviceName)) {
3030
return std::make_unique<AidlComposer>(serviceName);
3131
}
3232

0 commit comments

Comments
 (0)