Delay initialization of a ConsumerBase instance to construction of a sp/wp.
This is needed to ensure correcntess of wp view of `this` which is
needed during initialization.
Bug: 393217449
Test: presubmit
Flag: EXEMPT_refactor
Change-Id: I1c4ec5d4487e9bb7f16a2654e38b27d32d2b82f3
diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp
index 0266a3f..117a362 100644
--- a/libs/gui/ConsumerBase.cpp
+++ b/libs/gui/ConsumerBase.cpp
@@ -31,6 +31,7 @@
#include <gui/BufferItem.h>
#include <gui/BufferQueue.h>
#include <gui/ConsumerBase.h>
+#include <gui/IConsumerListener.h>
#include <gui/ISurfaceComposer.h>
#include <gui/Surface.h>
#include <gui/SurfaceComposerClient.h>
@@ -68,8 +69,8 @@
#endif
mAbandoned(false),
mConsumer(bufferQueue),
- mPrevFinalReleaseFence(Fence::NO_FENCE) {
- initialize(controlledByApp);
+ mPrevFinalReleaseFence(Fence::NO_FENCE),
+ mIsControlledByApp(controlledByApp) {
}
#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ)
@@ -79,11 +80,11 @@
mSlots(BufferQueueDefs::NUM_BUFFER_SLOTS),
#endif
mAbandoned(false),
- mPrevFinalReleaseFence(Fence::NO_FENCE) {
+ mPrevFinalReleaseFence(Fence::NO_FENCE),
+ mIsControlledByApp(controlledByApp) {
sp<IGraphicBufferProducer> producer;
BufferQueue::createBufferQueue(&producer, &mConsumer, consumerIsSurfaceFlinger);
mSurface = sp<Surface>::make(producer, controlledByApp);
- initialize(controlledByApp);
}
ConsumerBase::ConsumerBase(const sp<IGraphicBufferProducer>& producer,
@@ -95,24 +96,27 @@
mAbandoned(false),
mConsumer(consumer),
mSurface(sp<Surface>::make(producer, controlledByApp)),
- mPrevFinalReleaseFence(Fence::NO_FENCE) {
- initialize(controlledByApp);
+ mPrevFinalReleaseFence(Fence::NO_FENCE),
+ mIsControlledByApp(controlledByApp) {
}
#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ)
-void ConsumerBase::initialize(bool controlledByApp) {
+void ConsumerBase::onFirstRef() {
+ ConsumerListener::onFirstRef();
+ initialize();
+}
+
+void ConsumerBase::initialize() {
// Choose a name using the PID and a process-unique ID.
mName = String8::format("unnamed-%d-%d", getpid(), createProcessUniqueId());
- // Note that we can't create an sp<...>(this) in a ctor that will not keep a
- // reference once the ctor ends, as that would cause the refcount of 'this'
- // dropping to 0 at the end of the ctor. Since all we need is a wp<...>
- // that's what we create.
- wp<ConsumerListener> listener = static_cast<ConsumerListener*>(this);
- sp<IConsumerListener> proxy = new BufferQueue::ProxyConsumerListener(listener);
+ // Here we depend on an sp/wp having been created for `this`. For this
+ // reason, initialize() cannot be called from a ctor.
+ wp<ConsumerListener> listener = wp<ConsumerListener>::fromExisting(this);
+ sp<IConsumerListener> proxy = sp<BufferQueue::ProxyConsumerListener>::make(listener);
- status_t err = mConsumer->consumerConnect(proxy, controlledByApp);
+ status_t err = mConsumer->consumerConnect(proxy, mIsControlledByApp);
if (err != NO_ERROR) {
CB_LOGE("ConsumerBase: error connecting to BufferQueue: %s (%d)",
strerror(-err), err);
diff --git a/libs/gui/include/gui/ConsumerBase.h b/libs/gui/include/gui/ConsumerBase.h
index 477d98d..fd67f09 100644
--- a/libs/gui/include/gui/ConsumerBase.h
+++ b/libs/gui/include/gui/ConsumerBase.h
@@ -139,7 +139,8 @@
ConsumerBase(const ConsumerBase&);
void operator=(const ConsumerBase&);
- void initialize(bool controlledByApp);
+ // Requires `this` to be sp/wp so must not be called from ctor.
+ void initialize();
protected:
// ConsumerBase constructs a new ConsumerBase object to consume image
@@ -252,6 +253,10 @@
const sp<GraphicBuffer> graphicBuffer,
EGLDisplay display = EGL_NO_DISPLAY, EGLSyncKHR eglFence = EGL_NO_SYNC_KHR);
#endif
+ // Required to complete initialization, so `final` lest overrides forget to
+ // delegate.
+ void onFirstRef() override final;
+
// returns true iff the slot still has the graphicBuffer in it.
bool stillTracking(int slot, const sp<GraphicBuffer> graphicBuffer);
@@ -327,6 +332,8 @@
// releaseBufferLocked.
sp<Fence> mPrevFinalReleaseFence;
+ const bool mIsControlledByApp;
+
// mMutex is the mutex used to prevent concurrent access to the member
// variables of ConsumerBase objects. It must be locked whenever the
// member variables are accessed or when any of the *Locked methods are