Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4efd0d93 authored by Anton Ivanov's avatar Anton Ivanov
Browse files

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
parent af9a9ce3
Loading
Loading
Loading
Loading
+18 −14
Original line number Original line Diff line number Diff line
@@ -31,6 +31,7 @@
#include <gui/BufferItem.h>
#include <gui/BufferItem.h>
#include <gui/BufferQueue.h>
#include <gui/BufferQueue.h>
#include <gui/ConsumerBase.h>
#include <gui/ConsumerBase.h>
#include <gui/IConsumerListener.h>
#include <gui/ISurfaceComposer.h>
#include <gui/ISurfaceComposer.h>
#include <gui/Surface.h>
#include <gui/Surface.h>
#include <gui/SurfaceComposerClient.h>
#include <gui/SurfaceComposerClient.h>
@@ -68,8 +69,8 @@ ConsumerBase::ConsumerBase(const sp<IGraphicBufferConsumer>& bufferQueue, bool c
#endif
#endif
        mAbandoned(false),
        mAbandoned(false),
        mConsumer(bufferQueue),
        mConsumer(bufferQueue),
        mPrevFinalReleaseFence(Fence::NO_FENCE) {
        mPrevFinalReleaseFence(Fence::NO_FENCE),
    initialize(controlledByApp);
        mIsControlledByApp(controlledByApp) {
}
}


#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ)
#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ)
@@ -79,11 +80,11 @@ ConsumerBase::ConsumerBase(bool controlledByApp, bool consumerIsSurfaceFlinger)
        mSlots(BufferQueueDefs::NUM_BUFFER_SLOTS),
        mSlots(BufferQueueDefs::NUM_BUFFER_SLOTS),
#endif
#endif
        mAbandoned(false),
        mAbandoned(false),
        mPrevFinalReleaseFence(Fence::NO_FENCE) {
        mPrevFinalReleaseFence(Fence::NO_FENCE),
        mIsControlledByApp(controlledByApp) {
    sp<IGraphicBufferProducer> producer;
    sp<IGraphicBufferProducer> producer;
    BufferQueue::createBufferQueue(&producer, &mConsumer, consumerIsSurfaceFlinger);
    BufferQueue::createBufferQueue(&producer, &mConsumer, consumerIsSurfaceFlinger);
    mSurface = sp<Surface>::make(producer, controlledByApp);
    mSurface = sp<Surface>::make(producer, controlledByApp);
    initialize(controlledByApp);
}
}


ConsumerBase::ConsumerBase(const sp<IGraphicBufferProducer>& producer,
ConsumerBase::ConsumerBase(const sp<IGraphicBufferProducer>& producer,
@@ -95,24 +96,27 @@ ConsumerBase::ConsumerBase(const sp<IGraphicBufferProducer>& producer,
        mAbandoned(false),
        mAbandoned(false),
        mConsumer(consumer),
        mConsumer(consumer),
        mSurface(sp<Surface>::make(producer, controlledByApp)),
        mSurface(sp<Surface>::make(producer, controlledByApp)),
        mPrevFinalReleaseFence(Fence::NO_FENCE) {
        mPrevFinalReleaseFence(Fence::NO_FENCE),
    initialize(controlledByApp);
        mIsControlledByApp(controlledByApp) {
}
}


#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ)
#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.
    // Choose a name using the PID and a process-unique ID.
    mName = String8::format("unnamed-%d-%d", getpid(), createProcessUniqueId());
    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
    // Here we depend on an sp/wp having been created for `this`.  For this
    // reference once the ctor ends, as that would cause the refcount of 'this'
    // reason, initialize() cannot be called from a ctor.
    // dropping to 0 at the end of the ctor.  Since all we need is a wp<...>
    wp<ConsumerListener> listener = wp<ConsumerListener>::fromExisting(this);
    // that's what we create.
    sp<IConsumerListener> proxy = sp<BufferQueue::ProxyConsumerListener>::make(listener);
    wp<ConsumerListener> listener = static_cast<ConsumerListener*>(this);
    sp<IConsumerListener> proxy = new BufferQueue::ProxyConsumerListener(listener);


    status_t err = mConsumer->consumerConnect(proxy, controlledByApp);
    status_t err = mConsumer->consumerConnect(proxy, mIsControlledByApp);
    if (err != NO_ERROR) {
    if (err != NO_ERROR) {
        CB_LOGE("ConsumerBase: error connecting to BufferQueue: %s (%d)",
        CB_LOGE("ConsumerBase: error connecting to BufferQueue: %s (%d)",
                strerror(-err), err);
                strerror(-err), err);
+8 −1
Original line number Original line Diff line number Diff line
@@ -139,7 +139,8 @@ private:
    ConsumerBase(const ConsumerBase&);
    ConsumerBase(const ConsumerBase&);
    void operator=(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:
protected:
    // ConsumerBase constructs a new ConsumerBase object to consume image
    // ConsumerBase constructs a new ConsumerBase object to consume image
@@ -252,6 +253,10 @@ protected:
            const sp<GraphicBuffer> graphicBuffer,
            const sp<GraphicBuffer> graphicBuffer,
            EGLDisplay display = EGL_NO_DISPLAY, EGLSyncKHR eglFence = EGL_NO_SYNC_KHR);
            EGLDisplay display = EGL_NO_DISPLAY, EGLSyncKHR eglFence = EGL_NO_SYNC_KHR);
#endif
#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.
    // returns true iff the slot still has the graphicBuffer in it.
    bool stillTracking(int slot, const sp<GraphicBuffer> graphicBuffer);
    bool stillTracking(int slot, const sp<GraphicBuffer> graphicBuffer);


@@ -327,6 +332,8 @@ protected:
    // releaseBufferLocked.
    // releaseBufferLocked.
    sp<Fence> mPrevFinalReleaseFence;
    sp<Fence> mPrevFinalReleaseFence;


    const bool mIsControlledByApp;

    // mMutex is the mutex used to prevent concurrent access to the member
    // mMutex is the mutex used to prevent concurrent access to the member
    // variables of ConsumerBase objects. It must be locked whenever the
    // variables of ConsumerBase objects. It must be locked whenever the
    // member variables are accessed or when any of the *Locked methods are
    // member variables are accessed or when any of the *Locked methods are