Merge changes Id23fafaf,I50450881
* changes:
SF: Fix UAF due to outliving idle timer
SF: Fix leak in unit tests
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
index 71d5631..a94952f 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.cpp
@@ -714,20 +714,19 @@
void RefreshRateConfigs::initializeIdleTimer() {
if (mConfig.idleTimerTimeoutMs > 0) {
- const auto getCallback = [this]() -> std::optional<IdleTimerCallbacks::Callbacks> {
- std::scoped_lock lock(mIdleTimerCallbacksMutex);
- if (!mIdleTimerCallbacks.has_value()) return {};
- return mConfig.supportKernelIdleTimer ? mIdleTimerCallbacks->kernel
- : mIdleTimerCallbacks->platform;
- };
-
mIdleTimer.emplace(
"IdleTimer", std::chrono::milliseconds(mConfig.idleTimerTimeoutMs),
- [getCallback] {
- if (const auto callback = getCallback()) callback->onReset();
+ [this] {
+ std::scoped_lock lock(mIdleTimerCallbacksMutex);
+ if (const auto callbacks = getIdleTimerCallbacks()) {
+ callbacks->onReset();
+ }
},
- [getCallback] {
- if (const auto callback = getCallback()) callback->onExpired();
+ [this] {
+ std::scoped_lock lock(mIdleTimerCallbacksMutex);
+ if (const auto callbacks = getIdleTimerCallbacks()) {
+ callbacks->onExpired();
+ }
});
}
}
diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
index 4bbdab6..fc45d2b 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h
@@ -348,16 +348,24 @@
bool supportsKernelIdleTimer() const { return mConfig.supportKernelIdleTimer; }
- void setIdleTimerCallbacks(std::function<void()> platformTimerReset,
- std::function<void()> platformTimerExpired,
- std::function<void()> kernelTimerReset,
- std::function<void()> kernelTimerExpired) {
+ struct IdleTimerCallbacks {
+ struct Callbacks {
+ std::function<void()> onReset;
+ std::function<void()> onExpired;
+ };
+
+ Callbacks platform;
+ Callbacks kernel;
+ };
+
+ void setIdleTimerCallbacks(IdleTimerCallbacks callbacks) EXCLUDES(mIdleTimerCallbacksMutex) {
std::scoped_lock lock(mIdleTimerCallbacksMutex);
- mIdleTimerCallbacks.emplace();
- mIdleTimerCallbacks->platform.onReset = std::move(platformTimerReset);
- mIdleTimerCallbacks->platform.onExpired = std::move(platformTimerExpired);
- mIdleTimerCallbacks->kernel.onReset = std::move(kernelTimerReset);
- mIdleTimerCallbacks->kernel.onExpired = std::move(kernelTimerExpired);
+ mIdleTimerCallbacks = std::move(callbacks);
+ }
+
+ void clearIdleTimerCallbacks() EXCLUDES(mIdleTimerCallbacksMutex) {
+ std::scoped_lock lock(mIdleTimerCallbacksMutex);
+ mIdleTimerCallbacks.reset();
}
void startIdleTimer() {
@@ -380,7 +388,7 @@
return;
}
mIdleTimer->reset();
- };
+ }
void dump(std::string& result) const EXCLUDES(mLock);
@@ -448,6 +456,13 @@
void initializeIdleTimer();
+ std::optional<IdleTimerCallbacks::Callbacks> getIdleTimerCallbacks() const
+ REQUIRES(mIdleTimerCallbacksMutex) {
+ if (!mIdleTimerCallbacks) return {};
+ return mConfig.supportKernelIdleTimer ? mIdleTimerCallbacks->kernel
+ : mIdleTimerCallbacks->platform;
+ }
+
// The list of refresh rates, indexed by display modes ID. This may change after this
// object is initialized.
AllRefreshRatesMapType mRefreshRates GUARDED_BY(mLock);
@@ -492,21 +507,11 @@
mutable std::optional<GetBestRefreshRateInvocation> lastBestRefreshRateInvocation
GUARDED_BY(mLock);
- // Timer that records time between requests for next vsync.
- std::optional<scheduler::OneShotTimer> mIdleTimer;
-
- struct IdleTimerCallbacks {
- struct Callbacks {
- std::function<void()> onReset;
- std::function<void()> onExpired;
- };
-
- Callbacks platform;
- Callbacks kernel;
- };
-
+ // Declare mIdleTimer last to ensure its thread joins before the mutex/callbacks are destroyed.
std::mutex mIdleTimerCallbacksMutex;
std::optional<IdleTimerCallbacks> mIdleTimerCallbacks GUARDED_BY(mIdleTimerCallbacksMutex);
+ // Used to detect (lack of) frame activity.
+ std::optional<scheduler::OneShotTimer> mIdleTimer;
};
} // namespace android::scheduler
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index cbe4552..4173088 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -62,6 +62,15 @@
Scheduler::Scheduler(ICompositor& compositor, ISchedulerCallback& callback, FeatureFlags features)
: impl::MessageQueue(compositor), mFeatures(features), mSchedulerCallback(callback) {}
+Scheduler::~Scheduler() {
+ // Stop timers and wait for their threads to exit.
+ mDisplayPowerTimer.reset();
+ mTouchTimer.reset();
+
+ // Stop idle timer and clear callbacks, as the RefreshRateConfigs may outlive the Scheduler.
+ setRefreshRateConfigs(nullptr);
+}
+
void Scheduler::startTimers() {
using namespace sysprop;
using namespace std::string_literals;
@@ -84,11 +93,32 @@
}
}
-Scheduler::~Scheduler() {
- // Ensure the OneShotTimer threads are joined before we start destroying state.
- mDisplayPowerTimer.reset();
- mTouchTimer.reset();
- mRefreshRateConfigs.reset();
+void Scheduler::setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs> configs) {
+ {
+ // The current RefreshRateConfigs instance may outlive this call, so unbind its idle timer.
+ std::scoped_lock lock(mRefreshRateConfigsLock);
+ if (mRefreshRateConfigs) {
+ mRefreshRateConfigs->stopIdleTimer();
+ mRefreshRateConfigs->clearIdleTimerCallbacks();
+ }
+ }
+ {
+ // Clear state that depends on the current instance.
+ std::scoped_lock lock(mPolicyLock);
+ mPolicy = {};
+ }
+
+ std::scoped_lock lock(mRefreshRateConfigsLock);
+ mRefreshRateConfigs = std::move(configs);
+ if (!mRefreshRateConfigs) return;
+
+ mRefreshRateConfigs->setIdleTimerCallbacks(
+ {.platform = {.onReset = [this] { idleTimerCallback(TimerState::Reset); },
+ .onExpired = [this] { idleTimerCallback(TimerState::Expired); }},
+ .kernel = {.onReset = [this] { kernelIdleTimerCallback(TimerState::Reset); },
+ .onExpired = [this] { kernelIdleTimerCallback(TimerState::Expired); }}});
+
+ mRefreshRateConfigs->startIdleTimer();
}
void Scheduler::run() {
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 818f1ed..548c34b 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -74,11 +74,15 @@
public:
Scheduler(ICompositor&, ISchedulerCallback&, FeatureFlags);
- ~Scheduler();
+ virtual ~Scheduler();
+
+ void startTimers();
+ void setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs>)
+ EXCLUDES(mRefreshRateConfigsLock);
+
+ void run();
void createVsyncSchedule(FeatureFlags);
- void startTimers();
- void run();
using Impl::initVsync;
using Impl::setInjector;
@@ -198,36 +202,6 @@
std::optional<Fps> getFrameRateOverride(uid_t uid) const
EXCLUDES(mRefreshRateConfigsLock, mFrameRateOverridesLock);
- void setRefreshRateConfigs(std::shared_ptr<RefreshRateConfigs> refreshRateConfigs)
- EXCLUDES(mRefreshRateConfigsLock) {
- // We need to stop the idle timer on the previous RefreshRateConfigs instance
- // and cleanup the scheduler's state before we switch to the other RefreshRateConfigs.
- {
- std::scoped_lock lock(mRefreshRateConfigsLock);
- if (mRefreshRateConfigs) mRefreshRateConfigs->stopIdleTimer();
- }
- {
- std::scoped_lock lock(mPolicyLock);
- mPolicy = {};
- }
- {
- std::scoped_lock lock(mRefreshRateConfigsLock);
- mRefreshRateConfigs = std::move(refreshRateConfigs);
- mRefreshRateConfigs->setIdleTimerCallbacks(
- [this] { std::invoke(&Scheduler::idleTimerCallback, this, TimerState::Reset); },
- [this] {
- std::invoke(&Scheduler::idleTimerCallback, this, TimerState::Expired);
- },
- [this] {
- std::invoke(&Scheduler::kernelIdleTimerCallback, this, TimerState::Reset);
- },
- [this] {
- std::invoke(&Scheduler::kernelIdleTimerCallback, this, TimerState::Expired);
- });
- mRefreshRateConfigs->startIdleTimer();
- }
- }
-
nsecs_t getVsyncPeriodFromRefreshRateConfigs() const EXCLUDES(mRefreshRateConfigsLock) {
std::scoped_lock lock(mRefreshRateConfigsLock);
return mRefreshRateConfigs->getCurrentRefreshRate().getVsyncPeriod();
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 64a18c6..f8c7aec 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -384,7 +384,7 @@
mInternalDisplayDensity(getDensityFromProperty("ro.sf.lcd_density", true)),
mEmulatedDisplayDensity(getDensityFromProperty("qemu.sf.lcd_density", false)),
mPowerAdvisor(*this),
- mWindowInfosListenerInvoker(new WindowInfosListenerInvoker(this)) {
+ mWindowInfosListenerInvoker(sp<WindowInfosListenerInvoker>::make(*this)) {
ALOGI("Using HWComposer service: %s", mHwcServiceName.c_str());
}
diff --git a/services/surfaceflinger/WindowInfosListenerInvoker.cpp b/services/surfaceflinger/WindowInfosListenerInvoker.cpp
index b93d127..72434e9 100644
--- a/services/surfaceflinger/WindowInfosListenerInvoker.cpp
+++ b/services/surfaceflinger/WindowInfosListenerInvoker.cpp
@@ -25,24 +25,21 @@
using gui::IWindowInfosListener;
using gui::WindowInfo;
-struct WindowInfosReportedListener : gui::BnWindowInfosReportedListener {
- explicit WindowInfosReportedListener(std::function<void()> listenerCb)
- : mListenerCb(listenerCb) {}
+struct WindowInfosListenerInvoker::WindowInfosReportedListener
+ : gui::BnWindowInfosReportedListener {
+ explicit WindowInfosReportedListener(WindowInfosListenerInvoker& invoker) : mInvoker(invoker) {}
binder::Status onWindowInfosReported() override {
- if (mListenerCb != nullptr) {
- mListenerCb();
- }
+ mInvoker.windowInfosReported();
return binder::Status::ok();
}
- std::function<void()> mListenerCb;
+ WindowInfosListenerInvoker& mInvoker;
};
-WindowInfosListenerInvoker::WindowInfosListenerInvoker(const sp<SurfaceFlinger>& sf) : mSf(sf) {
- mWindowInfosReportedListener =
- new WindowInfosReportedListener([&]() { windowInfosReported(); });
-}
+WindowInfosListenerInvoker::WindowInfosListenerInvoker(SurfaceFlinger& flinger)
+ : mFlinger(flinger),
+ mWindowInfosReportedListener(sp<WindowInfosReportedListener>::make(*this)) {}
void WindowInfosListenerInvoker::addWindowInfosListener(
const sp<IWindowInfosListener>& windowInfosListener) {
@@ -91,8 +88,8 @@
void WindowInfosListenerInvoker::windowInfosReported() {
mCallbacksPending--;
if (mCallbacksPending == 0) {
- mSf->windowInfosReported();
+ mFlinger.windowInfosReported();
}
}
-} // namespace android
\ No newline at end of file
+} // namespace android
diff --git a/services/surfaceflinger/WindowInfosListenerInvoker.h b/services/surfaceflinger/WindowInfosListenerInvoker.h
index 4e08393..2eabf48 100644
--- a/services/surfaceflinger/WindowInfosListenerInvoker.h
+++ b/services/surfaceflinger/WindowInfosListenerInvoker.h
@@ -31,7 +31,8 @@
class WindowInfosListenerInvoker : public IBinder::DeathRecipient {
public:
- WindowInfosListenerInvoker(const sp<SurfaceFlinger>& sf);
+ explicit WindowInfosListenerInvoker(SurfaceFlinger&);
+
void addWindowInfosListener(const sp<gui::IWindowInfosListener>& windowInfosListener);
void removeWindowInfosListener(const sp<gui::IWindowInfosListener>& windowInfosListener);
@@ -42,13 +43,15 @@
void binderDied(const wp<IBinder>& who) override;
private:
+ struct WindowInfosReportedListener;
void windowInfosReported();
- const sp<SurfaceFlinger> mSf;
+ SurfaceFlinger& mFlinger;
std::mutex mListenersMutex;
std::unordered_map<wp<IBinder>, const sp<gui::IWindowInfosListener>, WpHash>
mWindowInfosListeners GUARDED_BY(mListenersMutex);
sp<gui::IWindowInfosReportedListener> mWindowInfosReportedListener;
std::atomic<size_t> mCallbacksPending{0};
};
-} // namespace android
\ No newline at end of file
+
+} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 6c96d5f..dee2358 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -143,11 +143,10 @@
.WillRepeatedly(Return(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD));
EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0));
- constexpr scheduler::ISchedulerCallback* kCallback = nullptr;
- constexpr bool kHasMultipleConfigs = true;
mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker),
- std::move(eventThread), std::move(sfEventThread), kCallback,
- kHasMultipleConfigs);
+ std::move(eventThread), std::move(sfEventThread),
+ TestableSurfaceFlinger::SchedulerCallbackImpl::kNoOp,
+ TestableSurfaceFlinger::kTwoDisplayModes);
}
void setupForceGeometryDirty() {
diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
index 5a0033e..40a9b1a 100644
--- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp
@@ -44,8 +44,8 @@
mFlinger.onComposerHalHotplug(PrimaryDisplayVariant::HWC_DISPLAY_ID, Connection::CONNECTED);
mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this)
- .setSupportedModes({kDisplayMode60, kDisplayMode90, kDisplayMode120})
- .setActiveMode(kDisplayModeId60)
+ .setDisplayModes({kDisplayMode60, kDisplayMode90, kDisplayMode120},
+ kDisplayModeId60)
.inject();
}
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index c318e28..2425862 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -77,7 +77,8 @@
mFlinger.setupScheduler(std::unique_ptr<scheduler::VsyncController>(mVsyncController),
std::unique_ptr<scheduler::VSyncTracker>(mVSyncTracker),
std::unique_ptr<EventThread>(mEventThread),
- std::unique_ptr<EventThread>(mSFEventThread), &mSchedulerCallback);
+ std::unique_ptr<EventThread>(mSFEventThread),
+ TestableSurfaceFlinger::SchedulerCallbackImpl::kMock);
}
void DisplayTransactionTest::injectMockComposer(int virtualDisplayCount) {
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
index 69ac26e..45eceff 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
@@ -48,7 +48,6 @@
#include "mock/DisplayHardware/MockPowerAdvisor.h"
#include "mock/MockEventThread.h"
#include "mock/MockNativeWindowSurface.h"
-#include "mock/MockSchedulerCallback.h"
#include "mock/MockSurfaceInterceptor.h"
#include "mock/MockVsyncController.h"
#include "mock/system/window/MockNativeWindow.h"
@@ -121,7 +120,6 @@
mock::VsyncController* mVsyncController = new mock::VsyncController;
mock::VSyncTracker* mVSyncTracker = new mock::VSyncTracker;
- scheduler::mock::SchedulerCallback mSchedulerCallback;
mock::EventThread* mEventThread = new mock::EventThread;
mock::EventThread* mSFEventThread = new mock::EventThread;
diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
index fe5f9e0..2b69f13 100644
--- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp
@@ -164,8 +164,9 @@
.WillRepeatedly(Return(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD));
EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0));
mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker),
- std::move(eventThread), std::move(sfEventThread), /*callback*/ nullptr,
- /*hasMultipleModes*/ true);
+ std::move(eventThread), std::move(sfEventThread),
+ TestableSurfaceFlinger::SchedulerCallbackImpl::kNoOp,
+ TestableSurfaceFlinger::kTwoDisplayModes);
}
namespace {
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index 56a0506..3205952 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -42,13 +42,18 @@
mFlinger.onComposerHalHotplug(PrimaryDisplayVariant::HWC_DISPLAY_ID, Connection::CONNECTED);
- mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this)
- .setSupportedModes({kDisplayMode60, kDisplayMode90, kDisplayMode120,
- kDisplayMode90DifferentResolution})
- .setActiveMode(kDisplayModeId60)
- .inject();
+ {
+ DisplayModes modes = {kDisplayMode60, kDisplayMode90, kDisplayMode120,
+ kDisplayMode90DifferentResolution};
+ const DisplayModeId activeModeId = kDisplayModeId60;
+ auto configs = std::make_shared<scheduler::RefreshRateConfigs>(modes, activeModeId);
- setupScheduler();
+ mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this)
+ .setDisplayModes(modes, activeModeId, std::move(configs))
+ .inject();
+ }
+
+ setupScheduler(mDisplay->holdRefreshRateConfigs());
// isVsyncPeriodSwitchSupported should return true, otherwise the SF's HWC proxy
// will call setActiveConfig instead of setActiveConfigWithConstraints.
@@ -57,7 +62,7 @@
}
protected:
- void setupScheduler();
+ void setupScheduler(std::shared_ptr<scheduler::RefreshRateConfigs>);
void testChangeRefreshRate(bool isDisplayActive, bool isRefreshRequired);
sp<DisplayDevice> mDisplay;
@@ -108,7 +113,8 @@
.build();
};
-void DisplayModeSwitchingTest::setupScheduler() {
+void DisplayModeSwitchingTest::setupScheduler(
+ std::shared_ptr<scheduler::RefreshRateConfigs> configs) {
auto eventThread = std::make_unique<mock::EventThread>();
mAppEventThread = eventThread.get();
auto sfEventThread = std::make_unique<mock::EventThread>();
@@ -132,8 +138,9 @@
Return(TestableSurfaceFlinger::FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD));
EXPECT_CALL(*vsyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0));
mFlinger.setupScheduler(std::move(vsyncController), std::move(vsyncTracker),
- std::move(eventThread), std::move(sfEventThread), /*callback*/ nullptr,
- /*hasMultipleModes*/ true);
+ std::move(eventThread), std::move(sfEventThread),
+ TestableSurfaceFlinger::SchedulerCallbackImpl::kNoOp,
+ std::move(configs));
}
TEST_F(DisplayModeSwitchingTest, changeRefreshRate_OnActiveDisplay_WithRefreshRequired) {
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
index b57feff..7948e60 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
@@ -61,7 +61,7 @@
struct EventThreadBaseSupportedVariant {
static void setupVsyncAndEventThreadNoCallExpectations(DisplayTransactionTest* test) {
// The callback should not be notified to toggle VSYNC.
- EXPECT_CALL(test->mSchedulerCallback, setVsyncEnabled(_)).Times(0);
+ EXPECT_CALL(test->mFlinger.mockSchedulerCallback(), setVsyncEnabled(_)).Times(0);
// The event thread should not be notified.
EXPECT_CALL(*test->mEventThread, onScreenReleased()).Times(0);
@@ -88,7 +88,7 @@
struct EventThreadIsSupportedVariant : public EventThreadBaseSupportedVariant {
static void setupAcquireAndEnableVsyncCallExpectations(DisplayTransactionTest* test) {
// The callback should be notified to enable VSYNC.
- EXPECT_CALL(test->mSchedulerCallback, setVsyncEnabled(true)).Times(1);
+ EXPECT_CALL(test->mFlinger.mockSchedulerCallback(), setVsyncEnabled(true)).Times(1);
// The event thread should be notified that the screen was acquired.
EXPECT_CALL(*test->mEventThread, onScreenAcquired()).Times(1);
@@ -96,7 +96,7 @@
static void setupReleaseAndDisableVsyncCallExpectations(DisplayTransactionTest* test) {
// The callback should be notified to disable VSYNC.
- EXPECT_CALL(test->mSchedulerCallback, setVsyncEnabled(false)).Times(1);
+ EXPECT_CALL(test->mFlinger.mockSchedulerCallback(), setVsyncEnabled(false)).Times(1);
// The event thread should not be notified that the screen was released.
EXPECT_CALL(*test->mEventThread, onScreenReleased()).Times(1);
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 361d629..d292e08 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -16,6 +16,9 @@
#pragma once
+#include <algorithm>
+#include <variant>
+
#include <compositionengine/Display.h>
#include <compositionengine/LayerFECompositionState.h>
#include <compositionengine/OutputLayer.h>
@@ -24,7 +27,6 @@
#include <compositionengine/impl/OutputLayerCompositionState.h>
#include <compositionengine/mock/DisplaySurface.h>
#include <gui/ScreenCaptureResults.h>
-#include <algorithm>
#include "BufferQueueLayer.h"
#include "BufferStateLayer.h"
@@ -45,6 +47,7 @@
#include "mock/DisplayHardware/MockComposer.h"
#include "mock/MockFrameTimeline.h"
#include "mock/MockFrameTracer.h"
+#include "mock/MockSchedulerCallback.h"
namespace android {
@@ -170,7 +173,7 @@
} // namespace surfaceflinger::test
-class TestableSurfaceFlinger final : private scheduler::ISchedulerCallback {
+class TestableSurfaceFlinger {
public:
using HotplugEvent = SurfaceFlinger::HotplugEvent;
@@ -193,42 +196,64 @@
mFlinger->mCompositionEngine->setTimeStats(timeStats);
}
- // The ISchedulerCallback argument can be nullptr for a no-op implementation.
+ enum class SchedulerCallbackImpl { kNoOp, kMock };
+
+ static constexpr struct OneDisplayMode {
+ } kOneDisplayMode;
+
+ static constexpr struct TwoDisplayModes {
+ } kTwoDisplayModes;
+
+ using RefreshRateConfigsPtr = std::shared_ptr<scheduler::RefreshRateConfigs>;
+
+ using DisplayModesVariant =
+ std::variant<OneDisplayMode, TwoDisplayModes, RefreshRateConfigsPtr>;
+
void setupScheduler(std::unique_ptr<scheduler::VsyncController> vsyncController,
std::unique_ptr<scheduler::VSyncTracker> vsyncTracker,
std::unique_ptr<EventThread> appEventThread,
std::unique_ptr<EventThread> sfEventThread,
- scheduler::ISchedulerCallback* callback = nullptr,
- bool hasMultipleModes = false) {
- DisplayModes modes{DisplayMode::Builder(0)
- .setId(DisplayModeId(0))
- .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0))
- .setVsyncPeriod(16'666'667)
- .setGroup(0)
- .build()};
+ SchedulerCallbackImpl callbackImpl = SchedulerCallbackImpl::kNoOp,
+ DisplayModesVariant modesVariant = kOneDisplayMode) {
+ RefreshRateConfigsPtr configs;
+ if (std::holds_alternative<RefreshRateConfigsPtr>(modesVariant)) {
+ configs = std::move(std::get<RefreshRateConfigsPtr>(modesVariant));
+ } else {
+ DisplayModes modes = {DisplayMode::Builder(0)
+ .setId(DisplayModeId(0))
+ .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0))
+ .setVsyncPeriod(16'666'667)
+ .setGroup(0)
+ .build()};
- if (hasMultipleModes) {
- modes.emplace_back(DisplayMode::Builder(1)
- .setId(DisplayModeId(1))
- .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0))
- .setVsyncPeriod(11'111'111)
- .setGroup(0)
- .build());
+ if (std::holds_alternative<TwoDisplayModes>(modesVariant)) {
+ modes.emplace_back(DisplayMode::Builder(1)
+ .setId(DisplayModeId(1))
+ .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0))
+ .setVsyncPeriod(11'111'111)
+ .setGroup(0)
+ .build());
+ }
+
+ configs = std::make_shared<scheduler::RefreshRateConfigs>(modes, DisplayModeId(0));
}
- const auto currMode = DisplayModeId(0);
- mRefreshRateConfigs = std::make_shared<scheduler::RefreshRateConfigs>(modes, currMode);
- const auto currFps = mRefreshRateConfigs->getCurrentRefreshRate().getFps();
+ const auto currFps = configs->getCurrentRefreshRate().getFps();
mFlinger->mVsyncConfiguration = mFactory.createVsyncConfiguration(currFps);
mFlinger->mVsyncModulator = sp<scheduler::VsyncModulator>::make(
mFlinger->mVsyncConfiguration->getCurrentConfigs());
mFlinger->mRefreshRateStats =
std::make_unique<scheduler::RefreshRateStats>(*mFlinger->mTimeStats, currFps,
- /*powerMode=*/hal::PowerMode::OFF);
+ hal::PowerMode::OFF);
+
+ using Callback = scheduler::ISchedulerCallback;
+ Callback& callback = callbackImpl == SchedulerCallbackImpl::kNoOp
+ ? static_cast<Callback&>(mNoOpSchedulerCallback)
+ : static_cast<Callback&>(mSchedulerCallback);
mScheduler = new scheduler::TestableScheduler(std::move(vsyncController),
- std::move(vsyncTracker), mRefreshRateConfigs,
- *(callback ?: this));
+ std::move(vsyncTracker), std::move(configs),
+ callback);
mFlinger->mAppConnectionHandle = mScheduler->createConnection(std::move(appEventThread));
mFlinger->mSfConnectionHandle = mScheduler->createConnection(std::move(sfEventThread));
@@ -237,7 +262,8 @@
void resetScheduler(scheduler::Scheduler* scheduler) { mFlinger->mScheduler.reset(scheduler); }
- scheduler::TestableScheduler& mutableScheduler() const { return *mScheduler; }
+ scheduler::TestableScheduler& mutableScheduler() { return *mScheduler; }
+ scheduler::mock::SchedulerCallback& mockSchedulerCallback() { return mSchedulerCallback; }
using CreateBufferQueueFunction = surfaceflinger::test::Factory::CreateBufferQueueFunction;
void setCreateBufferQueueFunction(CreateBufferQueueFunction f) {
@@ -662,23 +688,6 @@
mHwcDisplayId(hwcDisplayId) {
mCreationArgs.connectionType = connectionType;
mCreationArgs.isPrimary = isPrimary;
-
- mCreationArgs.activeModeId = DisplayModeId(0);
- DisplayModePtr activeMode =
- DisplayMode::Builder(FakeHwcDisplayInjector::DEFAULT_ACTIVE_CONFIG)
- .setId(mCreationArgs.activeModeId)
- .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0))
- .setWidth(FakeHwcDisplayInjector::DEFAULT_WIDTH)
- .setHeight(FakeHwcDisplayInjector::DEFAULT_HEIGHT)
- .setVsyncPeriod(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD)
- .setDpiX(FakeHwcDisplayInjector::DEFAULT_DPI)
- .setDpiY(FakeHwcDisplayInjector::DEFAULT_DPI)
- .setGroup(0)
- .build();
-
- DisplayModes modes{activeMode};
- mCreationArgs.supportedModes = modes;
- mCreationArgs.refreshRateConfigs = flinger.mRefreshRateConfigs;
}
sp<IBinder> token() const { return mDisplayToken; }
@@ -701,13 +710,16 @@
auto& mutableDisplayDevice() { return mFlinger.mutableDisplays()[mDisplayToken]; }
- auto& setActiveMode(DisplayModeId mode) {
- mCreationArgs.activeModeId = mode;
- return *this;
- }
-
- auto& setSupportedModes(DisplayModes mode) {
- mCreationArgs.supportedModes = mode;
+ // If `configs` is nullptr, the injector creates RefreshRateConfigs from the `modes`.
+ // Otherwise, it uses `configs`, which the caller must create using the same `modes`.
+ //
+ // TODO(b/182939859): Once `modes` can be retrieved from RefreshRateConfigs, remove
+ // the `configs` parameter in favor of an alternative setRefreshRateConfigs API.
+ auto& setDisplayModes(DisplayModes modes, DisplayModeId activeModeId,
+ std::shared_ptr<scheduler::RefreshRateConfigs> configs = nullptr) {
+ mCreationArgs.supportedModes = std::move(modes);
+ mCreationArgs.activeModeId = activeModeId;
+ mCreationArgs.refreshRateConfigs = std::move(configs);
return *this;
}
@@ -749,39 +761,58 @@
}
sp<DisplayDevice> inject() NO_THREAD_SAFETY_ANALYSIS {
- const auto displayId = mCreationArgs.compositionDisplay->getDisplayId();
+ auto& modes = mCreationArgs.supportedModes;
+ auto& activeModeId = mCreationArgs.activeModeId;
+
+ if (!mCreationArgs.refreshRateConfigs) {
+ if (modes.empty()) {
+ activeModeId = DisplayModeId(0);
+ modes.emplace_back(
+ DisplayMode::Builder(FakeHwcDisplayInjector::DEFAULT_ACTIVE_CONFIG)
+ .setId(activeModeId)
+ .setPhysicalDisplayId(PhysicalDisplayId::fromPort(0))
+ .setWidth(FakeHwcDisplayInjector::DEFAULT_WIDTH)
+ .setHeight(FakeHwcDisplayInjector::DEFAULT_HEIGHT)
+ .setVsyncPeriod(FakeHwcDisplayInjector::DEFAULT_VSYNC_PERIOD)
+ .setDpiX(FakeHwcDisplayInjector::DEFAULT_DPI)
+ .setDpiY(FakeHwcDisplayInjector::DEFAULT_DPI)
+ .setGroup(0)
+ .build());
+ }
+
+ mCreationArgs.refreshRateConfigs =
+ std::make_shared<scheduler::RefreshRateConfigs>(modes, activeModeId);
+ }
DisplayDeviceState state;
if (const auto type = mCreationArgs.connectionType) {
+ const auto displayId = mCreationArgs.compositionDisplay->getDisplayId();
LOG_ALWAYS_FATAL_IF(!displayId);
const auto physicalId = PhysicalDisplayId::tryCast(*displayId);
LOG_ALWAYS_FATAL_IF(!physicalId);
LOG_ALWAYS_FATAL_IF(!mHwcDisplayId);
- const DisplayModePtr activeModePtr =
- *std::find_if(mCreationArgs.supportedModes.begin(),
- mCreationArgs.supportedModes.end(), [&](DisplayModePtr mode) {
- return mode->getId() == mCreationArgs.activeModeId;
- });
+ const auto it = std::find_if(modes.begin(), modes.end(),
+ [&activeModeId](const DisplayModePtr& mode) {
+ return mode->getId() == activeModeId;
+ });
+ LOG_ALWAYS_FATAL_IF(it == modes.end());
+
state.physical = {.id = *physicalId,
.type = *type,
.hwcDisplayId = *mHwcDisplayId,
.deviceProductInfo = {},
- .supportedModes = mCreationArgs.supportedModes,
- .activeMode = activeModePtr};
+ .supportedModes = modes,
+ .activeMode = *it};
}
state.isSecure = mCreationArgs.isSecure;
- mCreationArgs.refreshRateConfigs =
- std::make_shared<scheduler::RefreshRateConfigs>(mCreationArgs.supportedModes,
- mCreationArgs.activeModeId);
-
- sp<DisplayDevice> device = new DisplayDevice(mCreationArgs);
- if (!device->isVirtual()) {
- device->setActiveMode(mCreationArgs.activeModeId);
+ sp<DisplayDevice> display = sp<DisplayDevice>::make(mCreationArgs);
+ if (!display->isVirtual()) {
+ display->setActiveMode(activeModeId);
}
- mFlinger.mutableDisplays().emplace(mDisplayToken, device);
+ mFlinger.mutableDisplays().emplace(mDisplayToken, display);
mFlinger.mutableCurrentState().displays.add(mDisplayToken, state);
mFlinger.mutableDrawingState().displays.add(mDisplayToken, state);
@@ -789,7 +820,7 @@
mFlinger.mutablePhysicalDisplayTokens()[physical->id] = mDisplayToken;
}
- return device;
+ return display;
}
private:
@@ -800,16 +831,12 @@
};
private:
- void scheduleComposite(FrameHint) override {}
- void setVsyncEnabled(bool) override {}
- void changeRefreshRate(const RefreshRate&, DisplayModeEvent) override {}
- void kernelTimerChanged(bool) override {}
- void triggerOnFrameRateOverridesChanged() {}
-
surfaceflinger::test::Factory mFactory;
sp<SurfaceFlinger> mFlinger = new SurfaceFlinger(mFactory, SurfaceFlinger::SkipInitialization);
+
+ scheduler::mock::SchedulerCallback mSchedulerCallback;
+ scheduler::mock::NoOpSchedulerCallback mNoOpSchedulerCallback;
scheduler::TestableScheduler* mScheduler = nullptr;
- std::shared_ptr<scheduler::RefreshRateConfigs> mRefreshRateConfigs;
};
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
index 849e308..c90b8ed 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h
@@ -35,7 +35,7 @@
void setVsyncEnabled(bool) override {}
void changeRefreshRate(const RefreshRate&, DisplayModeEvent) override {}
void kernelTimerChanged(bool) override {}
- void triggerOnFrameRateOverridesChanged() {}
+ void triggerOnFrameRateOverridesChanged() override {}
};
} // namespace android::scheduler::mock