diff options
45 files changed, 605 insertions, 157 deletions
diff --git a/cmds/dumpsys/OWNERS b/cmds/dumpsys/OWNERS index 2a9b681318..4f6a89ebe5 100644 --- a/cmds/dumpsys/OWNERS +++ b/cmds/dumpsys/OWNERS @@ -2,3 +2,6 @@ set noparent nandana@google.com jsharkey@android.com + +# for ServiceManager mock +per-file dumpsys_test.cpp=smoreland@google.com diff --git a/cmds/dumpsys/tests/dumpsys_test.cpp b/cmds/dumpsys/tests/dumpsys_test.cpp index 39af7dfae0..c9d2dbb883 100644 --- a/cmds/dumpsys/tests/dumpsys_test.cpp +++ b/cmds/dumpsys/tests/dumpsys_test.cpp @@ -59,6 +59,7 @@ class ServiceManagerMock : public IServiceManager { MOCK_METHOD1(waitForService, sp<IBinder>(const String16&)); MOCK_METHOD1(isDeclared, bool(const String16&)); MOCK_METHOD1(getDeclaredInstances, Vector<String16>(const String16&)); + MOCK_METHOD1(updatableViaApex, std::optional<String16>(const String16&)); protected: MOCK_METHOD0(onAsBinder, IBinder*()); }; diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp index 2f5524940e..b429fb3440 100644 --- a/cmds/servicemanager/ServiceManager.cpp +++ b/cmds/servicemanager/ServiceManager.cpp @@ -58,22 +58,34 @@ static bool forEachManifest(const std::function<bool(const ManifestWithDescripti return false; } -static bool isVintfDeclared(const std::string& name) { - size_t firstSlash = name.find('/'); - size_t lastDot = name.rfind('.', firstSlash); - if (firstSlash == std::string::npos || lastDot == std::string::npos) { - LOG(ERROR) << "VINTF HALs require names in the format type/instance (e.g. " - << "some.package.foo.IFoo/default) but got: " << name; - return false; +struct AidlName { + std::string package; + std::string iface; + std::string instance; + + static bool fill(const std::string& name, AidlName* aname) { + size_t firstSlash = name.find('/'); + size_t lastDot = name.rfind('.', firstSlash); + if (firstSlash == std::string::npos || lastDot == std::string::npos) { + LOG(ERROR) << "VINTF HALs require names in the format type/instance (e.g. " + << "some.package.foo.IFoo/default) but got: " << name; + return false; + } + aname->package = name.substr(0, lastDot); + aname->iface = name.substr(lastDot + 1, firstSlash - lastDot - 1); + aname->instance = name.substr(firstSlash + 1); + return true; } - const std::string package = name.substr(0, lastDot); - const std::string iface = name.substr(lastDot+1, firstSlash-lastDot-1); - const std::string instance = name.substr(firstSlash+1); +}; + +static bool isVintfDeclared(const std::string& name) { + AidlName aname; + if (!AidlName::fill(name, &aname)) return false; - bool found = forEachManifest([&] (const ManifestWithDescription& mwd) { - if (mwd.manifest->hasAidlInstance(package, iface, instance)) { + bool found = forEachManifest([&](const ManifestWithDescription& mwd) { + if (mwd.manifest->hasAidlInstance(aname.package, aname.iface, aname.instance)) { LOG(INFO) << "Found " << name << " in " << mwd.description << " VINTF manifest."; - return true; + return true; // break } return false; // continue }); @@ -81,13 +93,34 @@ static bool isVintfDeclared(const std::string& name) { if (!found) { // Although it is tested, explicitly rebuilding qualified name, in case it // becomes something unexpected. - LOG(ERROR) << "Could not find " << package << "." << iface << "/" << instance - << " in the VINTF manifest."; + LOG(ERROR) << "Could not find " << aname.package << "." << aname.iface << "/" + << aname.instance << " in the VINTF manifest."; } return found; } +static std::optional<std::string> getVintfUpdatableApex(const std::string& name) { + AidlName aname; + if (!AidlName::fill(name, &aname)) return std::nullopt; + + std::optional<std::string> updatableViaApex; + + forEachManifest([&](const ManifestWithDescription& mwd) { + mwd.manifest->forEachInstance([&](const auto& manifestInstance) { + if (manifestInstance.format() != vintf::HalFormat::AIDL) return true; + if (manifestInstance.package() != aname.package) return true; + if (manifestInstance.interface() != aname.iface) return true; + if (manifestInstance.instance() != aname.instance) return true; + updatableViaApex = manifestInstance.updatableViaApex(); + return false; // break (libvintf uses opposite convention) + }); + return false; // continue + }); + + return updatableViaApex; +} + static std::vector<std::string> getVintfInstances(const std::string& interface) { size_t lastDot = interface.rfind('.'); if (lastDot == std::string::npos) { @@ -388,6 +421,22 @@ binder::Status ServiceManager::getDeclaredInstances(const std::string& interface return Status::ok(); } +Status ServiceManager::updatableViaApex(const std::string& name, + std::optional<std::string>* outReturn) { + auto ctx = mAccess->getCallingContext(); + + if (!mAccess->canFind(ctx, name)) { + return Status::fromExceptionCode(Status::EX_SECURITY); + } + + *outReturn = std::nullopt; + +#ifndef VENDORSERVICEMANAGER + *outReturn = getVintfUpdatableApex(name); +#endif + return Status::ok(); +} + void ServiceManager::removeRegistrationCallback(const wp<IBinder>& who, ServiceCallbackMap::iterator* it, bool* found) { diff --git a/cmds/servicemanager/ServiceManager.h b/cmds/servicemanager/ServiceManager.h index c0891152e6..4f23c21078 100644 --- a/cmds/servicemanager/ServiceManager.h +++ b/cmds/servicemanager/ServiceManager.h @@ -46,6 +46,8 @@ public: binder::Status isDeclared(const std::string& name, bool* outReturn) override; binder::Status getDeclaredInstances(const std::string& interface, std::vector<std::string>* outReturn) override; + binder::Status updatableViaApex(const std::string& name, + std::optional<std::string>* outReturn) override; binder::Status registerClientCallback(const std::string& name, const sp<IBinder>& service, const sp<IClientCallback>& cb) override; binder::Status tryUnregisterService(const std::string& name, const sp<IBinder>& binder) override; diff --git a/include/android/surface_control.h b/include/android/surface_control.h index c349024839..b7eafcd6cd 100644 --- a/include/android/surface_control.h +++ b/include/android/surface_control.h @@ -147,6 +147,28 @@ typedef struct ASurfaceTransactionStats ASurfaceTransactionStats; typedef void (*ASurfaceTransaction_OnComplete)(void* context, ASurfaceTransactionStats* stats) __INTRODUCED_IN(29); + +/** + * The ASurfaceTransaction_OnCommit callback is invoked when transaction is applied and the updates + * are ready to be presented. This callback will be invoked before the + * ASurfaceTransaction_OnComplete callback. + * + * \param context Optional context provided by the client that is passed into the callback. + * + * \param stats Opaque handle that can be passed to ASurfaceTransactionStats functions to query + * information about the transaction. The handle is only valid during the callback. + * Present and release fences are not available for this callback. Querying them using + * ASurfaceTransactionStats_getPresentFenceFd and ASurfaceTransactionStats_getPreviousReleaseFenceFd + * will result in failure. + * + * THREADING + * The transaction committed callback can be invoked on any thread. + * + * Available since API level 31. + */ +typedef void (*ASurfaceTransaction_OnCommit)(void* context, ASurfaceTransactionStats* stats) + __INTRODUCED_IN(31); + /** * Returns the timestamp of when the frame was latched by the framework. Once a frame is * latched by the framework, it is presented at the following hardware vsync. @@ -161,6 +183,8 @@ int64_t ASurfaceTransactionStats_getLatchTime(ASurfaceTransactionStats* surface_ * The recipient of the callback takes ownership of the fence and is responsible for closing * it. If a device does not support present fences, a -1 will be returned. * + * This query is not valid for ASurfaceTransaction_OnCommit callback. + * * Available since API level 29. */ int ASurfaceTransactionStats_getPresentFenceFd(ASurfaceTransactionStats* surface_transaction_stats) @@ -218,6 +242,8 @@ int64_t ASurfaceTransactionStats_getAcquireTime(ASurfaceTransactionStats* surfac * The client must ensure that all pending refs on a buffer are released before attempting to reuse * this buffer, otherwise synchronization errors may occur. * + * This query is not valid for ASurfaceTransaction_OnCommit callback. + * * Available since API level 29. */ int ASurfaceTransactionStats_getPreviousReleaseFenceFd( @@ -236,6 +262,16 @@ void ASurfaceTransaction_setOnComplete(ASurfaceTransaction* transaction, void* c ASurfaceTransaction_OnComplete func) __INTRODUCED_IN(29); /** + * Sets the callback that will be invoked when the updates from this transaction are applied and are + * ready to be presented. This callback will be invoked before the ASurfaceTransaction_OnComplete + * callback. + * + * Available since API level 31. + */ +void ASurfaceTransaction_setOnCommit(ASurfaceTransaction* transaction, void* context, + ASurfaceTransaction_OnCommit func) __INTRODUCED_IN(31); + +/** * Reparents the \a surface_control from its old parent to the \a new_parent surface control. * Any children of the reparented \a surface_control will remain children of the \a surface_control. * diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index 61f4581df3..f684cf672f 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -75,6 +75,7 @@ public: sp<IBinder> waitForService(const String16& name16) override; bool isDeclared(const String16& name) override; Vector<String16> getDeclaredInstances(const String16& interface) override; + std::optional<String16> updatableViaApex(const String16& name) override; // for legacy ABI const String16& getInterfaceDescriptor() const override { @@ -388,4 +389,12 @@ Vector<String16> ServiceManagerShim::getDeclaredInstances(const String16& interf return res; } +std::optional<String16> ServiceManagerShim::updatableViaApex(const String16& name) { + std::optional<std::string> declared; + if (!mTheRealServiceManager->updatableViaApex(String8(name).c_str(), &declared).isOk()) { + return std::nullopt; + } + return declared ? std::optional<String16>(String16(declared.value().c_str())) : std::nullopt; +} + } // namespace android diff --git a/libs/binder/aidl/android/os/IServiceManager.aidl b/libs/binder/aidl/android/os/IServiceManager.aidl index 2fabf947cd..75c4092559 100644 --- a/libs/binder/aidl/android/os/IServiceManager.aidl +++ b/libs/binder/aidl/android/os/IServiceManager.aidl @@ -108,6 +108,11 @@ interface IServiceManager { @utf8InCpp String[] getDeclaredInstances(@utf8InCpp String iface); /** + * If updatable-via-apex, returns the APEX via which this is updated. + */ + @nullable @utf8InCpp String updatableViaApex(@utf8InCpp String name); + + /** * Request a callback when the number of clients of the service changes. * Used by LazyServiceRegistrar to dynamically stop services that have no clients. */ diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h index 5f0d056c5d..3dbe2c49f4 100644 --- a/libs/binder/include/binder/IServiceManager.h +++ b/libs/binder/include/binder/IServiceManager.h @@ -20,6 +20,8 @@ #include <utils/Vector.h> #include <utils/String16.h> +#include <optional> + namespace android { // ---------------------------------------------------------------------- @@ -99,6 +101,12 @@ public: * Get all instances of a service as declared in the VINTF manifest */ virtual Vector<String16> getDeclaredInstances(const String16& interface) = 0; + + /** + * If this instance is updatable via an APEX, returns the APEX with which + * this can be updated. + */ + virtual std::optional<String16> updatableViaApex(const String16& name) = 0; }; 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 55169140df..a90b4aabca 100644 --- a/libs/binder/ndk/include_platform/android/binder_manager.h +++ b/libs/binder/ndk/include_platform/android/binder_manager.h @@ -124,6 +124,15 @@ void AServiceManager_forEachDeclaredInstance(const char* interface, void* contex __INTRODUCED_IN(31); /** + * Check if a service is updatable via an APEX module. + * + * \param instance identifier of the service + * + * \return whether the interface is updatable via APEX + */ +bool AServiceManager_isUpdatableViaApex(const char* instance) __INTRODUCED_IN(31); + +/** * Prevent lazy services without client from shutting down their process * * \param persist 'true' if the process should not exit. diff --git a/libs/binder/ndk/libbinder_ndk.map.txt b/libs/binder/ndk/libbinder_ndk.map.txt index 67c85b66d4..7d4b82e4b6 100644 --- a/libs/binder/ndk/libbinder_ndk.map.txt +++ b/libs/binder/ndk/libbinder_ndk.map.txt @@ -118,14 +118,15 @@ LIBBINDER_NDK31 { # introduced=31 AIBinder_getCallingSid; # apex AIBinder_setRequestingSid; # apex AParcel_markSensitive; # llndk - AServiceManager_isDeclared; # apex llndk AServiceManager_forEachDeclaredInstance; # apex llndk - AServiceManager_registerLazyService; # llndk - AServiceManager_waitForService; # apex llndk AServiceManager_forceLazyServicesPersist; # llndk + AServiceManager_isDeclared; # apex llndk + AServiceManager_isUpdatableViaApex; # apex + AServiceManager_reRegister; # llndk + AServiceManager_registerLazyService; # llndk AServiceManager_setActiveServicesCallback; # llndk AServiceManager_tryUnregister; # llndk - AServiceManager_reRegister; # llndk + AServiceManager_waitForService; # apex llndk AIBinder_forceDowngradeToSystemStability; # apex AIBinder_forceDowngradeToVendorStability; # llndk diff --git a/libs/binder/ndk/service_manager.cpp b/libs/binder/ndk/service_manager.cpp index 1ccd0d2a2b..7649a26ff6 100644 --- a/libs/binder/ndk/service_manager.cpp +++ b/libs/binder/ndk/service_manager.cpp @@ -105,6 +105,14 @@ void AServiceManager_forEachDeclaredInstance(const char* interface, void* contex callback(String8(instance).c_str(), context); } } +bool AServiceManager_isUpdatableViaApex(const char* instance) { + if (instance == nullptr) { + return false; + } + + sp<IServiceManager> sm = defaultServiceManager(); + return sm->updatableViaApex(String16(instance)) != std::nullopt; +} void AServiceManager_forceLazyServicesPersist(bool persist) { auto serviceRegistrar = android::binder::LazyServiceRegistrar::getInstance(); serviceRegistrar.forcePersist(persist); diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp index 6a88401962..62db3cfbd2 100644 --- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp +++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp @@ -300,6 +300,11 @@ TEST(NdkBinder, GetLazyService) { EXPECT_EQ(STATUS_OK, AIBinder_ping(binder.get())); } +TEST(NdkBinder, IsUpdatable) { + bool isUpdatable = AServiceManager_isUpdatableViaApex("android.hardware.light.ILights/default"); + EXPECT_EQ(isUpdatable, false); +} + // This is too slow TEST(NdkBinder, CheckLazyServiceShutDown) { ndk::SpAIBinder binder(AServiceManager_waitForService(kLazyBinderNdkUnitTestService)); diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs index 321b422185..695a83e414 100644 --- a/libs/binder/rust/src/binder.rs +++ b/libs/binder/rust/src/binder.rs @@ -548,6 +548,28 @@ unsafe impl<T, V: AsNative<T>> AsNative<T> for Option<V> { } } +/// The features to enable when creating a native Binder. +/// +/// This should always be initialised with a default value, e.g.: +/// ``` +/// # use binder::BinderFeatures; +/// BinderFeatures { +/// set_requesting_sid: true, +/// ..BinderFeatures::default(), +/// } +/// ``` +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct BinderFeatures { + /// Indicates that the service intends to receive caller security contexts. This must be true + /// for `ThreadState::with_calling_sid` to work. + pub set_requesting_sid: bool, + // Ensure that clients include a ..BinderFeatures::default() to preserve backwards compatibility + // when new fields are added. #[non_exhaustive] doesn't work because it prevents struct + // expressions entirely. + #[doc(hidden)] + pub _non_exhaustive: (), +} + /// Declare typed interfaces for a binder object. /// /// Given an interface trait and descriptor string, create a native and remote @@ -730,8 +752,9 @@ macro_rules! declare_binder_interface { impl $native { /// Create a new binder service. - pub fn new_binder<T: $interface + Sync + Send + 'static>(inner: T) -> $crate::Strong<dyn $interface> { - let binder = $crate::Binder::new_with_stability($native(Box::new(inner)), $stability); + pub fn new_binder<T: $interface + Sync + Send + 'static>(inner: T, features: $crate::BinderFeatures) -> $crate::Strong<dyn $interface> { + let mut binder = $crate::Binder::new_with_stability($native(Box::new(inner)), $stability); + $crate::IBinderInternal::set_requesting_sid(&mut binder, features.set_requesting_sid); $crate::Strong::new(Box::new(binder)) } } diff --git a/libs/binder/rust/src/lib.rs b/libs/binder/rust/src/lib.rs index 30928a5cff..2694cba870 100644 --- a/libs/binder/rust/src/lib.rs +++ b/libs/binder/rust/src/lib.rs @@ -107,10 +107,9 @@ use binder_ndk_sys as sys; pub mod parcel; pub use crate::binder::{ - FromIBinder, IBinder, IBinderInternal, Interface, InterfaceClass, Remotable, - Stability, Strong, TransactionCode, TransactionFlags, Weak, - FIRST_CALL_TRANSACTION, FLAG_CLEAR_BUF, FLAG_ONEWAY, FLAG_PRIVATE_LOCAL, - LAST_CALL_TRANSACTION, + BinderFeatures, FromIBinder, IBinder, IBinderInternal, Interface, InterfaceClass, Remotable, + Stability, Strong, TransactionCode, TransactionFlags, Weak, FIRST_CALL_TRANSACTION, + FLAG_CLEAR_BUF, FLAG_ONEWAY, FLAG_PRIVATE_LOCAL, LAST_CALL_TRANSACTION, }; pub use error::{status_t, ExceptionCode, Result, Status, StatusCode}; pub use native::add_service; @@ -125,8 +124,8 @@ pub mod public_api { pub use super::parcel::ParcelFileDescriptor; pub use super::{add_service, get_interface}; pub use super::{ - DeathRecipient, ExceptionCode, IBinder, Interface, ProcessState, SpIBinder, Status, - StatusCode, Strong, ThreadState, Weak, WpIBinder, + BinderFeatures, DeathRecipient, ExceptionCode, IBinder, Interface, ProcessState, SpIBinder, + Status, StatusCode, Strong, ThreadState, Weak, WpIBinder, }; /// Binder result containing a [`Status`] on error. diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs index 60e3502759..03320076cb 100644 --- a/libs/binder/rust/tests/integration.rs +++ b/libs/binder/rust/tests/integration.rs @@ -19,7 +19,7 @@ use binder::declare_binder_interface; use binder::parcel::Parcel; use binder::{ - Binder, IBinderInternal, Interface, StatusCode, ThreadState, TransactionCode, + Binder, BinderFeatures, IBinderInternal, Interface, StatusCode, ThreadState, TransactionCode, FIRST_CALL_TRANSACTION, }; use std::convert::{TryFrom, TryInto}; @@ -55,7 +55,8 @@ fn main() -> Result<(), &'static str> { }))); service.set_requesting_sid(true); if let Some(extension_name) = extension_name { - let extension = BnTest::new_binder(TestService { s: extension_name }); + let extension = + BnTest::new_binder(TestService { s: extension_name }, BinderFeatures::default()); service .set_extension(&mut extension.as_binder()) .expect("Could not add extension"); @@ -212,8 +213,8 @@ mod tests { use std::time::Duration; use binder::{ - Binder, DeathRecipient, FromIBinder, IBinder, IBinderInternal, Interface, SpIBinder, - StatusCode, Strong, + Binder, BinderFeatures, DeathRecipient, FromIBinder, IBinder, IBinderInternal, Interface, + SpIBinder, StatusCode, Strong, }; use super::{BnTest, ITest, ITestSameDescriptor, TestService, RUST_SERVICE_BINARY}; @@ -495,9 +496,12 @@ mod tests { #[test] fn reassociate_rust_binder() { let service_name = "testing_service"; - let service_ibinder = BnTest::new_binder(TestService { - s: service_name.to_string(), - }) + let service_ibinder = BnTest::new_binder( + TestService { + s: service_name.to_string(), + }, + BinderFeatures::default(), + ) .as_binder(); let service: Strong<dyn ITest> = service_ibinder @@ -510,9 +514,12 @@ mod tests { #[test] fn weak_binder_upgrade() { let service_name = "testing_service"; - let service = BnTest::new_binder(TestService { - s: service_name.to_string(), - }); + let service = BnTest::new_binder( + TestService { + s: service_name.to_string(), + }, + BinderFeatures::default(), + ); let weak = Strong::downgrade(&service); @@ -525,9 +532,12 @@ mod tests { fn weak_binder_upgrade_dead() { let service_name = "testing_service"; let weak = { - let service = BnTest::new_binder(TestService { - s: service_name.to_string(), - }); + let service = BnTest::new_binder( + TestService { + s: service_name.to_string(), + }, + BinderFeatures::default(), + ); Strong::downgrade(&service) }; @@ -538,9 +548,12 @@ mod tests { #[test] fn weak_binder_clone() { let service_name = "testing_service"; - let service = BnTest::new_binder(TestService { - s: service_name.to_string(), - }); + let service = BnTest::new_binder( + TestService { + s: service_name.to_string(), + }, + BinderFeatures::default(), + ); let weak = Strong::downgrade(&service); let cloned = weak.clone(); @@ -556,12 +569,18 @@ mod tests { #[test] #[allow(clippy::eq_op)] fn binder_ord() { - let service1 = BnTest::new_binder(TestService { - s: "testing_service1".to_string(), - }); - let service2 = BnTest::new_binder(TestService { - s: "testing_service2".to_string(), - }); + let service1 = BnTest::new_binder( + TestService { + s: "testing_service1".to_string(), + }, + BinderFeatures::default(), + ); + let service2 = BnTest::new_binder( + TestService { + s: "testing_service2".to_string(), + }, + BinderFeatures::default(), + ); assert!(!(service1 < service1)); assert!(!(service1 > service1)); diff --git a/libs/binder/rust/tests/ndk_rust_interop.rs b/libs/binder/rust/tests/ndk_rust_interop.rs index ce75ab7125..4702e45f1f 100644 --- a/libs/binder/rust/tests/ndk_rust_interop.rs +++ b/libs/binder/rust/tests/ndk_rust_interop.rs @@ -16,15 +16,13 @@ //! Rust Binder NDK interop tests -use std::ffi::CStr; -use std::os::raw::{c_char, c_int}; -use ::IBinderRustNdkInteropTest::binder::{self, Interface, StatusCode}; use ::IBinderRustNdkInteropTest::aidl::IBinderRustNdkInteropTest::{ BnBinderRustNdkInteropTest, IBinderRustNdkInteropTest, }; -use ::IBinderRustNdkInteropTest::aidl::IBinderRustNdkInteropTestOther::{ - IBinderRustNdkInteropTestOther, -}; +use ::IBinderRustNdkInteropTest::aidl::IBinderRustNdkInteropTestOther::IBinderRustNdkInteropTestOther; +use ::IBinderRustNdkInteropTest::binder::{self, BinderFeatures, Interface, StatusCode}; +use std::ffi::CStr; +use std::os::raw::{c_char, c_int}; /// Look up the provided AIDL service and call its echo method. /// @@ -37,18 +35,21 @@ pub unsafe extern "C" fn rust_call_ndk(service_name: *const c_char) -> c_int { // The Rust class descriptor pointer will not match the NDK one, but the // descriptor strings match so this needs to still associate. - let service: binder::Strong<dyn IBinderRustNdkInteropTest> = match binder::get_interface(service_name) { - Err(e) => { - eprintln!("Could not find Ndk service {}: {:?}", service_name, e); - return StatusCode::NAME_NOT_FOUND as c_int; - } - Ok(service) => service, - }; + let service: binder::Strong<dyn IBinderRustNdkInteropTest> = + match binder::get_interface(service_name) { + Err(e) => { + eprintln!("Could not find Ndk service {}: {:?}", service_name, e); + return StatusCode::NAME_NOT_FOUND as c_int; + } + Ok(service) => service, + }; match service.echo("testing") { - Ok(s) => if s != "testing" { - return StatusCode::BAD_VALUE as c_int; - }, + Ok(s) => { + if s != "testing" { + return StatusCode::BAD_VALUE as c_int; + } + } Err(e) => return e.into(), } @@ -88,7 +89,7 @@ impl IBinderRustNdkInteropTest for Service { #[no_mangle] pub unsafe extern "C" fn rust_start_service(service_name: *const c_char) -> c_int { let service_name = CStr::from_ptr(service_name).to_str().unwrap(); - let service = BnBinderRustNdkInteropTest::new_binder(Service); + let service = BnBinderRustNdkInteropTest::new_binder(Service, BinderFeatures::default()); match binder::add_service(&service_name, service.as_binder()) { Ok(_) => StatusCode::OK as c_int, Err(e) => e as c_int, diff --git a/libs/binder/rust/tests/serialization.rs b/libs/binder/rust/tests/serialization.rs index f1b068ee43..66ba846c2d 100644 --- a/libs/binder/rust/tests/serialization.rs +++ b/libs/binder/rust/tests/serialization.rs @@ -18,11 +18,11 @@ //! access. use binder::declare_binder_interface; +use binder::parcel::ParcelFileDescriptor; use binder::{ - Binder, ExceptionCode, Interface, Parcel, Result, SpIBinder, Status, + Binder, BinderFeatures, ExceptionCode, Interface, Parcel, Result, SpIBinder, Status, StatusCode, TransactionCode, }; -use binder::parcel::ParcelFileDescriptor; use std::ffi::{c_void, CStr, CString}; use std::sync::Once; @@ -85,7 +85,7 @@ static mut SERVICE: Option<SpIBinder> = None; pub extern "C" fn rust_service() -> *mut c_void { unsafe { SERVICE_ONCE.call_once(|| { - SERVICE = Some(BnReadParcelTest::new_binder(()).as_binder()); + SERVICE = Some(BnReadParcelTest::new_binder((), BinderFeatures::default()).as_binder()); }); SERVICE.as_ref().unwrap().as_raw().cast() } @@ -108,8 +108,12 @@ impl ReadParcelTest for BpReadParcelTest {} impl ReadParcelTest for () {} #[allow(clippy::float_cmp)] -fn on_transact(_service: &dyn ReadParcelTest, code: TransactionCode, - parcel: &Parcel, reply: &mut Parcel) -> Result<()> { +fn on_transact( + _service: &dyn ReadParcelTest, + code: TransactionCode, + parcel: &Parcel, + reply: &mut Parcel, +) -> Result<()> { match code { bindings::Transaction_TEST_BOOL => { assert_eq!(parcel.read::<bool>()?, true); diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index e2193fadab..dc8c0f157e 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -88,6 +88,7 @@ enum BinderLibTestTranscationCode { BINDER_LIB_TEST_GETPID, BINDER_LIB_TEST_ECHO_VECTOR, BINDER_LIB_TEST_REJECT_BUF, + BINDER_LIB_TEST_CAN_GET_SID, }; pid_t start_server_process(int arg2, bool usePoll = false) @@ -1192,6 +1193,14 @@ TEST_F(BinderLibTest, BufRejected) { EXPECT_NE(NO_ERROR, ret); } +TEST_F(BinderLibTest, GotSid) { + sp<IBinder> server = addServer(); + + Parcel data; + status_t ret = server->transact(BINDER_LIB_TEST_CAN_GET_SID, data, nullptr); + EXPECT_EQ(OK, ret); +} + class BinderLibTestService : public BBinder { public: @@ -1494,6 +1503,9 @@ class BinderLibTestService : public BBinder case BINDER_LIB_TEST_REJECT_BUF: { return data.objectsCount() == 0 ? BAD_VALUE : NO_ERROR; } + case BINDER_LIB_TEST_CAN_GET_SID: { + return IPCThreadState::self()->getCallingSid() == nullptr ? BAD_VALUE : NO_ERROR; + } default: return UNKNOWN_TRANSACTION; }; diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index ec4ced299b..dd68fdbc6d 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -80,11 +80,10 @@ public: sp<RpcConnection> connection; Status sendString(const std::string& str) override { - std::cout << "Child received string: " << str << std::endl; + (void)str; return Status::ok(); } Status doubleString(const std::string& str, std::string* strstr) override { - std::cout << "Child received string to double: " << str << std::endl; *strstr = str + str; return Status::ok(); } @@ -749,7 +748,7 @@ TEST_P(BinderRpc, ThreadingStressTest) { threads.push_back(std::thread([&] { for (size_t j = 0; j < kNumCalls; j++) { sp<IBinder> out; - proc.rootIface->repeatBinder(proc.rootBinder, &out); + EXPECT_OK(proc.rootIface->repeatBinder(proc.rootBinder, &out)); EXPECT_EQ(proc.rootBinder, out); } })); @@ -758,6 +757,28 @@ TEST_P(BinderRpc, ThreadingStressTest) { for (auto& t : threads) t.join(); } +TEST_P(BinderRpc, OnewayStressTest) { + constexpr size_t kNumClientThreads = 10; + constexpr size_t kNumServerThreads = 10; + constexpr size_t kNumCalls = 100; + + auto proc = createRpcTestSocketServerProcess(kNumServerThreads); + + std::vector<std::thread> threads; + for (size_t i = 0; i < kNumClientThreads; i++) { + threads.push_back(std::thread([&] { + for (size_t j = 0; j < kNumCalls; j++) { + EXPECT_OK(proc.rootIface->sendString("a")); + } + + // check threads are not stuck + EXPECT_OK(proc.rootIface->sleepMs(250)); + })); + } + + for (auto& t : threads) t.join(); +} + TEST_P(BinderRpc, OnewayCallDoesNotWait) { constexpr size_t kReallyLongTimeMs = 100; constexpr size_t kSleepMs = kReallyLongTimeMs * 5; diff --git a/libs/binder/tests/binderStabilityTest.cpp b/libs/binder/tests/binderStabilityTest.cpp index cb309bdd81..2ce13df4b3 100644 --- a/libs/binder/tests/binderStabilityTest.cpp +++ b/libs/binder/tests/binderStabilityTest.cpp @@ -192,6 +192,8 @@ TEST(BinderStability, VintfStabilityServerMustBeDeclaredInManifest) { EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT, android::defaultServiceManager()->addService(String16("."), vintfServer)) << instance8; EXPECT_FALSE(android::defaultServiceManager()->isDeclared(instance)) << instance8; + EXPECT_EQ(std::nullopt, android::defaultServiceManager()->updatableViaApex(instance)) + << instance8; } } diff --git a/libs/fakeservicemanager/ServiceManager.cpp b/libs/fakeservicemanager/ServiceManager.cpp index 4ecbe531c2..761e45c967 100644 --- a/libs/fakeservicemanager/ServiceManager.cpp +++ b/libs/fakeservicemanager/ServiceManager.cpp @@ -73,4 +73,9 @@ Vector<String16> ServiceManager::getDeclaredInstances(const String16& name) { return out; } +std::optional<String16> ServiceManager::updatableViaApex(const String16& name) { + (void)name; + return std::nullopt; +} + } // namespace android diff --git a/libs/fakeservicemanager/ServiceManager.h b/libs/fakeservicemanager/ServiceManager.h index 4ef47fb043..e26c21b9e9 100644 --- a/libs/fakeservicemanager/ServiceManager.h +++ b/libs/fakeservicemanager/ServiceManager.h @@ -19,6 +19,7 @@ #include <binder/IServiceManager.h> #include <map> +#include <optional> namespace android { @@ -48,6 +49,8 @@ public: Vector<String16> getDeclaredInstances(const String16& iface) override; + std::optional<String16> updatableViaApex(const String16& name) override; + private: std::map<String16, sp<IBinder>> mNameToService; }; diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index 6ca248748f..53721cf6f7 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -99,7 +99,7 @@ public: SAFE_PARCEL(data.writeVectorSize, listenerCallbacks); for (const auto& [listener, callbackIds] : listenerCallbacks) { SAFE_PARCEL(data.writeStrongBinder, listener); - SAFE_PARCEL(data.writeInt64Vector, callbackIds); + SAFE_PARCEL(data.writeParcelableVector, callbackIds); } SAFE_PARCEL(data.writeUint64, transactionId); @@ -1268,7 +1268,7 @@ status_t BnSurfaceComposer::onTransact( for (int32_t i = 0; i < listenersSize; i++) { SAFE_PARCEL(data.readStrongBinder, &tmpBinder); std::vector<CallbackId> callbackIds; - SAFE_PARCEL(data.readInt64Vector, &callbackIds); + SAFE_PARCEL(data.readParcelableVector, &callbackIds); listenerCallbacks.emplace_back(tmpBinder, callbackIds); } diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp index b42793b8ac..f74f91e034 100644 --- a/libs/gui/ITransactionCompletedListener.cpp +++ b/libs/gui/ITransactionCompletedListener.cpp @@ -152,7 +152,7 @@ status_t SurfaceStats::readFromParcel(const Parcel* input) { } status_t TransactionStats::writeToParcel(Parcel* output) const { - status_t err = output->writeInt64Vector(callbackIds); + status_t err = output->writeParcelableVector(callbackIds); if (err != NO_ERROR) { return err; } @@ -176,7 +176,7 @@ status_t TransactionStats::writeToParcel(Parcel* output) const { } status_t TransactionStats::readFromParcel(const Parcel* input) { - status_t err = input->readInt64Vector(&callbackIds); + status_t err = input->readParcelableVector(&callbackIds); if (err != NO_ERROR) { return err; } @@ -227,8 +227,9 @@ status_t ListenerStats::readFromParcel(const Parcel* input) { return NO_ERROR; } -ListenerStats ListenerStats::createEmpty(const sp<IBinder>& listener, - const std::unordered_set<CallbackId>& callbackIds) { +ListenerStats ListenerStats::createEmpty( + const sp<IBinder>& listener, + const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds) { ListenerStats listenerStats; listenerStats.listener = listener; listenerStats.transactionStats.emplace_back(callbackIds); @@ -278,4 +279,28 @@ status_t BnTransactionCompletedListener::onTransact(uint32_t code, const Parcel& } } +ListenerCallbacks ListenerCallbacks::filter(CallbackId::Type type) const { + std::vector<CallbackId> filteredCallbackIds; + for (const auto& callbackId : callbackIds) { + if (callbackId.type == type) { + filteredCallbackIds.push_back(callbackId); + } + } + return ListenerCallbacks(transactionCompletedListener, filteredCallbackIds); +} + +status_t CallbackId::writeToParcel(Parcel* output) const { + SAFE_PARCEL(output->writeInt64, id); + SAFE_PARCEL(output->writeInt32, static_cast<int32_t>(type)); + return NO_ERROR; +} + +status_t CallbackId::readFromParcel(const Parcel* input) { + SAFE_PARCEL(input->readInt64, &id); + int32_t typeAsInt; + SAFE_PARCEL(input->readInt32, &typeAsInt); + type = static_cast<CallbackId::Type>(typeAsInt); + return NO_ERROR; +} + }; // namespace android diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 55ed7fe298..517b49e6b5 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -140,7 +140,7 @@ status_t layer_state_t::write(Parcel& output) const for (auto listener : listeners) { SAFE_PARCEL(output.writeStrongBinder, listener.transactionCompletedListener); - SAFE_PARCEL(output.writeInt64Vector, listener.callbackIds); + SAFE_PARCEL(output.writeParcelableVector, listener.callbackIds); } SAFE_PARCEL(output.writeFloat, shadowRadius); SAFE_PARCEL(output.writeInt32, frameRateSelectionPriority); @@ -258,7 +258,7 @@ status_t layer_state_t::read(const Parcel& input) sp<IBinder> listener; std::vector<CallbackId> callbackIds; SAFE_PARCEL(input.readNullableStrongBinder, &listener); - SAFE_PARCEL(input.readInt64Vector, &callbackIds); + SAFE_PARCEL(input.readParcelableVector, &callbackIds); listeners.emplace_back(listener, callbackIds); } SAFE_PARCEL(input.readFloat, &shadowRadius); diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 9b2e10b838..5db0eae416 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -139,7 +139,7 @@ JankDataListener::~JankDataListener() { // 0 is an invalid callback id TransactionCompletedListener::TransactionCompletedListener() : mCallbackIdCounter(1) {} -CallbackId TransactionCompletedListener::getNextIdLocked() { +int64_t TransactionCompletedListener::getNextIdLocked() { return mCallbackIdCounter++; } @@ -163,13 +163,13 @@ void TransactionCompletedListener::startListeningLocked() { CallbackId TransactionCompletedListener::addCallbackFunction( const TransactionCompletedCallback& callbackFunction, const std::unordered_set<sp<SurfaceControl>, SurfaceComposerClient::SCHash>& - surfaceControls) { + surfaceControls, + CallbackId::Type callbackType) { std::lock_guard<std::mutex> lock(mMutex); startListeningLocked(); - CallbackId callbackId = getNextIdLocked(); + CallbackId callbackId(getNextIdLocked(), callbackType); mCallbacks[callbackId].callbackFunction = callbackFunction; - auto& callbackSurfaceControls = mCallbacks[callbackId].surfaceControls; for (const auto& surfaceControl : surfaceControls) { @@ -228,7 +228,7 @@ void TransactionCompletedListener::removeSurfaceStatsListener(void* context, voi void TransactionCompletedListener::addSurfaceControlToCallbacks( const sp<SurfaceControl>& surfaceControl, - const std::unordered_set<CallbackId>& callbackIds) { + const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds) { std::lock_guard<std::mutex> lock(mMutex); for (auto callbackId : callbackIds) { @@ -240,7 +240,7 @@ void TransactionCompletedListener::addSurfaceControlToCallbacks( } void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) { - std::unordered_map<CallbackId, CallbackTranslation> callbacksMap; + std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> callbacksMap; std::multimap<sp<IBinder>, sp<JankDataListener>> jankListenersMap; std::multimap<sp<IBinder>, SurfaceStatsCallbackEntry> surfaceListeners; { @@ -267,7 +267,36 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener } } for (const auto& transactionStats : listenerStats.transactionStats) { + // handle on commit callbacks + for (auto callbackId : transactionStats.callbackIds) { + if (callbackId.type != CallbackId::Type::ON_COMMIT) { + continue; + } + auto& [callbackFunction, callbackSurfaceControls] = callbacksMap[callbackId]; + if (!callbackFunction) { + ALOGE("cannot call null callback function, skipping"); + continue; + } + std::vector<SurfaceControlStats> surfaceControlStats; + for (const auto& surfaceStats : transactionStats.surfaceStats) { + surfaceControlStats + .emplace_back(callbacksMap[callbackId] + .surfaceControls[surfaceStats.surfaceControl], + transactionStats.latchTime, surfaceStats.acquireTime, + transactionStats.presentFence, + surfaceStats.previousReleaseFence, surfaceStats.transformHint, + surfaceStats.eventStats); + } + + callbackFunction(transactionStats.latchTime, transactionStats.presentFence, + surfaceControlStats); + } + + // handle on complete callbacks for (auto callbackId : transactionStats.callbackIds) { + if (callbackId.type != CallbackId::Type::ON_COMPLETE) { + continue; + } auto& [callbackFunction, callbackSurfaceControls] = callbacksMap[callbackId]; if (!callbackFunction) { ALOGE("cannot call null callback function, skipping"); @@ -542,7 +571,9 @@ status_t SurfaceComposerClient::Transaction::readFromParcel(const Parcel* parcel return BAD_VALUE; } for (size_t j = 0; j < numCallbackIds; j++) { - listenerCallbacks[listener].callbackIds.insert(parcel->readInt64()); + CallbackId id; + parcel->readParcelable(&id); + listenerCallbacks[listener].callbackIds.insert(id); } size_t numSurfaces = parcel->readUint32(); if (numSurfaces > parcel->dataSize()) { @@ -628,7 +659,7 @@ status_t SurfaceComposerClient::Transaction::writeToParcel(Parcel* parcel) const parcel->writeStrongBinder(ITransactionCompletedListener::asBinder(listener)); parcel->writeUint32(static_cast<uint32_t>(callbackInfo.callbackIds.size())); for (auto callbackId : callbackInfo.callbackIds) { - parcel->writeInt64(callbackId); + parcel->writeParcelable(callbackId); } parcel->writeUint32(static_cast<uint32_t>(callbackInfo.surfaceControls.size())); for (auto surfaceControl : callbackInfo.surfaceControls) { @@ -1389,9 +1420,9 @@ SurfaceComposerClient::Transaction::setFrameRateSelectionPriority(const sp<Surfa return *this; } -SurfaceComposerClient::Transaction& -SurfaceComposerClient::Transaction::addTransactionCompletedCallback( - TransactionCompletedCallbackTakesContext callback, void* callbackContext) { +SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::addTransactionCallback( + TransactionCompletedCallbackTakesContext callback, void* callbackContext, + CallbackId::Type callbackType) { auto listener = TransactionCompletedListener::getInstance(); auto callbackWithContext = std::bind(callback, callbackContext, std::placeholders::_1, @@ -1399,13 +1430,26 @@ SurfaceComposerClient::Transaction::addTransactionCompletedCallback( const auto& surfaceControls = mListenerCallbacks[TransactionCompletedListener::getIInstance()].surfaceControls; - CallbackId callbackId = listener->addCallbackFunction(callbackWithContext, surfaceControls); + CallbackId callbackId = + listener->addCallbackFunction(callbackWithContext, surfaceControls, callbackType); mListenerCallbacks[TransactionCompletedListener::getIInstance()].callbackIds.emplace( callbackId); return *this; } +SurfaceComposerClient::Transaction& +SurfaceComposerClient::Transaction::addTransactionCompletedCallback( + TransactionCompletedCallbackTakesContext callback, void* callbackContext) { + return addTransactionCallback(callback, callbackContext, CallbackId::Type::ON_COMPLETE); +} + +SurfaceComposerClient::Transaction& +SurfaceComposerClient::Transaction::addTransactionCommittedCallback( + TransactionCompletedCallbackTakesContext callback, void* callbackContext) { + return addTransactionCallback(callback, callbackContext, CallbackId::Type::ON_COMMIT); +} + SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::notifyProducerDisconnect( const sp<SurfaceControl>& sc) { layer_state_t* s = getLayerState(sc); @@ -1796,7 +1840,8 @@ status_t SurfaceComposerClient::createSurfaceChecked(const String8& name, uint32 } ALOGE_IF(err, "SurfaceComposerClient::createSurface error %s", strerror(-err)); if (err == NO_ERROR) { - *outSurface = new SurfaceControl(this, handle, gbp, id, w, h, format, transformHint); + *outSurface = + new SurfaceControl(this, handle, gbp, id, w, h, format, transformHint, flags); } } return err; diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h index 098760e89d..2d71194f70 100644 --- a/libs/gui/include/gui/ITransactionCompletedListener.h +++ b/libs/gui/include/gui/ITransactionCompletedListener.h @@ -36,7 +36,22 @@ namespace android { class ITransactionCompletedListener; class ListenerCallbacks; -using CallbackId = int64_t; +class CallbackId : public Parcelable { +public: + int64_t id; + enum class Type : int32_t { ON_COMPLETE, ON_COMMIT } type; + + CallbackId() {} + CallbackId(int64_t id, Type type) : id(id), type(type) {} + status_t writeToParcel(Parcel* output) const override; + status_t readFromParcel(const Parcel* input) override; + + bool operator==(const CallbackId& rhs) const { return id == rhs.id && type == rhs.type; } +}; + +struct CallbackIdHash { + std::size_t operator()(const CallbackId& key) const { return std::hash<int64_t>()(key.id); } +}; class FrameEventHistoryStats : public Parcelable { public: @@ -112,7 +127,7 @@ public: TransactionStats() = default; TransactionStats(const std::vector<CallbackId>& ids) : callbackIds(ids) {} - TransactionStats(const std::unordered_set<CallbackId>& ids) + TransactionStats(const std::unordered_set<CallbackId, CallbackIdHash>& ids) : callbackIds(ids.begin(), ids.end()) {} TransactionStats(const std::vector<CallbackId>& ids, nsecs_t latch, const sp<Fence>& present, const std::vector<SurfaceStats>& surfaces) @@ -129,8 +144,9 @@ public: status_t writeToParcel(Parcel* output) const override; status_t readFromParcel(const Parcel* input) override; - static ListenerStats createEmpty(const sp<IBinder>& listener, - const std::unordered_set<CallbackId>& callbackIds); + static ListenerStats createEmpty( + const sp<IBinder>& listener, + const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds); sp<IBinder> listener; std::vector<TransactionStats> transactionStats; @@ -156,7 +172,8 @@ public: class ListenerCallbacks { public: - ListenerCallbacks(const sp<IBinder>& listener, const std::unordered_set<CallbackId>& callbacks) + ListenerCallbacks(const sp<IBinder>& listener, + const std::unordered_set<CallbackId, CallbackIdHash>& callbacks) : transactionCompletedListener(listener), callbackIds(callbacks.begin(), callbacks.end()) {} @@ -170,9 +187,12 @@ public: if (callbackIds.empty()) { return rhs.callbackIds.empty(); } - return callbackIds.front() == rhs.callbackIds.front(); + return callbackIds.front().id == rhs.callbackIds.front().id; } + // Returns a new ListenerCallbacks filtered by type + ListenerCallbacks filter(CallbackId::Type type) const; + sp<IBinder> transactionCompletedListener; std::vector<CallbackId> callbackIds; }; @@ -191,7 +211,7 @@ struct CallbackIdsHash { // same members. It is sufficient to just check the first CallbackId in the vectors. If // they match, they are the same. If they do not match, they are not the same. std::size_t operator()(const std::vector<CallbackId>& callbackIds) const { - return std::hash<CallbackId>{}((callbackIds.empty()) ? 0 : callbackIds.front()); + return std::hash<int64_t>{}((callbackIds.empty()) ? 0 : callbackIds.front().id); } }; diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index d3f6ff0697..5bbd8e3d85 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -336,7 +336,7 @@ public: struct CallbackInfo { // All the callbacks that have been requested for a TransactionCompletedListener in the // Transaction - std::unordered_set<CallbackId> callbackIds; + std::unordered_set<CallbackId, CallbackIdHash> callbackIds; // All the SurfaceControls that have been modified in this TransactionCompletedListener's // process that require a callback if there is one or more callbackIds set. std::unordered_set<sp<SurfaceControl>, SCHash> surfaceControls; @@ -484,9 +484,15 @@ public: // Sets information about the priority of the frame. Transaction& setFrameRateSelectionPriority(const sp<SurfaceControl>& sc, int32_t priority); + Transaction& addTransactionCallback(TransactionCompletedCallbackTakesContext callback, + void* callbackContext, CallbackId::Type callbackType); + Transaction& addTransactionCompletedCallback( TransactionCompletedCallbackTakesContext callback, void* callbackContext); + Transaction& addTransactionCommittedCallback( + TransactionCompletedCallbackTakesContext callback, void* callbackContext); + // ONLY FOR BLAST ADAPTER Transaction& notifyProducerDisconnect(const sp<SurfaceControl>& sc); // Set the framenumber generated by the graphics producer to mimic BufferQueue behaviour. @@ -621,14 +627,13 @@ public: class TransactionCompletedListener : public BnTransactionCompletedListener { TransactionCompletedListener(); - CallbackId getNextIdLocked() REQUIRES(mMutex); + int64_t getNextIdLocked() REQUIRES(mMutex); std::mutex mMutex; bool mListening GUARDED_BY(mMutex) = false; - CallbackId mCallbackIdCounter GUARDED_BY(mMutex) = 1; - + int64_t mCallbackIdCounter GUARDED_BY(mMutex) = 1; struct CallbackTranslation { TransactionCompletedCallback callbackFunction; std::unordered_map<sp<IBinder>, sp<SurfaceControl>, SurfaceComposerClient::IBinderHash> @@ -646,7 +651,8 @@ class TransactionCompletedListener : public BnTransactionCompletedListener { SurfaceStatsCallback callback; }; - std::unordered_map<CallbackId, CallbackTranslation> mCallbacks GUARDED_BY(mMutex); + std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> mCallbacks + GUARDED_BY(mMutex); std::multimap<sp<IBinder>, sp<JankDataListener>> mJankListeners GUARDED_BY(mMutex); std::unordered_map<uint64_t /* graphicsBufferId */, ReleaseBufferCallback> mReleaseBufferCallbacks GUARDED_BY(mMutex); @@ -662,10 +668,12 @@ public: CallbackId addCallbackFunction( const TransactionCompletedCallback& callbackFunction, const std::unordered_set<sp<SurfaceControl>, SurfaceComposerClient::SCHash>& - surfaceControls); + surfaceControls, + CallbackId::Type callbackType); - void addSurfaceControlToCallbacks(const sp<SurfaceControl>& surfaceControl, - const std::unordered_set<CallbackId>& callbackIds); + void addSurfaceControlToCallbacks( + const sp<SurfaceControl>& surfaceControl, + const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds); /* * Adds a jank listener to be informed about SurfaceFlinger's jank classification for a specific diff --git a/libs/permission/Android.bp b/libs/permission/Android.bp index dd38224a60..a5712b319f 100644 --- a/libs/permission/Android.bp +++ b/libs/permission/Android.bp @@ -1,3 +1,12 @@ +package { + // See: http://go/android-license-faq + // A large-scale-change added 'default_applicable_licenses' to import + // all of the 'license_kinds' from "frameworks_native_license" + // to get the below license kinds: + // SPDX-license-identifier-Apache-2.0 + default_applicable_licenses: ["frameworks_native_license"], +} + cc_library_shared { name: "libpermission", srcs: [ diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp index d63c88bc02..34ef0a4f08 100644 --- a/libs/renderengine/tests/RenderEngineTest.cpp +++ b/libs/renderengine/tests/RenderEngineTest.cpp @@ -1402,7 +1402,8 @@ TEST_P(RenderEngineTest, drawLayers_fillBufferColorTransformZeroLayerAlpha_color fillBufferColorTransformZeroLayerAlpha<ColorSourceVariant>(); } -TEST_P(RenderEngineTest, drawLayers_fillBufferAndBlurBackground_colorSource) { +// TODO(b/186010146): reenable once swiftshader is happy with this test +TEST_P(RenderEngineTest, DISABLED_drawLayers_fillBufferAndBlurBackground_colorSource) { initializeRenderEngine(); fillBufferAndBlurBackground<ColorSourceVariant>(); } @@ -1477,7 +1478,8 @@ TEST_P(RenderEngineTest, drawLayers_fillBufferColorTransformZeroLayerAlpha_opaqu fillBufferColorTransformZeroLayerAlpha<BufferSourceVariant<ForceOpaqueBufferVariant>>(); } -TEST_P(RenderEngineTest, drawLayers_fillBufferAndBlurBackground_opaqueBufferSource) { +// TODO(b/186010146): reenable once swiftshader is happy with this test +TEST_P(RenderEngineTest, DISABLED_drawLayers_fillBufferAndBlurBackground_opaqueBufferSource) { initializeRenderEngine(); fillBufferAndBlurBackground<BufferSourceVariant<ForceOpaqueBufferVariant>>(); } @@ -1552,7 +1554,8 @@ TEST_P(RenderEngineTest, drawLayers_fillBufferColorTransformZeroLayerAlpha_buffe fillBufferColorTransformZeroLayerAlpha<BufferSourceVariant<RelaxOpaqueBufferVariant>>(); } -TEST_P(RenderEngineTest, drawLayers_fillBufferAndBlurBackground_bufferSource) { +// TODO(b/186010146): reenable once swiftshader is happy with this test +TEST_P(RenderEngineTest, DISABLED_drawLayers_fillBufferAndBlurBackground_bufferSource) { initializeRenderEngine(); fillBufferAndBlurBackground<BufferSourceVariant<RelaxOpaqueBufferVariant>>(); } diff --git a/libs/sensorprivacy/SensorPrivacyManager.cpp b/libs/sensorprivacy/SensorPrivacyManager.cpp index 47144696f2..b3537ce444 100644 --- a/libs/sensorprivacy/SensorPrivacyManager.cpp +++ b/libs/sensorprivacy/SensorPrivacyManager.cpp @@ -55,6 +55,22 @@ sp<hardware::ISensorPrivacyManager> SensorPrivacyManager::getService() return service; } +bool SensorPrivacyManager::supportsSensorToggle(int sensor) { + if (mSupportedCache.find(sensor) == mSupportedCache.end()) { + sp<hardware::ISensorPrivacyManager> service = getService(); + if (service != nullptr) { + bool result; + service->supportsSensorToggle(sensor, &result); + mSupportedCache[sensor] = result; + return result; + } + // if the SensorPrivacyManager is not available then assume sensor privacy feature isn't + // supported + return false; + } + return mSupportedCache[sensor]; +} + void SensorPrivacyManager::addSensorPrivacyListener( const sp<hardware::ISensorPrivacyListener>& listener) { diff --git a/libs/sensorprivacy/aidl/android/hardware/ISensorPrivacyManager.aidl b/libs/sensorprivacy/aidl/android/hardware/ISensorPrivacyManager.aidl index 629b8c2093..c8ceeb89af 100644 --- a/libs/sensorprivacy/aidl/android/hardware/ISensorPrivacyManager.aidl +++ b/libs/sensorprivacy/aidl/android/hardware/ISensorPrivacyManager.aidl @@ -20,6 +20,8 @@ import android.hardware.ISensorPrivacyListener; /** @hide */ interface ISensorPrivacyManager { + boolean supportsSensorToggle(int sensor); + void addSensorPrivacyListener(in ISensorPrivacyListener listener); void addIndividualSensorPrivacyListener(int userId, int sensor, in ISensorPrivacyListener listener); diff --git a/libs/sensorprivacy/include/sensorprivacy/SensorPrivacyManager.h b/libs/sensorprivacy/include/sensorprivacy/SensorPrivacyManager.h index 12778e16b6..fb4cbe9d55 100644 --- a/libs/sensorprivacy/include/sensorprivacy/SensorPrivacyManager.h +++ b/libs/sensorprivacy/include/sensorprivacy/SensorPrivacyManager.h @@ -22,6 +22,8 @@ #include <utils/threads.h> +#include <unordered_map> + // --------------------------------------------------------------------------- namespace android { @@ -35,6 +37,7 @@ public: SensorPrivacyManager(); + bool supportsSensorToggle(int sensor); void addSensorPrivacyListener(const sp<hardware::ISensorPrivacyListener>& listener); status_t addIndividualSensorPrivacyListener(int userId, int sensor, const sp<hardware::ISensorPrivacyListener>& listener); @@ -50,6 +53,8 @@ private: Mutex mLock; sp<hardware::ISensorPrivacyManager> mService; sp<hardware::ISensorPrivacyManager> getService(); + + std::unordered_map<int, bool> mSupportedCache; }; diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index a2915c903f..7a5b20d78e 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -674,6 +674,11 @@ status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nse latchTime); } + std::deque<sp<CallbackHandle>> remainingHandles; + mFlinger->getTransactionCallbackInvoker() + .finalizeOnCommitCallbackHandles(mDrawingState.callbackHandles, remainingHandles); + mDrawingState.callbackHandles = remainingHandles; + mCurrentStateModified = false; return NO_ERROR; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h index a0606b48f0..289cb119ca 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h @@ -79,6 +79,9 @@ struct CompositionRefreshArgs { // If set, causes the dirty regions to flash with the delay std::optional<std::chrono::microseconds> devOptFlashDirtyRegionsDelay; + + // The earliest time to send the present command to the HAL + std::chrono::steady_clock::time_point earliestPresentTime; }; } // namespace android::compositionengine diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h index 8f767d37f6..f0ef6d6e7d 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h @@ -115,6 +115,9 @@ struct OutputCompositionState { // Current target dataspace ui::Dataspace targetDataspace{ui::Dataspace::UNKNOWN}; + // The earliest time to send the present command to the HAL + std::chrono::steady_clock::time_point earliestPresentTime; + // Debugging void dump(std::string& result) const; }; diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp index a605fe1dc3..1ffb1c8e2d 100644 --- a/services/surfaceflinger/CompositionEngine/src/Display.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp @@ -367,6 +367,11 @@ compositionengine::Output::FrameFences Display::presentAndGetFrameFences() { return fences; } + { + ATRACE_NAME("wait for earliest present time"); + std::this_thread::sleep_until(getState().earliestPresentTime); + } + auto& hwc = getCompositionEngine().getHwComposer(); hwc.presentAndGetReleaseFences(*halDisplayIdOpt); diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index 3468b204fc..faa4b74c28 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -711,6 +711,8 @@ void Output::writeCompositionState(const compositionengine::CompositionRefreshAr return; } + editState().earliestPresentTime = refreshArgs.earliestPresentTime; + sp<GraphicBuffer> previousOverride = nullptr; for (auto* layer : getOutputLayersOrderedByZ()) { bool skipLayer = false; diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 1d25c7247f..57bd045945 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -927,4 +927,11 @@ void Scheduler::setPreferredRefreshRateForUid(FrameRateOverride frameRateOverrid } } +std::chrono::steady_clock::time_point Scheduler::getPreviousVsyncFrom( + nsecs_t expectedPresentTime) const { + const auto presentTime = std::chrono::nanoseconds(expectedPresentTime); + const auto vsyncPeriod = std::chrono::nanoseconds(mVsyncSchedule.tracker->currentPeriod()); + return std::chrono::steady_clock::time_point(presentTime - vsyncPeriod); +} + } // namespace android diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 4d1f3c6572..49d3d93f36 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -149,6 +149,8 @@ public: bool isVsyncValid(nsecs_t expectedVsyncTimestamp, uid_t uid) const EXCLUDES(mFrameRateOverridesMutex); + std::chrono::steady_clock::time_point getPreviousVsyncFrom(nsecs_t expectedPresentTime) const; + void dump(std::string&) const; void dump(ConnectionHandle, std::string&) const; void dumpVsync(std::string&) const; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index b6179eaabf..b04868225e 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1972,6 +1972,8 @@ void SurfaceFlinger::onMessageRefresh() { std::chrono::milliseconds(mDebugRegion > 1 ? mDebugRegion : 0); } + refreshArgs.earliestPresentTime = mScheduler->getPreviousVsyncFrom(mExpectedPresentTime); + mGeometryInvalid = false; // Store the present time just before calling to the composition engine so we could notify @@ -2032,6 +2034,9 @@ bool SurfaceFlinger::handleMessageInvalidate() { ATRACE_CALL(); bool refreshNeeded = handlePageFlip(); + // Send on commit callbacks + mTransactionCallbackInvoker.sendCallbacks(); + if (mVisibleRegionsDirty) { computeLayerBounds(); } @@ -3761,14 +3766,30 @@ bool SurfaceFlinger::callingThreadHasUnscopedSurfaceFlingerAccess(bool usePermis uint32_t SurfaceFlinger::setClientStateLocked( const FrameTimelineInfo& frameTimelineInfo, const ComposerState& composerState, int64_t desiredPresentTime, bool isAutoTimestamp, int64_t postTime, uint32_t permissions, - std::unordered_set<ListenerCallbacks, ListenerCallbacksHash>& listenerCallbacks) { + std::unordered_set<ListenerCallbacks, ListenerCallbacksHash>& outListenerCallbacks) { const layer_state_t& s = composerState.state; const bool privileged = permissions & Permission::ACCESS_SURFACE_FLINGER; + + std::vector<ListenerCallbacks> filteredListeners; for (auto& listener : s.listeners) { + // Starts a registration but separates the callback ids according to callback type. This + // allows the callback invoker to send on latch callbacks earlier. // note that startRegistration will not re-register if the listener has // already be registered for a prior surface control - mTransactionCallbackInvoker.startRegistration(listener); - listenerCallbacks.insert(listener); + + ListenerCallbacks onCommitCallbacks = listener.filter(CallbackId::Type::ON_COMMIT); + if (!onCommitCallbacks.callbackIds.empty()) { + mTransactionCallbackInvoker.startRegistration(onCommitCallbacks); + filteredListeners.push_back(onCommitCallbacks); + outListenerCallbacks.insert(onCommitCallbacks); + } + + ListenerCallbacks onCompleteCallbacks = listener.filter(CallbackId::Type::ON_COMPLETE); + if (!onCompleteCallbacks.callbackIds.empty()) { + mTransactionCallbackInvoker.startRegistration(onCompleteCallbacks); + filteredListeners.push_back(onCompleteCallbacks); + outListenerCallbacks.insert(onCompleteCallbacks); + } } const uint64_t what = s.what; @@ -4029,8 +4050,8 @@ uint32_t SurfaceFlinger::setClientStateLocked( } } std::vector<sp<CallbackHandle>> callbackHandles; - if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!s.listeners.empty())) { - for (auto& [listener, callbackIds] : s.listeners) { + if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!filteredListeners.empty())) { + for (auto& [listener, callbackIds] : filteredListeners) { callbackHandles.emplace_back(new CallbackHandle(listener, callbackIds, s.surface)); } } @@ -5093,17 +5114,15 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) { // captureLayers and captureDisplay will handle the permission check in the function case CAPTURE_LAYERS: case CAPTURE_DISPLAY: - case SET_DISPLAY_BRIGHTNESS: case SET_FRAME_TIMELINE_INFO: case GET_GPU_CONTEXT_PRIORITY: case GET_EXTRA_BUFFER_COUNT: { // This is not sensitive information, so should not require permission control. return OK; } + case SET_DISPLAY_BRIGHTNESS: case ADD_HDR_LAYER_INFO_LISTENER: case REMOVE_HDR_LAYER_INFO_LISTENER: { - // TODO (b/183985553): Should getting & setting brightness be part of this...? - // codes that require permission check IPCThreadState* ipc = IPCThreadState::self(); const int pid = ipc->getCallingPid(); const int uid = ipc->getCallingUid(); diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp index 16c6bb33c8..3d82afa43a 100644 --- a/services/surfaceflinger/TimeStats/TimeStats.cpp +++ b/services/surfaceflinger/TimeStats/TimeStats.cpp @@ -745,7 +745,7 @@ void TimeStats::setPresentFence(int32_t layerId, uint64_t frameNumber, static const constexpr int32_t kValidJankyReason = JankType::DisplayHAL | JankType::SurfaceFlingerCpuDeadlineMissed | JankType::SurfaceFlingerGpuDeadlineMissed | JankType::AppDeadlineMissed | JankType::PredictionError | - JankType::SurfaceFlingerScheduling | JankType::BufferStuffing; + JankType::SurfaceFlingerScheduling; template <class T> static void updateJankPayload(T& t, int32_t reasons) { @@ -771,9 +771,11 @@ static void updateJankPayload(T& t, int32_t reasons) { if ((reasons & JankType::SurfaceFlingerScheduling) != 0) { t.jankPayload.totalSFScheduling++; } - if ((reasons & JankType::BufferStuffing) != 0) { - t.jankPayload.totalAppBufferStuffing++; - } + } + + // We want to track BufferStuffing separately as it can provide info on latency issues + if (reasons & JankType::BufferStuffing) { + t.jankPayload.totalAppBufferStuffing++; } } diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index 3590e76cb9..4f4c02be6c 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -36,13 +36,17 @@ namespace android { // <0 if the first id that doesn't match is lower in c2 or all ids match but c2 is shorter // >0 if the first id that doesn't match is greater in c2 or all ids match but c2 is longer // -// See CallbackIdsHash for a explaniation of why this works +// See CallbackIdsHash for a explanation of why this works static int compareCallbackIds(const std::vector<CallbackId>& c1, const std::vector<CallbackId>& c2) { if (c1.empty()) { return !c2.empty(); } - return c1.front() - c2.front(); + return c1.front().id - c2.front().id; +} + +static bool containsOnCommitCallbacks(const std::vector<CallbackId>& callbacks) { + return !callbacks.empty() && callbacks.front().type == CallbackId::Type::ON_COMMIT; } TransactionCallbackInvoker::~TransactionCallbackInvoker() { @@ -114,39 +118,69 @@ status_t TransactionCallbackInvoker::registerPendingCallbackHandle( return NO_ERROR; } -status_t TransactionCallbackInvoker::finalizePendingCallbackHandles( - const std::deque<sp<CallbackHandle>>& handles, const std::vector<JankData>& jankData) { +status_t TransactionCallbackInvoker::finalizeCallbackHandle(const sp<CallbackHandle>& handle, + const std::vector<JankData>& jankData) { + auto listener = mPendingTransactions.find(handle->listener); + if (listener != mPendingTransactions.end()) { + auto& pendingCallbacks = listener->second; + auto pendingCallback = pendingCallbacks.find(handle->callbackIds); + + if (pendingCallback != pendingCallbacks.end()) { + auto& pendingCount = pendingCallback->second; + + // Decrease the pending count for this listener + if (--pendingCount == 0) { + pendingCallbacks.erase(pendingCallback); + } + } else { + ALOGW("there are more latched callbacks than there were registered callbacks"); + } + if (listener->second.size() == 0) { + mPendingTransactions.erase(listener); + } + } else { + ALOGW("cannot find listener in mPendingTransactions"); + } + + status_t err = addCallbackHandle(handle, jankData); + if (err != NO_ERROR) { + ALOGE("could not add callback handle"); + return err; + } + return NO_ERROR; +} + +status_t TransactionCallbackInvoker::finalizeOnCommitCallbackHandles( + const std::deque<sp<CallbackHandle>>& handles, + std::deque<sp<CallbackHandle>>& outRemainingHandles) { if (handles.empty()) { return NO_ERROR; } std::lock_guard lock(mMutex); - + const std::vector<JankData>& jankData = std::vector<JankData>(); for (const auto& handle : handles) { - auto listener = mPendingTransactions.find(handle->listener); - if (listener != mPendingTransactions.end()) { - auto& pendingCallbacks = listener->second; - auto pendingCallback = pendingCallbacks.find(handle->callbackIds); - - if (pendingCallback != pendingCallbacks.end()) { - auto& pendingCount = pendingCallback->second; - - // Decrease the pending count for this listener - if (--pendingCount == 0) { - pendingCallbacks.erase(pendingCallback); - } - } else { - ALOGW("there are more latched callbacks than there were registered callbacks"); - } - if (listener->second.size() == 0) { - mPendingTransactions.erase(listener); - } - } else { - ALOGW("cannot find listener in mPendingTransactions"); + if (!containsOnCommitCallbacks(handle->callbackIds)) { + outRemainingHandles.push_back(handle); + continue; } + status_t err = finalizeCallbackHandle(handle, jankData); + if (err != NO_ERROR) { + return err; + } + } - status_t err = addCallbackHandle(handle, jankData); + return NO_ERROR; +} + +status_t TransactionCallbackInvoker::finalizePendingCallbackHandles( + const std::deque<sp<CallbackHandle>>& handles, const std::vector<JankData>& jankData) { + if (handles.empty()) { + return NO_ERROR; + } + std::lock_guard lock(mMutex); + for (const auto& handle : handles) { + status_t err = finalizeCallbackHandle(handle, jankData); if (err != NO_ERROR) { - ALOGE("could not add callback handle"); return err; } } @@ -243,7 +277,8 @@ void TransactionCallbackInvoker::sendCallbacks() { } // If the transaction has been latched - if (transactionStats.latchTime >= 0) { + if (transactionStats.latchTime >= 0 && + !containsOnCommitCallbacks(transactionStats.callbackIds)) { if (!mPresentFence) { break; } diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index caa8a4fb45..184b15103e 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -74,6 +74,8 @@ public: // Notifies the TransactionCallbackInvoker that a pending CallbackHandle has been presented. status_t finalizePendingCallbackHandles(const std::deque<sp<CallbackHandle>>& handles, const std::vector<JankData>& jankData); + status_t finalizeOnCommitCallbackHandles(const std::deque<sp<CallbackHandle>>& handles, + std::deque<sp<CallbackHandle>>& outRemainingHandles); // Adds the Transaction CallbackHandle from a layer that does not need to be relatched and // presented this frame. @@ -95,6 +97,9 @@ private: status_t addCallbackHandle(const sp<CallbackHandle>& handle, const std::vector<JankData>& jankData) REQUIRES(mMutex); + status_t finalizeCallbackHandle(const sp<CallbackHandle>& handle, + const std::vector<JankData>& jankData) REQUIRES(mMutex); + class CallbackDeathRecipient : public IBinder::DeathRecipient { public: // This function is a no-op. isBinderAlive needs a linked DeathRecipient to work. diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp index 4e73cbc1c0..188ea758d4 100644 --- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp +++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp @@ -1051,6 +1051,8 @@ TEST_F(TimeStatsTest, globalStatsCallback) { JankType::AppDeadlineMissed | JankType::BufferStuffing, 1, 2, 3}); mTimeStats->incrementJankyFrames({kRefreshRate0, kRenderRate0, UID_0, genLayerName(LAYER_ID_0), + JankType::BufferStuffing, 1, 2, 3}); + mTimeStats->incrementJankyFrames({kRefreshRate0, kRenderRate0, UID_0, genLayerName(LAYER_ID_0), JankType::None, 1, 2, 3}); std::string pulledData; @@ -1069,7 +1071,7 @@ TEST_F(TimeStatsTest, globalStatsCallback) { EXPECT_EQ(atom.event_connection_count(), DISPLAY_EVENT_CONNECTIONS); EXPECT_THAT(atom.frame_duration(), HistogramEq(buildExpectedHistogram({2}, {1}))); EXPECT_THAT(atom.render_engine_timing(), HistogramEq(buildExpectedHistogram({1, 2}, {1, 1}))); - EXPECT_EQ(atom.total_timeline_frames(), 8); + EXPECT_EQ(atom.total_timeline_frames(), 9); EXPECT_EQ(atom.total_janky_frames(), 7); EXPECT_EQ(atom.total_janky_frames_with_long_cpu(), 1); EXPECT_EQ(atom.total_janky_frames_with_long_gpu(), 1); @@ -1077,7 +1079,7 @@ TEST_F(TimeStatsTest, globalStatsCallback) { EXPECT_EQ(atom.total_janky_frames_app_unattributed(), 2); EXPECT_EQ(atom.total_janky_frames_sf_scheduling(), 1); EXPECT_EQ(atom.total_jank_frames_sf_prediction_error(), 1); - EXPECT_EQ(atom.total_jank_frames_app_buffer_stuffing(), 1); + EXPECT_EQ(atom.total_jank_frames_app_buffer_stuffing(), 2); EXPECT_EQ(atom.display_refresh_rate_bucket(), REFRESH_RATE_BUCKET_0); EXPECT_THAT(atom.sf_deadline_misses(), HistogramEq(buildExpectedHistogram({1}, {7}))); EXPECT_THAT(atom.sf_prediction_errors(), HistogramEq(buildExpectedHistogram({2}, {7}))); @@ -1096,7 +1098,7 @@ TEST_F(TimeStatsTest, globalStatsCallback) { const std::string result(inputCommand(InputCommand::DUMP_ALL, FMT_STRING)); std::string expectedResult = "totalTimelineFrames = " + std::to_string(0); EXPECT_THAT(result, HasSubstr(expectedResult)); - expectedResult = "totalTimelineFrames = " + std::to_string(8); + expectedResult = "totalTimelineFrames = " + std::to_string(9); EXPECT_THAT(result, HasSubstr(expectedResult)); expectedResult = "jankyFrames = " + std::to_string(0); EXPECT_THAT(result, HasSubstr(expectedResult)); @@ -1128,7 +1130,7 @@ TEST_F(TimeStatsTest, globalStatsCallback) { EXPECT_THAT(result, HasSubstr(expectedResult)); expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(0); EXPECT_THAT(result, HasSubstr(expectedResult)); - expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(1); + expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(2); EXPECT_THAT(result, HasSubstr(expectedResult)); } |