diff options
| -rw-r--r-- | services/surfaceflinger/Barrier.h | 59 | ||||
| -rw-r--r-- | services/surfaceflinger/MonitoredProducer.cpp | 8 | ||||
| -rw-r--r-- | services/surfaceflinger/Scheduler/MessageQueue.cpp | 40 | ||||
| -rw-r--r-- | services/surfaceflinger/Scheduler/MessageQueue.h | 68 | ||||
| -rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 314 | ||||
| -rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 13 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/unittests/CompositionTest.cpp | 8 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/unittests/mock/MockMessageQueue.cpp | 15 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/unittests/mock/MockMessageQueue.h | 8 |
9 files changed, 183 insertions, 350 deletions
diff --git a/services/surfaceflinger/Barrier.h b/services/surfaceflinger/Barrier.h deleted file mode 100644 index 97028a89fa..0000000000 --- a/services/surfaceflinger/Barrier.h +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright (C) 2007 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. - */ - -#ifndef ANDROID_BARRIER_H -#define ANDROID_BARRIER_H - -#include <stdint.h> -#include <condition_variable> -#include <mutex> - -namespace android { - -class Barrier -{ -public: - // Release any threads waiting at the Barrier. - // Provides release semantics: preceding loads and stores will be visible - // to other threads before they wake up. - void open() { - std::lock_guard<std::mutex> lock(mMutex); - mIsOpen = true; - mCondition.notify_all(); - } - - // Reset the Barrier, so wait() will block until open() has been called. - void close() { - std::lock_guard<std::mutex> lock(mMutex); - mIsOpen = false; - } - - // Wait until the Barrier is OPEN. - // Provides acquire semantics: no subsequent loads or stores will occur - // until wait() returns. - void wait() const { - std::unique_lock<std::mutex> lock(mMutex); - mCondition.wait(lock, [this]() NO_THREAD_SAFETY_ANALYSIS { return mIsOpen; }); - } -private: - mutable std::mutex mMutex; - mutable std::condition_variable mCondition; - int mIsOpen GUARDED_BY(mMutex){false}; -}; - -}; // namespace android - -#endif // ANDROID_BARRIER_H diff --git a/services/surfaceflinger/MonitoredProducer.cpp b/services/surfaceflinger/MonitoredProducer.cpp index 5009e10532..18a28918fd 100644 --- a/services/surfaceflinger/MonitoredProducer.cpp +++ b/services/surfaceflinger/MonitoredProducer.cpp @@ -38,13 +38,7 @@ MonitoredProducer::~MonitoredProducer() { // because we don't know where this destructor is called from. It could be // called with the mStateLock held, leading to a dead-lock (it actually // happens). - sp<LambdaMessage> cleanUpListMessage = - new LambdaMessage([flinger = mFlinger, asBinder = wp<IBinder>(onAsBinder())]() { - Mutex::Autolock lock(flinger->mStateLock); - flinger->mGraphicBufferProducerList.erase(asBinder); - }); - - mFlinger->postMessageAsync(cleanUpListMessage); + mFlinger->removeGraphicBufferProducerAsync(onAsBinder()); } status_t MonitoredProducer::requestBuffer(int slot, sp<GraphicBuffer>* buf) { diff --git a/services/surfaceflinger/Scheduler/MessageQueue.cpp b/services/surfaceflinger/Scheduler/MessageQueue.cpp index 9d6e1d864a..6067e69ea0 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.cpp +++ b/services/surfaceflinger/Scheduler/MessageQueue.cpp @@ -18,10 +18,6 @@ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wconversion" -#include <errno.h> -#include <stdint.h> -#include <sys/types.h> - #include <binder/IPCThreadState.h> #include <utils/Log.h> @@ -35,26 +31,7 @@ #include "MessageQueue.h" #include "SurfaceFlinger.h" -namespace android { - -// --------------------------------------------------------------------------- - -MessageBase::MessageBase() : MessageHandler() {} - -MessageBase::~MessageBase() {} - -void MessageBase::handleMessage(const Message&) { - this->handler(); - barrier.open(); -}; - -// --------------------------------------------------------------------------- - -MessageQueue::~MessageQueue() = default; - -// --------------------------------------------------------------------------- - -namespace impl { +namespace android::impl { void MessageQueue::Handler::dispatchRefresh() { if ((android_atomic_or(eventMaskRefresh, &mEventMask) & eventMaskRefresh) == 0) { @@ -123,14 +100,8 @@ void MessageQueue::waitMessage() { } while (true); } -status_t MessageQueue::postMessage(const sp<MessageBase>& messageHandler, nsecs_t relTime) { - const Message dummyMessage; - if (relTime > 0) { - mLooper->sendMessageDelayed(relTime, messageHandler, dummyMessage); - } else { - mLooper->sendMessage(messageHandler, dummyMessage); - } - return NO_ERROR; +void MessageQueue::postMessage(sp<MessageHandler>&& handler) { + mLooper->sendMessage(handler, Message()); } void MessageQueue::invalidate() { @@ -160,10 +131,7 @@ int MessageQueue::eventReceiver(int /*fd*/, int /*events*/) { return 1; } -// --------------------------------------------------------------------------- - -} // namespace impl -} // namespace android +} // namespace android::impl // TODO(b/129481165): remove the #pragma below and fix conversion issues #pragma clang diagnostic pop // ignored "-Wconversion" diff --git a/services/surfaceflinger/Scheduler/MessageQueue.h b/services/surfaceflinger/Scheduler/MessageQueue.h index ebc4315ec9..132b416c88 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.h +++ b/services/surfaceflinger/Scheduler/MessageQueue.h @@ -14,12 +14,12 @@ * limitations under the License. */ -#ifndef ANDROID_MESSAGE_QUEUE_H -#define ANDROID_MESSAGE_QUEUE_H +#pragma once -#include <errno.h> -#include <stdint.h> -#include <sys/types.h> +#include <cstdint> +#include <future> +#include <type_traits> +#include <utility> #include <utils/Looper.h> #include <utils/Timers.h> @@ -28,52 +28,30 @@ #include <gui/IDisplayEventConnection.h> #include <private/gui/BitTube.h> -#include "Barrier.h" #include "EventThread.h" -#include <functional> - namespace android { class SurfaceFlinger; -// --------------------------------------------------------------------------- - -class MessageBase : public MessageHandler { -public: - MessageBase(); - - // return true if message has a handler - virtual bool handler() = 0; - - // waits for the handler to be processed - void wait() const { barrier.wait(); } +template <typename F> +class Task : public MessageHandler { + template <typename G> + friend auto makeTask(G&&); -protected: - virtual ~MessageBase(); + explicit Task(F&& f) : mTask(std::move(f)) {} -private: - virtual void handleMessage(const Message& message); + void handleMessage(const Message&) override { mTask(); } - mutable Barrier barrier; + using T = std::invoke_result_t<F>; + std::packaged_task<T()> mTask; }; -class LambdaMessage : public MessageBase { -public: - explicit LambdaMessage(std::function<void()> handler) - : MessageBase(), mHandler(std::move(handler)) {} - - bool handler() override { - mHandler(); - // This return value is no longer checked, so it's always safe to return true - return true; - } - -private: - const std::function<void()> mHandler; -}; - -// --------------------------------------------------------------------------- +template <typename F> +inline auto makeTask(F&& f) { + sp<Task<F>> task = new Task<F>(std::move(f)); + return std::make_pair(task, task->mTask.get_future()); +} class MessageQueue { public: @@ -82,12 +60,12 @@ public: REFRESH = 1, }; - virtual ~MessageQueue(); + virtual ~MessageQueue() = default; virtual void init(const sp<SurfaceFlinger>& flinger) = 0; virtual void setEventConnection(const sp<EventThreadConnection>& connection) = 0; virtual void waitMessage() = 0; - virtual status_t postMessage(const sp<MessageBase>& message, nsecs_t reltime = 0) = 0; + virtual void postMessage(sp<MessageHandler>&&) = 0; virtual void invalidate() = 0; virtual void refresh() = 0; }; @@ -127,7 +105,7 @@ public: void setEventConnection(const sp<EventThreadConnection>& connection) override; void waitMessage() override; - status_t postMessage(const sp<MessageBase>& message, nsecs_t reltime = 0) override; + void postMessage(sp<MessageHandler>&&) override; // sends INVALIDATE message at next VSYNC void invalidate() override; @@ -136,9 +114,5 @@ public: void refresh() override; }; -// --------------------------------------------------------------------------- - } // namespace impl } // namespace android - -#endif /* ANDROID_MESSAGE_QUEUE_H */ diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 380634303a..29434990f6 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -410,15 +410,13 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory) : SurfaceFlinger(factory, SkipI useFrameRateApi = use_frame_rate_api(true); } -void SurfaceFlinger::onFirstRef() -{ +SurfaceFlinger::~SurfaceFlinger() = default; + +void SurfaceFlinger::onFirstRef() { mEventQueue->init(this); } -SurfaceFlinger::~SurfaceFlinger() = default; - -void SurfaceFlinger::binderDied(const wp<IBinder>& /* who */) -{ +void SurfaceFlinger::binderDied(const wp<IBinder>&) { // the window manager died on us. prepare its eulogy. mBootFinished = false; @@ -429,21 +427,25 @@ void SurfaceFlinger::binderDied(const wp<IBinder>& /* who */) startBootAnim(); } -static sp<ISurfaceComposerClient> initClient(const sp<Client>& client) { - status_t err = client->initCheck(); - if (err == NO_ERROR) { - return client; +void SurfaceFlinger::run() { + while (true) { + mEventQueue->waitMessage(); } - return nullptr; +} + +template <typename F, typename T> +inline std::future<T> SurfaceFlinger::schedule(F&& f) { + auto [task, future] = makeTask(std::move(f)); + mEventQueue->postMessage(std::move(task)); + return std::move(future); } sp<ISurfaceComposerClient> SurfaceFlinger::createConnection() { - return initClient(new Client(this)); + const sp<Client> client = new Client(this); + return client->initCheck() == NO_ERROR ? client : nullptr; } -sp<IBinder> SurfaceFlinger::createDisplay(const String8& displayName, - bool secure) -{ +sp<IBinder> SurfaceFlinger::createDisplay(const String8& displayName, bool secure) { class DisplayToken : public BBinder { sp<SurfaceFlinger> flinger; virtual ~DisplayToken() { @@ -579,7 +581,7 @@ void SurfaceFlinger::bootFinished() LOG_EVENT_LONG(LOGTAG_SF_STOP_BOOTANIM, ns2ms(systemTime(SYSTEM_TIME_MONOTONIC))); - postMessageAsync(new LambdaMessage([this]() NO_THREAD_SAFETY_ANALYSIS { + static_cast<void>(schedule([this]() NO_THREAD_SAFETY_ANALYSIS { readPersistentProperties(); mPowerAdvisor.onBootFinished(); mBootStage = BootStage::FINISHED; @@ -607,9 +609,12 @@ uint32_t SurfaceFlinger::getNewTexture() { // The pool was empty, so we need to get a new texture name directly using a // blocking call to the main thread - uint32_t name = 0; - postMessageSync(new LambdaMessage([&]() { getRenderEngine().genTextures(1, &name); })); - return name; + return schedule([this] { + uint32_t name = 0; + getRenderEngine().genTextures(1, &name); + return name; + }) + .get(); } void SurfaceFlinger::deleteTextureAsync(uint32_t texture) { @@ -663,7 +668,7 @@ void SurfaceFlinger::init() { // mStateLock from the vr flinger dispatch thread might trigger a // deadlock in surface flinger (see b/66916578), so post a message // to be handled on the main thread instead. - postMessageAsync(new LambdaMessage([=] { + static_cast<void>(schedule([=] { ALOGI("VR request display mode: requestDisplay=%d", requestDisplay); mVrFlingerRequestsDisplay = requestDisplay; signalTransaction(); @@ -985,29 +990,26 @@ status_t SurfaceFlinger::setActiveConfig(const sp<IBinder>& displayToken, int mo return BAD_VALUE; } - status_t result = NAME_NOT_FOUND; - - postMessageSync(new LambdaMessage([&]() { + auto future = schedule([=]() -> status_t { const auto display = getDisplayDeviceLocked(displayToken); if (!display) { ALOGE("Attempt to set allowed display configs for invalid display token %p", displayToken.get()); + return NAME_NOT_FOUND; } else if (display->isVirtual()) { ALOGW("Attempt to set allowed display configs for virtual display"); - result = INVALID_OPERATION; + return INVALID_OPERATION; } else { - HwcConfigIndexType config(mode); - const auto& refreshRate = mRefreshRateConfigs->getRefreshRateFromConfigId(config); - result = setDesiredDisplayConfigSpecsInternal(display, - scheduler::RefreshRateConfigs:: - Policy{config, - refreshRate.getFps(), - refreshRate.getFps()}, - /*overridePolicy=*/false); + const HwcConfigIndexType config(mode); + const float fps = mRefreshRateConfigs->getRefreshRateFromConfigId(config).getFps(); + const scheduler::RefreshRateConfigs::Policy policy{config, fps, fps}; + constexpr bool kOverridePolicy = false; + + return setDesiredDisplayConfigSpecsInternal(display, policy, kOverridePolicy); } - })); + }); - return result; + return future.get(); } void SurfaceFlinger::setActiveConfigInternal() { @@ -1181,7 +1183,7 @@ ColorMode SurfaceFlinger::getActiveColorMode(const sp<IBinder>& displayToken) { } status_t SurfaceFlinger::setActiveColorMode(const sp<IBinder>& displayToken, ColorMode mode) { - postMessageSync(new LambdaMessage([&] { + schedule([=] { Vector<ColorMode> modes; getDisplayColorModes(displayToken, &modes); bool exists = std::find(std::begin(modes), std::end(modes), mode) != std::end(modes); @@ -1203,7 +1205,7 @@ status_t SurfaceFlinger::setActiveColorMode(const sp<IBinder>& displayToken, Col RenderIntent::COLORIMETRIC, Dataspace::UNKNOWN}); } - })); + }).wait(); return NO_ERROR; } @@ -1227,7 +1229,7 @@ status_t SurfaceFlinger::getAutoLowLatencyModeSupport(const sp<IBinder>& display } void SurfaceFlinger::setAutoLowLatencyMode(const sp<IBinder>& displayToken, bool on) { - postMessageAsync(new LambdaMessage([=] { + static_cast<void>(schedule([=] { if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) { getHwComposer().setAutoLowLatencyMode(*displayId, on); } else { @@ -1258,7 +1260,7 @@ status_t SurfaceFlinger::getGameContentTypeSupport(const sp<IBinder>& displayTok } void SurfaceFlinger::setGameContentType(const sp<IBinder>& displayToken, bool on) { - postMessageAsync(new LambdaMessage([=] { + static_cast<void>(schedule([=] { if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) { const auto type = on ? hal::ContentType::GAME : hal::ContentType::NONE; getHwComposer().setContentType(*displayId, type); @@ -1348,18 +1350,17 @@ status_t SurfaceFlinger::getDisplayedContentSamplingAttributes(const sp<IBinder> status_t SurfaceFlinger::setDisplayContentSamplingEnabled(const sp<IBinder>& displayToken, bool enable, uint8_t componentMask, uint64_t maxFrames) { - status_t result = NAME_NOT_FOUND; - - postMessageSync(new LambdaMessage([&] { - if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) { - result = getHwComposer().setDisplayContentSamplingEnabled(*displayId, enable, - componentMask, maxFrames); - } else { - ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get()); - } - })); - - return result; + return schedule([=]() -> status_t { + if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) { + return getHwComposer().setDisplayContentSamplingEnabled(*displayId, enable, + componentMask, + maxFrames); + } else { + ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get()); + return NAME_NOT_FOUND; + } + }) + .get(); } status_t SurfaceFlinger::getDisplayedContentSample(const sp<IBinder>& displayToken, @@ -1401,14 +1402,14 @@ status_t SurfaceFlinger::isWideColorDisplay(const sp<IBinder>& displayToken, } status_t SurfaceFlinger::enableVSyncInjections(bool enable) { - postMessageSync(new LambdaMessage([&] { + schedule([=] { Mutex::Autolock lock(mStateLock); if (const auto handle = mScheduler->enableVSyncInjection(enable)) { mEventQueue->setEventConnection( mScheduler->getEventConnection(enable ? handle : mSfConnectionHandle)); } - })); + }).wait(); return NO_ERROR; } @@ -1490,17 +1491,15 @@ status_t SurfaceFlinger::setDisplayBrightness(const sp<IBinder>& displayToken, f return BAD_VALUE; } - status_t result = NAME_NOT_FOUND; - - postMessageSync(new LambdaMessage([&] { - if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) { - result = getHwComposer().setDisplayBrightness(*displayId, brightness); - } else { - ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get()); - } - })); - - return result; + return schedule([=]() -> status_t { + if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) { + return getHwComposer().setDisplayBrightness(*displayId, brightness); + } else { + ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get()); + return NAME_NOT_FOUND; + } + }) + .get(); } status_t SurfaceFlinger::notifyPowerHint(int32_t hintId) { @@ -1523,12 +1522,6 @@ sp<IDisplayEventConnection> SurfaceFlinger::createDisplayEventConnection( return mScheduler->createDisplayEventConnection(handle, configChanged); } -// ---------------------------------------------------------------------------- - -void SurfaceFlinger::waitForEvent() { - mEventQueue->waitMessage(); -} - void SurfaceFlinger::signalTransaction() { mScheduler->resetIdleTimer(); mPowerAdvisor.notifyDisplayUpdateImminent(); @@ -1546,26 +1539,6 @@ void SurfaceFlinger::signalRefresh() { mEventQueue->refresh(); } -status_t SurfaceFlinger::postMessageAsync(const sp<MessageBase>& msg, - nsecs_t reltime, uint32_t /* flags */) { - return mEventQueue->postMessage(msg, reltime); -} - -status_t SurfaceFlinger::postMessageSync(const sp<MessageBase>& msg, - nsecs_t reltime, uint32_t /* flags */) { - status_t res = mEventQueue->postMessage(msg, reltime); - if (res == NO_ERROR) { - msg->wait(); - } - return res; -} - -void SurfaceFlinger::run() { - do { - waitForEvent(); - } while (true); -} - nsecs_t SurfaceFlinger::getVsyncPeriod() const { const auto displayId = getInternalDisplayIdLocked(); if (!displayId || !getHwComposer().isConnected(*displayId)) { @@ -1683,8 +1656,8 @@ void SurfaceFlinger::setPrimaryVsyncEnabled(bool enabled) { // Enable / Disable HWVsync from the main thread to avoid race conditions with // display power state. - postMessageAsync(new LambdaMessage( - [=]() NO_THREAD_SAFETY_ANALYSIS { setPrimaryVsyncEnabledInternal(enabled); })); + static_cast<void>( + schedule([=]() NO_THREAD_SAFETY_ANALYSIS { setPrimaryVsyncEnabledInternal(enabled); })); } void SurfaceFlinger::setPrimaryVsyncEnabledInternal(bool enabled) { @@ -3210,6 +3183,13 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBind return NO_ERROR; } +void SurfaceFlinger::removeGraphicBufferProducerAsync(const wp<IBinder>& binder) { + static_cast<void>(schedule([=] { + Mutex::Autolock lock(mStateLock); + mGraphicBufferProducerList.erase(binder); + })); +} + uint32_t SurfaceFlinger::peekTransactionFlags() { return mTransactionFlags; } @@ -4133,8 +4113,7 @@ void SurfaceFlinger::onInitializeDisplays() { void SurfaceFlinger::initializeDisplays() { // Async since we may be called from the main thread. - postMessageAsync( - new LambdaMessage([this]() NO_THREAD_SAFETY_ANALYSIS { onInitializeDisplays(); })); + static_cast<void>(schedule([this]() NO_THREAD_SAFETY_ANALYSIS { onInitializeDisplays(); })); } void SurfaceFlinger::setVsyncEnabledInHWC(DisplayId displayId, hal::Vsync enabled) { @@ -4225,7 +4204,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal: } void SurfaceFlinger::setPowerMode(const sp<IBinder>& displayToken, int mode) { - postMessageSync(new LambdaMessage([&]() NO_THREAD_SAFETY_ANALYSIS { + schedule([=]() NO_THREAD_SAFETY_ANALYSIS { const auto display = getDisplayDeviceLocked(displayToken); if (!display) { ALOGE("Attempt to set power mode %d for invalid display token %p", mode, @@ -4235,11 +4214,9 @@ void SurfaceFlinger::setPowerMode(const sp<IBinder>& displayToken, int mode) { } else { setPowerModeInternal(display, static_cast<hal::PowerMode>(mode)); } - })); + }).wait(); } -// --------------------------------------------------------------------------- - status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) NO_THREAD_SAFETY_ANALYSIS { std::string result; @@ -4600,20 +4577,21 @@ void SurfaceFlinger::dumpOffscreenLayersProto(LayersProto& layersProto, uint32_t } LayersProto SurfaceFlinger::dumpProtoFromMainThread(uint32_t traceFlags) { - LayersProto layersProto; - postMessageSync(new LambdaMessage([&] { layersProto = dumpDrawingStateProto(traceFlags); })); - return layersProto; + return schedule([=] { return dumpDrawingStateProto(traceFlags); }).get(); } void SurfaceFlinger::dumpOffscreenLayers(std::string& result) { result.append("Offscreen Layers:\n"); - postMessageSync(new LambdaMessage([&]() { - for (Layer* offscreenLayer : mOffscreenLayers) { - offscreenLayer->traverse(LayerVector::StateSet::Drawing, [&](Layer* layer) { - layer->dumpCallingUidPid(result); - }); - } - })); + result.append(schedule([this] { + std::string result; + for (Layer* offscreenLayer : mOffscreenLayers) { + offscreenLayer->traverse(LayerVector::StateSet::Drawing, + [&](Layer* layer) { + layer->dumpCallingUidPid(result); + }); + } + return result; + }).get()); } void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) const { @@ -5296,7 +5274,7 @@ void SurfaceFlinger::kernelTimerChanged(bool expired) { // Update the overlay on the main thread to avoid race conditions with // mRefreshRateConfigs->getCurrentRefreshRate() - postMessageAsync(new LambdaMessage([this, expired]() NO_THREAD_SAFETY_ANALYSIS { + static_cast<void>(schedule([=]() NO_THREAD_SAFETY_ANALYSIS { if (mRefreshRateOverlay) { const auto kernelTimerEnabled = property_get_bool(KERNEL_IDLE_TIMER_PROP, false); const bool timerExpired = kernelTimerEnabled && expired; @@ -5680,60 +5658,33 @@ status_t SurfaceFlinger::captureScreenCommon(RenderArea& renderArea, const sp<GraphicBuffer>& buffer, bool useIdentityTransform, bool regionSampling, bool& outCapturedSecureLayers) { - // This mutex protects syncFd and captureResult for communication of the return values from the - // main thread back to this Binder thread - std::mutex captureMutex; - std::condition_variable captureCondition; - std::unique_lock<std::mutex> captureLock(captureMutex); - int syncFd = -1; - std::optional<status_t> captureResult; - const int uid = IPCThreadState::self()->getCallingUid(); const bool forSystem = uid == AID_GRAPHICS || uid == AID_SYSTEM; - sp<LambdaMessage> message = new LambdaMessage([&] { - // If there is a refresh pending, bug out early and tell the binder thread to try again - // after the refresh. - if (mRefreshPending) { - ATRACE_NAME("Skipping screenshot for now"); - std::unique_lock<std::mutex> captureLock(captureMutex); - captureResult = std::make_optional<status_t>(EAGAIN); - captureCondition.notify_one(); - return; - } + status_t result; + int syncFd; - status_t result = NO_ERROR; - int fd = -1; - { - Mutex::Autolock _l(mStateLock); - renderArea.render([&] { - result = captureScreenImplLocked(renderArea, traverseLayers, buffer.get(), - useIdentityTransform, forSystem, &fd, - regionSampling, outCapturedSecureLayers); - }); - } + do { + std::tie(result, syncFd) = + schedule([&] { + if (mRefreshPending) { + ATRACE_NAME("Skipping screenshot for now"); + return std::make_pair(EAGAIN, -1); + } - { - std::unique_lock<std::mutex> captureLock(captureMutex); - syncFd = fd; - captureResult = std::make_optional<status_t>(result); - captureCondition.notify_one(); - } - }); + status_t result = NO_ERROR; + int fd = -1; - status_t result = postMessageAsync(message); - if (result == NO_ERROR) { - captureCondition.wait(captureLock, [&] { return captureResult; }); - while (*captureResult == EAGAIN) { - captureResult.reset(); - result = postMessageAsync(message); - if (result != NO_ERROR) { - return result; - } - captureCondition.wait(captureLock, [&] { return captureResult; }); - } - result = *captureResult; - } + Mutex::Autolock lock(mStateLock); + renderArea.render([&] { + result = captureScreenImplLocked(renderArea, traverseLayers, buffer.get(), + useIdentityTransform, forSystem, &fd, + regionSampling, outCapturedSecureLayers); + }); + + return std::make_pair(result, fd); + }).get(); + } while (result == EAGAIN); if (result == NO_ERROR) { sync_wait(syncFd, -1); @@ -6008,28 +5959,25 @@ status_t SurfaceFlinger::setDesiredDisplayConfigSpecs(const sp<IBinder>& display return BAD_VALUE; } - status_t result = NAME_NOT_FOUND; - - postMessageSync(new LambdaMessage([&]() { + auto future = schedule([=]() -> status_t { const auto display = getDisplayDeviceLocked(displayToken); if (!display) { ALOGE("Attempt to set desired display configs for invalid display token %p", displayToken.get()); + return NAME_NOT_FOUND; } else if (display->isVirtual()) { ALOGW("Attempt to set desired display configs for virtual display"); - result = INVALID_OPERATION; + return INVALID_OPERATION; } else { - result = setDesiredDisplayConfigSpecsInternal(display, - scheduler::RefreshRateConfigs:: - Policy{HwcConfigIndexType( - defaultConfig), - minRefreshRate, - maxRefreshRate}, - /*overridePolicy=*/false); + using Policy = scheduler::RefreshRateConfigs::Policy; + const Policy policy{HwcConfigIndexType(defaultConfig), minRefreshRate, maxRefreshRate}; + constexpr bool kOverridePolicy = false; + + return setDesiredDisplayConfigSpecsInternal(display, policy, kOverridePolicy); } - })); + }); - return result; + return future.get(); } status_t SurfaceFlinger::getDesiredDisplayConfigSpecs(const sp<IBinder>& displayToken, @@ -6158,7 +6106,7 @@ status_t SurfaceFlinger::setFrameRate(const sp<IGraphicBufferProducer>& surface, return BAD_VALUE; } - postMessageAsync(new LambdaMessage([=]() NO_THREAD_SAFETY_ANALYSIS { + static_cast<void>(schedule([=]() NO_THREAD_SAFETY_ANALYSIS { Mutex::Autolock lock(mStateLock); if (authenticateSurfaceTextureLocked(surface)) { sp<Layer> layer = (static_cast<MonitoredProducer*>(surface.get()))->getLayer(); @@ -6181,8 +6129,11 @@ status_t SurfaceFlinger::acquireFrameRateFlexibilityToken(sp<IBinder>* outToken) if (!outToken) { return BAD_VALUE; } - status_t result = NO_ERROR; - postMessageSync(new LambdaMessage([&]() { + + auto future = schedule([this] { + status_t result = NO_ERROR; + sp<IBinder> token; + if (mFrameRateFlexibilityTokenCount == 0) { // |mStateLock| not needed as we are on the main thread const auto display = getDefaultDisplayDeviceLocked(); @@ -6196,8 +6147,8 @@ status_t SurfaceFlinger::acquireFrameRateFlexibilityToken(sp<IBinder>* outToken) overridePolicy.defaultConfig = mRefreshRateConfigs->getDisplayManagerPolicy().defaultConfig; overridePolicy.allowGroupSwitching = true; - result = setDesiredDisplayConfigSpecsInternal(display, overridePolicy, - /*overridePolicy=*/true); + constexpr bool kOverridePolicy = true; + result = setDesiredDisplayConfigSpecsInternal(display, overridePolicy, kOverridePolicy); } if (result == NO_ERROR) { @@ -6214,17 +6165,22 @@ status_t SurfaceFlinger::acquireFrameRateFlexibilityToken(sp<IBinder>* outToken) // 2. The frame rate flexibility token is acquired/released only by CTS tests, so even // if condition 1 were changed, the problem would only show up when running CTS tests, // not on end user devices, so we could spot it and fix it without serious impact. - *outToken = new FrameRateFlexibilityToken( + token = new FrameRateFlexibilityToken( [this]() { onFrameRateFlexibilityTokenReleased(); }); ALOGD("Frame rate flexibility token acquired. count=%d", mFrameRateFlexibilityTokenCount); } - })); + + return std::make_pair(result, token); + }); + + status_t result; + std::tie(result, *outToken) = future.get(); return result; } void SurfaceFlinger::onFrameRateFlexibilityTokenReleased() { - postMessageAsync(new LambdaMessage([&]() { + static_cast<void>(schedule([this] { LOG_ALWAYS_FATAL_IF(mFrameRateFlexibilityTokenCount == 0, "Failed tracking frame rate flexibility tokens"); mFrameRateFlexibilityTokenCount--; @@ -6232,8 +6188,8 @@ void SurfaceFlinger::onFrameRateFlexibilityTokenReleased() { if (mFrameRateFlexibilityTokenCount == 0) { // |mStateLock| not needed as we are on the main thread const auto display = getDefaultDisplayDeviceLocked(); - status_t result = - setDesiredDisplayConfigSpecsInternal(display, {}, /*overridePolicy=*/true); + constexpr bool kOverridePolicy = true; + status_t result = setDesiredDisplayConfigSpecsInternal(display, {}, kOverridePolicy); LOG_ALWAYS_FATAL_IF(result < 0, "Failed releasing frame rate flexibility token"); } })); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index eb269b4d40..b65e4f6899 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -68,6 +68,7 @@ #include <atomic> #include <cstdint> #include <functional> +#include <future> #include <map> #include <memory> #include <mutex> @@ -276,12 +277,9 @@ public: // starts SurfaceFlinger main loop in the current thread void run() ANDROID_API; - // post an asynchronous message to the main thread - status_t postMessageAsync(const sp<MessageBase>& msg, nsecs_t reltime = 0, uint32_t flags = 0); - - // post a synchronous message to the main thread - status_t postMessageSync(const sp<MessageBase>& msg, nsecs_t reltime = 0, uint32_t flags = 0) - EXCLUDES(mStateLock); + // Schedule an asynchronous or synchronous task on the main thread. + template <typename F, typename T = std::invoke_result_t<F>> + [[nodiscard]] std::future<T> schedule(F&&); // force full composition on all displays void repaintEverything(); @@ -542,7 +540,6 @@ private: /* ------------------------------------------------------------------------ * Message handling */ - void waitForEvent(); // Can only be called from the main thread or with mStateLock held void signalTransaction(); // Can only be called from the main thread or with mStateLock held @@ -999,6 +996,8 @@ private: std::set<wp<IBinder>> mGraphicBufferProducerList; size_t mMaxGraphicBufferProducerListSize = ISurfaceComposer::MAX_LAYERS; + void removeGraphicBufferProducerAsync(const wp<IBinder>&); + // protected by mStateLock (but we could use another lock) bool mLayersRemoved = false; bool mLayersAdded = false; diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index e0dd0d9f07..8713b2bd49 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -542,7 +542,7 @@ struct BaseLayerProperties { EXPECT_CALL(*test->mMessageQueue, invalidate()).Times(1); enqueueBuffer(test, layer); - Mock::VerifyAndClear(test->mMessageQueue); + Mock::VerifyAndClearExpectations(test->mMessageQueue); EXPECT_CALL(*test->mRenderEngine, useNativeFenceSync()).WillRepeatedly(Return(true)); bool ignoredRecomputeVisibleRegions; @@ -834,7 +834,7 @@ template <typename LayerProperties> struct BaseLayerVariant { template <typename L, typename F> static sp<L> createLayerWithFactory(CompositionTest* test, F factory) { - EXPECT_CALL(*test->mMessageQueue, postMessage(_, 0)).Times(0); + EXPECT_CALL(*test->mMessageQueue, postMessage(_)).Times(0); sp<L> layer = factory(); @@ -843,7 +843,7 @@ struct BaseLayerVariant { Mock::VerifyAndClear(test->mComposer); Mock::VerifyAndClear(test->mRenderEngine); - Mock::VerifyAndClear(test->mMessageQueue); + Mock::VerifyAndClearExpectations(test->mMessageQueue); auto& layerDrawingState = test->mFlinger.mutableLayerDrawingState(layer); layerDrawingState.layerStack = DEFAULT_LAYER_STACK; @@ -944,7 +944,7 @@ struct BufferLayerVariant : public BaseLayerVariant<LayerProperties> { } static void cleanupInjectedLayers(CompositionTest* test) { - EXPECT_CALL(*test->mMessageQueue, postMessage(_, 0)).Times(1); + EXPECT_CALL(*test->mMessageQueue, postMessage(_)).Times(1); Base::cleanupInjectedLayers(test); } diff --git a/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.cpp b/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.cpp index 97a13e4d3e..5fb06fd65f 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.cpp +++ b/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.cpp @@ -16,12 +16,15 @@ #include "mock/MockMessageQueue.h" -namespace android { -namespace mock { +namespace android::mock { + +MessageQueue::MessageQueue() { + ON_CALL(*this, postMessage).WillByDefault([](sp<MessageHandler>&& handler) { + // Execute task to prevent broken promise exception on destruction. + handler->handleMessage(Message()); + }); +} -// Explicit default instantiation is recommended. -MessageQueue::MessageQueue() = default; MessageQueue::~MessageQueue() = default; -} // namespace mock -} // namespace android
\ No newline at end of file +} // namespace android::mock diff --git a/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.h b/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.h index e781c0a10d..a82b583d6a 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.h +++ b/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.h @@ -21,8 +21,7 @@ #include "Scheduler/EventThread.h" #include "Scheduler/MessageQueue.h" -namespace android { -namespace mock { +namespace android::mock { class MessageQueue : public android::MessageQueue { public: @@ -32,10 +31,9 @@ public: MOCK_METHOD1(init, void(const sp<SurfaceFlinger>&)); MOCK_METHOD1(setEventConnection, void(const sp<EventThreadConnection>& connection)); MOCK_METHOD0(waitMessage, void()); - MOCK_METHOD2(postMessage, status_t(const sp<MessageBase>&, nsecs_t)); + MOCK_METHOD1(postMessage, void(sp<MessageHandler>&&)); MOCK_METHOD0(invalidate, void()); MOCK_METHOD0(refresh, void()); }; -} // namespace mock -} // namespace android +} // namespace android::mock |