diff options
28 files changed, 444 insertions, 71 deletions
diff --git a/cmds/dumpsys/tests/dumpsys_test.cpp b/cmds/dumpsys/tests/dumpsys_test.cpp index b8e5ce1a63..17ae0aa2bf 100644 --- a/cmds/dumpsys/tests/dumpsys_test.cpp +++ b/cmds/dumpsys/tests/dumpsys_test.cpp @@ -67,6 +67,8 @@ class ServiceManagerMock : public IServiceManager { MOCK_METHOD2(unregisterForNotifications, status_t(const String16&, const sp<LocalRegistrationCallback>&)); MOCK_METHOD0(getServiceDebugInfo, std::vector<ServiceDebugInfo>()); + MOCK_METHOD1(enableAddServiceCache, void(bool)); + protected: MOCK_METHOD0(onAsBinder, IBinder*()); }; diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp index fa7cb64f3a..38a125bb54 100644 --- a/cmds/servicemanager/ServiceManager.cpp +++ b/cmds/servicemanager/ServiceManager.cpp @@ -396,7 +396,7 @@ Status ServiceManager::getService(const std::string& name, sp<IBinder>* outBinde SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); - *outBinder = tryGetBinder(name, true); + *outBinder = tryGetBinder(name, true).service; // returns ok regardless of result for legacy reasons return Status::ok(); } @@ -430,13 +430,15 @@ os::Service ServiceManager::tryGetService(const std::string& name, bool startIfN return os::Service::make<os::Service::Tag::accessor>(nullptr); } return os::Service::make<os::Service::Tag::accessor>( - tryGetBinder(*accessorName, startIfNotFound)); + tryGetBinder(*accessorName, startIfNotFound).service); } else { - return os::Service::make<os::Service::Tag::binder>(tryGetBinder(name, startIfNotFound)); + return os::Service::make<os::Service::Tag::serviceWithMetadata>( + tryGetBinder(name, startIfNotFound)); } } -sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNotFound) { +os::ServiceWithMetadata ServiceManager::tryGetBinder(const std::string& name, + bool startIfNotFound) { SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_PROTO_FIELDS( PERFETTO_TE_PROTO_FIELD_CSTR(kProtoServiceName, name.c_str()))); @@ -450,13 +452,13 @@ sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNo if (!service->allowIsolated && is_multiuser_uid_isolated(ctx.uid)) { LOG(WARNING) << "Isolated app with UID " << ctx.uid << " requested '" << name << "', but the service is not allowed for isolated apps."; - return nullptr; + return os::ServiceWithMetadata(); } out = service->binder; } if (!mAccess->canFind(ctx, name)) { - return nullptr; + return os::ServiceWithMetadata(); } if (!out && startIfNotFound) { @@ -473,8 +475,11 @@ sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNo CHECK(handleServiceClientCallback(2 /* sm + transaction */, name, false)); service->guaranteeClient = true; } - - return out; + os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata(); + serviceWithMetadata.service = out; + serviceWithMetadata.isLazyService = + service ? service->dumpPriority & FLAG_IS_LAZY_SERVICE : false; + return serviceWithMetadata; } bool isValidServiceName(const std::string& name) { diff --git a/cmds/servicemanager/ServiceManager.h b/cmds/servicemanager/ServiceManager.h index c92141b393..964abee6af 100644 --- a/cmds/servicemanager/ServiceManager.h +++ b/cmds/servicemanager/ServiceManager.h @@ -114,7 +114,7 @@ private: void removeClientCallback(const wp<IBinder>& who, ClientCallbackMap::iterator* it); os::Service tryGetService(const std::string& name, bool startIfNotFound); - sp<IBinder> tryGetBinder(const std::string& name, bool startIfNotFound); + os::ServiceWithMetadata tryGetBinder(const std::string& name, bool startIfNotFound); binder::Status canAddService(const Access::CallingContext& ctx, const std::string& name, std::optional<std::string>* accessor); binder::Status canFindService(const Access::CallingContext& ctx, const std::string& name, diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp index 95f459f7f0..e620770e18 100644 --- a/cmds/servicemanager/test_sm.cpp +++ b/cmds/servicemanager/test_sm.cpp @@ -92,6 +92,11 @@ TEST(AddService, HappyHappy) { auto sm = getPermissiveServiceManager(); EXPECT_TRUE(sm->addService("foo", getBinder(), false /*allowIsolated*/, IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk()); + + EXPECT_TRUE(sm->addService("lazyfoo", getBinder(), false /*allowIsolated*/, + IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT | + IServiceManager::FLAG_IS_LAZY_SERVICE) + .isOk()); } TEST(AddService, EmptyNameDisallowed) { @@ -156,7 +161,7 @@ TEST(AddService, OverwriteExistingService) { Service outA; EXPECT_TRUE(sm->getService2("foo", &outA).isOk()); - EXPECT_EQ(serviceA, outA.get<Service::Tag::binder>()); + EXPECT_EQ(serviceA, outA.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinderA; EXPECT_TRUE(sm->getService("foo", &outBinderA).isOk()); EXPECT_EQ(serviceA, outBinderA); @@ -168,7 +173,7 @@ TEST(AddService, OverwriteExistingService) { Service outB; EXPECT_TRUE(sm->getService2("foo", &outB).isOk()); - EXPECT_EQ(serviceB, outB.get<Service::Tag::binder>()); + EXPECT_EQ(serviceB, outB.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinderB; EXPECT_TRUE(sm->getService("foo", &outBinderB).isOk()); EXPECT_EQ(serviceB, outBinderB); @@ -195,7 +200,7 @@ TEST(GetService, HappyHappy) { Service out; EXPECT_TRUE(sm->getService2("foo", &out).isOk()); - EXPECT_EQ(service, out.get<Service::Tag::binder>()); + EXPECT_EQ(service, out.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinder; EXPECT_TRUE(sm->getService("foo", &outBinder).isOk()); EXPECT_EQ(service, outBinder); @@ -206,7 +211,7 @@ TEST(GetService, NonExistant) { Service out; EXPECT_TRUE(sm->getService2("foo", &out).isOk()); - EXPECT_EQ(nullptr, out.get<Service::Tag::binder>()); + EXPECT_EQ(nullptr, out.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinder; EXPECT_TRUE(sm->getService("foo", &outBinder).isOk()); EXPECT_EQ(nullptr, outBinder); @@ -227,7 +232,7 @@ TEST(GetService, NoPermissionsForGettingService) { Service out; // returns nullptr but has OK status for legacy compatibility EXPECT_TRUE(sm->getService2("foo", &out).isOk()); - EXPECT_EQ(nullptr, out.get<Service::Tag::binder>()); + EXPECT_EQ(nullptr, out.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinder; EXPECT_TRUE(sm->getService("foo", &outBinder).isOk()); EXPECT_EQ(nullptr, outBinder); @@ -257,7 +262,7 @@ TEST(GetService, AllowedFromIsolated) { Service out; EXPECT_TRUE(sm->getService2("foo", &out).isOk()); - EXPECT_EQ(service, out.get<Service::Tag::binder>()); + EXPECT_EQ(service, out.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinder; EXPECT_TRUE(sm->getService("foo", &outBinder).isOk()); EXPECT_EQ(service, outBinder); @@ -289,7 +294,7 @@ TEST(GetService, NotAllowedFromIsolated) { Service out; // returns nullptr but has OK status for legacy compatibility EXPECT_TRUE(sm->getService2("foo", &out).isOk()); - EXPECT_EQ(nullptr, out.get<Service::Tag::binder>()); + EXPECT_EQ(nullptr, out.get<Service::Tag::serviceWithMetadata>().service); sp<IBinder> outBinder; EXPECT_TRUE(sm->getService("foo", &outBinder).isOk()); EXPECT_EQ(nullptr, outBinder); diff --git a/include/android/OWNERS b/include/android/OWNERS index 5b014d0425..9dfa27995d 100644 --- a/include/android/OWNERS +++ b/include/android/OWNERS @@ -9,4 +9,5 @@ per-file *luts* = file:platform/frameworks/base:/graphics/java/android/graphics/ # ADPF per-file performance_hint.h = file:platform/frameworks/base:/ADPF_OWNERS +per-file system_health.h = file:platform/frameworks/base:/ADPF_OWNERS per-file thermal.h = file:platform/frameworks/base:/ADPF_OWNERS diff --git a/include/android/thermal.h b/include/android/thermal.h index 7f9d2edfc7..1d7ad12294 100644 --- a/include/android/thermal.h +++ b/include/android/thermal.h @@ -254,7 +254,7 @@ typedef struct AThermalHeadroomThreshold AThermalHeadroomThreshold; * The headroom threshold is used to interpret the possible thermal throttling status based on * the headroom prediction. For example, if the headroom threshold for * {@link ATHERMAL_STATUS_LIGHT} is 0.7, and a headroom prediction in 10s returns 0.75 - * (or {@code AThermal_getThermalHeadroom(10)=0.75}), one can expect that in 10 seconds the system + * (or `AThermal_getThermalHeadroom(10)=0.75`), one can expect that in 10 seconds the system * could be in lightly throttled state if the workload remains the same. The app can consider * taking actions according to the nearest throttling status the difference between the headroom and * the threshold. @@ -262,10 +262,10 @@ typedef struct AThermalHeadroomThreshold AThermalHeadroomThreshold; * For new devices it's guaranteed to have a single sensor, but for older devices with multiple * sensors reporting different threshold values, the minimum threshold is taken to be conservative * on predictions. Thus, when reading real-time headroom, it's not guaranteed that a real-time value - * of 0.75 (or {@code AThermal_getThermalHeadroom(0)}=0.75) exceeding the threshold of 0.7 above + * of 0.75 (or `AThermal_getThermalHeadroom(0)`=0.75) exceeding the threshold of 0.7 above * will always come with lightly throttled state - * (or {@code AThermal_getCurrentThermalStatus()=ATHERMAL_STATUS_LIGHT}) but it can be lower - * (or {@code AThermal_getCurrentThermalStatus()=ATHERMAL_STATUS_NONE}). + * (or `AThermal_getCurrentThermalStatus()=ATHERMAL_STATUS_LIGHT`) but it can be lower + * (or `AThermal_getCurrentThermalStatus()=ATHERMAL_STATUS_NONE`). * While it's always guaranteed that the device won't be throttled heavier than the unmet * threshold's state, so a real-time headroom of 0.75 will never come with * {@link #ATHERMAL_STATUS_MODERATE} but always lower, and 0.65 will never come with diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index 0a6117808a..aac369ddd1 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -456,6 +456,28 @@ cc_library_shared { } soong_config_module_type { + name: "libbinder_remove_cache_static_list_config", + module_type: "cc_defaults", + config_namespace: "libbinder", + bool_variables: ["release_libbinder_remove_cache_static_list"], + properties: [ + "cflags", + ], +} + +libbinder_remove_cache_static_list_config { + name: "libbinder_remove_cache_static_list_flag", + soong_config_variables: { + release_libbinder_remove_cache_static_list: { + cflags: ["-DLIBBINDER_REMOVE_CACHE_STATIC_LIST"], + conditions_default: { + cflags: ["-DNO_LIBBINDER_REMOVE_CACHE_STATIC_LIST"], + }, + }, + }, +} + +soong_config_module_type { name: "libbinder_client_cache_config", module_type: "cc_defaults", config_namespace: "libbinder", @@ -477,9 +499,35 @@ libbinder_client_cache_config { }, } +soong_config_module_type { + name: "libbinder_addservice_cache_config", + module_type: "cc_defaults", + config_namespace: "libbinder", + bool_variables: ["release_libbinder_addservice_cache"], + properties: [ + "cflags", + ], +} + +libbinder_addservice_cache_config { + name: "libbinder_addservice_cache_flag", + soong_config_variables: { + release_libbinder_addservice_cache: { + cflags: ["-DLIBBINDER_ADDSERVICE_CACHE"], + conditions_default: { + cflags: ["-DNO_LIBBINDER_ADDSERVICE_CACHE"], + }, + }, + }, +} + cc_defaults { name: "libbinder_kernel_defaults", - defaults: ["libbinder_client_cache_flag"], + defaults: [ + "libbinder_client_cache_flag", + "libbinder_addservice_cache_flag", + "libbinder_remove_cache_static_list_flag", + ], srcs: [ "BufferedTextOutput.cpp", "BackendUnifiedServiceManager.cpp", @@ -801,6 +849,7 @@ filegroup { "aidl/android/os/IServiceCallback.aidl", "aidl/android/os/IServiceManager.aidl", "aidl/android/os/Service.aidl", + "aidl/android/os/ServiceWithMetadata.aidl", "aidl/android/os/ServiceDebugInfo.aidl", ], path: "aidl", diff --git a/libs/binder/BackendUnifiedServiceManager.cpp b/libs/binder/BackendUnifiedServiceManager.cpp index d32eecdc60..03d687a9cc 100644 --- a/libs/binder/BackendUnifiedServiceManager.cpp +++ b/libs/binder/BackendUnifiedServiceManager.cpp @@ -16,6 +16,7 @@ #include "BackendUnifiedServiceManager.h" #include <android/os/IAccessor.h> +#include <android/os/IServiceManager.h> #include <binder/RpcSession.h> #if defined(__BIONIC__) && !defined(__ANDROID_VNDK__) @@ -30,6 +31,18 @@ constexpr bool kUseCache = true; constexpr bool kUseCache = false; #endif +#ifdef LIBBINDER_ADDSERVICE_CACHE +constexpr bool kUseCacheInAddService = true; +#else +constexpr bool kUseCacheInAddService = false; +#endif + +#ifdef LIBBINDER_REMOVE_CACHE_STATIC_LIST +constexpr bool kRemoveStaticList = true; +#else +constexpr bool kRemoveStaticList = false; +#endif + using AidlServiceManager = android::os::IServiceManager; using android::os::IAccessor; using binder::Status; @@ -104,6 +117,13 @@ static const char* kStaticCachableList[] = { // go/keep-sorted end }; +os::ServiceWithMetadata createServiceWithMetadata(const sp<IBinder>& service, bool isLazyService) { + os::ServiceWithMetadata out = os::ServiceWithMetadata(); + out.service = service; + out.isLazyService = isLazyService; + return out; +} + bool BinderCacheWithInvalidation::isClientSideCachingEnabled(const std::string& serviceName) { sp<ProcessState> self = ProcessState::selfOrNull(); if (!self || self->getThreadPoolMaxTotalThreadCount() <= 0) { @@ -125,31 +145,44 @@ Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName, if (!kUseCache) { return Status::ok(); } + + if (service.getTag() == os::Service::Tag::serviceWithMetadata) { + auto serviceWithMetadata = service.get<os::Service::Tag::serviceWithMetadata>(); + return updateCache(serviceName, serviceWithMetadata.service, + serviceWithMetadata.isLazyService); + } + return Status::ok(); +} + +Status BackendUnifiedServiceManager::updateCache(const std::string& serviceName, + const sp<IBinder>& binder, bool isServiceLazy) { std::string traceStr; + // Don't cache if service is lazy + if (kRemoveStaticList && isServiceLazy) { + return Status::ok(); + } if (atrace_is_tag_enabled(ATRACE_TAG_AIDL)) { traceStr = "BinderCacheWithInvalidation::updateCache : " + serviceName; } binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, traceStr.c_str()); - - if (service.getTag() == os::Service::Tag::binder) { - sp<IBinder> binder = service.get<os::Service::Tag::binder>(); - if (!binder) { - binder::ScopedTrace - aidlTrace(ATRACE_TAG_AIDL, - "BinderCacheWithInvalidation::updateCache failed: binder_null"); - } else if (!binder->isBinderAlive()) { - binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, - "BinderCacheWithInvalidation::updateCache failed: " - "isBinderAlive_false"); - } else if (mCacheForGetService->isClientSideCachingEnabled(serviceName)) { - binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, - "BinderCacheWithInvalidation::updateCache successful"); - return mCacheForGetService->setItem(serviceName, binder); - } else { - binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, - "BinderCacheWithInvalidation::updateCache failed: " - "caching_not_enabled"); - } + if (!binder) { + binder::ScopedTrace + aidlTrace(ATRACE_TAG_AIDL, + "BinderCacheWithInvalidation::updateCache failed: binder_null"); + } else if (!binder->isBinderAlive()) { + binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, + "BinderCacheWithInvalidation::updateCache failed: " + "isBinderAlive_false"); + } + // If we reach here with kRemoveStaticList=true then we know service isn't lazy + else if (kRemoveStaticList || mCacheForGetService->isClientSideCachingEnabled(serviceName)) { + binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, + "BinderCacheWithInvalidation::updateCache successful"); + return mCacheForGetService->setItem(serviceName, binder); + } else { + binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, + "BinderCacheWithInvalidation::updateCache failed: " + "caching_not_enabled"); } return Status::ok(); } @@ -162,7 +195,7 @@ bool BackendUnifiedServiceManager::returnIfCached(const std::string& serviceName sp<IBinder> item = mCacheForGetService->getItem(serviceName); // TODO(b/363177618): Enable caching for binders which are always null. if (item != nullptr && item->isBinderAlive()) { - *_out = os::Service::make<os::Service::Tag::binder>(item); + *_out = createServiceWithMetadata(item, false); return true; } return false; @@ -177,7 +210,7 @@ Status BackendUnifiedServiceManager::getService(const ::std::string& name, sp<IBinder>* _aidl_return) { os::Service service; Status status = getService2(name, &service); - *_aidl_return = service.get<os::Service::Tag::binder>(); + *_aidl_return = service.get<os::Service::Tag::serviceWithMetadata>().service; return status; } @@ -216,14 +249,16 @@ Status BackendUnifiedServiceManager::checkService(const ::std::string& name, os: Status BackendUnifiedServiceManager::toBinderService(const ::std::string& name, const os::Service& in, os::Service* _out) { switch (in.getTag()) { - case os::Service::Tag::binder: { - if (in.get<os::Service::Tag::binder>() == nullptr) { + case os::Service::Tag::serviceWithMetadata: { + auto serviceWithMetadata = in.get<os::Service::Tag::serviceWithMetadata>(); + if (serviceWithMetadata.service == nullptr) { // failed to find a service. Check to see if we have any local // injected Accessors for this service. os::Service accessor; Status status = getInjectedAccessor(name, &accessor); if (!status.isOk()) { - *_out = os::Service::make<os::Service::Tag::binder>(nullptr); + *_out = os::Service::make<os::Service::Tag::serviceWithMetadata>( + createServiceWithMetadata(nullptr, false)); return status; } if (accessor.getTag() == os::Service::Tag::accessor && @@ -244,7 +279,8 @@ Status BackendUnifiedServiceManager::toBinderService(const ::std::string& name, sp<IAccessor> accessor = interface_cast<IAccessor>(accessorBinder); if (accessor == nullptr) { ALOGE("Service#accessor doesn't have accessor. VM is maybe starting..."); - *_out = os::Service::make<os::Service::Tag::binder>(nullptr); + *_out = os::Service::make<os::Service::Tag::serviceWithMetadata>( + createServiceWithMetadata(nullptr, false)); return Status::ok(); } auto request = [=] { @@ -265,7 +301,8 @@ Status BackendUnifiedServiceManager::toBinderService(const ::std::string& name, return Status::fromStatusT(status); } session->setSessionSpecificRoot(accessorBinder); - *_out = os::Service::make<os::Service::Tag::binder>(session->getRootObject()); + *_out = os::Service::make<os::Service::Tag::serviceWithMetadata>( + createServiceWithMetadata(session->getRootObject(), false)); return Status::ok(); } default: { @@ -277,7 +314,13 @@ Status BackendUnifiedServiceManager::toBinderService(const ::std::string& name, Status BackendUnifiedServiceManager::addService(const ::std::string& name, const sp<IBinder>& service, bool allowIsolated, int32_t dumpPriority) { - return mTheRealServiceManager->addService(name, service, allowIsolated, dumpPriority); + Status status = mTheRealServiceManager->addService(name, service, allowIsolated, dumpPriority); + // mEnableAddServiceCache is true by default. + if (kUseCacheInAddService && mEnableAddServiceCache && status.isOk()) { + return updateCache(name, service, + dumpPriority & android::os::IServiceManager::FLAG_IS_LAZY_SERVICE); + } + return status; } Status BackendUnifiedServiceManager::listServices(int32_t dumpPriority, ::std::vector<::std::string>* _aidl_return) { diff --git a/libs/binder/BackendUnifiedServiceManager.h b/libs/binder/BackendUnifiedServiceManager.h index abc0eda7eb..6a0d06a079 100644 --- a/libs/binder/BackendUnifiedServiceManager.h +++ b/libs/binder/BackendUnifiedServiceManager.h @@ -105,8 +105,7 @@ public: binder::ScopedTrace aidlTrace(ATRACE_TAG_AIDL, "BinderCacheWithInvalidation::setItem Successfully Cached"); std::lock_guard<std::mutex> lock(mCacheMutex); - Entry entry = {.service = item, .deathRecipient = deathRecipient}; - mCache[key] = entry; + mCache[key] = {.service = item, .deathRecipient = deathRecipient}; return binder::Status::ok(); } @@ -147,17 +146,21 @@ public: const sp<IBinder>& service) override; binder::Status getServiceDebugInfo(::std::vector<os::ServiceDebugInfo>* _aidl_return) override; + void enableAddServiceCache(bool value) { mEnableAddServiceCache = value; } // for legacy ABI const String16& getInterfaceDescriptor() const override { return mTheRealServiceManager->getInterfaceDescriptor(); } private: + bool mEnableAddServiceCache = true; std::shared_ptr<BinderCacheWithInvalidation> mCacheForGetService; sp<os::IServiceManager> mTheRealServiceManager; binder::Status toBinderService(const ::std::string& name, const os::Service& in, os::Service* _out); binder::Status updateCache(const std::string& serviceName, const os::Service& service); + binder::Status updateCache(const std::string& serviceName, const sp<IBinder>& binder, + bool isLazyService); bool returnIfCached(const std::string& serviceName, os::Service* _out); }; diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index 39d8c2446e..53435c357b 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -127,6 +127,8 @@ public: } IBinder* onAsBinder() override { return IInterface::asBinder(mUnifiedServiceManager).get(); } + void enableAddServiceCache(bool value) { mUnifiedServiceManager->enableAddServiceCache(value); } + protected: sp<BackendUnifiedServiceManager> mUnifiedServiceManager; // AidlRegistrationCallback -> services that its been registered for @@ -150,7 +152,8 @@ protected: virtual Status realGetService(const std::string& name, sp<IBinder>* _aidl_return) { Service service; Status status = mUnifiedServiceManager->getService2(name, &service); - *_aidl_return = service.get<Service::Tag::binder>(); + auto serviceWithMetadata = service.get<Service::Tag::serviceWithMetadata>(); + *_aidl_return = serviceWithMetadata.service; return status; } }; @@ -605,7 +608,7 @@ sp<IBinder> CppBackendShim::checkService(const String16& name) const { if (!mUnifiedServiceManager->checkService(String8(name).c_str(), &ret).isOk()) { return nullptr; } - return ret.get<Service::Tag::binder>(); + return ret.get<Service::Tag::serviceWithMetadata>().service; } status_t CppBackendShim::addService(const String16& name, const sp<IBinder>& service, diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp index 0f0af0b142..27cfe0cf21 100644 --- a/libs/binder/LazyServiceRegistrar.cpp +++ b/libs/binder/LazyServiceRegistrar.cpp @@ -19,7 +19,6 @@ #include <android/os/BnClientCallback.h> #include <android/os/IServiceManager.h> #include <binder/IPCThreadState.h> -#include <binder/IServiceManager.h> #include <binder/LazyServiceRegistrar.h> namespace android { @@ -134,6 +133,12 @@ bool ClientCounterCallbackImpl::registerServiceLocked(const sp<IBinder>& service std::string regStr = (reRegister) ? "Re-registering" : "Registering"; ALOGI("%s service %s", regStr.c_str(), name.c_str()); + if (dumpFlags & android::os::IServiceManager::FLAG_IS_LAZY_SERVICE) { + ALOGW("FLAG_IS_LAZY_SERVICE flag already set. This should only be set by " + "ClientCounterCallbackImpl in LazyServiceRegistrar"); + } + dumpFlags |= android::os::IServiceManager::FLAG_IS_LAZY_SERVICE; + if (Status status = manager->addService(name.c_str(), service, allowIsolated, dumpFlags); !status.isOk()) { ALOGE("Failed to register service %s (%s)", name.c_str(), status.toString8().c_str()); diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING index 99a9c911bf..9e5e79f89b 100644 --- a/libs/binder/TEST_MAPPING +++ b/libs/binder/TEST_MAPPING @@ -70,6 +70,9 @@ "name": "CtsOsTestCases_ParcelAndBinderTests" }, { + "name": "FrameworksCoreTests_all_binder" + }, + { "name": "libbinder_rs-internal_test" }, { diff --git a/libs/binder/aidl/android/os/IServiceManager.aidl b/libs/binder/aidl/android/os/IServiceManager.aidl index 1d1f84fc01..69edef86aa 100644 --- a/libs/binder/aidl/android/os/IServiceManager.aidl +++ b/libs/binder/aidl/android/os/IServiceManager.aidl @@ -49,6 +49,8 @@ interface IServiceManager { DUMP_FLAG_PRIORITY_CRITICAL | DUMP_FLAG_PRIORITY_HIGH | DUMP_FLAG_PRIORITY_NORMAL | DUMP_FLAG_PRIORITY_DEFAULT; + const int FLAG_IS_LAZY_SERVICE = 1 << 30; + /* Allows services to dump sections in protobuf format. */ const int DUMP_FLAG_PROTO = 1 << 4; @@ -61,7 +63,8 @@ interface IServiceManager { * * Returns null if the service does not exist. * - * @deprecated TODO(b/355394904): Use getService2 instead. + * @deprecated TODO(b/355394904): Use getService2 instead. This does not return metadata + * that is included in ServiceWithMetadata */ @UnsupportedAppUsage @nullable IBinder getService(@utf8InCpp String name); diff --git a/libs/binder/aidl/android/os/Service.aidl b/libs/binder/aidl/android/os/Service.aidl index 4c5210911c..3bc658833b 100644 --- a/libs/binder/aidl/android/os/Service.aidl +++ b/libs/binder/aidl/android/os/Service.aidl @@ -16,6 +16,8 @@ package android.os; +import android.os.ServiceWithMetadata; + /** * Service is a union of different service types that can be returned * by the internal {@link ServiceManager#getService(name)} API. @@ -23,6 +25,6 @@ package android.os; * @hide */ union Service { - @nullable IBinder binder; + ServiceWithMetadata serviceWithMetadata; @nullable IBinder accessor; }
\ No newline at end of file diff --git a/libs/binder/aidl/android/os/ServiceWithMetadata.aidl b/libs/binder/aidl/android/os/ServiceWithMetadata.aidl new file mode 100644 index 0000000000..96f76ffe78 --- /dev/null +++ b/libs/binder/aidl/android/os/ServiceWithMetadata.aidl @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.os; + +/** + * Service binder with metadata. + * @hide + */ +parcelable ServiceWithMetadata { + /** + * IBinder to service + */ + @nullable IBinder service; + /** + * boolean if the IBinder can be cached by client. + */ + boolean isLazyService; +} diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h index 81f7cdbcb1..7d79baa34d 100644 --- a/libs/binder/include/binder/IServiceManager.h +++ b/libs/binder/include/binder/IServiceManager.h @@ -150,6 +150,12 @@ public: int pid; }; virtual std::vector<ServiceDebugInfo> getServiceDebugInfo() = 0; + + /** + * Directly enable or disable caching binder during addService calls. + * Only used for testing. This is enabled by default. + */ + virtual void enableAddServiceCache(bool value) = 0; }; LIBBINDER_EXPORTED sp<IServiceManager> defaultServiceManager(); diff --git a/libs/binder/ndk/include_platform/android/binder_manager.h b/libs/binder/ndk/include_platform/android/binder_manager.h index cc4943b9c3..f5df8d5c2f 100644 --- a/libs/binder/ndk/include_platform/android/binder_manager.h +++ b/libs/binder/ndk/include_platform/android/binder_manager.h @@ -34,6 +34,7 @@ enum AServiceManager_AddServiceFlag : uint32_t { ADD_SERVICE_DUMP_FLAG_PRIORITY_HIGH = 1 << 2, ADD_SERVICE_DUMP_FLAG_PRIORITY_NORMAL = 1 << 3, ADD_SERVICE_DUMP_FLAG_PRIORITY_DEFAULT = 1 << 4, + // All other bits are reserved for internal usage }; /** diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index 28a3f654b8..c21d7c61cd 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -70,7 +70,11 @@ cc_test { static_libs: [ "libfakeservicemanager", ], - defaults: ["libbinder_client_cache_flag"], + defaults: [ + "libbinder_client_cache_flag", + "libbinder_addservice_cache_flag", + "libbinder_remove_cache_static_list_flag", + ], test_suites: ["general-tests"], require_root: true, } diff --git a/libs/binder/tests/binderCacheUnitTest.cpp b/libs/binder/tests/binderCacheUnitTest.cpp index c5ad79391c..19395c2dcd 100644 --- a/libs/binder/tests/binderCacheUnitTest.cpp +++ b/libs/binder/tests/binderCacheUnitTest.cpp @@ -34,6 +34,18 @@ constexpr bool kUseLibbinderCache = true; constexpr bool kUseLibbinderCache = false; #endif +#ifdef LIBBINDER_ADDSERVICE_CACHE +constexpr bool kUseCacheInAddService = true; +#else +constexpr bool kUseCacheInAddService = false; +#endif + +#ifdef LIBBINDER_REMOVE_CACHE_STATIC_LIST +constexpr bool kRemoveStaticList = true; +#else +constexpr bool kRemoveStaticList = false; +#endif + // A service name which is in the static list of cachable services const String16 kCachedServiceName = String16("isub"); @@ -63,8 +75,10 @@ public: MockAidlServiceManager() : innerSm() {} binder::Status checkService(const ::std::string& name, os::Service* _out) override { - sp<IBinder> binder = innerSm.getService(String16(name.c_str())); - *_out = os::Service::make<os::Service::Tag::binder>(binder); + os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata(); + serviceWithMetadata.service = innerSm.getService(String16(name.c_str())); + serviceWithMetadata.isLazyService = false; + *_out = os::Service::make<os::Service::Tag::serviceWithMetadata>(serviceWithMetadata); return binder::Status::ok(); } @@ -74,14 +88,127 @@ public: innerSm.addService(String16(name.c_str()), service, allowIsolated, dumpPriority)); } + void clearServices() { innerSm.clear(); } + FakeServiceManager innerSm; }; +// Returns services with isLazyService flag as true. +class MockAidlServiceManager2 : public os::IServiceManagerDefault { +public: + MockAidlServiceManager2() : innerSm() {} + + binder::Status checkService(const ::std::string& name, os::Service* _out) override { + os::ServiceWithMetadata serviceWithMetadata = os::ServiceWithMetadata(); + serviceWithMetadata.service = innerSm.getService(String16(name.c_str())); + serviceWithMetadata.isLazyService = true; + *_out = os::Service::make<os::Service::Tag::serviceWithMetadata>(serviceWithMetadata); + return binder::Status::ok(); + } + + binder::Status addService(const std::string& name, const sp<IBinder>& service, + bool allowIsolated, int32_t dumpPriority) override { + return binder::Status::fromStatusT( + innerSm.addService(String16(name.c_str()), service, allowIsolated, dumpPriority)); + } + + void clearServices() { innerSm.clear(); } + + FakeServiceManager innerSm; +}; + +class LibbinderCacheRemoveStaticList : public ::testing::Test { +protected: + void SetUp() override { + fakeServiceManager = sp<MockAidlServiceManager2>::make(); + mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(fakeServiceManager); + mServiceManager->enableAddServiceCache(true); + } + void TearDown() override {} + +public: + void cacheAddServiceAndConfirmCacheMiss(const sp<IBinder>& binder1) { + // Add a service. This shouldn't cache it. + EXPECT_EQ(OK, + mServiceManager->addService(kCachedServiceName, binder1, + /*allowIsolated = */ false, + android::os::IServiceManager::FLAG_IS_LAZY_SERVICE)); + // Try to populate cache. Cache shouldn't be updated. + EXPECT_EQ(binder1, mServiceManager->checkService(kCachedServiceName)); + fakeServiceManager->clearServices(); + EXPECT_EQ(nullptr, mServiceManager->checkService(kCachedServiceName)); + } + + sp<MockAidlServiceManager2> fakeServiceManager; + sp<android::IServiceManager> mServiceManager; +}; + +TEST_F(LibbinderCacheRemoveStaticList, AddLocalServiceAndConfirmCacheMiss) { + if (!kRemoveStaticList) { + GTEST_SKIP() << "Skipping as feature is not enabled"; + return; + } + sp<IBinder> binder1 = sp<BBinder>::make(); + cacheAddServiceAndConfirmCacheMiss(binder1); +} + +TEST_F(LibbinderCacheRemoveStaticList, AddRemoteServiceAndConfirmCacheMiss) { + if (!kRemoveStaticList) { + GTEST_SKIP() << "Skipping as feature is not enabled"; + return; + } + sp<IBinder> binder1 = defaultServiceManager()->checkService(kServerName); + ASSERT_NE(binder1, nullptr); + cacheAddServiceAndConfirmCacheMiss(binder1); +} + +class LibbinderCacheAddServiceTest : public ::testing::Test { +protected: + void SetUp() override { + fakeServiceManager = sp<MockAidlServiceManager>::make(); + mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(fakeServiceManager); + mServiceManager->enableAddServiceCache(true); + } + + void TearDown() override {} + +public: + void cacheAddServiceAndConfirmCacheHit(const sp<IBinder>& binder1) { + // Add a service. This also caches it. + EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder1)); + // remove services from fakeservicemanager + fakeServiceManager->clearServices(); + + sp<IBinder> result = mServiceManager->checkService(kCachedServiceName); + if (kUseCacheInAddService && kUseLibbinderCache) { + // If cache is enabled, we should get the binder. + EXPECT_EQ(binder1, result); + } else { + // If cache is disabled, then we should get the null binder + EXPECT_EQ(nullptr, result); + } + } + sp<MockAidlServiceManager> fakeServiceManager; + sp<android::IServiceManager> mServiceManager; +}; + +TEST_F(LibbinderCacheAddServiceTest, AddLocalServiceAndConfirmCacheHit) { + sp<IBinder> binder1 = sp<BBinder>::make(); + cacheAddServiceAndConfirmCacheHit(binder1); +} + +TEST_F(LibbinderCacheAddServiceTest, AddRemoteServiceAndConfirmCacheHit) { + sp<IBinder> binder1 = defaultServiceManager()->checkService(kServerName); + ASSERT_NE(binder1, nullptr); + cacheAddServiceAndConfirmCacheHit(binder1); +} + class LibbinderCacheTest : public ::testing::Test { protected: void SetUp() override { - sp<MockAidlServiceManager> sm = sp<MockAidlServiceManager>::make(); - mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(sm); + fakeServiceManager = sp<MockAidlServiceManager>::make(); + mServiceManager = getServiceManagerShimFromAidlServiceManagerForTests(fakeServiceManager); + mServiceManager->enableAddServiceCache(false); } void TearDown() override {} @@ -108,6 +235,7 @@ public: } } + sp<MockAidlServiceManager> fakeServiceManager; sp<android::IServiceManager> mServiceManager; }; @@ -180,7 +308,13 @@ TEST_F(LibbinderCacheTest, NullBinderNotCached) { EXPECT_EQ(binder2, result); } +// TODO(b/333854840): Remove this test removing the static list TEST_F(LibbinderCacheTest, DoNotCacheServiceNotInList) { + if (kRemoveStaticList) { + GTEST_SKIP() << "Skipping test as static list is disabled"; + return; + } + sp<IBinder> binder1 = sp<BBinder>::make(); sp<IBinder> binder2 = sp<BBinder>::make(); String16 serviceName = String16("NewLibbinderCacheTest"); diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index ec2f50ca16..e56b7cf004 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -220,6 +220,8 @@ class BinderLibTestEnv : public ::testing::Environment { ASSERT_GT(m_serverpid, 0); sp<IServiceManager> sm = defaultServiceManager(); + // disable caching during addService. + sm->enableAddServiceCache(false); //printf("%s: pid %d, get service\n", __func__, m_pid); LIBBINDER_IGNORE("-Wdeprecated-declarations") m_server = sm->getService(binderLibTestServiceName); @@ -295,6 +297,9 @@ class BinderLibTest : public ::testing::Test { virtual void SetUp() { m_server = static_cast<BinderLibTestEnv *>(binder_env)->getServer(); IPCThreadState::self()->restoreCallingWorkSource(0); + sp<IServiceManager> sm = defaultServiceManager(); + // disable caching during addService. + sm->enableAddServiceCache(false); } virtual void TearDown() { } @@ -1644,6 +1649,7 @@ TEST_F(BinderLibTest, TooManyFdsFlattenable) { TEST(ServiceNotifications, Unregister) { auto sm = defaultServiceManager(); + sm->enableAddServiceCache(false); using LocalRegistrationCallback = IServiceManager::LocalRegistrationCallback; class LocalRegistrationCallbackImpl : public virtual LocalRegistrationCallback { void onServiceRegistration(const String16 &, const sp<IBinder> &) override {} @@ -2374,6 +2380,8 @@ int run_server(int index, int readypipefd, bool usePoll) status_t ret; sp<IServiceManager> sm = defaultServiceManager(); + sm->enableAddServiceCache(false); + BinderLibTestService* testServicePtr; { sp<BinderLibTestService> testService = new BinderLibTestService(index); diff --git a/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h b/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h index f62241d9c2..f2b2aa78fc 100644 --- a/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h +++ b/libs/fakeservicemanager/include/fakeservicemanager/FakeServiceManager.h @@ -65,6 +65,7 @@ public: std::vector<IServiceManager::ServiceDebugInfo> getServiceDebugInfo() override; + void enableAddServiceCache(bool /*value*/) override {} // Clear all of the registered services void clear(); diff --git a/libs/nativewindow/rust/src/lib.rs b/libs/nativewindow/rust/src/lib.rs index 9876362cec..d760285918 100644 --- a/libs/nativewindow/rust/src/lib.rs +++ b/libs/nativewindow/rust/src/lib.rs @@ -30,8 +30,8 @@ use binder::{ StatusCode, }; use ffi::{ - AHardwareBuffer, AHardwareBuffer_Desc, AHardwareBuffer_readFromParcel, - AHardwareBuffer_writeToParcel, ARect, + AHardwareBuffer, AHardwareBuffer_Desc, AHardwareBuffer_Plane, AHardwareBuffer_Planes, + AHardwareBuffer_readFromParcel, AHardwareBuffer_writeToParcel, ARect, }; use std::ffi::c_void; use std::fmt::{self, Debug, Formatter}; @@ -313,6 +313,57 @@ impl HardwareBuffer { }) } + /// Lock a potentially multi-planar hardware buffer for direct CPU access. + /// + /// # Safety + /// + /// - If `fence` is `None`, the caller must ensure that all writes to the buffer have completed + /// before calling this function. + /// - If the buffer has `AHARDWAREBUFFER_FORMAT_BLOB`, multiple threads or process may lock the + /// buffer simultaneously, but the caller must ensure that they don't access it simultaneously + /// and break Rust's aliasing rules, like any other shared memory. + /// - Otherwise if `usage` includes `AHARDWAREBUFFER_USAGE_CPU_WRITE_RARELY` or + /// `AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN`, the caller must ensure that no other threads or + /// processes lock the buffer simultaneously for any usage. + /// - Otherwise, the caller must ensure that no other threads lock the buffer for writing + /// simultaneously. + /// - If `rect` is not `None`, the caller must not modify the buffer outside of that rectangle. + pub unsafe fn lock_planes<'a>( + &'a self, + usage: AHardwareBuffer_UsageFlags, + fence: Option<BorrowedFd>, + rect: Option<&ARect>, + ) -> Result<Vec<PlaneGuard<'a>>, StatusCode> { + let fence = if let Some(fence) = fence { fence.as_raw_fd() } else { -1 }; + let rect = rect.map(ptr::from_ref).unwrap_or(null()); + let mut planes = AHardwareBuffer_Planes { + planeCount: 0, + planes: [const { AHardwareBuffer_Plane { data: null_mut(), pixelStride: 0, rowStride: 0 } }; + 4], + }; + + // SAFETY: The `AHardwareBuffer` pointer we wrap is always valid, and the various out + // pointers are valid because they come from references. Our caller promises that writes have + // completed and there will be no simultaneous read/write locks. + let status = unsafe { + ffi::AHardwareBuffer_lockPlanes(self.0.as_ptr(), usage.0, fence, rect, &mut planes) + }; + status_result(status)?; + let plane_count = planes.planeCount.try_into().unwrap(); + Ok(planes.planes[..plane_count] + .iter() + .map(|plane| PlaneGuard { + guard: HardwareBufferGuard { + buffer: self, + address: NonNull::new(plane.data) + .expect("AHardwareBuffer_lockAndGetInfo set a null outVirtualAddress"), + }, + pixel_stride: plane.pixelStride, + row_stride: plane.rowStride, + }) + .collect()) + } + /// Locks the hardware buffer for direct CPU access, returning information about the bytes per /// pixel and stride as well. /// @@ -510,6 +561,18 @@ pub struct LockedBufferInfo<'a> { pub stride: u32, } +/// A guard for a single plane of a locked `HardwareBuffer`, with additional information about the +/// stride. +#[derive(Debug)] +pub struct PlaneGuard<'a> { + /// The locked buffer guard. + pub guard: HardwareBufferGuard<'a>, + /// The stride in bytes between the color channel for one pixel to the next pixel. + pub pixel_stride: u32, + /// The stride in bytes between rows in the buffer. + pub row_stride: u32, +} + #[cfg(test)] mod test { use super::*; diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp index e62640eb85..cf995bf9b4 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaRenderEngine.cpp @@ -970,7 +970,7 @@ void SkiaRenderEngine::drawLayersInternal( const auto& item = layer.source.buffer; auto imageTextureRef = getOrCreateBackendTexture(item.buffer->getBuffer(), false); - // if the layer's buffer has a fence, then we must must respect the fence prior to using + // if the layer's buffer has a fence, then we must respect the fence prior to using // the buffer. if (layer.source.buffer.fence != nullptr) { waitFence(context, layer.source.buffer.fence->get()); diff --git a/services/surfaceflinger/Scheduler/ISchedulerCallback.h b/services/surfaceflinger/Scheduler/ISchedulerCallback.h index f430526b76..eae8d6d095 100644 --- a/services/surfaceflinger/Scheduler/ISchedulerCallback.h +++ b/services/surfaceflinger/Scheduler/ISchedulerCallback.h @@ -32,7 +32,7 @@ struct ISchedulerCallback { virtual void onChoreographerAttached() = 0; virtual void onExpectedPresentTimePosted(TimePoint, ftl::NonNull<DisplayModePtr>, Fps renderRate) = 0; - virtual void onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) = 0; + virtual void onCommitNotComposited() = 0; virtual void vrrDisplayIdle(bool idle) = 0; protected: diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 5ec7e48332..60f11b81e8 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -234,7 +234,7 @@ void Scheduler::onFrameSignal(ICompositor& compositor, VsyncId vsyncId, if (FlagManager::getInstance().vrr_config()) { compositor.sendNotifyExpectedPresentHint(pacesetterPtr->displayId); } - mSchedulerCallback.onCommitNotComposited(pacesetterPtr->displayId); + mSchedulerCallback.onCommitNotComposited(); return; } } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index d4d32aa37a..8fa7e3a80f 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2580,7 +2580,7 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, } { - ATRACE_NAME("LLM:commitChanges"); + ATRACE_NAME("LayerLifecycleManager:commitChanges"); mLayerLifecycleManager.commitChanges(); } @@ -4472,7 +4472,7 @@ void SurfaceFlinger::sendNotifyExpectedPresentHint(PhysicalDisplayId displayId) scheduleNotifyExpectedPresentHint(displayId); } -void SurfaceFlinger::onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) { +void SurfaceFlinger::onCommitNotComposited() { if (FlagManager::getInstance().commit_not_composited()) { mFrameTimeline->onCommitNotComposited(); } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 2369043821..da797f8161 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -701,7 +701,7 @@ private: void onChoreographerAttached() override; void onExpectedPresentTimePosted(TimePoint expectedPresentTime, ftl::NonNull<DisplayModePtr>, Fps renderRate) override; - void onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) override + void onCommitNotComposited() override REQUIRES(kMainThreadContext); void vrrDisplayIdle(bool idle) override; diff --git a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h index 8f21cdbaa6..0ac4b68933 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h +++ b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h @@ -30,7 +30,7 @@ struct SchedulerCallback final : ISchedulerCallback { MOCK_METHOD(void, onChoreographerAttached, (), (override)); MOCK_METHOD(void, onExpectedPresentTimePosted, (TimePoint, ftl::NonNull<DisplayModePtr>, Fps), (override)); - MOCK_METHOD(void, onCommitNotComposited, (PhysicalDisplayId), (override)); + MOCK_METHOD(void, onCommitNotComposited, (), (override)); MOCK_METHOD(void, vrrDisplayIdle, (bool), (override)); }; @@ -41,7 +41,7 @@ struct NoOpSchedulerCallback final : ISchedulerCallback { void triggerOnFrameRateOverridesChanged() override {} void onChoreographerAttached() override {} void onExpectedPresentTimePosted(TimePoint, ftl::NonNull<DisplayModePtr>, Fps) override {} - void onCommitNotComposited(PhysicalDisplayId) override {} + void onCommitNotComposited() override {} void vrrDisplayIdle(bool) override {} }; |