SF: Clean up Scheduler
Remove dynamic allocation for Scheduler::{Connection,ConnectionHandle},
as well as ref-counting for the latter. Also, remove dead code and make
members private.
Bug: 130554049
Test: libsurfaceflinger_unittest
Change-Id: Ibb9dc8d4cb66451a4172c852a36032bbc0a54411
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 82dd3c7..2e64a78 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -124,7 +124,16 @@
auto sfEventThread = std::make_unique<mock::EventThread>();
EXPECT_CALL(*eventThread, registerDisplayEventConnection(_));
+ EXPECT_CALL(*eventThread, createEventConnection(_, _))
+ .WillOnce(Return(
+ new EventThreadConnection(eventThread.get(), ResyncCallback(),
+ ISurfaceComposer::eConfigChangedSuppress)));
+
EXPECT_CALL(*sfEventThread, registerDisplayEventConnection(_));
+ EXPECT_CALL(*sfEventThread, createEventConnection(_, _))
+ .WillOnce(Return(
+ new EventThreadConnection(sfEventThread.get(), ResyncCallback(),
+ ISurfaceComposer::eConfigChangedSuppress)));
auto primaryDispSync = std::make_unique<mock::DispSync>();
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index 8f6f3ec..c858cc0 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -179,7 +179,14 @@
void DisplayTransactionTest::injectMockScheduler() {
EXPECT_CALL(*mEventThread, registerDisplayEventConnection(_));
+ EXPECT_CALL(*mEventThread, createEventConnection(_, _))
+ .WillOnce(Return(new EventThreadConnection(mEventThread, ResyncCallback(),
+ ISurfaceComposer::eConfigChangedSuppress)));
+
EXPECT_CALL(*mSFEventThread, registerDisplayEventConnection(_));
+ EXPECT_CALL(*mSFEventThread, createEventConnection(_, _))
+ .WillOnce(Return(new EventThreadConnection(mSFEventThread, ResyncCallback(),
+ ISurfaceComposer::eConfigChangedSuppress)));
mFlinger.setupScheduler(std::unique_ptr<DispSync>(mPrimaryDispSync),
std::unique_ptr<EventControlThread>(mEventControlThread),
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 740115e..ebcb9d8 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -3,14 +3,13 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
-
#include <log/log.h>
#include <mutex>
#include "Scheduler/EventControlThread.h"
#include "Scheduler/EventThread.h"
-#include "Scheduler/Scheduler.h"
+#include "TestableScheduler.h"
#include "mock/MockEventThread.h"
using testing::_;
@@ -34,37 +33,14 @@
MOCK_METHOD0(requestNextVsync, void());
};
- scheduler::RefreshRateConfigs mRefreshRateConfigs;
-
- /**
- * This mock Scheduler class uses implementation of mock::EventThread but keeps everything else
- * the same.
- */
- class MockScheduler : public android::Scheduler {
- public:
- MockScheduler(const scheduler::RefreshRateConfigs& refreshRateConfigs,
- std::unique_ptr<EventThread> eventThread)
- : Scheduler([](bool) {}, refreshRateConfigs), mEventThread(std::move(eventThread)) {}
-
- std::unique_ptr<EventThread> makeEventThread(
- const char* /* connectionName */, DispSync* /* dispSync */,
- nsecs_t /* phaseOffsetNs */, nsecs_t /* offsetThresholdForNextVsync */,
- impl::EventThread::InterceptVSyncsCallback /* interceptCallback */) override {
- return std::move(mEventThread);
- }
-
- MockScheduler() = default;
- ~MockScheduler() override = default;
-
- std::unique_ptr<EventThread> mEventThread;
- };
-
SchedulerTest();
~SchedulerTest() override;
- sp<Scheduler::ConnectionHandle> mConnectionHandle;
+ scheduler::RefreshRateConfigs mRefreshRateConfigs;
+ TestableScheduler mScheduler{mRefreshRateConfigs};
+
+ Scheduler::ConnectionHandle mConnectionHandle;
mock::EventThread* mEventThread;
- std::unique_ptr<MockScheduler> mScheduler;
sp<MockEventThreadConnection> mEventThreadConnection;
};
@@ -73,9 +49,8 @@
::testing::UnitTest::GetInstance()->current_test_info();
ALOGD("**** Setting up for %s.%s\n", test_info->test_case_name(), test_info->name());
- std::unique_ptr<mock::EventThread> eventThread = std::make_unique<mock::EventThread>();
+ auto eventThread = std::make_unique<mock::EventThread>();
mEventThread = eventThread.get();
- mScheduler = std::make_unique<MockScheduler>(mRefreshRateConfigs, std::move(eventThread));
EXPECT_CALL(*mEventThread, registerDisplayEventConnection(_)).WillOnce(Return(0));
mEventThreadConnection = new MockEventThreadConnection(mEventThread);
@@ -85,9 +60,8 @@
EXPECT_CALL(*mEventThread, createEventConnection(_, _))
.WillRepeatedly(Return(mEventThreadConnection));
- mConnectionHandle = mScheduler->createConnection("appConnection", 16, 16, ResyncCallback(),
- impl::EventThread::InterceptVSyncsCallback());
- EXPECT_TRUE(mConnectionHandle != nullptr);
+ mConnectionHandle = mScheduler.createConnection(std::move(eventThread));
+ EXPECT_TRUE(mConnectionHandle);
}
SchedulerTest::~SchedulerTest() {
@@ -101,92 +75,67 @@
* Test cases
*/
-TEST_F(SchedulerTest, testNullPtr) {
- // Passing a null pointer for ConnectionHandle is a valid argument. The code doesn't throw any
- // exceptions, just gracefully continues.
- sp<IDisplayEventConnection> returnedValue;
- ASSERT_NO_FATAL_FAILURE(
- returnedValue =
- mScheduler->createDisplayEventConnection(nullptr, ResyncCallback(),
- ISurfaceComposer::
- eConfigChangedSuppress));
- EXPECT_TRUE(returnedValue == nullptr);
- EXPECT_TRUE(mScheduler->getEventThread(nullptr) == nullptr);
- EXPECT_TRUE(mScheduler->getEventConnection(nullptr) == nullptr);
- ASSERT_NO_FATAL_FAILURE(mScheduler->hotplugReceived(nullptr, PHYSICAL_DISPLAY_ID, false));
- ASSERT_NO_FATAL_FAILURE(mScheduler->onScreenAcquired(nullptr));
- ASSERT_NO_FATAL_FAILURE(mScheduler->onScreenReleased(nullptr));
- std::string testString;
- ASSERT_NO_FATAL_FAILURE(mScheduler->dump(nullptr, testString));
- EXPECT_TRUE(testString == "");
- ASSERT_NO_FATAL_FAILURE(mScheduler->setPhaseOffset(nullptr, 10));
-}
-
TEST_F(SchedulerTest, invalidConnectionHandle) {
- // Passing an invalid ConnectionHandle is a valid argument. The code doesn't throw any
- // exceptions, just gracefully continues.
- sp<Scheduler::ConnectionHandle> connectionHandle = new Scheduler::ConnectionHandle(20);
+ Scheduler::ConnectionHandle handle;
- sp<IDisplayEventConnection> returnedValue;
+ sp<IDisplayEventConnection> connection;
ASSERT_NO_FATAL_FAILURE(
- returnedValue =
- mScheduler->createDisplayEventConnection(connectionHandle, ResyncCallback(),
- ISurfaceComposer::
- eConfigChangedSuppress));
- EXPECT_TRUE(returnedValue == nullptr);
- EXPECT_TRUE(mScheduler->getEventThread(connectionHandle) == nullptr);
- EXPECT_TRUE(mScheduler->getEventConnection(connectionHandle) == nullptr);
+ connection = mScheduler.createDisplayEventConnection(handle, ResyncCallback(),
+ ISurfaceComposer::
+ eConfigChangedSuppress));
+ EXPECT_FALSE(connection);
+ EXPECT_FALSE(mScheduler.getEventThread(handle));
+ EXPECT_FALSE(mScheduler.getEventConnection(handle));
// The EXPECT_CALLS make sure we don't call the functions on the subsequent event threads.
EXPECT_CALL(*mEventThread, onHotplugReceived(_, _)).Times(0);
- ASSERT_NO_FATAL_FAILURE(
- mScheduler->hotplugReceived(connectionHandle, PHYSICAL_DISPLAY_ID, false));
+ ASSERT_NO_FATAL_FAILURE(mScheduler.onHotplugReceived(handle, PHYSICAL_DISPLAY_ID, false));
EXPECT_CALL(*mEventThread, onScreenAcquired()).Times(0);
- ASSERT_NO_FATAL_FAILURE(mScheduler->onScreenAcquired(connectionHandle));
+ ASSERT_NO_FATAL_FAILURE(mScheduler.onScreenAcquired(handle));
EXPECT_CALL(*mEventThread, onScreenReleased()).Times(0);
- ASSERT_NO_FATAL_FAILURE(mScheduler->onScreenReleased(connectionHandle));
+ ASSERT_NO_FATAL_FAILURE(mScheduler.onScreenReleased(handle));
- std::string testString;
+ std::string output;
EXPECT_CALL(*mEventThread, dump(_)).Times(0);
- ASSERT_NO_FATAL_FAILURE(mScheduler->dump(connectionHandle, testString));
- EXPECT_TRUE(testString == "");
+ ASSERT_NO_FATAL_FAILURE(mScheduler.dump(handle, output));
+ EXPECT_TRUE(output.empty());
EXPECT_CALL(*mEventThread, setPhaseOffset(_)).Times(0);
- ASSERT_NO_FATAL_FAILURE(mScheduler->setPhaseOffset(connectionHandle, 10));
+ ASSERT_NO_FATAL_FAILURE(mScheduler.setPhaseOffset(handle, 10));
}
TEST_F(SchedulerTest, validConnectionHandle) {
- sp<IDisplayEventConnection> returnedValue;
+ sp<IDisplayEventConnection> connection;
ASSERT_NO_FATAL_FAILURE(
- returnedValue =
- mScheduler->createDisplayEventConnection(mConnectionHandle, ResyncCallback(),
- ISurfaceComposer::
- eConfigChangedSuppress));
- EXPECT_TRUE(returnedValue != nullptr);
- ASSERT_EQ(returnedValue, mEventThreadConnection);
+ connection =
+ mScheduler.createDisplayEventConnection(mConnectionHandle, ResyncCallback(),
+ ISurfaceComposer::
+ eConfigChangedSuppress));
+ ASSERT_EQ(mEventThreadConnection, connection);
- EXPECT_TRUE(mScheduler->getEventThread(mConnectionHandle) != nullptr);
- EXPECT_TRUE(mScheduler->getEventConnection(mConnectionHandle) != nullptr);
+ EXPECT_TRUE(mScheduler.getEventThread(mConnectionHandle));
+ EXPECT_TRUE(mScheduler.getEventConnection(mConnectionHandle));
EXPECT_CALL(*mEventThread, onHotplugReceived(PHYSICAL_DISPLAY_ID, false)).Times(1);
ASSERT_NO_FATAL_FAILURE(
- mScheduler->hotplugReceived(mConnectionHandle, PHYSICAL_DISPLAY_ID, false));
+ mScheduler.onHotplugReceived(mConnectionHandle, PHYSICAL_DISPLAY_ID, false));
EXPECT_CALL(*mEventThread, onScreenAcquired()).Times(1);
- ASSERT_NO_FATAL_FAILURE(mScheduler->onScreenAcquired(mConnectionHandle));
+ ASSERT_NO_FATAL_FAILURE(mScheduler.onScreenAcquired(mConnectionHandle));
EXPECT_CALL(*mEventThread, onScreenReleased()).Times(1);
- ASSERT_NO_FATAL_FAILURE(mScheduler->onScreenReleased(mConnectionHandle));
+ ASSERT_NO_FATAL_FAILURE(mScheduler.onScreenReleased(mConnectionHandle));
- std::string testString("dump");
- EXPECT_CALL(*mEventThread, dump(testString)).Times(1);
- ASSERT_NO_FATAL_FAILURE(mScheduler->dump(mConnectionHandle, testString));
- EXPECT_TRUE(testString != "");
+ std::string output("dump");
+ EXPECT_CALL(*mEventThread, dump(output)).Times(1);
+ ASSERT_NO_FATAL_FAILURE(mScheduler.dump(mConnectionHandle, output));
+ EXPECT_FALSE(output.empty());
EXPECT_CALL(*mEventThread, setPhaseOffset(10)).Times(1);
- ASSERT_NO_FATAL_FAILURE(mScheduler->setPhaseOffset(mConnectionHandle, 10));
+ ASSERT_NO_FATAL_FAILURE(mScheduler.setPhaseOffset(mConnectionHandle, 10));
}
+
} // namespace
} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index 5157cc4..780b608 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -19,6 +19,7 @@
#include <gmock/gmock.h>
#include <gui/ISurfaceComposer.h>
+#include "Scheduler/DispSync.h"
#include "Scheduler/EventThread.h"
#include "Scheduler/Scheduler.h"
@@ -26,24 +27,17 @@
class TestableScheduler : public Scheduler {
public:
+ explicit TestableScheduler(const scheduler::RefreshRateConfigs& configs)
+ : Scheduler([](bool) {}, configs) {}
+
TestableScheduler(std::unique_ptr<DispSync> primaryDispSync,
std::unique_ptr<EventControlThread> eventControlThread,
const scheduler::RefreshRateConfigs& configs)
: Scheduler(std::move(primaryDispSync), std::move(eventControlThread), configs) {}
- // Creates EventThreadConnection with the given eventThread. Creates Scheduler::Connection
- // and adds it to the list of connectins. Returns the ConnectionHandle for the
- // Scheduler::Connection. This allows plugging in mock::EventThread.
- sp<Scheduler::ConnectionHandle> addConnection(std::unique_ptr<EventThread> eventThread) {
- sp<EventThreadConnection> eventThreadConnection =
- new EventThreadConnection(eventThread.get(), ResyncCallback(),
- ISurfaceComposer::eConfigChangedSuppress);
- const int64_t id = sNextId++;
- mConnections.emplace(id,
- std::make_unique<Scheduler::Connection>(new ConnectionHandle(id),
- eventThreadConnection,
- std::move(eventThread)));
- return mConnections[id]->handle;
+ // Used to inject mock event thread.
+ ConnectionHandle createConnection(std::unique_ptr<EventThread> eventThread) {
+ return Scheduler::createConnection(std::move(eventThread), ResyncCallback());
}
/* ------------------------------------------------------------------------
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 27a119b..9536dd1 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -61,7 +61,7 @@
public:
~Factory() = default;
- std::unique_ptr<DispSync> createDispSync(const char*, bool, int64_t) override {
+ std::unique_ptr<DispSync> createDispSync(const char*, bool) override {
// TODO: Use test-fixture controlled factory
return nullptr;
}
@@ -198,8 +198,8 @@
new TestableScheduler(std::move(primaryDispSync), std::move(eventControlThread),
mFlinger->mRefreshRateConfigs);
- mFlinger->mAppConnectionHandle = mScheduler->addConnection(std::move(appEventThread));
- mFlinger->mSfConnectionHandle = mScheduler->addConnection(std::move(sfEventThread));
+ mFlinger->mAppConnectionHandle = mScheduler->createConnection(std::move(appEventThread));
+ mFlinger->mSfConnectionHandle = mScheduler->createConnection(std::move(sfEventThread));
mFlinger->mScheduler.reset(mScheduler);
mFlinger->mVSyncModulator.emplace(*mScheduler, mFlinger->mAppConnectionHandle,