diff options
62 files changed, 1613 insertions, 960 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/lshal/ListCommand.cpp b/cmds/lshal/ListCommand.cpp index d5110f6203..2722e214e8 100644 --- a/cmds/lshal/ListCommand.cpp +++ b/cmds/lshal/ListCommand.cpp @@ -417,7 +417,7 @@ void ListCommand::dumpVintf(const NullableOStream<std::ostream>& out) const { } } out << "-->" << std::endl; - out << vintf::gHalManifestConverter(manifest, vintf::SerializeFlags::HALS_ONLY); + out << vintf::toXml(manifest, vintf::SerializeFlags::HALS_ONLY); } std::string ListCommand::INIT_VINTF_NOTES{ diff --git a/cmds/lshal/test.cpp b/cmds/lshal/test.cpp index 7c1ca91528..6f08f74690 100644 --- a/cmds/lshal/test.cpp +++ b/cmds/lshal/test.cpp @@ -47,8 +47,6 @@ using ::android::hardware::hidl_vec; using ::android::hardware::Void; using android::vintf::Arch; using android::vintf::CompatibilityMatrix; -using android::vintf::gCompatibilityMatrixConverter; -using android::vintf::gHalManifestConverter; using android::vintf::HalManifest; using android::vintf::Transport; using android::vintf::VintfObject; @@ -510,7 +508,7 @@ TEST_F(ListTest, DumpVintf) { std::string error; vintf::HalManifest m; - EXPECT_EQ(true, vintf::gHalManifestConverter(&m, out.str(), &error)) + EXPECT_EQ(true, vintf::fromXml(&m, out.str(), &error)) << "--init-vintf does not emit valid HAL manifest: " << error; } @@ -775,10 +773,10 @@ TEST_F(ListTest, Vintf) { auto deviceMatrix = std::make_shared<CompatibilityMatrix>(); auto frameworkMatrix = std::make_shared<CompatibilityMatrix>(); - ASSERT_TRUE(gHalManifestConverter(deviceManifest.get(), deviceManifestXml)); - ASSERT_TRUE(gHalManifestConverter(frameworkManifest.get(), frameworkManifestXml)); - ASSERT_TRUE(gCompatibilityMatrixConverter(deviceMatrix.get(), deviceMatrixXml)); - ASSERT_TRUE(gCompatibilityMatrixConverter(frameworkMatrix.get(), frameworkMatrixXml)); + ASSERT_TRUE(fromXml(deviceManifest.get(), deviceManifestXml)); + ASSERT_TRUE(fromXml(frameworkManifest.get(), frameworkManifestXml)); + ASSERT_TRUE(fromXml(deviceMatrix.get(), deviceMatrixXml)); + ASSERT_TRUE(fromXml(frameworkMatrix.get(), frameworkMatrixXml)); ON_CALL(*mockList, getDeviceManifest()).WillByDefault(Return(deviceManifest)); ON_CALL(*mockList, getDeviceMatrix()).WillByDefault(Return(deviceMatrix)); @@ -964,7 +962,7 @@ public: " </hal>\n" "</manifest>"; auto manifest = std::make_shared<HalManifest>(); - EXPECT_TRUE(gHalManifestConverter(manifest.get(), mockManifestXml)); + EXPECT_TRUE(fromXml(manifest.get(), mockManifestXml)); EXPECT_CALL(*mockList, getDeviceManifest()) .Times(AnyNumber()) .WillRepeatedly(Return(manifest)); 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/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index 406bd54e6f..6fb1227f63 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -90,6 +90,8 @@ static const char *kReturnStrings[] = { "BR_DEAD_BINDER", "BR_CLEAR_DEATH_NOTIFICATION_DONE", "BR_FAILED_REPLY", + "BR_FROZEN_REPLY", + "BR_ONEWAY_SPAM_SUSPECT", "BR_TRANSACTION_SEC_CTX", }; @@ -894,6 +896,11 @@ status_t IPCThreadState::waitForResponse(Parcel *reply, status_t *acquireResult) } switch (cmd) { + case BR_ONEWAY_SPAM_SUSPECT: + ALOGE("Process seems to be sending too many oneway calls."); + CallStack::logStack("oneway spamming", CallStack::getCurrent().get(), + ANDROID_LOG_ERROR); + [[fallthrough]]; case BR_TRANSACTION_COMPLETE: if (!reply && !acquireResult) goto finish; break; 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/ProcessState.cpp b/libs/binder/ProcessState.cpp index 7647a8c0df..0414e76bfe 100644 --- a/libs/binder/ProcessState.cpp +++ b/libs/binder/ProcessState.cpp @@ -43,6 +43,7 @@ #define BINDER_VM_SIZE ((1 * 1024 * 1024) - sysconf(_SC_PAGE_SIZE) * 2) #define DEFAULT_MAX_BINDER_THREADS 15 +#define DEFAULT_ENABLE_ONEWAY_SPAM_DETECTION 1 #ifdef __ANDROID_VNDK__ const char* kDefaultDriver = "/dev/vndbinder"; @@ -358,6 +359,15 @@ status_t ProcessState::setThreadPoolMaxThreadCount(size_t maxThreads) { return result; } +status_t ProcessState::enableOnewaySpamDetection(bool enable) { + uint32_t enableDetection = enable ? 1 : 0; + if (ioctl(mDriverFD, BINDER_ENABLE_ONEWAY_SPAM_DETECTION, &enableDetection) == -1) { + ALOGE("Binder ioctl to enable oneway spam detection failed: %s", strerror(errno)); + return -errno; + } + return NO_ERROR; +} + void ProcessState::giveThreadPoolName() { androidSetThreadName( makeBinderThreadName().string() ); } @@ -388,6 +398,11 @@ static int open_driver(const char *driver) if (result == -1) { ALOGE("Binder ioctl to set max threads failed: %s", strerror(errno)); } + uint32_t enable = DEFAULT_ENABLE_ONEWAY_SPAM_DETECTION; + result = ioctl(fd, BINDER_ENABLE_ONEWAY_SPAM_DETECTION, &enable); + if (result == -1) { + ALOGE("Binder ioctl to enable oneway spam detection failed: %s", strerror(errno)); + } } else { ALOGW("Opening '%s' failed: %s\n", driver, strerror(errno)); } diff --git a/libs/binder/RpcConnection.cpp b/libs/binder/RpcConnection.cpp index 4aff92b58a..1388a801e4 100644 --- a/libs/binder/RpcConnection.cpp +++ b/libs/binder/RpcConnection.cpp @@ -18,6 +18,16 @@ #include <binder/RpcConnection.h> +#include <arpa/inet.h> +#include <netdb.h> +#include <netinet/in.h> +#include <sys/socket.h> +#include <sys/types.h> +#include <sys/un.h> +#include <unistd.h> + +#include <string_view> + #include <binder/Parcel.h> #include <binder/Stability.h> #include <utils/String8.h> @@ -25,11 +35,6 @@ #include "RpcState.h" #include "RpcWireFormat.h" -#include <sys/socket.h> -#include <sys/types.h> -#include <sys/un.h> -#include <unistd.h> - #ifdef __GLIBC__ extern "C" pid_t gettid(); #endif @@ -41,6 +46,7 @@ extern "C" pid_t gettid(); namespace android { using base::unique_fd; +using AddrInfo = std::unique_ptr<addrinfo, decltype(&freeaddrinfo)>; RpcConnection::SocketAddress::~SocketAddress() {} @@ -51,6 +57,10 @@ RpcConnection::RpcConnection() { } RpcConnection::~RpcConnection() { LOG_RPC_DETAIL("RpcConnection destroyed %p", this); + + std::lock_guard<std::mutex> _l(mSocketMutex); + LOG_ALWAYS_FATAL_IF(mServers.size() != 0, + "Should not be able to destroy a connection with servers in use."); } sp<RpcConnection> RpcConnection::make() { @@ -120,6 +130,70 @@ bool RpcConnection::addVsockClient(unsigned int cid, unsigned int port) { #endif // __BIONIC__ +class SocketAddressImpl : public RpcConnection::SocketAddress { +public: + SocketAddressImpl(const sockaddr* addr, size_t size, const String8& desc) + : mAddr(addr), mSize(size), mDesc(desc) {} + [[nodiscard]] std::string toString() const override { + return std::string(mDesc.c_str(), mDesc.size()); + } + [[nodiscard]] const sockaddr* addr() const override { return mAddr; } + [[nodiscard]] size_t addrSize() const override { return mSize; } + void set(const sockaddr* addr, size_t size) { + mAddr = addr; + mSize = size; + } + +private: + const sockaddr* mAddr = nullptr; + size_t mSize = 0; + String8 mDesc; +}; + +AddrInfo GetAddrInfo(const char* addr, unsigned int port) { + addrinfo hint{ + .ai_flags = 0, + .ai_family = AF_UNSPEC, + .ai_socktype = SOCK_STREAM, + .ai_protocol = 0, + }; + addrinfo* aiStart = nullptr; + if (int rc = getaddrinfo(addr, std::to_string(port).data(), &hint, &aiStart); 0 != rc) { + ALOGE("Unable to resolve %s:%u: %s", addr, port, gai_strerror(rc)); + return AddrInfo(nullptr, nullptr); + } + if (aiStart == nullptr) { + ALOGE("Unable to resolve %s:%u: getaddrinfo returns null", addr, port); + return AddrInfo(nullptr, nullptr); + } + return AddrInfo(aiStart, &freeaddrinfo); +} + +bool RpcConnection::setupInetServer(unsigned int port) { + auto aiStart = GetAddrInfo("127.0.0.1", port); + if (aiStart == nullptr) return false; + SocketAddressImpl socketAddress(nullptr, 0, String8::format("127.0.0.1:%u", port)); + for (auto ai = aiStart.get(); ai != nullptr; ai = ai->ai_next) { + socketAddress.set(ai->ai_addr, ai->ai_addrlen); + if (setupSocketServer(socketAddress)) return true; + } + ALOGE("None of the socket address resolved for 127.0.0.1:%u can be set up as inet server.", + port); + return false; +} + +bool RpcConnection::addInetClient(const char* addr, unsigned int port) { + auto aiStart = GetAddrInfo(addr, port); + if (aiStart == nullptr) return false; + SocketAddressImpl socketAddress(nullptr, 0, String8::format("%s:%u", addr, port)); + for (auto ai = aiStart.get(); ai != nullptr; ai = ai->ai_next) { + socketAddress.set(ai->ai_addr, ai->ai_addrlen); + if (addSocketClient(socketAddress)) return true; + } + ALOGE("None of the socket address resolved for %s:%u can be added as inet client.", addr, port); + return false; +} + bool RpcConnection::addNullDebuggingClient() { unique_fd serverFd(TEMP_FAILURE_RETRY(open("/dev/null", O_WRONLY | O_CLOEXEC))); @@ -152,36 +226,35 @@ status_t RpcConnection::sendDecStrong(const RpcAddress& address) { } void RpcConnection::join() { - // establish a connection - { - unique_fd clientFd( - TEMP_FAILURE_RETRY(accept4(mServer.get(), nullptr, 0 /*length*/, SOCK_CLOEXEC))); - if (clientFd < 0) { - // If this log becomes confusing, should save more state from setupUnixDomainServer - // in order to output here. - ALOGE("Could not accept4 socket: %s", strerror(errno)); - return; - } - - LOG_RPC_DETAIL("accept4 on fd %d yields fd %d", mServer.get(), clientFd.get()); - - assignServerToThisThread(std::move(clientFd)); + // TODO(b/185167543): do this dynamically, instead of from a static number + // of threads + unique_fd clientFd( + TEMP_FAILURE_RETRY(accept4(mServer.get(), nullptr, 0 /*length*/, SOCK_CLOEXEC))); + if (clientFd < 0) { + // If this log becomes confusing, should save more state from setupUnixDomainServer + // in order to output here. + ALOGE("Could not accept4 socket: %s", strerror(errno)); + return; } - // We may not use the connection we just established (two threads might - // establish connections for each other), but for now, just use one - // server/socket connection. - ExclusiveSocket socket(sp<RpcConnection>::fromExisting(this), SocketUse::SERVER); + LOG_RPC_DETAIL("accept4 on fd %d yields fd %d", mServer.get(), clientFd.get()); + + // must be registered to allow arbitrary client code executing commands to + // be able to do nested calls (we can't only read from it) + sp<ConnectionSocket> socket = assignServerToThisThread(std::move(clientFd)); while (true) { status_t error = - state()->getAndExecuteCommand(socket.fd(), sp<RpcConnection>::fromExisting(this)); + state()->getAndExecuteCommand(socket->fd, sp<RpcConnection>::fromExisting(this)); if (error != OK) { ALOGI("Binder socket thread closing w/ status %s", statusToString(error).c_str()); - return; + break; } } + + LOG_ALWAYS_FATAL_IF(!removeServerSocket(socket), + "bad state: socket object guaranteed to be in list"); } void RpcConnection::setForServer(const wp<RpcServer>& server) { @@ -246,11 +319,23 @@ void RpcConnection::addClient(unique_fd&& fd) { mClients.push_back(connection); } -void RpcConnection::assignServerToThisThread(unique_fd&& fd) { +sp<RpcConnection::ConnectionSocket> RpcConnection::assignServerToThisThread(unique_fd&& fd) { std::lock_guard<std::mutex> _l(mSocketMutex); sp<ConnectionSocket> connection = sp<ConnectionSocket>::make(); connection->fd = std::move(fd); + connection->exclusiveTid = gettid(); mServers.push_back(connection); + + return connection; +} + +bool RpcConnection::removeServerSocket(const sp<ConnectionSocket>& socket) { + std::lock_guard<std::mutex> _l(mSocketMutex); + if (auto it = std::find(mServers.begin(), mServers.end(), socket); it != mServers.end()) { + mServers.erase(it); + return true; + } + return false; } RpcConnection::ExclusiveSocket::ExclusiveSocket(const sp<RpcConnection>& connection, SocketUse use) @@ -265,37 +350,31 @@ RpcConnection::ExclusiveSocket::ExclusiveSocket(const sp<RpcConnection>& connect // CHECK FOR DEDICATED CLIENT SOCKET // - // A server/looper should always use a dedicated connection. - if (use != SocketUse::SERVER) { - findSocket(tid, &exclusive, &available, mConnection->mClients, - mConnection->mClientsOffset); - - // WARNING: this assumes a server cannot request its client to send - // a transaction, as mServers is excluded below. - // - // Imagine we have more than one thread in play, and a single thread - // sends a synchronous, then an asynchronous command. Imagine the - // asynchronous command is sent on the first client socket. Then, if - // we naively send a synchronous command to that same socket, the - // thread on the far side might be busy processing the asynchronous - // command. So, we move to considering the second available thread - // for subsequent calls. - if (use == SocketUse::CLIENT_ASYNC && (exclusive != nullptr || available != nullptr)) { - mConnection->mClientsOffset = - (mConnection->mClientsOffset + 1) % mConnection->mClients.size(); - } + // A server/looper should always use a dedicated connection if available + findSocket(tid, &exclusive, &available, mConnection->mClients, mConnection->mClientsOffset); + + // WARNING: this assumes a server cannot request its client to send + // a transaction, as mServers is excluded below. + // + // Imagine we have more than one thread in play, and a single thread + // sends a synchronous, then an asynchronous command. Imagine the + // asynchronous command is sent on the first client socket. Then, if + // we naively send a synchronous command to that same socket, the + // thread on the far side might be busy processing the asynchronous + // command. So, we move to considering the second available thread + // for subsequent calls. + if (use == SocketUse::CLIENT_ASYNC && (exclusive != nullptr || available != nullptr)) { + mConnection->mClientsOffset = + (mConnection->mClientsOffset + 1) % mConnection->mClients.size(); } - // USE SERVING SOCKET (to start serving or for nested transaction) + // USE SERVING SOCKET (for nested transaction) // // asynchronous calls cannot be nested if (use != SocketUse::CLIENT_ASYNC) { - // servers should start serving on an available thread only - // otherwise, this should only be a nested call - bool useAvailable = use == SocketUse::SERVER; - - findSocket(tid, &exclusive, (useAvailable ? &available : nullptr), - mConnection->mServers, 0 /* index hint */); + // server sockets are always assigned to a thread + findSocket(tid, &exclusive, nullptr /*available*/, mConnection->mServers, + 0 /* index hint */); } // if our thread is already using a connection, prioritize using that @@ -309,8 +388,6 @@ RpcConnection::ExclusiveSocket::ExclusiveSocket(const sp<RpcConnection>& connect break; } - LOG_ALWAYS_FATAL_IF(use == SocketUse::SERVER, "Must create connection to join one."); - // in regular binder, this would usually be a deadlock :) LOG_ALWAYS_FATAL_IF(mConnection->mClients.size() == 0, "Not a client of any connection. You must create a connection to an " 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/include/binder/ProcessState.h b/libs/binder/include/binder/ProcessState.h index 0919648541..b9db5d726c 100644 --- a/libs/binder/include/binder/ProcessState.h +++ b/libs/binder/include/binder/ProcessState.h @@ -58,6 +58,7 @@ public: void spawnPooledThread(bool isMain); status_t setThreadPoolMaxThreadCount(size_t maxThreads); + status_t enableOnewaySpamDetection(bool enable); void giveThreadPoolName(); String8 getDriverName(); diff --git a/libs/binder/include/binder/RpcConnection.h b/libs/binder/include/binder/RpcConnection.h index dba47b4d15..2395e78de2 100644 --- a/libs/binder/include/binder/RpcConnection.h +++ b/libs/binder/include/binder/RpcConnection.h @@ -74,6 +74,16 @@ public: #endif // __BIONIC__ /** + * Creates an RPC server at the current port. + */ + [[nodiscard]] bool setupInetServer(unsigned int port); + + /** + * Connects to an RPC server at the given address and port. + */ + [[nodiscard]] bool addInetClient(const char* addr, unsigned int port); + + /** * For debugging! * * Sets up an empty socket. All queries to this socket which require a @@ -118,11 +128,6 @@ private: friend sp<RpcConnection>; RpcConnection(); - bool setupSocketServer(const SocketAddress& address); - bool addSocketClient(const SocketAddress& address); - void addClient(base::unique_fd&& fd); - void assignServerToThisThread(base::unique_fd&& fd); - struct ConnectionSocket : public RefBase { base::unique_fd fd; @@ -131,11 +136,16 @@ private: std::optional<pid_t> exclusiveTid; }; + bool setupSocketServer(const SocketAddress& address); + bool addSocketClient(const SocketAddress& address); + void addClient(base::unique_fd&& fd); + sp<ConnectionSocket> assignServerToThisThread(base::unique_fd&& fd); + bool removeServerSocket(const sp<ConnectionSocket>& socket); + enum class SocketUse { CLIENT, CLIENT_ASYNC, CLIENT_REFCOUNT, - SERVER, }; // RAII object for connection socket diff --git a/libs/binder/include/private/binder/binder_module.h b/libs/binder/include/private/binder/binder_module.h index 157919903f..151235c16b 100644 --- a/libs/binder/include/private/binder/binder_module.h +++ b/libs/binder/include/private/binder/binder_module.h @@ -32,10 +32,6 @@ #include <sys/ioctl.h> #include <linux/android/binder.h> -#ifdef __cplusplus -namespace android { -#endif - #ifndef BR_FROZEN_REPLY // Temporary definition of BR_FROZEN_REPLY. For production // this will come from UAPI binder.h @@ -88,8 +84,18 @@ struct binder_frozen_status_info { }; #endif //BINDER_GET_FROZEN_INFO -#ifdef __cplusplus -} // namespace android -#endif +#ifndef BR_ONEWAY_SPAM_SUSPECT +// Temporary definition of BR_ONEWAY_SPAM_SUSPECT. For production +// this will come from UAPI binder.h +#define BR_ONEWAY_SPAM_SUSPECT _IO('r', 19) +#endif //BR_ONEWAY_SPAM_SUSPECT + +#ifndef BINDER_ENABLE_ONEWAY_SPAM_DETECTION +/* + * Temporary definitions for oneway spam detection support. For the final version + * these will be defined in the UAPI binder.h file from upstream kernel. + */ +#define BINDER_ENABLE_ONEWAY_SPAM_DETECTION _IOW('b', 16, __u32) +#endif //BINDER_ENABLE_ONEWAY_SPAM_DETECTION #endif // _BINDER_MODULE_H_ 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/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 33402937ab..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(); } @@ -257,6 +256,7 @@ enum class SocketType { #ifdef __BIONIC__ VSOCK, #endif // __BIONIC__ + INET, }; static inline std::string PrintSocketType(const testing::TestParamInfo<SocketType>& info) { switch (info.param) { @@ -266,6 +266,8 @@ static inline std::string PrintSocketType(const testing::TestParamInfo<SocketTyp case SocketType::VSOCK: return "vm_socket"; #endif // __BIONIC__ + case SocketType::INET: + return "inet_socket"; default: LOG_ALWAYS_FATAL("Unknown socket type"); return ""; @@ -305,6 +307,9 @@ public: CHECK(connection->setupVsockServer(port)); break; #endif // __BIONIC__ + case SocketType::INET: + CHECK(connection->setupInetServer(port)); + break; default: LOG_ALWAYS_FATAL("Unknown socket type"); } @@ -335,6 +340,9 @@ public: if (ret.connection->addVsockClient(VMADDR_CID_LOCAL, port)) goto success; break; #endif // __BIONIC__ + case SocketType::INET: + if (ret.connection->addInetClient("127.0.0.1", port)) goto success; + break; default: LOG_ALWAYS_FATAL("Unknown socket type"); } @@ -740,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); } })); @@ -749,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; @@ -852,12 +882,13 @@ TEST_P(BinderRpc, Fds) { } INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc, - ::testing::Values(SocketType::UNIX + ::testing::ValuesIn({ + SocketType::UNIX, #ifdef __BIONIC__ - , - SocketType::VSOCK + SocketType::VSOCK, #endif // __BIONIC__ - ), + SocketType::INET, + }), PrintSocketType); } // namespace android 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/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index dbc1a7ed22..3d854c2b92 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -186,6 +186,7 @@ BLASTBufferQueue::~BLASTBufferQueue() { void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width, uint32_t height, int32_t format) { std::unique_lock _lock{mMutex}; + BQA_LOGV("update width=%d height=%d format=%d", width, height, format); if (mFormat != format) { mFormat = format; mBufferItemConsumer->setDefaultBufferFormat(convertBufferFormat(format)); @@ -397,10 +398,11 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { // Ensure BLASTBufferQueue stays alive until we receive the transaction complete callback. incStrong((void*)transactionCallbackThunk); + Rect crop = computeCrop(bufferItem); mLastAcquiredFrameNumber = bufferItem.mFrameNumber; mLastBufferInfo.update(true /* hasBuffer */, bufferItem.mGraphicBuffer->getWidth(), bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, - bufferItem.mScalingMode); + bufferItem.mScalingMode, crop); auto releaseBufferCallback = std::bind(releaseBufferCallbackThunk, wp<BLASTBufferQueue>(this) /* callbackContext */, @@ -415,7 +417,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { mSurfaceControlsWithPendingCallback.push(mSurfaceControl); setMatrix(t, mLastBufferInfo); - t->setCrop(mSurfaceControl, computeCrop(bufferItem)); + t->setCrop(mSurfaceControl, crop); t->setTransform(mSurfaceControl, bufferItem.mTransform); t->setTransformToDisplayInverse(mSurfaceControl, bufferItem.mTransformToDisplayInverse); if (!bufferItem.mIsAutoTimestamp) { @@ -543,13 +545,15 @@ bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { void BLASTBufferQueue::setMatrix(SurfaceComposerClient::Transaction* t, const BufferInfo& bufferInfo) { - uint32_t bufWidth = bufferInfo.width; - uint32_t bufHeight = bufferInfo.height; + uint32_t bufWidth = bufferInfo.crop.getWidth(); + uint32_t bufHeight = bufferInfo.crop.getHeight(); - float dsdx = mSize.width / static_cast<float>(bufWidth); - float dsdy = mSize.height / static_cast<float>(bufHeight); + float sx = mSize.width / static_cast<float>(bufWidth); + float sy = mSize.height / static_cast<float>(bufHeight); - t->setMatrix(mSurfaceControl, dsdx, 0, 0, dsdy); + t->setMatrix(mSurfaceControl, sx, 0, 0, sy); + // Update position based on crop. + t->setPosition(mSurfaceControl, bufferInfo.crop.left * sx * -1, bufferInfo.crop.top * sy * -1); } void BLASTBufferQueue::setTransactionCompleteCallback( diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index 0e28966a9b..53721cf6f7 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -497,6 +497,28 @@ public: return result; } + status_t onPullAtom(const int32_t atomId, std::string* pulledData, bool* success) { + Parcel data, reply; + SAFE_PARCEL(data.writeInterfaceToken, ISurfaceComposer::getInterfaceDescriptor()); + SAFE_PARCEL(data.writeInt32, atomId); + + status_t err = remote()->transact(BnSurfaceComposer::ON_PULL_ATOM, data, &reply); + if (err != NO_ERROR) { + ALOGE("onPullAtom failed to transact: %d", err); + return err; + } + + int32_t size = 0; + SAFE_PARCEL(reply.readInt32, &size); + const void* dataPtr = reply.readInplace(size); + if (dataPtr == nullptr) { + return UNEXPECTED_NULL; + } + pulledData->assign((const char*)dataPtr, size); + SAFE_PARCEL(reply.readBool, success); + return NO_ERROR; + } + status_t enableVSyncInjections(bool enable) override { Parcel data, reply; status_t result = data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor()); @@ -2021,6 +2043,19 @@ status_t BnSurfaceComposer::onTransact( } return overrideHdrTypes(display, hdrTypesVector); } + case ON_PULL_ATOM: { + CHECK_INTERFACE(ISurfaceComposer, data, reply); + int32_t atomId = 0; + SAFE_PARCEL(data.readInt32, &atomId); + + std::string pulledData; + bool success; + status_t err = onPullAtom(atomId, &pulledData, &success); + SAFE_PARCEL(reply->writeByteArray, pulledData.size(), + reinterpret_cast<const uint8_t*>(pulledData.data())); + SAFE_PARCEL(reply->writeBool, success); + return err; + } default: { return BBinder::onTransact(code, data, reply, flags); } diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 49a10300b8..83124cf0cd 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -1993,6 +1993,11 @@ status_t SurfaceComposerClient::overrideHdrTypes(const sp<IBinder>& display, return ComposerService::getComposerService()->overrideHdrTypes(display, hdrTypes); } +status_t SurfaceComposerClient::onPullAtom(const int32_t atomId, std::string* outData, + bool* success) { + return ComposerService::getComposerService()->onPullAtom(atomId, outData, success); +} + status_t SurfaceComposerClient::getDisplayedContentSamplingAttributes(const sp<IBinder>& display, ui::PixelFormat* outFormat, ui::Dataspace* outDataspace, diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 8a23223660..139dbb7f6b 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -151,14 +151,20 @@ private: // we get the next buffer. This will support scenarios where the layer can change sizes // and the buffer will scale to fit the new size. uint32_t scalingMode = NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW; + Rect crop; void update(bool hasBuffer, uint32_t width, uint32_t height, uint32_t transform, - uint32_t scalingMode) { + uint32_t scalingMode, const Rect& crop) { this->hasBuffer = hasBuffer; this->width = width; this->height = height; this->transform = transform; this->scalingMode = scalingMode; + if (!crop.isEmpty()) { + this->crop = crop; + } else { + this->crop = Rect(width, height); + } } }; diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index ccd6d4e754..cb0468901a 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -275,6 +275,12 @@ public: virtual status_t overrideHdrTypes(const sp<IBinder>& display, const std::vector<ui::Hdr>& hdrTypes) = 0; + /* Pulls surfaceflinger atoms global stats and layer stats to pipe to statsd. + * + * Requires the calling uid be from system server. + */ + virtual status_t onPullAtom(const int32_t atomId, std::string* outData, bool* success) = 0; + virtual status_t enableVSyncInjections(bool enable) = 0; virtual status_t injectVSync(nsecs_t when) = 0; @@ -600,6 +606,7 @@ public: OVERRIDE_HDR_TYPES, ADD_HDR_LAYER_INFO_LISTENER, REMOVE_HDR_LAYER_INFO_LISTENER, + ON_PULL_ATOM, // Always append new enum to the end. }; diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index d2d1e5b91c..b4f62f2206 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -84,6 +84,7 @@ struct layer_state_t { eLayerStackChanged = 0x00000080, eReleaseBufferListenerChanged = 0x00000400, eShadowRadiusChanged = 0x00000800, + eLayerCreated = 0x00001000, /* was eDetachChildren, now available 0x00002000, */ eRelativeLayerChanged = 0x00004000, eReparent = 0x00008000, diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 171c05e0c5..5bbd8e3d85 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -573,6 +573,8 @@ public: static status_t overrideHdrTypes(const sp<IBinder>& display, const std::vector<ui::Hdr>& hdrTypes); + static status_t onPullAtom(const int32_t atomId, std::string* outData, bool* success); + static void setDisplayProjection(const sp<IBinder>& token, ui::Rotation orientation, const Rect& layerStackRect, const Rect& displayRect); diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index a44f44fbe6..5a5da97599 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -522,16 +522,146 @@ TEST_F(BLASTBufferQueueTest, SetCrop_ScalingModeScaleCrop) { adapter.waitForCallbacks(); // capture screen and verify that it is red ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(r, g, b, + {10, 10, (int32_t)bufferSideLength - 10, + (int32_t)bufferSideLength - 10})); + ASSERT_NO_FATAL_FAILURE( + checkScreenCapture(0, 0, 0, + {0, 0, (int32_t)bufferSideLength, (int32_t)bufferSideLength}, + /*border*/ 0, /*outsideRegion*/ true)); +} - Rect bounds; - bounds.left = finalCropSideLength / 2; - bounds.top = 0; - bounds.right = bounds.left + finalCropSideLength; - bounds.bottom = finalCropSideLength; +TEST_F(BLASTBufferQueueTest, ScaleCroppedBufferToBufferSize) { + // add black background + auto bg = mClient->createSurface(String8("BGTest"), 0, 0, PIXEL_FORMAT_RGBA_8888, + ISurfaceComposerClient::eFXSurfaceEffect); + ASSERT_NE(nullptr, bg.get()); + Transaction t; + t.setLayerStack(bg, 0) + .setCrop(bg, Rect(0, 0, mDisplayWidth, mDisplayHeight)) + .setColor(bg, half3{0, 0, 0}) + .setLayer(bg, 0) + .apply(); - ASSERT_NO_FATAL_FAILURE(checkScreenCapture(r, g, b, bounds)); - ASSERT_NO_FATAL_FAILURE( - checkScreenCapture(0, 0, 0, bounds, /*border*/ 0, /*outsideRegion*/ true)); + Rect windowSize(1000, 1000); + Rect bufferSize(windowSize); + Rect bufferCrop(200, 200, 700, 700); + + BLASTBufferQueueHelper adapter(mSurfaceControl, windowSize.getWidth(), windowSize.getHeight()); + sp<IGraphicBufferProducer> igbProducer; + setUpProducer(adapter, igbProducer); + int slot; + sp<Fence> fence; + sp<GraphicBuffer> buf; + auto ret = igbProducer->dequeueBuffer(&slot, &fence, bufferSize.getWidth(), + bufferSize.getHeight(), PIXEL_FORMAT_RGBA_8888, + GRALLOC_USAGE_SW_WRITE_OFTEN, nullptr, nullptr); + ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, ret); + ASSERT_EQ(OK, igbProducer->requestBuffer(slot, &buf)); + + uint32_t* bufData; + buf->lock(static_cast<uint32_t>(GraphicBuffer::USAGE_SW_WRITE_OFTEN), + reinterpret_cast<void**>(&bufData)); + // fill buffer with grey + fillBuffer(bufData, bufferSize, buf->getStride(), 127, 127, 127); + + // fill crop area with different colors so we can verify the cropped region has been scaled + // correctly. + fillBuffer(bufData, Rect(200, 200, 450, 450), buf->getStride(), /* rgb */ 255, 0, 0); + fillBuffer(bufData, Rect(200, 451, 450, 700), buf->getStride(), /* rgb */ 0, 255, 0); + fillBuffer(bufData, Rect(451, 200, 700, 450), buf->getStride(), /* rgb */ 0, 0, 255); + fillBuffer(bufData, Rect(451, 451, 700, 700), buf->getStride(), /* rgb */ 255, 0, 0); + buf->unlock(); + + IGraphicBufferProducer::QueueBufferOutput qbOutput; + IGraphicBufferProducer::QueueBufferInput input(systemTime(), true /* autotimestamp */, + HAL_DATASPACE_UNKNOWN, + bufferCrop /* Rect::INVALID_RECT */, + NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW, 0, + Fence::NO_FENCE); + igbProducer->queueBuffer(slot, input, &qbOutput); + ASSERT_NE(ui::Transform::ROT_INVALID, qbOutput.transformHint); + + adapter.waitForCallbacks(); + + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + + // Verify cropped region is scaled correctly. + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(255, 0, 0, {10, 10, 490, 490})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 255, 0, {10, 510, 490, 990})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 0, 255, {510, 10, 990, 490})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(255, 0, 0, {510, 510, 990, 990})); + // Verify outside region is black. + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 0, 0, + {0, 0, (int32_t)windowSize.getWidth(), + (int32_t)windowSize.getHeight()}, + /*border*/ 0, /*outsideRegion*/ true)); +} + +TEST_F(BLASTBufferQueueTest, ScaleCroppedBufferToWindowSize) { + // add black background + auto bg = mClient->createSurface(String8("BGTest"), 0, 0, PIXEL_FORMAT_RGBA_8888, + ISurfaceComposerClient::eFXSurfaceEffect); + ASSERT_NE(nullptr, bg.get()); + Transaction t; + t.setLayerStack(bg, 0) + .setCrop(bg, Rect(0, 0, mDisplayWidth, mDisplayHeight)) + .setColor(bg, half3{0, 0, 0}) + .setLayer(bg, 0) + .apply(); + + Rect windowSize(1000, 1000); + Rect bufferSize(500, 500); + Rect bufferCrop(100, 100, 350, 350); + + BLASTBufferQueueHelper adapter(mSurfaceControl, windowSize.getWidth(), windowSize.getHeight()); + sp<IGraphicBufferProducer> igbProducer; + setUpProducer(adapter, igbProducer); + int slot; + sp<Fence> fence; + sp<GraphicBuffer> buf; + auto ret = igbProducer->dequeueBuffer(&slot, &fence, bufferSize.getWidth(), + bufferSize.getHeight(), PIXEL_FORMAT_RGBA_8888, + GRALLOC_USAGE_SW_WRITE_OFTEN, nullptr, nullptr); + ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, ret); + ASSERT_EQ(OK, igbProducer->requestBuffer(slot, &buf)); + + uint32_t* bufData; + buf->lock(static_cast<uint32_t>(GraphicBuffer::USAGE_SW_WRITE_OFTEN), + reinterpret_cast<void**>(&bufData)); + // fill buffer with grey + fillBuffer(bufData, bufferSize, buf->getStride(), 127, 127, 127); + + // fill crop area with different colors so we can verify the cropped region has been scaled + // correctly. + fillBuffer(bufData, Rect(100, 100, 225, 225), buf->getStride(), /* rgb */ 255, 0, 0); + fillBuffer(bufData, Rect(100, 226, 225, 350), buf->getStride(), /* rgb */ 0, 255, 0); + fillBuffer(bufData, Rect(226, 100, 350, 225), buf->getStride(), /* rgb */ 0, 0, 255); + fillBuffer(bufData, Rect(226, 226, 350, 350), buf->getStride(), /* rgb */ 255, 0, 0); + buf->unlock(); + + IGraphicBufferProducer::QueueBufferOutput qbOutput; + IGraphicBufferProducer::QueueBufferInput input(systemTime(), true /* autotimestamp */, + HAL_DATASPACE_UNKNOWN, + bufferCrop /* Rect::INVALID_RECT */, + NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW, 0, + Fence::NO_FENCE); + igbProducer->queueBuffer(slot, input, &qbOutput); + ASSERT_NE(ui::Transform::ROT_INVALID, qbOutput.transformHint); + + adapter.waitForCallbacks(); + + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + // Verify cropped region is scaled correctly. + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(255, 0, 0, {10, 10, 490, 490})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 255, 0, {10, 510, 490, 990})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 0, 255, {510, 10, 990, 490})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(255, 0, 0, {510, 510, 990, 990})); + // Verify outside region is black. + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 0, 0, + {0, 0, (int32_t)windowSize.getWidth(), + (int32_t)windowSize.getHeight()}, + /*border*/ 0, /*outsideRegion*/ true)); } class TestProducerListener : public BnProducerListener { diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 751b95af66..ea8c2957a1 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -102,14 +102,13 @@ protected: // test flakiness. mSurfaceControl = mComposerClient->createSurface( String8("Test Surface"), 32, 32, PIXEL_FORMAT_RGBA_8888, 0); + SurfaceComposerClient::Transaction().apply(true); ASSERT_TRUE(mSurfaceControl != nullptr); ASSERT_TRUE(mSurfaceControl->isValid()); Transaction t; - ASSERT_EQ(NO_ERROR, t.setLayer(mSurfaceControl, 0x7fffffff) - .show(mSurfaceControl) - .apply()); + ASSERT_EQ(NO_ERROR, t.setLayer(mSurfaceControl, 0x7fffffff).show(mSurfaceControl).apply()); mSurface = mSurfaceControl->getSurface(); ASSERT_TRUE(mSurface != nullptr); @@ -776,6 +775,10 @@ public: const std::vector<ui::Hdr>& /*hdrTypes*/) override { return NO_ERROR; } + status_t onPullAtom(const int32_t /*atomId*/, std::string* /*outData*/, + bool* /*success*/) override { + return NO_ERROR; + } status_t enableVSyncInjections(bool /*enable*/) override { return NO_ERROR; } 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/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h index c8a0f0a034..ddaa7c7783 100644 --- a/libs/renderengine/include/renderengine/RenderEngine.h +++ b/libs/renderengine/include/renderengine/RenderEngine.h @@ -308,7 +308,8 @@ private: bool precacheToneMapperShaderOnly = false; bool supportsBackgroundBlur = false; RenderEngine::ContextPriority contextPriority = RenderEngine::ContextPriority::MEDIUM; - RenderEngine::RenderEngineType renderEngineType = RenderEngine::RenderEngineType::GLES; + RenderEngine::RenderEngineType renderEngineType = + RenderEngine::RenderEngineType::SKIA_GL_THREADED; }; } // namespace renderengine diff --git a/libs/renderengine/skia/AutoBackendTexture.cpp b/libs/renderengine/skia/AutoBackendTexture.cpp index c535597aea..9ed759f6bf 100644 --- a/libs/renderengine/skia/AutoBackendTexture.cpp +++ b/libs/renderengine/skia/AutoBackendTexture.cpp @@ -29,7 +29,8 @@ namespace renderengine { namespace skia { AutoBackendTexture::AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer, - bool isRender) { + bool isOutputBuffer) + : mIsOutputBuffer(isOutputBuffer) { ATRACE_CALL(); AHardwareBuffer_Desc desc; AHardwareBuffer_describe(buffer, &desc); @@ -40,8 +41,12 @@ AutoBackendTexture::AutoBackendTexture(GrDirectContext* context, AHardwareBuffer GrAHardwareBufferUtils::MakeBackendTexture(context, buffer, desc.width, desc.height, &mDeleteProc, &mUpdateProc, &mImageCtx, createProtectedImage, backendFormat, - isRender); + isOutputBuffer); mColorType = GrAHardwareBufferUtils::GetSkColorTypeFromBufferFormat(desc.format); + ALOGE_IF(!mBackendTexture.isValid(), + "Failed to create a valid texture. [%p]:[%d,%d] isProtected:%d isWriteable:%d " + "format:%d", + this, desc.width, desc.height, isOutputBuffer, createProtectedImage, desc.format); } void AutoBackendTexture::unref(bool releaseLocalResources) { @@ -92,13 +97,16 @@ sk_sp<SkImage> AutoBackendTexture::makeImage(ui::Dataspace dataspace, SkAlphaTyp mImage = image; mDataspace = dataspace; - LOG_ALWAYS_FATAL_IF(mImage == nullptr, "Unable to generate SkImage from buffer"); + LOG_ALWAYS_FATAL_IF(mImage == nullptr, + "Unable to generate SkImage. isTextureValid:%d dataspace:%d", + mBackendTexture.isValid(), dataspace); return mImage; } sk_sp<SkSurface> AutoBackendTexture::getOrCreateSurface(ui::Dataspace dataspace, GrDirectContext* context) { ATRACE_CALL(); + LOG_ALWAYS_FATAL_IF(!mIsOutputBuffer, "You can't generate a SkSurface for a read-only texture"); if (!mSurface.get() || mDataspace != dataspace) { sk_sp<SkSurface> surface = SkSurface::MakeFromBackendTexture(context, mBackendTexture, @@ -113,7 +121,9 @@ sk_sp<SkSurface> AutoBackendTexture::getOrCreateSurface(ui::Dataspace dataspace, } mDataspace = dataspace; - LOG_ALWAYS_FATAL_IF(mSurface == nullptr, "Unable to generate SkSurface"); + LOG_ALWAYS_FATAL_IF(mSurface == nullptr, + "Unable to generate SkSurface. isTextureValid:%d dataspace:%d", + mBackendTexture.isValid(), dataspace); return mSurface; } diff --git a/libs/renderengine/skia/AutoBackendTexture.h b/libs/renderengine/skia/AutoBackendTexture.h index 2d61cf854b..3133de60b5 100644 --- a/libs/renderengine/skia/AutoBackendTexture.h +++ b/libs/renderengine/skia/AutoBackendTexture.h @@ -41,34 +41,42 @@ public: // of shared ownership with Skia objects, so we wrap it here instead. class LocalRef { public: - LocalRef(AutoBackendTexture* texture) { setTexture(texture); } + LocalRef(GrDirectContext* context, AHardwareBuffer* buffer, bool isOutputBuffer) { + mTexture = new AutoBackendTexture(context, buffer, isOutputBuffer); + mTexture->ref(); + } ~LocalRef() { - // Destroying the texture is the same as setting it to null - setTexture(nullptr); + if (mTexture != nullptr) { + mTexture->unref(true); + } } - AutoBackendTexture* getTexture() const { return mTexture; } + // Makes a new SkImage from the texture content. + // As SkImages are immutable but buffer content is not, we create + // a new SkImage every time. + sk_sp<SkImage> makeImage(ui::Dataspace dataspace, SkAlphaType alphaType, + GrDirectContext* context) { + return mTexture->makeImage(dataspace, alphaType, context); + } + + // Makes a new SkSurface from the texture content, if needed. + sk_sp<SkSurface> getOrCreateSurface(ui::Dataspace dataspace, GrDirectContext* context) { + return mTexture->getOrCreateSurface(dataspace, context); + } DISALLOW_COPY_AND_ASSIGN(LocalRef); private: - // Sets the texture to locally ref-track. - void setTexture(AutoBackendTexture* texture) { - if (mTexture != nullptr) { - mTexture->unref(true); - } - - mTexture = texture; - if (mTexture != nullptr) { - mTexture->ref(); - } - } AutoBackendTexture* mTexture = nullptr; }; +private: // Creates a GrBackendTexture whose contents come from the provided buffer. - AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer, bool isRender); + AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer, bool isOutputBuffer); + + // The only way to invoke dtor is with unref, when mUsageCount is 0. + ~AutoBackendTexture() {} void ref() { mUsageCount++; } @@ -85,10 +93,6 @@ public: // Makes a new SkSurface from the texture content, if needed. sk_sp<SkSurface> getOrCreateSurface(ui::Dataspace dataspace, GrDirectContext* context); -private: - // The only way to invoke dtor is with unref, when mUsageCount is 0. - ~AutoBackendTexture() {} - GrBackendTexture mBackendTexture; GrAHardwareBufferUtils::DeleteImageProc mDeleteProc; GrAHardwareBufferUtils::UpdateImageProc mUpdateProc; @@ -99,6 +103,7 @@ private: int mUsageCount = 0; + const bool mIsOutputBuffer; sk_sp<SkImage> mImage = nullptr; sk_sp<SkSurface> mSurface = nullptr; ui::Dataspace mDataspace = ui::Dataspace::UNKNOWN; diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp index 37d98a3a8f..0a84754d22 100644 --- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp @@ -508,9 +508,9 @@ void SkiaGLRenderEngine::mapExternalTextureBuffer(const sp<GraphicBuffer>& buffe if (const auto& iter = cache.find(buffer->getId()); iter == cache.end()) { std::shared_ptr<AutoBackendTexture::LocalRef> imageTextureRef = - std::make_shared<AutoBackendTexture::LocalRef>( - new AutoBackendTexture(grContext.get(), buffer->toAHardwareBuffer(), - isRenderable)); + std::make_shared<AutoBackendTexture::LocalRef>(grContext.get(), + buffer->toAHardwareBuffer(), + isRenderable); cache.insert({buffer->getId(), imageTextureRef}); } // restore the original state of the protected context if necessary @@ -669,15 +669,17 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, if (const auto& it = cache.find(buffer->getBuffer()->getId()); it != cache.end()) { surfaceTextureRef = it->second; } else { - surfaceTextureRef = std::make_shared<AutoBackendTexture::LocalRef>( - new AutoBackendTexture(grContext.get(), buffer->getBuffer()->toAHardwareBuffer(), - true)); + surfaceTextureRef = + std::make_shared<AutoBackendTexture::LocalRef>(grContext.get(), + buffer->getBuffer() + ->toAHardwareBuffer(), + true); } const ui::Dataspace dstDataspace = mUseColorManagement ? display.outputDataspace : ui::Dataspace::UNKNOWN; sk_sp<SkSurface> dstSurface = - surfaceTextureRef->getTexture()->getOrCreateSurface(dstDataspace, grContext.get()); + surfaceTextureRef->getOrCreateSurface(dstDataspace, grContext.get()); SkCanvas* dstCanvas = mCapture->tryCapture(dstSurface.get()); if (dstCanvas == nullptr) { @@ -889,18 +891,17 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, // it. If we're using skia, we're guaranteed to run on a dedicated GPU thread so if // we didn't find anything in the cache then we intentionally did not cache this // buffer's resources. - imageTextureRef = std::make_shared<AutoBackendTexture::LocalRef>( - new AutoBackendTexture(grContext.get(), - item.buffer->getBuffer()->toAHardwareBuffer(), - false)); + imageTextureRef = std::make_shared< + AutoBackendTexture::LocalRef>(grContext.get(), + item.buffer->getBuffer()->toAHardwareBuffer(), + false); } sk_sp<SkImage> image = - imageTextureRef->getTexture()->makeImage(layerDataspace, - item.usePremultipliedAlpha - ? kPremul_SkAlphaType - : kUnpremul_SkAlphaType, - grContext.get()); + imageTextureRef->makeImage(layerDataspace, + item.usePremultipliedAlpha ? kPremul_SkAlphaType + : kUnpremul_SkAlphaType, + grContext.get()); auto texMatrix = getSkM44(item.textureTransform).asM33(); // textureTansform was intended to be passed directly into a shader, so when 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/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index 627ff53abf..7a5b20d78e 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -795,7 +795,7 @@ void BufferStateLayer::gatherBufferInfo() { mBufferInfo.mFence = s.acquireFence; mBufferInfo.mTransform = s.bufferTransform; mBufferInfo.mDataspace = translateDataspace(s.dataspace); - mBufferInfo.mCrop = computeCrop(s); + mBufferInfo.mCrop = computeBufferCrop(s); mBufferInfo.mScaleMode = NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW; mBufferInfo.mSurfaceDamage = s.surfaceDamageRegion; mBufferInfo.mHdrMetadata = s.hdrMetadata; @@ -808,27 +808,11 @@ uint32_t BufferStateLayer::getEffectiveScalingMode() const { return NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW; } -Rect BufferStateLayer::computeCrop(const State& s) { - if (s.crop.isEmpty() && s.buffer) { +Rect BufferStateLayer::computeBufferCrop(const State& s) { + if (s.buffer) { return s.buffer->getBuffer()->getBounds(); - } else if (s.buffer) { - Rect crop = s.crop; - crop.left = std::max(crop.left, 0); - crop.top = std::max(crop.top, 0); - uint32_t bufferWidth = s.buffer->getBuffer()->getWidth(); - uint32_t bufferHeight = s.buffer->getBuffer()->getHeight(); - if (bufferHeight <= std::numeric_limits<int32_t>::max() && - bufferWidth <= std::numeric_limits<int32_t>::max()) { - crop.right = std::min(crop.right, static_cast<int32_t>(bufferWidth)); - crop.bottom = std::min(crop.bottom, static_cast<int32_t>(bufferHeight)); - } - if (!crop.isValid()) { - // Crop rect is out of bounds, return whole buffer - return s.buffer->getBuffer()->getBounds(); - } - return crop; } - return s.crop; + return Rect::INVALID_RECT; } sp<Layer> BufferStateLayer::createClone() { diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h index 4171092b58..af4fcae7ba 100644 --- a/services/surfaceflinger/BufferStateLayer.h +++ b/services/surfaceflinger/BufferStateLayer.h @@ -140,7 +140,7 @@ private: sp<Layer> createClone() override; // Crop that applies to the buffer - Rect computeCrop(const State& s); + Rect computeBufferCrop(const State& s); bool willPresentCurrentTransaction() const; 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/OneShotTimer.cpp b/services/surfaceflinger/Scheduler/OneShotTimer.cpp index ce3b0c6cfa..d659398394 100644 --- a/services/surfaceflinger/Scheduler/OneShotTimer.cpp +++ b/services/surfaceflinger/Scheduler/OneShotTimer.cpp @@ -40,13 +40,21 @@ void calculateTimeoutTime(std::chrono::nanoseconds timestamp, timespec* spec) { namespace android { namespace scheduler { +std::chrono::steady_clock::time_point OneShotTimer::Clock::now() const { + return std::chrono::steady_clock::now(); +} + OneShotTimer::OneShotTimer(std::string name, const Interval& interval, const ResetCallback& resetCallback, - const TimeoutCallback& timeoutCallback) - : mName(std::move(name)), + const TimeoutCallback& timeoutCallback, + std::unique_ptr<OneShotTimer::Clock> clock) + : mClock(std::move(clock)), + mName(std::move(name)), mInterval(interval), mResetCallback(resetCallback), - mTimeoutCallback(timeoutCallback) {} + mTimeoutCallback(timeoutCallback) { + LOG_ALWAYS_FATAL_IF(!mClock, "Clock must not be provided"); +} OneShotTimer::~OneShotTimer() { stop(); @@ -112,7 +120,7 @@ void OneShotTimer::loop() { break; } - auto triggerTime = std::chrono::steady_clock::now() + mInterval; + auto triggerTime = mClock->now() + mInterval; state = TimerState::WAITING; while (state == TimerState::WAITING) { constexpr auto zero = std::chrono::steady_clock::duration::zero(); @@ -128,10 +136,9 @@ void OneShotTimer::loop() { state = checkForResetAndStop(state); if (state == TimerState::RESET) { - triggerTime = std::chrono::steady_clock::now() + mInterval; + triggerTime = mClock->now() + mInterval; state = TimerState::WAITING; - } else if (state == TimerState::WAITING && - (triggerTime - std::chrono::steady_clock::now()) <= zero) { + } else if (state == TimerState::WAITING && (triggerTime - mClock->now()) <= zero) { triggerTimeout = true; state = TimerState::IDLE; } diff --git a/services/surfaceflinger/Scheduler/OneShotTimer.h b/services/surfaceflinger/Scheduler/OneShotTimer.h index 3690ce7542..7285427abb 100644 --- a/services/surfaceflinger/Scheduler/OneShotTimer.h +++ b/services/surfaceflinger/Scheduler/OneShotTimer.h @@ -36,8 +36,17 @@ public: using ResetCallback = std::function<void()>; using TimeoutCallback = std::function<void()>; + class Clock { + public: + Clock() = default; + virtual ~Clock() = default; + + virtual std::chrono::steady_clock::time_point now() const; + }; + OneShotTimer(std::string name, const Interval& interval, const ResetCallback& resetCallback, - const TimeoutCallback& timeoutCallback); + const TimeoutCallback& timeoutCallback, + std::unique_ptr<OneShotTimer::Clock> = std::make_unique<OneShotTimer::Clock>()); ~OneShotTimer(); // Initializes and turns on the idle timer. @@ -78,6 +87,9 @@ private: // Thread waiting for timer to expire. std::thread mThread; + // Clock object for the timer. Mocked in unit tests. + std::unique_ptr<Clock> mClock; + // Semaphore to keep mThread synchronized. sem_t mSemaphore; 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 8e2d5e5253..2da8ed4ab0 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -64,7 +64,6 @@ #include <private/android_filesystem_config.h> #include <private/gui/SyncFeatures.h> #include <renderengine/RenderEngine.h> -#include <statslog.h> #include <sys/types.h> #include <ui/ColorSpace.h> #include <ui/DebugUtils.h> @@ -627,7 +626,6 @@ void SurfaceFlinger::bootFinished() { ALOGI("Boot is finished (%ld ms)", long(ns2ms(duration)) ); mFrameTracer->initialize(); - mTimeStats->onBootFinished(); mFrameTimeline->onBootFinished(); // wait patiently for the window manager death @@ -1299,6 +1297,11 @@ status_t SurfaceFlinger::overrideHdrTypes(const sp<IBinder>& displayToken, return NO_ERROR; } +status_t SurfaceFlinger::onPullAtom(const int32_t atomId, std::string* pulledData, bool* success) { + *success = mTimeStats->onPullAtom(atomId, pulledData); + return NO_ERROR; +} + status_t SurfaceFlinger::getDisplayedContentSamplingAttributes(const sp<IBinder>& displayToken, ui::PixelFormat* outFormat, ui::Dataspace* outDataspace, @@ -1969,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 @@ -2754,6 +2759,7 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken, (currentState.orientedDisplaySpaceRect != drawingState.orientedDisplaySpaceRect)) { display->setProjection(currentState.orientation, currentState.layerStackSpaceRect, currentState.orientedDisplaySpaceRect); + mDefaultDisplayTransformHint = display->getTransformHint(); } if (currentState.width != drawingState.width || currentState.height != drawingState.height) { @@ -3260,70 +3266,38 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBind const sp<IBinder>& parentHandle, const sp<Layer>& parentLayer, bool addToCurrentState, uint32_t* outTransformHint) { - // add this layer to the current state list - { - Mutex::Autolock _l(mStateLock); - sp<Layer> parent; - if (parentHandle != nullptr) { - parent = fromHandleLocked(parentHandle).promote(); - if (parent == nullptr) { - return NAME_NOT_FOUND; - } - } else { - parent = parentLayer; - } - - if (mNumLayers >= ISurfaceComposer::MAX_LAYERS) { - ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(), - ISurfaceComposer::MAX_LAYERS); - return NO_MEMORY; - } + if (mNumLayers >= ISurfaceComposer::MAX_LAYERS) { + ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(), + ISurfaceComposer::MAX_LAYERS); + return NO_MEMORY; + } - mLayersByLocalBinderToken.emplace(handle->localBinder(), lbc); + wp<IBinder> initialProducer; + if (gbc != nullptr) { + initialProducer = IInterface::asBinder(gbc); + } + setLayerCreatedState(handle, lbc, parentHandle, parentLayer, initialProducer); - if (parent == nullptr && addToCurrentState) { - mCurrentState.layersSortedByZ.add(lbc); - } else if (parent == nullptr) { - lbc->onRemovedFromCurrentState(); - } else if (parent->isRemovedFromCurrentState()) { - parent->addChild(lbc); - lbc->onRemovedFromCurrentState(); - } else { - parent->addChild(lbc); - } - - if (gbc != nullptr) { - mGraphicBufferProducerList.insert(IInterface::asBinder(gbc).get()); - LOG_ALWAYS_FATAL_IF(mGraphicBufferProducerList.size() > - mMaxGraphicBufferProducerListSize, - "Suspected IGBP leak: %zu IGBPs (%zu max), %zu Layers", - mGraphicBufferProducerList.size(), - mMaxGraphicBufferProducerListSize, mNumLayers.load()); - if (mGraphicBufferProducerList.size() > mGraphicBufferProducerListSizeLogThreshold) { - ALOGW("Suspected IGBP leak: %zu IGBPs (%zu max), %zu Layers", - mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize, - mNumLayers.load()); - } - } + // Create a transaction includes the initial parent and producer. + Vector<ComposerState> states; + Vector<DisplayState> displays; - if (const auto token = getInternalDisplayTokenLocked()) { - const ssize_t index = mCurrentState.displays.indexOfKey(token); - if (index >= 0) { - const DisplayDeviceState& state = mCurrentState.displays.valueAt(index); - lbc->updateTransformHint(ui::Transform::toRotationFlags(state.orientation)); - } - } - if (outTransformHint) { - *outTransformHint = lbc->getTransformHint(); - } + ComposerState composerState; + composerState.state.what = layer_state_t::eLayerCreated; + composerState.state.surface = handle; + states.add(composerState); - mLayersAdded = true; + lbc->updateTransformHint(mDefaultDisplayTransformHint); + if (outTransformHint) { + *outTransformHint = mDefaultDisplayTransformHint; } - // attach this layer to the client client->attachLayer(handle, lbc); - return NO_ERROR; + return setTransactionState(FrameTimelineInfo{}, states, displays, 0 /* flags */, nullptr, + InputWindowCommands{}, -1 /* desiredPresentTime */, + true /* isAutoTimestamp */, {}, false /* hasListenerCallbacks */, {}, + 0 /* Undefined transactionId */); } void SurfaceFlinger::removeGraphicBufferProducerAsync(const wp<IBinder>& binder) { @@ -3818,9 +3792,21 @@ uint32_t SurfaceFlinger::setClientStateLocked( } } + const uint64_t what = s.what; + uint32_t flags = 0; sp<Layer> layer = nullptr; if (s.surface) { - layer = fromHandleLocked(s.surface).promote(); + if (what & layer_state_t::eLayerCreated) { + layer = handleLayerCreatedLocked(s.surface, privileged); + if (layer) { + // put the created layer into mLayersByLocalBinderToken. + mLayersByLocalBinderToken.emplace(s.surface->localBinder(), layer); + flags |= eTransactionNeeded | eTraversalNeeded; + mLayersAdded = true; + } + } else { + layer = fromHandleLocked(s.surface).promote(); + } } else { // The client may provide us a null handle. Treat it as if the layer was removed. ALOGW("Attempt to set client state with a null layer handle"); @@ -3833,10 +3819,6 @@ uint32_t SurfaceFlinger::setClientStateLocked( return 0; } - uint32_t flags = 0; - - const uint64_t what = s.what; - // Only set by BLAST adapter layers if (what & layer_state_t::eProducerDisconnect) { layer->onDisconnect(); @@ -4290,14 +4272,7 @@ status_t SurfaceFlinger::createBufferStateLayer(const sp<Client>& client, std::s sp<Layer>* outLayer) { LayerCreationArgs args(this, client, std::move(name), w, h, flags, std::move(metadata)); args.textureName = getNewTexture(); - sp<BufferStateLayer> layer; - { - // TODO (b/173538294): Investigate why we need mStateLock here and above in - // createBufferQueue layer. Is it the renderengine::Image? - Mutex::Autolock lock(mStateLock); - layer = getFactory().createBufferStateLayer(args); - - } + sp<BufferStateLayer> layer = getFactory().createBufferStateLayer(args); *handle = layer->getHandle(); *outLayer = layer; @@ -4385,7 +4360,7 @@ void SurfaceFlinger::onInitializeDisplays() { setPowerModeInternal(display, hal::PowerMode::ON); const nsecs_t vsyncPeriod = mRefreshRateConfigs->getCurrentRefreshRate().getVsyncPeriod(); mAnimFrameTracker.setDisplayRefreshPeriod(vsyncPeriod); - + mDefaultDisplayTransformHint = display->getTransformHint(); // Use phase of 0 since phase is not known. // Use latency of 0, which will snap to the ideal latency. DisplayStatInfo stats{0 /* vsyncTime */, vsyncPeriod}; @@ -5184,6 +5159,13 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) { } return PERMISSION_DENIED; } + case ON_PULL_ATOM: { + const int uid = IPCThreadState::self()->getCallingUid(); + if (uid == AID_SYSTEM) { + return OK; + } + return PERMISSION_DENIED; + } } // These codes are used for the IBinder protocol to either interrogate the recipient @@ -6641,6 +6623,87 @@ void SurfaceFlinger::TransactionState::traverseStatesWithBuffers( } } +void SurfaceFlinger::setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer, + const wp<IBinder>& parent, const wp<Layer> parentLayer, + const wp<IBinder>& producer) { + Mutex::Autolock lock(mCreatedLayersLock); + mCreatedLayers[handle->localBinder()] = + std::make_unique<LayerCreatedState>(layer, parent, parentLayer, producer); +} + +auto SurfaceFlinger::getLayerCreatedState(const sp<IBinder>& handle) { + Mutex::Autolock lock(mCreatedLayersLock); + BBinder* b = nullptr; + if (handle) { + b = handle->localBinder(); + } + + if (b == nullptr) { + return std::unique_ptr<LayerCreatedState>(nullptr); + } + + auto it = mCreatedLayers.find(b); + if (it == mCreatedLayers.end()) { + ALOGE("Can't find layer from handle %p", handle.get()); + return std::unique_ptr<LayerCreatedState>(nullptr); + } + + auto state = std::move(it->second); + mCreatedLayers.erase(it); + return state; +} + +sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle, bool privileged) { + const auto& state = getLayerCreatedState(handle); + if (!state) { + return nullptr; + } + + sp<Layer> layer = state->layer.promote(); + if (!layer) { + ALOGE("Invalid layer %p", state->layer.unsafe_get()); + return nullptr; + } + + sp<Layer> parent; + bool allowAddRoot = privileged; + if (state->initialParent != nullptr) { + parent = fromHandleLocked(state->initialParent.promote()).promote(); + if (parent == nullptr) { + ALOGE("Invalid parent %p", state->initialParent.unsafe_get()); + allowAddRoot = false; + } + } else if (state->initialParentLayer != nullptr) { + parent = state->initialParentLayer.promote(); + allowAddRoot = false; + } + + if (parent == nullptr && allowAddRoot) { + mCurrentState.layersSortedByZ.add(layer); + } else if (parent == nullptr) { + layer->onRemovedFromCurrentState(); + } else if (parent->isRemovedFromCurrentState()) { + parent->addChild(layer); + layer->onRemovedFromCurrentState(); + } else { + parent->addChild(layer); + } + + if (state->initialProducer != nullptr) { + mGraphicBufferProducerList.insert(state->initialProducer); + LOG_ALWAYS_FATAL_IF(mGraphicBufferProducerList.size() > mMaxGraphicBufferProducerListSize, + "Suspected IGBP leak: %zu IGBPs (%zu max), %zu Layers", + mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize, + mNumLayers.load()); + if (mGraphicBufferProducerList.size() > mGraphicBufferProducerListSizeLogThreshold) { + ALOGW("Suspected IGBP leak: %zu IGBPs (%zu max), %zu Layers", + mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize, + mNumLayers.load()); + } + } + + return layer; +} } // namespace android #if defined(__gl_h_) diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 893b3d8751..cf1a545342 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -637,6 +637,7 @@ private: status_t getAnimationFrameStats(FrameStats* outStats) const override; status_t overrideHdrTypes(const sp<IBinder>& displayToken, const std::vector<ui::Hdr>& hdrTypes) override; + status_t onPullAtom(const int32_t atomId, std::string* pulledData, bool* success) override; status_t enableVSyncInjections(bool enable) override; status_t injectVSync(nsecs_t when) override; status_t getLayerDebugInfo(std::vector<LayerDebugInfo>* outLayers) override; @@ -1391,6 +1392,35 @@ private: std::unordered_map<DisplayId, sp<HdrLayerInfoReporter>> mHdrLayerInfoListeners GUARDED_BY(mStateLock); + mutable Mutex mCreatedLayersLock; + struct LayerCreatedState { + LayerCreatedState(const wp<Layer>& layer, const wp<IBinder>& parent, + const wp<Layer> parentLayer, const wp<IBinder>& producer) + : layer(layer), + initialParent(parent), + initialParentLayer(parentLayer), + initialProducer(producer) {} + wp<Layer> layer; + // Indicates the initial parent of the created layer, only used for creating layer in + // SurfaceFlinger. If nullptr, it may add the created layer into the current root layers. + wp<IBinder> initialParent; + wp<Layer> initialParentLayer; + // Indicates the initial graphic buffer producer of the created layer, only used for + // creating layer in SurfaceFlinger. + wp<IBinder> initialProducer; + }; + + // A temporay pool that store the created layers and will be added to current state in main + // thread. + std::unordered_map<BBinder*, std::unique_ptr<LayerCreatedState>> mCreatedLayers; + void setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer, + const wp<IBinder>& parent, const wp<Layer> parentLayer, + const wp<IBinder>& producer); + auto getLayerCreatedState(const sp<IBinder>& handle); + sp<Layer> handleLayerCreatedLocked(const sp<IBinder>& handle, bool privileged) + REQUIRES(mStateLock); + + std::atomic<ui::Transform::RotationFlags> mDefaultDisplayTransformHint; }; } // namespace android diff --git a/services/surfaceflinger/TimeStats/Android.bp b/services/surfaceflinger/TimeStats/Android.bp index 62fddb4f47..bcc3e4e52a 100644 --- a/services/surfaceflinger/TimeStats/Android.bp +++ b/services/surfaceflinger/TimeStats/Android.bp @@ -18,20 +18,13 @@ cc_library { "libcutils", "liblog", "libprotobuf-cpp-lite", - "libprotoutil", - "libstatslog", - "libstatspull", - "libstatssocket", + "libtimestats_atoms_proto", "libtimestats_proto", "libui", "libutils", ], export_include_dirs: ["."], export_shared_lib_headers: [ - "libprotoutil", - "libstatslog", - "libstatspull", - "libstatssocket", "libtimestats_proto", ], cppflags: [ diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp index 2094972ed0..16c6bb33c8 100644 --- a/services/surfaceflinger/TimeStats/TimeStats.cpp +++ b/services/surfaceflinger/TimeStats/TimeStats.cpp @@ -19,11 +19,9 @@ #define LOG_TAG "TimeStats" #define ATRACE_TAG ATRACE_TAG_GRAPHICS -#include "TimeStats.h" - #include <android-base/stringprintf.h> -#include <android/util/ProtoOutputStream.h> #include <log/log.h> +#include <timestatsatomsproto/TimeStatsAtomsProtoHeader.h> #include <utils/String8.h> #include <utils/Timers.h> #include <utils/Trace.h> @@ -31,147 +29,102 @@ #include <algorithm> #include <chrono> +#include "TimeStats.h" #include "timestatsproto/TimeStatsHelper.h" namespace android { namespace impl { -AStatsManager_PullAtomCallbackReturn TimeStats::pullAtomCallback(int32_t atom_tag, - AStatsEventList* data, - void* cookie) { - impl::TimeStats* timeStats = reinterpret_cast<impl::TimeStats*>(cookie); - AStatsManager_PullAtomCallbackReturn result = AStatsManager_PULL_SKIP; - if (atom_tag == android::util::SURFACEFLINGER_STATS_GLOBAL_INFO) { - result = timeStats->populateGlobalAtom(data); - } else if (atom_tag == android::util::SURFACEFLINGER_STATS_LAYER_INFO) { - result = timeStats->populateLayerAtom(data); - } - - // Enable timestats now. The first full pull for a given build is expected to - // have empty or very little stats, as stats are first enabled after the - // first pull is completed for either the global or layer stats. - timeStats->enable(); - return result; -} - namespace { -// Histograms align with the order of fields in SurfaceflingerStatsLayerInfo. -const std::array<std::string, 6> kHistogramNames = { - "present2present", "post2present", "acquire2present", - "latch2present", "desired2present", "post2acquire", -}; - -std::string histogramToProtoByteString(const std::unordered_map<int32_t, int32_t>& histogram, - size_t maxPulledHistogramBuckets) { + +FrameTimingHistogram histogramToProto(const std::unordered_map<int32_t, int32_t>& histogram, + size_t maxPulledHistogramBuckets) { auto buckets = std::vector<std::pair<int32_t, int32_t>>(histogram.begin(), histogram.end()); std::sort(buckets.begin(), buckets.end(), [](std::pair<int32_t, int32_t>& left, std::pair<int32_t, int32_t>& right) { return left.second > right.second; }); - util::ProtoOutputStream proto; + FrameTimingHistogram histogramProto; int histogramSize = 0; for (const auto& bucket : buckets) { if (++histogramSize > maxPulledHistogramBuckets) { break; } - proto.write(android::util::FIELD_TYPE_INT32 | android::util::FIELD_COUNT_REPEATED | - 1 /* field id */, - (int32_t)bucket.first); - proto.write(android::util::FIELD_TYPE_INT64 | android::util::FIELD_COUNT_REPEATED | - 2 /* field id */, - (int64_t)bucket.second); + histogramProto.add_time_millis_buckets((int32_t)bucket.first); + histogramProto.add_frame_counts((int64_t)bucket.second); } - - std::string byteString; - proto.serializeToString(&byteString); - return byteString; + return histogramProto; } -std::string frameRateVoteToProtoByteString( - float refreshRate, - TimeStats::SetFrameRateVote::FrameRateCompatibility frameRateCompatibility, - TimeStats::SetFrameRateVote::Seamlessness seamlessness) { - util::ProtoOutputStream proto; - proto.write(android::util::FIELD_TYPE_FLOAT | 1 /* field id */, refreshRate); - proto.write(android::util::FIELD_TYPE_ENUM | 2 /* field id */, - static_cast<int>(frameRateCompatibility)); - proto.write(android::util::FIELD_TYPE_ENUM | 3 /* field id */, static_cast<int>(seamlessness)); - - std::string byteString; - proto.serializeToString(&byteString); - return byteString; +SurfaceflingerStatsLayerInfo_SetFrameRateVote frameRateVoteToProto( + const TimeStats::SetFrameRateVote& setFrameRateVote) { + using FrameRateCompatibilityEnum = + SurfaceflingerStatsLayerInfo::SetFrameRateVote::FrameRateCompatibility; + using SeamlessnessEnum = SurfaceflingerStatsLayerInfo::SetFrameRateVote::Seamlessness; + + SurfaceflingerStatsLayerInfo_SetFrameRateVote proto; + proto.set_frame_rate(setFrameRateVote.frameRate); + proto.set_frame_rate_compatibility( + static_cast<FrameRateCompatibilityEnum>(setFrameRateVote.frameRateCompatibility)); + proto.set_seamlessness(static_cast<SeamlessnessEnum>(setFrameRateVote.seamlessness)); + return proto; } } // namespace -AStatsManager_PullAtomCallbackReturn TimeStats::populateGlobalAtom(AStatsEventList* data) { +bool TimeStats::populateGlobalAtom(std::string* pulledData) { std::lock_guard<std::mutex> lock(mMutex); if (mTimeStats.statsStartLegacy == 0) { - return AStatsManager_PULL_SKIP; + return false; } flushPowerTimeLocked(); - + SurfaceflingerStatsGlobalInfoWrapper atomList; for (const auto& globalSlice : mTimeStats.stats) { - AStatsEvent* event = mStatsDelegate->addStatsEventToPullData(data); - mStatsDelegate->statsEventSetAtomId(event, android::util::SURFACEFLINGER_STATS_GLOBAL_INFO); - mStatsDelegate->statsEventWriteInt64(event, mTimeStats.totalFramesLegacy); - mStatsDelegate->statsEventWriteInt64(event, mTimeStats.missedFramesLegacy); - mStatsDelegate->statsEventWriteInt64(event, mTimeStats.clientCompositionFramesLegacy); - mStatsDelegate->statsEventWriteInt64(event, mTimeStats.displayOnTimeLegacy); - mStatsDelegate->statsEventWriteInt64(event, mTimeStats.presentToPresentLegacy.totalTime()); - mStatsDelegate->statsEventWriteInt32(event, mTimeStats.displayEventConnectionsCountLegacy); - std::string frameDurationBytes = - histogramToProtoByteString(mTimeStats.frameDurationLegacy.hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, (const uint8_t*)frameDurationBytes.c_str(), - frameDurationBytes.size()); - std::string renderEngineTimingBytes = - histogramToProtoByteString(mTimeStats.renderEngineTimingLegacy.hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, - (const uint8_t*)renderEngineTimingBytes.c_str(), - renderEngineTimingBytes.size()); - - mStatsDelegate->statsEventWriteInt32(event, globalSlice.second.jankPayload.totalFrames); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalJankyFrames); - mStatsDelegate->statsEventWriteInt32(event, globalSlice.second.jankPayload.totalSFLongCpu); - mStatsDelegate->statsEventWriteInt32(event, globalSlice.second.jankPayload.totalSFLongGpu); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalSFUnattributed); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalAppUnattributed); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalSFScheduling); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalSFPredictionError); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalAppBufferStuffing); - mStatsDelegate->statsEventWriteInt32(event, globalSlice.first.displayRefreshRateBucket); - std::string sfDeadlineMissedBytes = - histogramToProtoByteString(globalSlice.second.displayDeadlineDeltas.hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, - (const uint8_t*)sfDeadlineMissedBytes.c_str(), - sfDeadlineMissedBytes.size()); - std::string sfPredictionErrorBytes = - histogramToProtoByteString(globalSlice.second.displayPresentDeltas.hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, - (const uint8_t*)sfPredictionErrorBytes.c_str(), - sfPredictionErrorBytes.size()); - mStatsDelegate->statsEventWriteInt32(event, globalSlice.first.renderRateBucket); - mStatsDelegate->statsEventBuild(event); + SurfaceflingerStatsGlobalInfo* atom = atomList.add_atom(); + atom->set_total_frames(mTimeStats.totalFramesLegacy); + atom->set_missed_frames(mTimeStats.missedFramesLegacy); + atom->set_client_composition_frames(mTimeStats.clientCompositionFramesLegacy); + atom->set_display_on_millis(mTimeStats.displayOnTimeLegacy); + atom->set_animation_millis(mTimeStats.presentToPresentLegacy.totalTime()); + atom->set_event_connection_count(mTimeStats.displayEventConnectionsCountLegacy); + *atom->mutable_frame_duration() = + histogramToProto(mTimeStats.frameDurationLegacy.hist, mMaxPulledHistogramBuckets); + *atom->mutable_render_engine_timing() = + histogramToProto(mTimeStats.renderEngineTimingLegacy.hist, + mMaxPulledHistogramBuckets); + atom->set_total_timeline_frames(globalSlice.second.jankPayload.totalFrames); + atom->set_total_janky_frames(globalSlice.second.jankPayload.totalJankyFrames); + atom->set_total_janky_frames_with_long_cpu(globalSlice.second.jankPayload.totalSFLongCpu); + atom->set_total_janky_frames_with_long_gpu(globalSlice.second.jankPayload.totalSFLongGpu); + atom->set_total_janky_frames_sf_unattributed( + globalSlice.second.jankPayload.totalSFUnattributed); + atom->set_total_janky_frames_app_unattributed( + globalSlice.second.jankPayload.totalAppUnattributed); + atom->set_total_janky_frames_sf_scheduling( + globalSlice.second.jankPayload.totalSFScheduling); + atom->set_total_jank_frames_sf_prediction_error( + globalSlice.second.jankPayload.totalSFPredictionError); + atom->set_total_jank_frames_app_buffer_stuffing( + globalSlice.second.jankPayload.totalAppBufferStuffing); + atom->set_display_refresh_rate_bucket(globalSlice.first.displayRefreshRateBucket); + *atom->mutable_sf_deadline_misses() = + histogramToProto(globalSlice.second.displayDeadlineDeltas.hist, + mMaxPulledHistogramBuckets); + *atom->mutable_sf_prediction_errors() = + histogramToProto(globalSlice.second.displayPresentDeltas.hist, + mMaxPulledHistogramBuckets); + atom->set_render_rate_bucket(globalSlice.first.renderRateBucket); } + // Always clear data. clearGlobalLocked(); - return AStatsManager_PULL_SUCCESS; + return atomList.SerializeToString(pulledData); } -AStatsManager_PullAtomCallbackReturn TimeStats::populateLayerAtom(AStatsEventList* data) { +bool TimeStats::populateLayerAtom(std::string* pulledData) { std::lock_guard<std::mutex> lock(mMutex); std::vector<TimeStatsHelper::TimeStatsLayer*> dumpStats; @@ -198,69 +151,73 @@ AStatsManager_PullAtomCallbackReturn TimeStats::populateLayerAtom(AStatsEventLis dumpStats.resize(mMaxPulledLayers); } + SurfaceflingerStatsLayerInfoWrapper atomList; for (auto& layer : dumpStats) { - AStatsEvent* event = mStatsDelegate->addStatsEventToPullData(data); - mStatsDelegate->statsEventSetAtomId(event, android::util::SURFACEFLINGER_STATS_LAYER_INFO); - mStatsDelegate->statsEventWriteString8(event, layer->layerName.c_str()); - mStatsDelegate->statsEventWriteInt64(event, layer->totalFrames); - mStatsDelegate->statsEventWriteInt64(event, layer->droppedFrames); - - for (const auto& name : kHistogramNames) { - const auto& histogram = layer->deltas.find(name); - if (histogram == layer->deltas.cend()) { - mStatsDelegate->statsEventWriteByteArray(event, nullptr, 0); - } else { - std::string bytes = histogramToProtoByteString(histogram->second.hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, (const uint8_t*)bytes.c_str(), - bytes.size()); - } + SurfaceflingerStatsLayerInfo* atom = atomList.add_atom(); + atom->set_layer_name(layer->layerName); + atom->set_total_frames(layer->totalFrames); + atom->set_dropped_frames(layer->droppedFrames); + const auto& present2PresentHist = layer->deltas.find("present2present"); + if (present2PresentHist != layer->deltas.cend()) { + *atom->mutable_present_to_present() = + histogramToProto(present2PresentHist->second.hist, mMaxPulledHistogramBuckets); + } + const auto& post2presentHist = layer->deltas.find("post2present"); + if (post2presentHist != layer->deltas.cend()) { + *atom->mutable_post_to_present() = + histogramToProto(post2presentHist->second.hist, mMaxPulledHistogramBuckets); + } + const auto& acquire2presentHist = layer->deltas.find("acquire2present"); + if (acquire2presentHist != layer->deltas.cend()) { + *atom->mutable_acquire_to_present() = + histogramToProto(acquire2presentHist->second.hist, mMaxPulledHistogramBuckets); + } + const auto& latch2presentHist = layer->deltas.find("latch2present"); + if (latch2presentHist != layer->deltas.cend()) { + *atom->mutable_latch_to_present() = + histogramToProto(latch2presentHist->second.hist, mMaxPulledHistogramBuckets); + } + const auto& desired2presentHist = layer->deltas.find("desired2present"); + if (desired2presentHist != layer->deltas.cend()) { + *atom->mutable_desired_to_present() = + histogramToProto(desired2presentHist->second.hist, mMaxPulledHistogramBuckets); + } + const auto& post2acquireHist = layer->deltas.find("post2acquire"); + if (post2acquireHist != layer->deltas.cend()) { + *atom->mutable_post_to_acquire() = + histogramToProto(post2acquireHist->second.hist, mMaxPulledHistogramBuckets); } - mStatsDelegate->statsEventWriteInt64(event, layer->lateAcquireFrames); - mStatsDelegate->statsEventWriteInt64(event, layer->badDesiredPresentFrames); - mStatsDelegate->statsEventWriteInt32(event, layer->uid); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalFrames); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalJankyFrames); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalSFLongCpu); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalSFLongGpu); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalSFUnattributed); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalAppUnattributed); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalSFScheduling); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalSFPredictionError); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalAppBufferStuffing); - mStatsDelegate->statsEventWriteInt32( - event, layer->displayRefreshRateBucket); // display_refresh_rate_bucket - mStatsDelegate->statsEventWriteInt32(event, layer->renderRateBucket); // render_rate_bucket - std::string frameRateVoteBytes = - frameRateVoteToProtoByteString(layer->setFrameRateVote.frameRate, - layer->setFrameRateVote.frameRateCompatibility, - layer->setFrameRateVote.seamlessness); - mStatsDelegate->statsEventWriteByteArray(event, (const uint8_t*)frameRateVoteBytes.c_str(), - frameRateVoteBytes.size()); // set_frame_rate_vote - std::string appDeadlineMissedBytes = - histogramToProtoByteString(layer->deltas["appDeadlineDeltas"].hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, - (const uint8_t*)appDeadlineMissedBytes.c_str(), - appDeadlineMissedBytes.size()); - - mStatsDelegate->statsEventBuild(event); + atom->set_late_acquire_frames(layer->lateAcquireFrames); + atom->set_bad_desired_present_frames(layer->badDesiredPresentFrames); + atom->set_uid(layer->uid); + atom->set_total_timeline_frames(layer->jankPayload.totalFrames); + atom->set_total_janky_frames(layer->jankPayload.totalJankyFrames); + atom->set_total_janky_frames_with_long_cpu(layer->jankPayload.totalSFLongCpu); + atom->set_total_janky_frames_with_long_gpu(layer->jankPayload.totalSFLongGpu); + atom->set_total_janky_frames_sf_unattributed(layer->jankPayload.totalSFUnattributed); + atom->set_total_janky_frames_app_unattributed(layer->jankPayload.totalAppUnattributed); + atom->set_total_janky_frames_sf_scheduling(layer->jankPayload.totalSFScheduling); + atom->set_total_jank_frames_sf_prediction_error(layer->jankPayload.totalSFPredictionError); + atom->set_total_jank_frames_app_buffer_stuffing(layer->jankPayload.totalAppBufferStuffing); + atom->set_display_refresh_rate_bucket(layer->displayRefreshRateBucket); + atom->set_render_rate_bucket(layer->renderRateBucket); + *atom->mutable_set_frame_rate_vote() = frameRateVoteToProto(layer->setFrameRateVote); + *atom->mutable_app_deadline_misses() = + histogramToProto(layer->deltas["appDeadlineDeltas"].hist, + mMaxPulledHistogramBuckets); } + + // Always clear data. clearLayersLocked(); - return AStatsManager_PULL_SUCCESS; + return atomList.SerializeToString(pulledData); } -TimeStats::TimeStats() : TimeStats(nullptr, std::nullopt, std::nullopt) {} +TimeStats::TimeStats() : TimeStats(std::nullopt, std::nullopt) {} -TimeStats::TimeStats(std::unique_ptr<StatsEventDelegate> statsDelegate, - std::optional<size_t> maxPulledLayers, +TimeStats::TimeStats(std::optional<size_t> maxPulledLayers, std::optional<size_t> maxPulledHistogramBuckets) { - if (statsDelegate != nullptr) { - mStatsDelegate = std::move(statsDelegate); - } - if (maxPulledLayers) { mMaxPulledLayers = *maxPulledLayers; } @@ -270,18 +227,19 @@ TimeStats::TimeStats(std::unique_ptr<StatsEventDelegate> statsDelegate, } } -TimeStats::~TimeStats() { - std::lock_guard<std::mutex> lock(mMutex); - mStatsDelegate->clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO); - mStatsDelegate->clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO); -} +bool TimeStats::onPullAtom(const int atomId, std::string* pulledData) { + bool success = false; + if (atomId == 10062) { // SURFACEFLINGER_STATS_GLOBAL_INFO + success = populateGlobalAtom(pulledData); + } else if (atomId == 10063) { // SURFACEFLINGER_STATS_LAYER_INFO + success = populateLayerAtom(pulledData); + } -void TimeStats::onBootFinished() { - std::lock_guard<std::mutex> lock(mMutex); - mStatsDelegate->setStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - nullptr, TimeStats::pullAtomCallback, this); - mStatsDelegate->setStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - nullptr, TimeStats::pullAtomCallback, this); + // Enable timestats now. The first full pull for a given build is expected to + // have empty or very little stats, as stats are first enabled after the + // first pull is completed for either the global or layer stats. + enable(); + return success; } void TimeStats::parseArgs(bool asProto, const Vector<String16>& args, std::string& result) { diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h index a87b7cb4a6..5b0f5bd13d 100644 --- a/services/surfaceflinger/TimeStats/TimeStats.h +++ b/services/surfaceflinger/TimeStats/TimeStats.h @@ -29,9 +29,6 @@ #include <../Fps.h> #include <gui/JankInfo.h> -#include <stats_event.h> -#include <stats_pull_atom_callback.h> -#include <statslog.h> #include <timestatsproto/TimeStatsHelper.h> #include <timestatsproto/TimeStatsProtoHeader.h> #include <ui/FenceTime.h> @@ -54,9 +51,8 @@ public: virtual ~TimeStats() = default; - // Called once boot has been finished to perform additional capabilities, - // e.g. registration to statsd. - virtual void onBootFinished() = 0; + // Process a pull request from statsd. + virtual bool onPullAtom(const int atomId, std::string* pulledData); virtual void parseArgs(bool asProto, const Vector<String16>& args, std::string& result) = 0; virtual bool isEnabled() = 0; @@ -232,58 +228,11 @@ class TimeStats : public android::TimeStats { public: TimeStats(); - - // Delegate to the statsd service and associated APIs. - // Production code may use this class directly, whereas unit test may define - // a subclass for ease of testing. - class StatsEventDelegate { - public: - virtual ~StatsEventDelegate() = default; - virtual AStatsEvent* addStatsEventToPullData(AStatsEventList* data) { - return AStatsEventList_addStatsEvent(data); - } - virtual void setStatsPullAtomCallback(int32_t atom_tag, - AStatsManager_PullAtomMetadata* metadata, - AStatsManager_PullAtomCallback callback, - void* cookie) { - return AStatsManager_setPullAtomCallback(atom_tag, metadata, callback, cookie); - } - - virtual void clearStatsPullAtomCallback(int32_t atom_tag) { - return AStatsManager_clearPullAtomCallback(atom_tag); - } - - virtual void statsEventSetAtomId(AStatsEvent* event, uint32_t atom_id) { - return AStatsEvent_setAtomId(event, atom_id); - } - - virtual void statsEventWriteInt32(AStatsEvent* event, int32_t field) { - return AStatsEvent_writeInt32(event, field); - } - - virtual void statsEventWriteInt64(AStatsEvent* event, int64_t field) { - return AStatsEvent_writeInt64(event, field); - } - - virtual void statsEventWriteString8(AStatsEvent* event, const char* field) { - return AStatsEvent_writeString(event, field); - } - - virtual void statsEventWriteByteArray(AStatsEvent* event, const uint8_t* buf, - size_t numBytes) { - return AStatsEvent_writeByteArray(event, buf, numBytes); - } - - virtual void statsEventBuild(AStatsEvent* event) { return AStatsEvent_build(event); } - }; // For testing only for injecting custom dependencies. - TimeStats(std::unique_ptr<StatsEventDelegate> statsDelegate, - std::optional<size_t> maxPulledLayers, + TimeStats(std::optional<size_t> maxPulledLayers, std::optional<size_t> maxPulledHistogramBuckets); - ~TimeStats() override; - - void onBootFinished() override; + bool onPullAtom(const int atomId, std::string* pulledData) override; void parseArgs(bool asProto, const Vector<String16>& args, std::string& result) override; bool isEnabled() override; std::string miniDump() override; @@ -332,11 +281,8 @@ public: static const size_t MAX_NUM_TIME_RECORDS = 64; private: - static AStatsManager_PullAtomCallbackReturn pullAtomCallback(int32_t atom_tag, - AStatsEventList* data, - void* cookie); - AStatsManager_PullAtomCallbackReturn populateGlobalAtom(AStatsEventList* data); - AStatsManager_PullAtomCallbackReturn populateLayerAtom(AStatsEventList* data); + bool populateGlobalAtom(std::string* pulledData); + bool populateLayerAtom(std::string* pulledData); bool recordReadyLocked(int32_t layerId, TimeRecord* timeRecord); void flushAvailableRecordsToStatsLocked(int32_t layerId, Fps displayRefreshRate, std::optional<Fps> renderRate, @@ -366,7 +312,6 @@ private: static const size_t RENDER_RATE_BUCKET_WIDTH = REFRESH_RATE_BUCKET_WIDTH; static const size_t MAX_NUM_LAYER_STATS = 200; static const size_t MAX_NUM_PULLED_LAYERS = MAX_NUM_LAYER_STATS; - std::unique_ptr<StatsEventDelegate> mStatsDelegate = std::make_unique<StatsEventDelegate>(); size_t mMaxPulledLayers = MAX_NUM_PULLED_LAYERS; size_t mMaxPulledHistogramBuckets = 6; }; diff --git a/services/surfaceflinger/TimeStats/timestatsatomsproto/Android.bp b/services/surfaceflinger/TimeStats/timestatsatomsproto/Android.bp new file mode 100644 index 0000000000..0cf086fe01 --- /dev/null +++ b/services/surfaceflinger/TimeStats/timestatsatomsproto/Android.bp @@ -0,0 +1,36 @@ +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 { + name: "libtimestats_atoms_proto", + export_include_dirs: ["include"], + + srcs: [ + "timestats_atoms.proto", + ], + + proto: { + type: "lite", + export_proto_headers: true, + }, + + cppflags: [ + "-Werror", + "-Wno-c++98-compat-pedantic", + "-Wno-disabled-macro-expansion", + "-Wno-float-conversion", + "-Wno-float-equal", + "-Wno-format", + "-Wno-old-style-cast", + "-Wno-padded", + "-Wno-sign-conversion", + "-Wno-undef", + "-Wno-unused-parameter", + ], +}
\ No newline at end of file diff --git a/services/surfaceflinger/TimeStats/timestatsatomsproto/include/timestatsatomsproto/TimeStatsAtomsProtoHeader.h b/services/surfaceflinger/TimeStats/timestatsatomsproto/include/timestatsatomsproto/TimeStatsAtomsProtoHeader.h new file mode 100644 index 0000000000..d305cb403c --- /dev/null +++ b/services/surfaceflinger/TimeStats/timestatsatomsproto/include/timestatsatomsproto/TimeStatsAtomsProtoHeader.h @@ -0,0 +1,23 @@ +/* + * Copyright 2021 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. + */ + +// pragma is used here to disable the warnings emitted from the protobuf +// headers. By adding #pragma before including layer.pb.h, it suppresses +// protobuf warnings, but allows the rest of the files to continuing using +// the current flags. +// This file should be included instead of directly including timestats_atoms.b.h +#pragma GCC system_header +#include <timestats_atoms.pb.h> diff --git a/services/surfaceflinger/TimeStats/timestatsatomsproto/timestats_atoms.proto b/services/surfaceflinger/TimeStats/timestatsatomsproto/timestats_atoms.proto new file mode 100644 index 0000000000..133a5419b5 --- /dev/null +++ b/services/surfaceflinger/TimeStats/timestatsatomsproto/timestats_atoms.proto @@ -0,0 +1,289 @@ +/* + * Copyright 2021 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. + */ +syntax = "proto2"; +option optimize_for = LITE_RUNTIME; +package android.surfaceflinger; + +// This is a copy of surfaceflinger's atoms from frameworks/proto_logging/stats/atoms.proto. +// Pulled atoms for surfaceflinger must be routed through system server since surfaceflinger is +// in the bootstrap namespace. This copy is used to pass the atoms as protos to system server using +// proto lite to serialize/deserialize the atoms. + +// These wrappers are so that we can pass a List<Atom> as a single byte string. +// They are not in atoms.proto +message SurfaceflingerStatsGlobalInfoWrapper { + repeated SurfaceflingerStatsGlobalInfo atom = 1; +} + +message SurfaceflingerStatsLayerInfoWrapper { + repeated SurfaceflingerStatsLayerInfo atom = 1; +} + +/** + * Global display pipeline metrics reported by SurfaceFlinger. + * Metrics exist beginning in Android 11. + * Pulled from: + * frameworks/native/services/surfaceflinger/TimeStats/TimeStats.cpp + */ +message SurfaceflingerStatsGlobalInfo { + // Aggregated refresh rate buckets that layers were presenting at. Buckets + // are defined in SurfaceFlinger and are tracked per device. + // Introduced in Android 12. + // This is intended to be used as a dimenstion in collecting per-refresh rate + // jank statistics. + optional int32 display_refresh_rate_bucket = 18; + // Aggregated render rate buckets that layers were overridden to run at. + // Buckets are defined in SurfaceFlinger and are tracked per device. + // Introduced in Android 12. + // This is intended to be used as a dimension in collecting per-render rate + // jank statistics. + optional int32 render_rate_bucket = 21; + // Total number of frames presented during the tracing period + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int64 total_frames = 1; + // Total number of frames missed + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int64 missed_frames = 2; + // Total number of frames that fell back to client composition + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int64 client_composition_frames = 3; + // Total time the display was turned on + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int64 display_on_millis = 4; + // Total time that was spent performing animations. + // This is derived from the present-to-present layer histogram. + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int64 animation_millis = 5; + // Total number of event connections tracked by SurfaceFlinger at the time + // of this pull. If this number grows prohibitively large, then this can + // cause jank due to resource contention. + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int32 event_connection_count = 6; + // Set of timings measured from when SurfaceFlinger began compositing a + // frame, until the frame was requested to be presented to the display. This + // measures SurfaceFlinger's total CPU walltime on the critical path per + // frame. + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional FrameTimingHistogram frame_duration = 7; + // Set of timings measured from when SurfaceFlinger first began using the + // GPU to composite a frame, until the GPU has finished compositing that + // frame. This measures the total additional time SurfaceFlinger needed to + // perform due to falling back into GPU composition. + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional FrameTimingHistogram render_engine_timing = 8; + // Number of frames where SF saw a frame, based on its frame timeline. + // Frame timelines may include transactions without updating buffer contents. + // Introduced in Android 12. + optional int32 total_timeline_frames = 9; + // Number of frames where SF saw a janky frame. + // Introduced in Android 12. + optional int32 total_janky_frames = 10; + // Number of janky frames where SF spent a long time on the CPU. + // Introduced in Android 12. + optional int32 total_janky_frames_with_long_cpu = 11; + // Number of janky frames where SF spent a long time on the GPU. + // Introduced in Android 12. + optional int32 total_janky_frames_with_long_gpu = 12; + // Number of janky frames where SF missed the frame deadline, but there + // was not an attributed reason (e.g., maybe HWC missed?) + // Introduced in Android 12. + optional int32 total_janky_frames_sf_unattributed = 13; + // Number of janky frames where the app missed the frame deadline, but + // there was not an attributed reason + // Introduced in Android 12. + optional int32 total_janky_frames_app_unattributed = 14; + // Number of janky frames that were caused because of scheduling errors in + // SF that resulted in early present (e.g., SF sending a buffer to the + // composition engine earlier than expected, resulting in a present that is + // one vsync early) + // Introduced in Android 12. + optional int32 total_janky_frames_sf_scheduling = 15; + // Number of frames that were classified as jank because of possible drift in + // vsync predictions. + // Introduced in Android 12. + optional int32 total_jank_frames_sf_prediction_error = 16; + // Number of janky frames where the app was in a buffer stuffed state (more + // than one buffer ready to be presented at the same vsync). Usually caused + // when the first frame is unusually long, the following frames enter into a + // stuffed state. + // Introduced in Android 12. + optional int32 total_jank_frames_app_buffer_stuffing = 17; + // Buckets of timings in ms by which SurfaceFlinger's deadline was missed + // while latching and presenting frames. + // Introduced in Android 12. + optional FrameTimingHistogram sf_deadline_misses = 19; + // Buckets of timings in ms by which the Vsync prediction drifted, when + // compared to the actual hardware vsync. + // Introduced in Android 12. + optional FrameTimingHistogram sf_prediction_errors = 20; + + // Next ID: 22 +} + +/** + * Per-layer display pipeline metrics reported by SurfaceFlinger. + * Metrics exist beginning in Android 11. + * The number of layers uploaded may be restricted due to size limitations. + * Pulled from: + * frameworks/native/services/surfaceflinger/TimeStats/TimeStats.cpp + */ +message SurfaceflingerStatsLayerInfo { + // UID of the application who submitted this layer for presentation + // This is intended to be used as a dimension for surfacing rendering + // statistics to applications. + // Introduced in Android 12. + optional int32 uid = 12; + // Refresh rate bucket that the layer was presenting at. Buckets are + // defined in SurfaceFlinger and are tracked per device. + // Introduced in Android 12. + // This is intended to be used as a dimension in collecting per-refresh rate + // jank statistics + optional int32 display_refresh_rate_bucket = 22; + // Render rate bucket that the layer was submitting frames at. Buckets are + // defined in SurfaceFlinger and are tracked per device. + // Introduced in Android 12. + // This is intended to be used as a dimension in collecting per-render rate + // jank statistics. + optional int32 render_rate_bucket = 23; + // The layer for this set of metrics + // In many scenarios the package name is included in the layer name, e.g., + // layers created by Window Manager. But this is not a guarantee - in the + // general case layer names are arbitrary debug names. + optional string layer_name = 1; + // Total number of frames presented + optional int64 total_frames = 2; + // Total number of dropped frames while latching a buffer for this layer. + optional int64 dropped_frames = 3; + // Set of timings measured between successive presentation timestamps. + optional FrameTimingHistogram present_to_present = 4; + // Set of timings measured from when an app queued a buffer for + // presentation, until the buffer was actually presented to the + // display. + optional FrameTimingHistogram post_to_present = 5; + // Set of timings measured from when a buffer is ready to be presented, + // until the buffer was actually presented to the display. + optional FrameTimingHistogram acquire_to_present = 6; + // Set of timings measured from when a buffer was latched by + // SurfaceFlinger, until the buffer was presented to the display + optional FrameTimingHistogram latch_to_present = 7; + // Set of timings measured from the desired presentation to the actual + // presentation time + optional FrameTimingHistogram desired_to_present = 8; + // Set of timings measured from when an app queued a buffer for + // presentation, until the buffer was ready to be presented. + optional FrameTimingHistogram post_to_acquire = 9; + // Frames missed latch because the acquire fence didn't fire + optional int64 late_acquire_frames = 10; + // Frames latched early because the desired present time was bad + optional int64 bad_desired_present_frames = 11; + // Number of frames where SF saw a frame, based on its frame timeline. + // Frame timelines may include transactions without updating buffer contents. + // Introduced in Android 12. + optional int32 total_timeline_frames = 13; + // Number of frames where SF saw a janky frame. + // Introduced in Android 12. + optional int32 total_janky_frames = 14; + // Number of janky frames where SF spent a long time on the CPU. + // Introduced in Android 12. + optional int32 total_janky_frames_with_long_cpu = 15; + // Number of janky frames where SF spent a long time on the GPU. + // Introduced in Android 12. + optional int32 total_janky_frames_with_long_gpu = 16; + // Number of janky frames where SF missed the frame deadline, but there + // was not an attributed reason (e.g., maybe HWC missed?) + // Introduced in Android 12. + optional int32 total_janky_frames_sf_unattributed = 17; + // Number of janky frames where the app missed the frame deadline, but + // there was not an attributed reason + // Introduced in Android 12. + optional int32 total_janky_frames_app_unattributed = 18; + // Number of janky frames that were caused because of scheduling errors in + // SF that resulted in early present (e.g., SF sending a buffer to the + // composition engine earlier than expected, resulting in a present that is + // one vsync early) + // Introduced in Android 12. + optional int32 total_janky_frames_sf_scheduling = 19; + // Number of frames that were classified as jank because of possible drift in + // vsync predictions. + // Introduced in Android 12. + optional int32 total_jank_frames_sf_prediction_error = 20; + // Number of janky frames where the app was in a buffer stuffed state (more + // than one buffer ready to be presented at the same vsync). Usually caused + // when the first frame is unusually long, the following frames enter into a + // stuffed state. + // Introduced in Android 12. + optional int32 total_jank_frames_app_buffer_stuffing = 21; + + /** + * Encapsulates the FrameRateVote information sent by the application while + * calling setFrameRate. + * Metrics exist beginning in Android 12. + */ + message SetFrameRateVote { + // The desired frame rate the application wishes to run on. + optional float frame_rate = 1; + + enum FrameRateCompatibility { + FRAME_RATE_UNDEFINED = 0; + FRAME_RATE_DEFAULT = 1; + FRAME_RATE_EXACT_OR_MULTIPLE = 2; + } + + // Specifies how to interpret the frame rate associated with the layer. + // Defined in Layer.h + optional FrameRateCompatibility frame_rate_compatibility = 2; + + enum Seamlessness { + SEAMLESS_UNDEFINED = 0; + SEAMLESS_SHOULD_BE_SEAMLESS = 1; + SEAMLESS_NOT_REQUIRED = 2; + } + // Indicates whether seamless refresh rate switch is required or not. + optional Seamlessness seamlessness = 3; + } + + // The last frame rate vote set by the application. + // Introduced in Android 12. + optional SetFrameRateVote set_frame_rate_vote = 24; + // Buckets of timings in ms by which the app deadline was missed while + // submitting work for a frame. + // Introduced in Android 12. + optional FrameTimingHistogram app_deadline_misses = 25; + + // Next ID: 26 +} + +/** + * Histogram of frame counts bucketed by time in milliseconds. + * Because of size limitations, we hard-cap the number of buckets, with + * buckets for corresponding to larger milliseconds being less precise. + */ +message FrameTimingHistogram { + // Timings in milliseconds that describes a set of histogram buckets + repeated int32 time_millis_buckets = 1; + // Number of frames that match to each time_millis, i.e. the bucket + // contents + // It's required that len(time_millis) == len(frame_count) + repeated int64 frame_counts = 2; +} diff --git a/services/surfaceflinger/tests/InvalidHandles_test.cpp b/services/surfaceflinger/tests/InvalidHandles_test.cpp index 58b039e5d5..9cf7c0909b 100644 --- a/services/surfaceflinger/tests/InvalidHandles_test.cpp +++ b/services/surfaceflinger/tests/InvalidHandles_test.cpp @@ -52,12 +52,15 @@ protected: } }; -TEST_F(InvalidHandleTest, createSurfaceInvalidHandle) { - auto notSc = makeNotSurfaceControl(); - ASSERT_EQ(nullptr, - mScc->createSurface(String8("lolcats"), 19, 47, PIXEL_FORMAT_RGBA_8888, 0, - notSc->getHandle()) - .get()); +TEST_F(InvalidHandleTest, createSurfaceInvalidParentHandle) { + // The createSurface is scheduled now, we could still get a created surface from createSurface. + // Should verify if it actually added into current state by checking the screenshot. + auto notSc = mScc->createSurface(String8("lolcats"), 19, 47, PIXEL_FORMAT_RGBA_8888, 0, + mNotSc->getHandle()); + LayerCaptureArgs args; + args.layerHandle = notSc->getHandle(); + ScreenCaptureResults captureResults; + ASSERT_EQ(NAME_NOT_FOUND, ScreenCapture::captureLayers(args, captureResults)); } TEST_F(InvalidHandleTest, captureLayersInvalidHandle) { diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index 4ac096b609..88fb811b52 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -127,6 +127,7 @@ cc_test { "librenderengine", "libserviceutils", "libtimestats", + "libtimestats_atoms_proto", "libtimestats_proto", "libtrace_proto", "perfetto_trace_protos", @@ -152,14 +153,10 @@ cc_test { "libnativewindow", "libprocessgroup", "libprotobuf-cpp-lite", - "libprotoutil", - "libstatslog", - "libstatssocket", "libSurfaceFlingerProp", "libsync", "libui", "libutils", - "libstatspull", ], header_libs: [ "android.hardware.graphics.composer@2.1-command-buffer", diff --git a/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp index cfbb3f5e9f..a1f0588fc7 100644 --- a/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp +++ b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp @@ -19,6 +19,7 @@ #include <gmock/gmock.h> #include <gtest/gtest.h> #include <utils/Log.h> +#include <utils/Timers.h> #include "AsyncCallRecorder.h" #include "Scheduler/OneShotTimer.h" @@ -28,21 +29,22 @@ using namespace std::chrono_literals; namespace android { namespace scheduler { +class FakeClock : public OneShotTimer::Clock { +public: + virtual ~FakeClock() = default; + std::chrono::steady_clock::time_point now() const override { return mNow; } + + void advanceTime(std::chrono::nanoseconds delta) { mNow += delta; } + +private: + std::chrono::steady_clock::time_point mNow; +}; + class OneShotTimerTest : public testing::Test { protected: OneShotTimerTest() = default; ~OneShotTimerTest() override = default; - // This timeout should be used when a 3ms callback is expected. - // While the tests typically request a callback after 3ms, the scheduler - // does not always cooperate, at it can take significantly longer (observed - // 30ms). - static constexpr auto waitTimeForExpected3msCallback = 100ms; - - // This timeout should be used when an 3ms callback is not expected. - // Note that there can be false-negatives if the callback happens later. - static constexpr auto waitTimeForUnexpected3msCallback = 6ms; - AsyncCallRecorder<void (*)()> mResetTimerCallback; AsyncCallRecorder<void (*)()> mExpiredTimerCallback; @@ -56,162 +58,179 @@ protected: namespace { TEST_F(OneShotTimerTest, createAndDestroyTest) { + FakeClock* clock = new FakeClock(); mIdleTimer = std::make_unique<scheduler::OneShotTimer>( - "TestTimer", 3ms, [] {}, [] {}); + "TestTimer", 3ms, [] {}, [] {}, std::unique_ptr<FakeClock>(clock)); } TEST_F(OneShotTimerTest, startStopTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 30ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); - auto startTime = std::chrono::steady_clock::now(); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - // The idle timer fires after 30ms, so there should be no callback within - // 25ms (waiting for a callback for the full 30ms would be problematic). - bool callbackCalled = mExpiredTimerCallback.waitForCall(25ms).has_value(); - // Under ideal conditions there should be no event. But occasionally - // it is possible that the wait just prior takes more than 30ms, and - // a callback is observed. We check the elapsed time since before the OneShotTimer - // thread was started as a sanity check to not have a flakey test. - EXPECT_FALSE(callbackCalled && std::chrono::steady_clock::now() - startTime < 30ms); - - std::this_thread::sleep_for(std::chrono::milliseconds(25)); - EXPECT_FALSE(mResetTimerCallback.waitForCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); + + clock->advanceTime(2ms); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->stop(); } TEST_F(OneShotTimerTest, resetTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 20ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); + mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - // Observe any event that happens in about 25ms. We don't care if one was - // observed or not. - mExpiredTimerCallback.waitForCall(25ms).has_value(); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); mIdleTimer->reset(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - // There may have been a race with the reset. Clear any callbacks we - // received right afterwards. - clearPendingCallbacks(); - // A single callback should be generated after 30ms - EXPECT_TRUE( - mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback + 30ms).has_value()); - // After one event, it should be idle, and not generate another. - EXPECT_FALSE( - mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback * 10).has_value()); - mIdleTimer->stop(); - // Final quick check that no more callback were observed. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall(0ms).has_value()); + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); + + clock->advanceTime(2ms); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, resetBackToBackTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 20ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); mIdleTimer->reset(); - EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value()); - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->reset(); - EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value()); - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->reset(); - EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value()); - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->reset(); - EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value()); - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); + + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); - // A single callback should be generated after 30ms - EXPECT_TRUE( - mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback + 30ms).has_value()); mIdleTimer->stop(); + clock->advanceTime(2ms); // Final quick check that no more callback were observed. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall(0ms).has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, startNotCalledTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); // The start hasn't happened, so the callback does not happen. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->stop(); + clock->advanceTime(2ms); // Final quick check that no more callback were observed. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall(0ms).has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, idleTimerIdlesTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); + + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); - // A callback should be generated after 3ms - EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value()); - // After one event, it should be idle, and not generate another. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); - // Once reset, it should generate another mIdleTimer->reset(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value()); + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); mIdleTimer->stop(); + clock->advanceTime(2ms); // Final quick check that no more callback were observed. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall(0ms).has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, timeoutCallbackExecutionTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value()); + + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); mIdleTimer->stop(); + clock->advanceTime(2ms); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, noCallbacksAfterStopAndResetTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value()); + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); mIdleTimer->stop(); - clearPendingCallbacks(); mIdleTimer->reset(); - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall().has_value()); + clock->advanceTime(2ms); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, noCallbacksAfterStopTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->stop(); - clearPendingCallbacks(); mIdleTimer->reset(); + clock->advanceTime(2ms); // No more idle events should be observed - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp index c1f0c4ef03..4e73cbc1c0 100644 --- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp +++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp @@ -23,10 +23,10 @@ #define LOG_TAG "LibSurfaceFlingerUnittests" #include <TimeStats/TimeStats.h> -#include <android/util/ProtoOutputStream.h> #include <gmock/gmock.h> #include <gtest/gtest.h> #include <log/log.h> +#include <timestatsatomsproto/TimeStatsAtomsProtoHeader.h> #include <utils/String16.h> #include <utils/Vector.h> @@ -156,44 +156,8 @@ public: } std::mt19937 mRandomEngine = std::mt19937(std::random_device()()); - - class FakeStatsEventDelegate : public impl::TimeStats::StatsEventDelegate { - public: - FakeStatsEventDelegate() = default; - ~FakeStatsEventDelegate() override = default; - - struct AStatsEvent* addStatsEventToPullData(AStatsEventList*) override { - return mEvent; - } - void setStatsPullAtomCallback(int32_t atom_tag, AStatsManager_PullAtomMetadata*, - AStatsManager_PullAtomCallback callback, - void* cookie) override { - mAtomTags.push_back(atom_tag); - mCallback = callback; - mCookie = cookie; - } - - AStatsManager_PullAtomCallbackReturn makePullAtomCallback(int32_t atom_tag, void* cookie) { - return (*mCallback)(atom_tag, nullptr, cookie); - } - - MOCK_METHOD1(clearStatsPullAtomCallback, void(int32_t)); - MOCK_METHOD2(statsEventSetAtomId, void(AStatsEvent*, uint32_t)); - MOCK_METHOD2(statsEventWriteInt32, void(AStatsEvent*, int32_t)); - MOCK_METHOD2(statsEventWriteInt64, void(AStatsEvent*, int64_t)); - MOCK_METHOD2(statsEventWriteString8, void(AStatsEvent*, const char*)); - MOCK_METHOD3(statsEventWriteByteArray, void(AStatsEvent*, const uint8_t*, size_t)); - MOCK_METHOD1(statsEventBuild, void(AStatsEvent*)); - - AStatsEvent* mEvent = AStatsEvent_obtain(); - std::vector<int32_t> mAtomTags; - AStatsManager_PullAtomCallback mCallback = nullptr; - void* mCookie = nullptr; - }; - FakeStatsEventDelegate* mDelegate = new FakeStatsEventDelegate; std::unique_ptr<TimeStats> mTimeStats = - std::make_unique<impl::TimeStats>(std::unique_ptr<FakeStatsEventDelegate>(mDelegate), - std::nullopt, std::nullopt); + std::make_unique<impl::TimeStats>(std::nullopt, std::nullopt); }; std::string TimeStatsTest::inputCommand(InputCommand cmd, bool useProto) { @@ -278,21 +242,6 @@ TEST_F(TimeStatsTest, disabledByDefault) { ASSERT_FALSE(mTimeStats->isEnabled()); } -TEST_F(TimeStatsTest, setsCallbacksAfterBoot) { - mTimeStats->onBootFinished(); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); -} - -TEST_F(TimeStatsTest, clearsCallbacksOnDestruction) { - EXPECT_CALL(*mDelegate, - clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO)); - EXPECT_CALL(*mDelegate, - clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - mTimeStats.reset(); -} - TEST_F(TimeStatsTest, canEnableAndDisableTimeStats) { EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); ASSERT_TRUE(mTimeStats->isEnabled()); @@ -1012,61 +961,49 @@ TEST_F(TimeStatsTest, noInfInAverageFPS) { } namespace { -std::string buildExpectedHistogramBytestring(const std::vector<int32_t>& times, - const std::vector<int32_t>& frameCounts) { - util::ProtoOutputStream proto; +FrameTimingHistogram buildExpectedHistogram(const std::vector<int32_t>& times, + const std::vector<int32_t>& frameCounts) { + FrameTimingHistogram histogram; for (int i = 0; i < times.size(); i++) { ALOGE("Writing time: %d", times[i]); - proto.write(util::FIELD_TYPE_INT32 | util::FIELD_COUNT_REPEATED | 1 /* field id */, - (int32_t)times[i]); + histogram.add_time_millis_buckets(times[i]); ALOGE("Writing count: %d", frameCounts[i]); - proto.write(util::FIELD_TYPE_INT64 | util::FIELD_COUNT_REPEATED | 2 /* field id */, - (int64_t)frameCounts[i]); + histogram.add_frame_counts((int64_t)frameCounts[i]); } - std::string byteString; - proto.serializeToString(&byteString); - return byteString; + return histogram; } - -std::string frameRateVoteToProtoByteString( - float refreshRate, - TimeStats::SetFrameRateVote::FrameRateCompatibility frameRateCompatibility, - TimeStats::SetFrameRateVote::Seamlessness seamlessness) { - util::ProtoOutputStream proto; - proto.write(android::util::FIELD_TYPE_FLOAT | 1 /* field id */, refreshRate); - proto.write(android::util::FIELD_TYPE_ENUM | 2 /* field id */, - static_cast<int>(frameRateCompatibility)); - proto.write(android::util::FIELD_TYPE_ENUM | 3 /* field id */, static_cast<int>(seamlessness)); - - std::string byteString; - proto.serializeToString(&byteString); - return byteString; -} - -std::string dumpByteStringHex(const std::string& str) { - std::stringstream ss; - ss << std::hex; - for (const char& c : str) { - ss << (int)c << " "; - } - - return ss.str(); -} - } // namespace -MATCHER_P2(BytesEq, bytes, size, "") { - std::string expected; - expected.append((const char*)bytes, size); - std::string actual; - actual.append((const char*)arg, size); +MATCHER_P(HistogramEq, expected, "") { + *result_listener << "Histograms are not equal! \n"; - *result_listener << "Bytes are not equal! \n"; - *result_listener << "size: " << size << "\n"; - *result_listener << "expected: " << dumpByteStringHex(expected).c_str() << "\n"; - *result_listener << "actual: " << dumpByteStringHex(actual).c_str() << "\n"; + if (arg.time_millis_buckets_size() != expected.time_millis_buckets_size()) { + *result_listener << "Time millis bucket are different sizes. Expected: " + << expected.time_millis_buckets_size() << ". Actual " + << arg.time_millis_buckets_size(); + return false; + } + if (arg.frame_counts_size() != expected.frame_counts_size()) { + *result_listener << "Frame counts are different sizes. Expected: " + << expected.frame_counts_size() << ". Actual " << arg.frame_counts_size(); + return false; + } - return expected == actual; + for (int i = 0; i < expected.time_millis_buckets_size(); i++) { + if (arg.time_millis_buckets(i) != expected.time_millis_buckets(i)) { + *result_listener << "time_millis_bucket[" << i + << "] is different. Expected: " << expected.time_millis_buckets(i) + << ". Actual: " << arg.time_millis_buckets(i); + return false; + } + if (arg.frame_counts(i) != expected.frame_counts(i)) { + *result_listener << "frame_counts[" << i + << "] is different. Expected: " << expected.frame_counts(i) + << ". Actual: " << arg.frame_counts(i); + return false; + } + } + return true; } TEST_F(TimeStatsTest, globalStatsCallback) { @@ -1075,7 +1012,6 @@ TEST_F(TimeStatsTest, globalStatsCallback) { constexpr size_t CLIENT_COMPOSITION_FRAMES = 3; constexpr size_t DISPLAY_EVENT_CONNECTIONS = 14; - mTimeStats->onBootFinished(); EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); for (size_t i = 0; i < TOTAL_FRAMES; i++) { @@ -1117,69 +1053,35 @@ TEST_F(TimeStatsTest, globalStatsCallback) { mTimeStats->incrementJankyFrames({kRefreshRate0, kRenderRate0, UID_0, genLayerName(LAYER_ID_0), JankType::None, 1, 2, 3}); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - std::string expectedFrameDuration = buildExpectedHistogramBytestring({2}, {1}); - std::string expectedRenderEngineTiming = buildExpectedHistogramBytestring({1, 2}, {1, 1}); - std::string expectedEmptyHistogram = buildExpectedHistogramBytestring({}, {}); - std::string expectedSfDeadlineMissed = buildExpectedHistogramBytestring({1}, {7}); - std::string expectedSfPredictionErrors = buildExpectedHistogramBytestring({2}, {7}); - - { - InSequence seq; - EXPECT_CALL(*mDelegate, - statsEventSetAtomId(mDelegate->mEvent, - android::util::SURFACEFLINGER_STATS_GLOBAL_INFO)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, TOTAL_FRAMES)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, MISSED_FRAMES)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, CLIENT_COMPOSITION_FRAMES)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, _)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, 2)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, DISPLAY_EVENT_CONNECTIONS)); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*)expectedFrameDuration.c_str(), - expectedFrameDuration.size()), - expectedFrameDuration.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedRenderEngineTiming.c_str(), - expectedRenderEngineTiming.size()), - expectedRenderEngineTiming.size())); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 8)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 7)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 2)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, REFRESH_RATE_BUCKET_0)); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedSfDeadlineMissed.c_str(), - expectedSfDeadlineMissed.size()), - expectedSfDeadlineMissed.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedSfPredictionErrors.c_str(), - expectedSfPredictionErrors.size()), - expectedSfPredictionErrors.size())); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, RENDER_RATE_BUCKET_0)); - - EXPECT_CALL(*mDelegate, statsEventBuild(mDelegate->mEvent)); - } - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - mDelegate->mCookie)); + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10062 /*SURFACEFLINGER_STATS_GLOBAL_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsGlobalInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 1); + const android::surfaceflinger::SurfaceflingerStatsGlobalInfo& atom = atomList.atom(0); + + EXPECT_EQ(atom.total_frames(), TOTAL_FRAMES); + EXPECT_EQ(atom.missed_frames(), MISSED_FRAMES); + EXPECT_EQ(atom.client_composition_frames(), CLIENT_COMPOSITION_FRAMES); + // Display on millis is not checked. + EXPECT_EQ(atom.animation_millis(), 2); + 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_janky_frames(), 7); + EXPECT_EQ(atom.total_janky_frames_with_long_cpu(), 1); + EXPECT_EQ(atom.total_janky_frames_with_long_gpu(), 1); + EXPECT_EQ(atom.total_janky_frames_sf_unattributed(), 1); + 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.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}))); + EXPECT_EQ(atom.render_rate_bucket(), RENDER_RATE_BUCKET_0); SFTimeStatsGlobalProto globalProto; ASSERT_TRUE(globalProto.ParseFromString(inputCommand(InputCommand::DUMP_ALL, FMT_PROTO))); @@ -1235,8 +1137,6 @@ TEST_F(TimeStatsTest, layerStatsCallback_pullsAllAndClears) { constexpr size_t BAD_DESIRED_PRESENT_FRAMES = 3; EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); - mTimeStats->onBootFinished(); - insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000); for (size_t i = 0; i < LATE_ACQUIRE_FRAMES; i++) { mTimeStats->incrementLatchSkipped(LAYER_ID_0, TimeStats::LatchSkipReason::LateAcquire); @@ -1270,99 +1170,42 @@ TEST_F(TimeStatsTest, layerStatsCallback_pullsAllAndClears) { mTimeStats->incrementJankyFrames({kRefreshRate0, kRenderRate0, UID_0, genLayerName(LAYER_ID_0), JankType::None, 1, 2, 3}); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - std::string expectedPresentToPresent = buildExpectedHistogramBytestring({1}, {1}); - std::string expectedPostToPresent = buildExpectedHistogramBytestring({4}, {1}); - std::string expectedAcquireToPresent = buildExpectedHistogramBytestring({3}, {1}); - std::string expectedLatchToPresent = buildExpectedHistogramBytestring({2}, {1}); - std::string expectedDesiredToPresent = buildExpectedHistogramBytestring({1}, {1}); - std::string expectedPostToAcquire = buildExpectedHistogramBytestring({1}, {1}); - std::string expectedFrameRateOverride = - frameRateVoteToProtoByteString(frameRate60.frameRate, - frameRate60.frameRateCompatibility, - frameRate60.seamlessness); - std::string expectedAppDeadlineMissed = buildExpectedHistogramBytestring({3, 2}, {4, 3}); - { - InSequence seq; - EXPECT_CALL(*mDelegate, - statsEventSetAtomId(mDelegate->mEvent, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_CALL(*mDelegate, - statsEventWriteString8(mDelegate->mEvent, - StrEq(genLayerName(LAYER_ID_0).c_str()))); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, 0)); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedPresentToPresent.c_str(), - expectedPresentToPresent.size()), - expectedPresentToPresent.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*)expectedPostToPresent.c_str(), - expectedPostToPresent.size()), - expectedPostToPresent.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedAcquireToPresent.c_str(), - expectedAcquireToPresent.size()), - expectedAcquireToPresent.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*)expectedLatchToPresent.c_str(), - expectedLatchToPresent.size()), - expectedLatchToPresent.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedDesiredToPresent.c_str(), - expectedDesiredToPresent.size()), - expectedDesiredToPresent.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*)expectedPostToAcquire.c_str(), - expectedPostToAcquire.size()), - expectedPostToAcquire.size())); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, LATE_ACQUIRE_FRAMES)); - EXPECT_CALL(*mDelegate, - statsEventWriteInt64(mDelegate->mEvent, BAD_DESIRED_PRESENT_FRAMES)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, UID_0)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 8)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 7)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 2)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, REFRESH_RATE_BUCKET_0)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, RENDER_RATE_BUCKET_0)); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedFrameRateOverride.c_str(), - expectedFrameRateOverride.size()), - expectedFrameRateOverride.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedAppDeadlineMissed.c_str(), - expectedAppDeadlineMissed.size()), - expectedAppDeadlineMissed.size())); - - EXPECT_CALL(*mDelegate, statsEventBuild(mDelegate->mEvent)); - } - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - mDelegate->mCookie)); + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsLayerInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 1); + const android::surfaceflinger::SurfaceflingerStatsLayerInfo& atom = atomList.atom(0); + + EXPECT_EQ(atom.layer_name(), genLayerName(LAYER_ID_0)); + EXPECT_EQ(atom.total_frames(), 1); + EXPECT_EQ(atom.dropped_frames(), 0); + EXPECT_THAT(atom.present_to_present(), HistogramEq(buildExpectedHistogram({1}, {1}))); + EXPECT_THAT(atom.post_to_present(), HistogramEq(buildExpectedHistogram({4}, {1}))); + EXPECT_THAT(atom.acquire_to_present(), HistogramEq(buildExpectedHistogram({3}, {1}))); + EXPECT_THAT(atom.latch_to_present(), HistogramEq(buildExpectedHistogram({2}, {1}))); + EXPECT_THAT(atom.desired_to_present(), HistogramEq(buildExpectedHistogram({1}, {1}))); + EXPECT_THAT(atom.post_to_acquire(), HistogramEq(buildExpectedHistogram({1}, {1}))); + EXPECT_EQ(atom.late_acquire_frames(), LATE_ACQUIRE_FRAMES); + EXPECT_EQ(atom.bad_desired_present_frames(), BAD_DESIRED_PRESENT_FRAMES); + EXPECT_EQ(atom.uid(), UID_0); + EXPECT_EQ(atom.total_timeline_frames(), 8); + 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); + EXPECT_EQ(atom.total_janky_frames_sf_unattributed(), 1); + 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.display_refresh_rate_bucket(), REFRESH_RATE_BUCKET_0); + EXPECT_EQ(atom.render_rate_bucket(), RENDER_RATE_BUCKET_0); + EXPECT_THAT(atom.set_frame_rate_vote().frame_rate(), testing::FloatEq(frameRate60.frameRate)); + EXPECT_EQ((int)atom.set_frame_rate_vote().frame_rate_compatibility(), + (int)frameRate60.frameRateCompatibility); + EXPECT_EQ((int)atom.set_frame_rate_vote().seamlessness(), (int)frameRate60.seamlessness); + EXPECT_THAT(atom.app_deadline_misses(), HistogramEq(buildExpectedHistogram({3, 2}, {4, 3}))); SFTimeStatsGlobalProto globalProto; ASSERT_TRUE(globalProto.ParseFromString(inputCommand(InputCommand::DUMP_ALL, FMT_PROTO))); @@ -1398,37 +1241,26 @@ TEST_F(TimeStatsTest, layerStatsCallback_pullsAllAndClears) { TEST_F(TimeStatsTest, layerStatsCallback_pullsMultipleLayers) { EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); - mTimeStats->onBootFinished(); - insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 2, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 1, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 2, 3000000); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - EXPECT_CALL(*mDelegate, - statsEventSetAtomId(mDelegate->mEvent, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)) - .Times(2); - EXPECT_CALL(*mDelegate, - statsEventWriteString8(mDelegate->mEvent, StrEq(genLayerName(LAYER_ID_0).c_str()))); - EXPECT_CALL(*mDelegate, - statsEventWriteString8(mDelegate->mEvent, StrEq(genLayerName(LAYER_ID_1).c_str()))); - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - mDelegate->mCookie)); + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsLayerInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 2); + std::vector<std::string> actualLayerNames = {atomList.atom(0).layer_name(), + atomList.atom(1).layer_name()}; + EXPECT_THAT(actualLayerNames, + UnorderedElementsAre(genLayerName(LAYER_ID_0), genLayerName(LAYER_ID_1))); } TEST_F(TimeStatsTest, layerStatsCallback_pullsMultipleBuckets) { EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); - mTimeStats->onBootFinished(); - insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 2, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 3, 4000000); @@ -1438,102 +1270,53 @@ TEST_F(TimeStatsTest, layerStatsCallback_pullsMultipleBuckets) { mTimeStats->setPowerMode(PowerMode::ON); mTimeStats->setPresentFenceGlobal(std::make_shared<FenceTime>(3000000)); mTimeStats->setPresentFenceGlobal(std::make_shared<FenceTime>(5000000)); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - std::string expectedPresentToPresent = buildExpectedHistogramBytestring({1, 2}, {2, 1}); - { - InSequence seq; - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedPresentToPresent.c_str(), - expectedPresentToPresent.size()), - expectedPresentToPresent.size())); - EXPECT_CALL(*mDelegate, statsEventWriteByteArray(mDelegate->mEvent, _, _)) - .Times(AnyNumber()); - } - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - mDelegate->mCookie)); + + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsLayerInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 1); + const android::surfaceflinger::SurfaceflingerStatsLayerInfo& atom = atomList.atom(0); + EXPECT_THAT(atom.present_to_present(), HistogramEq(buildExpectedHistogram({1, 2}, {2, 1}))); } TEST_F(TimeStatsTest, layerStatsCallback_limitsHistogramBuckets) { - mDelegate = new FakeStatsEventDelegate; - mTimeStats = - std::make_unique<impl::TimeStats>(std::unique_ptr<FakeStatsEventDelegate>(mDelegate), - std::nullopt, 1); + mTimeStats = std::make_unique<impl::TimeStats>(std::nullopt, 1); EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); - mTimeStats->onBootFinished(); - insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 2, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 3, 4000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 4, 5000000); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - std::string expectedPresentToPresent = buildExpectedHistogramBytestring({1}, {2}); - { - InSequence seq; - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedPresentToPresent.c_str(), - expectedPresentToPresent.size()), - expectedPresentToPresent.size())); - EXPECT_CALL(*mDelegate, statsEventWriteByteArray(mDelegate->mEvent, _, _)) - .Times(AnyNumber()); - } - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - mDelegate->mCookie)); + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsLayerInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 1); + const android::surfaceflinger::SurfaceflingerStatsLayerInfo& atom = atomList.atom(0); + EXPECT_THAT(atom.present_to_present(), HistogramEq(buildExpectedHistogram({1}, {2}))); } TEST_F(TimeStatsTest, layerStatsCallback_limitsLayers) { - mDelegate = new FakeStatsEventDelegate; - mTimeStats = - std::make_unique<impl::TimeStats>(std::unique_ptr<FakeStatsEventDelegate>(mDelegate), 1, - std::nullopt); + mTimeStats = std::make_unique<impl::TimeStats>(1, std::nullopt); EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); - mTimeStats->onBootFinished(); - insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 2, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 1, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 2, 3000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 4, 5000000); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - EXPECT_CALL(*mDelegate, - statsEventSetAtomId(mDelegate->mEvent, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)) - .Times(1); - EXPECT_CALL(*mDelegate, - statsEventWriteString8(mDelegate->mEvent, StrEq(genLayerName(LAYER_ID_1).c_str()))); - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - mDelegate->mCookie)); + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsLayerInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 1); + EXPECT_EQ(atomList.atom(0).layer_name(), genLayerName(LAYER_ID_1)); } TEST_F(TimeStatsTest, canSurviveMonkey) { diff --git a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h index 37b74ed3a7..526a847614 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h +++ b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h @@ -27,7 +27,7 @@ public: TimeStats(); ~TimeStats() override; - MOCK_METHOD0(onBootFinished, void()); + MOCK_METHOD2(onPullAtom, bool(const int, std::string*)); MOCK_METHOD3(parseArgs, void(bool, const Vector<String16>&, std::string&)); MOCK_METHOD0(isEnabled, bool()); MOCK_METHOD0(miniDump, std::string()); |