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

Commit 5de3ad2c authored by Valerie Hau's avatar Valerie Hau
Browse files

Switch from ITransactionCompletedListener to IBinder keys 

IBinder remains consistent across multiple transactions when passed
cross-process to SF.  However, interface_cast'ing it to an
ITransactionCompletedListener results in different underlying objects.
Switch to using IBinder as the keys for listener callbacks.

Bug: 139731321
Test: build, boot, SurfaceFlinger_test
Change-Id: I9bea76664087020eb43ceec258b5ecede0faaea5
parent 7704a15d

libs/gui/ISurfaceComposer.cpp

+2 −3
Original line number Diff line number Diff line
@@ -93,7 +93,7 @@ public: 


        if (data.writeVectorSize(listenerCallbacks) == NO_ERROR) {

            for (const auto& [listener, callbackIds] : listenerCallbacks) {

                data.writeStrongBinder(IInterface::asBinder(listener));

                data.writeStrongBinder(listener);

                data.writeInt64Vector(callbackIds);

            }

        }
@@ -1042,8 +1042,7 @@ status_t BnSurfaceComposer::onTransact( 
            std::vector<ListenerCallbacks> listenerCallbacks;

            int32_t listenersSize = data.readInt32();

            for (int32_t i = 0; i < listenersSize; i++) {

                auto listener =

                        interface_cast<ITransactionCompletedListener>(data.readStrongBinder());

                auto listener = data.readStrongBinder();

                std::vector<CallbackId> callbackIds;

                data.readInt64Vector(&callbackIds);

                listenerCallbacks.emplace_back(listener, callbackIds);

libs/gui/ITransactionCompletedListener.cpp

+1 −1
Original line number Diff line number Diff line
@@ -151,7 +151,7 @@ status_t ListenerStats::readFromParcel(const Parcel* input) { 
    return NO_ERROR;

}



ListenerStats ListenerStats::createEmpty(const sp<ITransactionCompletedListener>& listener,

ListenerStats ListenerStats::createEmpty(const sp<IBinder>& listener,

                                         const std::unordered_set<CallbackId>& callbackIds) {

    ListenerStats listenerStats;

    listenerStats.listener = listener;

libs/gui/SurfaceComposerClient.cpp

+1 −1
Original line number Diff line number Diff line
@@ -548,7 +548,7 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous) { 
            continue;

        }



        listenerCallbacks.emplace_back(listener, std::move(callbackIds));

        listenerCallbacks.emplace_back(IInterface::asBinder(listener), std::move(callbackIds));



        // If the listener has any SurfaceControls set on this Transaction update the surface state

        for (const auto& surfaceControl : surfaceControls) {

libs/gui/include/gui/ITransactionCompletedListener.h

+40 −7
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ 
namespace android {



class ITransactionCompletedListener;

class ListenerCallbacks;



using CallbackId = int64_t;
@@ -72,10 +73,10 @@ public: 
    status_t writeToParcel(Parcel* output) const override;

    status_t readFromParcel(const Parcel* input) override;



    static ListenerStats createEmpty(const sp<ITransactionCompletedListener>& listener,

    static ListenerStats createEmpty(const sp<IBinder>& listener,

                                     const std::unordered_set<CallbackId>& callbackIds);



    sp<ITransactionCompletedListener> listener;

    sp<IBinder> listener;

    std::vector<TransactionStats> transactionStats;

};
@@ -97,13 +98,11 @@ public: 


class ListenerCallbacks {

public:

    ListenerCallbacks(const sp<ITransactionCompletedListener>& listener,

                      const std::unordered_set<CallbackId>& callbacks)

    ListenerCallbacks(const sp<IBinder>& listener, const std::unordered_set<CallbackId>& callbacks)

          : transactionCompletedListener(listener),

            callbackIds(callbacks.begin(), callbacks.end()) {}



    ListenerCallbacks(const sp<ITransactionCompletedListener>& listener,

                      const std::vector<CallbackId>& ids)

    ListenerCallbacks(const sp<IBinder>& listener, const std::vector<CallbackId>& ids)

          : transactionCompletedListener(listener), callbackIds(ids) {}



    bool operator==(const ListenerCallbacks& rhs) const {
@@ -116,8 +115,42 @@ public: 
        return callbackIds.front() == rhs.callbackIds.front();

    }



    sp<ITransactionCompletedListener> transactionCompletedListener;

    sp<IBinder> transactionCompletedListener;

    std::vector<CallbackId> callbackIds;

};



struct IListenerHash {

    std::size_t operator()(const sp<IBinder>& strongPointer) const {

        return std::hash<IBinder*>{}(strongPointer.get());

    }

};



struct CallbackIdsHash {

    // CallbackId vectors have several properties that let us get away with this simple hash.

    // 1) CallbackIds are never 0 so if something has gone wrong and our CallbackId vector is

    // empty we can still hash 0.

    // 2) CallbackId vectors for the same listener either are identical or contain none of the

    // same members. It is sufficient to just check the first CallbackId in the vectors. If

    // they match, they are the same. If they do not match, they are not the same.

    std::size_t operator()(const std::vector<CallbackId>& callbackIds) const {

        return std::hash<CallbackId>{}((callbackIds.empty()) ? 0 : callbackIds.front());

    }

};



struct ListenerCallbacksHash {

    std::size_t HashCombine(size_t value1, size_t value2) const {

        return value1 ^ (value2 + 0x9e3779b9 + (value1 << 6) + (value1 >> 2));

    }



    std::size_t operator()(const ListenerCallbacks& listenerCallbacks) const {

        struct IListenerHash listenerHasher;

        struct CallbackIdsHash callbackIdsHasher;



        std::size_t listenerHash = listenerHasher(listenerCallbacks.transactionCompletedListener);

        std::size_t callbackIdsHash = callbackIdsHasher(listenerCallbacks.callbackIds);



        return HashCombine(listenerHash, callbackIdsHash);

    }

};



} // namespace android

services/surfaceflinger/SurfaceFlinger.h

+2 −6
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ 
#include <gui/FrameTimestamps.h>

#include <gui/ISurfaceComposer.h>

#include <gui/ISurfaceComposerClient.h>

#include <gui/ITransactionCompletedListener.h>

#include <gui/LayerState.h>

#include <gui/OccupancyTracker.h>

#include <hardware/hwcomposer_defs.h>
@@ -1025,11 +1026,6 @@ private: 
    uint32_t mTexturePoolSize = 0;

    std::vector<uint32_t> mTexturePool;



    struct IBinderHash {

        std::size_t operator()(const sp<IBinder>& strongPointer) const {

            return std::hash<IBinder*>{}(strongPointer.get());

        }

    };

    struct TransactionState {

        TransactionState(const Vector<ComposerState>& composerStates,

                         const Vector<DisplayState>& displayStates, uint32_t transactionFlags,
@@ -1054,7 +1050,7 @@ private: 
        const int64_t postTime;

        bool privileged;

    };

    std::unordered_map<sp<IBinder>, std::queue<TransactionState>, IBinderHash> mTransactionQueues;

    std::unordered_map<sp<IBinder>, std::queue<TransactionState>, IListenerHash> mTransactionQueues;



    /* ------------------------------------------------------------------------

     * Feature prototyping