SF: Cleanup EventThread Part 2
De-refbase EventThread and a bunch of related classes. Convert from
usign StrongPointer to std::unique_ptr to hold owned references, or bare
pointers for unowned references.
I did not see any need for using std::shared_ptr, or anything else, as
SurfaceFlinger appeared to own all the objects, and they were created
once and not really destroyed afterwards.
Test: Things seem to still work on a Pixel XL
Bug: None
Change-Id: Ifff32118d31bc1bb51e38df695ebe5cf86d2bb6d
diff --git a/services/surfaceflinger/DispSync.cpp b/services/surfaceflinger/DispSync.cpp
index b5fc365..9e01fd0 100644
--- a/services/surfaceflinger/DispSync.cpp
+++ b/services/surfaceflinger/DispSync.cpp
@@ -168,8 +168,7 @@
return false;
}
- status_t addEventListener(const char* name, nsecs_t phase,
- const sp<DispSync::Callback>& callback) {
+ status_t addEventListener(const char* name, nsecs_t phase, DispSync::Callback* callback) {
if (kTraceDetailedInfo) ATRACE_CALL();
Mutex::Autolock lock(mMutex);
@@ -195,7 +194,7 @@
return NO_ERROR;
}
- status_t removeEventListener(const sp<DispSync::Callback>& callback) {
+ status_t removeEventListener(DispSync::Callback* callback) {
if (kTraceDetailedInfo) ATRACE_CALL();
Mutex::Autolock lock(mMutex);
@@ -223,11 +222,11 @@
const char* mName;
nsecs_t mPhase;
nsecs_t mLastEventTime;
- sp<DispSync::Callback> mCallback;
+ DispSync::Callback* mCallback;
};
struct CallbackInvocation {
- sp<DispSync::Callback> mCallback;
+ DispSync::Callback* mCallback;
nsecs_t mEventTime;
};
@@ -388,7 +387,8 @@
// not needed because any time there is an event registered we will
// turn on the HW vsync events.
if (!mIgnorePresentFences && kEnableZeroPhaseTracer) {
- addEventListener("ZeroPhaseTracer", 0, new ZeroPhaseTracer());
+ mZeroPhaseTracer = std::make_unique<ZeroPhaseTracer>();
+ addEventListener("ZeroPhaseTracer", 0, mZeroPhaseTracer.get());
}
}
}
@@ -470,7 +470,7 @@
void DispSync::endResync() {}
-status_t DispSync::addEventListener(const char* name, nsecs_t phase, const sp<Callback>& callback) {
+status_t DispSync::addEventListener(const char* name, nsecs_t phase, Callback* callback) {
Mutex::Autolock lock(mMutex);
return mThread->addEventListener(name, phase, callback);
}
@@ -482,7 +482,7 @@
updateModelLocked();
}
-status_t DispSync::removeEventListener(const sp<Callback>& callback) {
+status_t DispSync::removeEventListener(Callback* callback) {
Mutex::Autolock lock(mMutex);
return mThread->removeEventListener(callback);
}
diff --git a/services/surfaceflinger/DispSync.h b/services/surfaceflinger/DispSync.h
index d066d55..9336f4d 100644
--- a/services/surfaceflinger/DispSync.h
+++ b/services/surfaceflinger/DispSync.h
@@ -48,7 +48,7 @@
// needed.
class DispSync {
public:
- class Callback : public virtual RefBase {
+ class Callback {
public:
virtual ~Callback(){};
virtual void onDispSyncEvent(nsecs_t when) = 0;
@@ -106,12 +106,12 @@
// given phase offset from the hardware vsync events. The callback is
// called from a separate thread and it should return reasonably quickly
// (i.e. within a few hundred microseconds).
- status_t addEventListener(const char* name, nsecs_t phase, const sp<Callback>& callback);
+ status_t addEventListener(const char* name, nsecs_t phase, Callback* callback);
// removeEventListener removes an already-registered event callback. Once
// this method returns that callback will no longer be called by the
// DispSync object.
- status_t removeEventListener(const sp<Callback>& callback);
+ status_t removeEventListener(Callback* callback);
// computeNextRefresh computes when the next refresh is expected to begin.
// The periodOffset value can be used to move forward or backward; an
@@ -188,6 +188,8 @@
// Ignore present (retire) fences if the device doesn't have support for the
// sync framework
bool mIgnorePresentFences;
+
+ std::unique_ptr<Callback> mZeroPhaseTracer;
};
} // namespace android
diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp
index 2cbc59e..53d95e2 100644
--- a/services/surfaceflinger/EventThread.cpp
+++ b/services/surfaceflinger/EventThread.cpp
@@ -43,7 +43,7 @@
// ---------------------------------------------------------------------------
-EventThread::EventThread(const sp<VSyncSource>& src, SurfaceFlinger& flinger, bool interceptVSyncs,
+EventThread::EventThread(VSyncSource* src, SurfaceFlinger& flinger, bool interceptVSyncs,
const char* threadName)
: mVSyncSource(src), mFlinger(flinger), mInterceptVSyncs(interceptVSyncs) {
for (auto& event : mVSyncEvent) {
@@ -339,7 +339,7 @@
// never enable h/w VSYNC when screen is off
if (!mVsyncEnabled) {
mVsyncEnabled = true;
- mVSyncSource->setCallback(static_cast<VSyncSource::Callback*>(this));
+ mVSyncSource->setCallback(this);
mVSyncSource->setVSyncEnabled(true);
}
}
@@ -370,7 +370,7 @@
// ---------------------------------------------------------------------------
-EventThread::Connection::Connection(const sp<EventThread>& eventThread)
+EventThread::Connection::Connection(EventThread* eventThread)
: count(-1), mEventThread(eventThread), mChannel(gui::BitTube::DefaultSize) {}
EventThread::Connection::~Connection() {
diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h
index 63cdb54..9ae8fb2 100644
--- a/services/surfaceflinger/EventThread.h
+++ b/services/surfaceflinger/EventThread.h
@@ -29,7 +29,6 @@
#include <private/gui/BitTube.h>
#include <utils/Errors.h>
-#include <utils/RefBase.h>
#include <utils/SortedVector.h>
#include "DisplayDevice.h"
@@ -43,9 +42,9 @@
// ---------------------------------------------------------------------------
-class VSyncSource : public virtual RefBase {
+class VSyncSource {
public:
- class Callback : public virtual RefBase {
+ class Callback {
public:
virtual ~Callback() {}
virtual void onVSyncEvent(nsecs_t when) = 0;
@@ -53,14 +52,14 @@
virtual ~VSyncSource() {}
virtual void setVSyncEnabled(bool enable) = 0;
- virtual void setCallback(const sp<Callback>& callback) = 0;
+ virtual void setCallback(Callback* callback) = 0;
virtual void setPhaseOffset(nsecs_t phaseOffset) = 0;
};
-class EventThread : public virtual RefBase, private VSyncSource::Callback {
+class EventThread : private VSyncSource::Callback {
class Connection : public BnDisplayEventConnection {
public:
- explicit Connection(const sp<EventThread>& eventThread);
+ explicit Connection(EventThread* eventThread);
status_t postEvent(const DisplayEventReceiver::Event& event);
// count >= 1 : continuous event. count is the vsync rate
@@ -74,12 +73,12 @@
status_t stealReceiveChannel(gui::BitTube* outChannel) override;
status_t setVsyncRate(uint32_t count) override;
void requestNextVsync() override; // asynchronous
- sp<EventThread> const mEventThread;
+ EventThread* const mEventThread;
gui::BitTube mChannel;
};
public:
- EventThread(const sp<VSyncSource>& src, SurfaceFlinger& flinger, bool interceptVSyncs,
+ EventThread(VSyncSource* src, SurfaceFlinger& flinger, bool interceptVSyncs,
const char* threadName);
~EventThread();
@@ -116,7 +115,7 @@
void onVSyncEvent(nsecs_t timestamp) override;
// constants
- sp<VSyncSource> mVSyncSource GUARDED_BY(mMutex);
+ VSyncSource* mVSyncSource GUARDED_BY(mMutex) = nullptr;
SurfaceFlinger& mFlinger;
std::thread mThread;
diff --git a/services/surfaceflinger/MessageQueue.cpp b/services/surfaceflinger/MessageQueue.cpp
index c9c3989..5a6ff4d 100644
--- a/services/surfaceflinger/MessageQueue.cpp
+++ b/services/surfaceflinger/MessageQueue.cpp
@@ -82,7 +82,7 @@
mHandler = new Handler(*this);
}
-void MessageQueue::setEventThread(const sp<EventThread>& eventThread) {
+void MessageQueue::setEventThread(EventThread* eventThread) {
if (mEventThread == eventThread) {
return;
}
diff --git a/services/surfaceflinger/MessageQueue.h b/services/surfaceflinger/MessageQueue.h
index fe1bf08..dcfc716 100644
--- a/services/surfaceflinger/MessageQueue.h
+++ b/services/surfaceflinger/MessageQueue.h
@@ -93,7 +93,7 @@
sp<SurfaceFlinger> mFlinger;
sp<Looper> mLooper;
- sp<EventThread> mEventThread;
+ EventThread* mEventThread;
sp<IDisplayEventConnection> mEvents;
gui::BitTube mEventTube;
sp<Handler> mHandler;
@@ -110,7 +110,7 @@
MessageQueue();
~MessageQueue();
void init(const sp<SurfaceFlinger>& flinger);
- void setEventThread(const sp<EventThread>& events);
+ void setEventThread(EventThread* events);
void waitMessage();
status_t postMessage(const sp<MessageBase>& message, nsecs_t reltime = 0);
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 3774ab1..1054c32 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -424,7 +424,7 @@
postMessageAsync(new MessageDestroyGLTexture(getRenderEngine(), texture));
}
-class DispSyncSource : public VSyncSource, private DispSync::Callback {
+class DispSyncSource final : public VSyncSource, private DispSync::Callback {
public:
DispSyncSource(DispSync* dispSync, nsecs_t phaseOffset, bool traceVsync,
const char* name) :
@@ -435,14 +435,13 @@
mVsyncEventLabel(String8::format("VSYNC-%s", name)),
mDispSync(dispSync),
mCallbackMutex(),
- mCallback(),
mVsyncMutex(),
mPhaseOffset(phaseOffset),
mEnabled(false) {}
- virtual ~DispSyncSource() {}
+ ~DispSyncSource() override = default;
- virtual void setVSyncEnabled(bool enable) {
+ void setVSyncEnabled(bool enable) override {
Mutex::Autolock lock(mVsyncMutex);
if (enable) {
status_t err = mDispSync->addEventListener(mName, mPhaseOffset,
@@ -464,12 +463,12 @@
mEnabled = enable;
}
- virtual void setCallback(const sp<VSyncSource::Callback>& callback) {
+ void setCallback(VSyncSource::Callback* callback) override{
Mutex::Autolock lock(mCallbackMutex);
mCallback = callback;
}
- virtual void setPhaseOffset(nsecs_t phaseOffset) {
+ void setPhaseOffset(nsecs_t phaseOffset) override {
Mutex::Autolock lock(mVsyncMutex);
// Normalize phaseOffset to [0, period)
@@ -506,7 +505,7 @@
private:
virtual void onDispSyncEvent(nsecs_t when) {
- sp<VSyncSource::Callback> callback;
+ VSyncSource::Callback* callback;
{
Mutex::Autolock lock(mCallbackMutex);
callback = mCallback;
@@ -533,37 +532,36 @@
DispSync* mDispSync;
Mutex mCallbackMutex; // Protects the following
- sp<VSyncSource::Callback> mCallback;
+ VSyncSource::Callback* mCallback = nullptr;
Mutex mVsyncMutex; // Protects the following
nsecs_t mPhaseOffset;
bool mEnabled;
};
-class InjectVSyncSource : public VSyncSource {
+class InjectVSyncSource final : public VSyncSource {
public:
- InjectVSyncSource() {}
+ InjectVSyncSource() = default;
+ ~InjectVSyncSource() override = default;
- virtual ~InjectVSyncSource() {}
-
- virtual void setCallback(const sp<VSyncSource::Callback>& callback) {
+ void setCallback(VSyncSource::Callback* callback) override {
std::lock_guard<std::mutex> lock(mCallbackMutex);
mCallback = callback;
}
- virtual void onInjectSyncEvent(nsecs_t when) {
+ void onInjectSyncEvent(nsecs_t when) {
std::lock_guard<std::mutex> lock(mCallbackMutex);
if (mCallback) {
mCallback->onVSyncEvent(when);
}
}
- virtual void setVSyncEnabled(bool) {}
- virtual void setPhaseOffset(nsecs_t) {}
+ void setVSyncEnabled(bool) override {}
+ void setPhaseOffset(nsecs_t) override {}
private:
std::mutex mCallbackMutex; // Protects the following
- sp<VSyncSource::Callback> mCallback;
+ VSyncSource::Callback* mCallback = nullptr;
};
// Do not call property_set on main thread which will be blocked by init
@@ -577,13 +575,16 @@
Mutex::Autolock _l(mStateLock);
// start the EventThread
- sp<VSyncSource> vsyncSrc =
- new DispSyncSource(&mPrimaryDispSync, SurfaceFlinger::vsyncPhaseOffsetNs, true, "app");
- mEventThread = new EventThread(vsyncSrc, *this, false, "appEventThread");
- sp<VSyncSource> sfVsyncSrc =
- new DispSyncSource(&mPrimaryDispSync, SurfaceFlinger::sfVsyncPhaseOffsetNs, true, "sf");
- mSFEventThread = new EventThread(sfVsyncSrc, *this, true, "sfEventThread");
- mEventQueue.setEventThread(mSFEventThread);
+
+ mEventThreadSource = std::make_unique<DispSyncSource>(
+ &mPrimaryDispSync, SurfaceFlinger::vsyncPhaseOffsetNs, true, "app");
+ mEventThread = std::make_unique<EventThread>(
+ mEventThreadSource.get(), *this, false, "sfEventThread");
+ mSfEventThreadSource = std::make_unique<DispSyncSource>(
+ &mPrimaryDispSync, SurfaceFlinger::sfVsyncPhaseOffsetNs, true, "sf");
+ mSFEventThread = std::make_unique<EventThread>(
+ mSfEventThreadSource.get(), *this, true, "appEventThread");
+ mEventQueue.setEventThread(mSFEventThread.get());
// Get a RenderEngine for the given display / config (can't fail)
getBE().mRenderEngine = RenderEngine::create(HAL_PIXEL_FORMAT_RGBA_8888,
@@ -1063,14 +1064,14 @@
if (enable) {
ALOGV("VSync Injections enabled");
if (mVSyncInjector.get() == nullptr) {
- mVSyncInjector = new InjectVSyncSource();
- mInjectorEventThread = new EventThread(mVSyncInjector, *this, false,
- "injEventThread");
+ mVSyncInjector = std::make_unique<InjectVSyncSource>();
+ mInjectorEventThread = std::make_unique<EventThread>(
+ mVSyncInjector.get(), *this, false, "injEvThread");
}
- mEventQueue.setEventThread(mInjectorEventThread);
+ mEventQueue.setEventThread(mInjectorEventThread.get());
} else {
ALOGV("VSync Injections disabled");
- mEventQueue.setEventThread(mSFEventThread);
+ mEventQueue.setEventThread(mSFEventThread.get());
}
mInjectVSyncs = enable;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 89f3930..19dd059 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -719,10 +719,12 @@
// constant members (no synchronization needed for access)
nsecs_t mBootTime;
bool mGpuToCpuSupported;
- sp<EventThread> mEventThread;
- sp<EventThread> mSFEventThread;
- sp<EventThread> mInjectorEventThread;
- sp<InjectVSyncSource> mVSyncInjector;
+ std::unique_ptr<EventThread> mEventThread;
+ std::unique_ptr<EventThread> mSFEventThread;
+ std::unique_ptr<EventThread> mInjectorEventThread;
+ std::unique_ptr<VSyncSource> mEventThreadSource;
+ std::unique_ptr<VSyncSource> mSfEventThreadSource;
+ std::unique_ptr<InjectVSyncSource> mVSyncInjector;
std::unique_ptr<EventControlThread> mEventControlThread;
sp<IBinder> mBuiltinDisplays[DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES];