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

Commit 3b4bdcf6 authored by chaviw's avatar chaviw
Browse files

Invoke sync transaction callback if overwritten 

If a caller invokes the syncNextTransaction it will overwrite any
callback that was set before. This can break since the caller expects to
receive the callback.

The change invokes the old callback when a new callback overwrites it.
It also invokes the callback when the blast buffer queue is torn down in
case there's one still pending.

Test: Chrome in split + rotation
Test: BLASTBufferQueueTest
Fixes: 223329500
Change-Id: I91d1d7c47f4eca014c66ba8c2f773ef8be5c674e
parent a49b7b35

libs/gui/BLASTBufferQueue.cpp

+28 −7
Original line number Diff line number Diff line
@@ -184,6 +184,10 @@ BLASTBufferQueue::~BLASTBufferQueue() { 
    mergePendingTransactions(&t, std::numeric_limits<uint64_t>::max() /* frameNumber */);

    // All transactions on our apply token are one-way. See comment on mAppliedLastTransaction

    t.setApplyToken(mApplyToken).apply(false, true);



    if (mTransactionReadyCallback) {

        mTransactionReadyCallback(mSyncTransaction);

    }

}



void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width, uint32_t height,
@@ -702,7 +706,19 @@ void BLASTBufferQueue::syncNextTransaction( 
        std::function<void(SurfaceComposerClient::Transaction*)> callback,

        bool acquireSingleBuffer) {

    BBQ_TRACE();



    std::function<void(SurfaceComposerClient::Transaction*)> prevCallback = nullptr;

    SurfaceComposerClient::Transaction* prevTransaction = nullptr;



    {

        std::lock_guard _lock{mMutex};

        // We're about to overwrite the previous call so we should invoke that callback

        // immediately.

        if (mTransactionReadyCallback) {

            prevCallback = mTransactionReadyCallback;

            prevTransaction = mSyncTransaction;

        }



        mTransactionReadyCallback = callback;

        if (callback) {

            mSyncTransaction = new SurfaceComposerClient::Transaction();
@@ -712,6 +728,11 @@ void BLASTBufferQueue::syncNextTransaction( 
        mAcquireSingleBuffer = mTransactionReadyCallback ? acquireSingleBuffer : true;

    }



    if (prevCallback) {

        prevCallback(prevTransaction);

    }

}



void BLASTBufferQueue::stopContinuousSyncTransaction() {

    std::function<void(SurfaceComposerClient::Transaction*)> prevCallback = nullptr;

    SurfaceComposerClient::Transaction* prevTransaction = nullptr;

libs/gui/tests/BLASTBufferQueue_test.cpp

+28 −0
Original line number Diff line number Diff line
@@ -1083,6 +1083,34 @@ TEST_F(BLASTBufferQueueTest, SyncNextTransactionAcquireMultipleBuffers) { 
            checkScreenCapture(255, 0, 0, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight}));

}



TEST_F(BLASTBufferQueueTest, SyncNextTransactionOverwrite) {

    std::mutex mutex;

    std::condition_variable callbackReceivedCv;

    bool receivedCallback = false;



    BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight);

    ASSERT_EQ(nullptr, adapter.getTransactionReadyCallback());

    auto callback = [&](Transaction*) {

        std::unique_lock<std::mutex> lock(mutex);

        receivedCallback = true;

        callbackReceivedCv.notify_one();

    };

    adapter.syncNextTransaction(callback);

    ASSERT_NE(nullptr, adapter.getTransactionReadyCallback());



    auto callback2 = [](Transaction*) {};

    adapter.syncNextTransaction(callback2);



    std::unique_lock<std::mutex> lock(mutex);

    if (!receivedCallback) {

        ASSERT_NE(callbackReceivedCv.wait_for(lock, std::chrono::seconds(3)),

                  std::cv_status::timeout)

                << "did not receive callback";

    }



    ASSERT_TRUE(receivedCallback);

}



// This test will currently fail because the old surfacecontrol will steal the last presented buffer

// until the old surface control is destroyed. This is not necessarily a bug but to document a

// limitation with the update API and to test any changes to make the api more robust. The current