From 39f4b1c202a5bfb57987ebac350b6cd93a69e147 Mon Sep 17 00:00:00 2001
From: Jorim Jaggi <jjaggi@google.com>
Date: Wed, 23 Jun 2021 13:39:41 +0200
Subject: Revert "Ensure reportFrameMetrics not being called on deleted
 instance"

This reverts commit 1283885c6478edabb3bcdf26a971818a9f510d40.

Reason: Creates deadlock, while not actually fixing the original
issue it was trying to fix.
Bug: 191581295

Change-Id: I84013239cc9d7ddfc6351639937ff98a21ee15a2
---
 libs/gui/SurfaceComposerClient.cpp | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

(limited to 'libs/gui/SurfaceComposerClient.cpp')

diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index c69435d328..fd78309462 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -182,12 +182,12 @@ CallbackId TransactionCompletedListener::addCallbackFunction(
 
 void TransactionCompletedListener::addJankListener(const sp<JankDataListener>& listener,
                                                    sp<SurfaceControl> surfaceControl) {
-    std::scoped_lock<std::recursive_mutex> lock(mJankListenerMutex);
+    std::lock_guard<std::mutex> lock(mMutex);
     mJankListeners.insert({surfaceControl->getHandle(), listener});
 }
 
 void TransactionCompletedListener::removeJankListener(const sp<JankDataListener>& listener) {
-    std::scoped_lock<std::recursive_mutex> lock(mJankListenerMutex);
+    std::lock_guard<std::mutex> lock(mMutex);
     for (auto it = mJankListeners.begin(); it != mJankListeners.end();) {
         if (it->second == listener) {
             it = mJankListeners.erase(it);
@@ -242,6 +242,7 @@ void TransactionCompletedListener::addSurfaceControlToCallbacks(
 
 void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) {
     std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> callbacksMap;
+    std::multimap<sp<IBinder>, sp<JankDataListener>> jankListenersMap;
     {
         std::lock_guard<std::mutex> lock(mMutex);
 
@@ -257,6 +258,7 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
          * sp<SurfaceControl> that could possibly exist for the callbacks.
          */
         callbacksMap = mCallbacks;
+        jankListenersMap = mJankListeners;
         for (const auto& transactionStats : listenerStats.transactionStats) {
             for (auto& callbackId : transactionStats.callbackIds) {
                 mCallbacks.erase(callbackId);
@@ -352,12 +354,7 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
             }
 
             if (surfaceStats.jankData.empty()) continue;
-
-            // Acquire jank listener lock such that we guarantee that after calling unregister,
-            // there won't be any further callback.
-            std::scoped_lock<std::recursive_mutex> lock(mJankListenerMutex);
-            auto copy = mJankListeners;
-            auto jankRange = copy.equal_range(surfaceStats.surfaceControl);
+            auto jankRange = jankListenersMap.equal_range(surfaceStats.surfaceControl);
             for (auto it = jankRange.first; it != jankRange.second; it++) {
                 it->second->onJankDataAvailable(surfaceStats.jankData);
             }
-- 
cgit v1.2.3-59-g8ed1b


From 4ba0c2e2a2e8a4870b4d5055bd765d5514a35ebc Mon Sep 17 00:00:00 2001
From: Vishnu Nair <vishnun@google.com>
Date: Thu, 24 Jun 2021 11:27:17 -0700
Subject: Blast: Use a unique id to track buffers

When buffer queue is configured in shared buffer mode, the client
can queue the same buffer over and over again and the consumer
can acquire the same buffer repeatedly. BBQ tracks acquired buffers
by graphic buffer id and SCC tracks release buffer callbacks by
graphic buffer ids. This breaks if a buffer is reused before release.
Fix this by using graphic buffer id and framenumber to track acquired
buffers and buffer release callbacks.

Test: atest CtsDeqpTestCases:dEQP-VK.wsi.android.shared_presentable_image.scale_none.identity.inherit.demand CtsDeqpTestCases:dEQP-VK.wsi.android.colorspace#basic
Bug: 191220669

Change-Id: Ibd54df9fa6780c26cd8de769bf9e43163abbed5a
---
 libs/gui/BLASTBufferQueue.cpp                      | 40 +++++------
 libs/gui/ITransactionCompletedListener.cpp         | 24 +++++--
 libs/gui/SurfaceComposerClient.cpp                 | 43 +++++++-----
 libs/gui/include/gui/BLASTBufferQueue.h            |  7 +-
 .../include/gui/ITransactionCompletedListener.h    | 38 +++++++++--
 libs/gui/include/gui/LayerState.h                  |  6 ++
 libs/gui/include/gui/SurfaceComposerClient.h       | 20 +++---
 services/surfaceflinger/BufferLayer.h              |  1 +
 services/surfaceflinger/BufferStateLayer.cpp       | 40 +++++------
 services/surfaceflinger/BufferStateLayer.h         |  9 +--
 .../surfaceflinger/TransactionCallbackInvoker.cpp  |  3 +-
 .../surfaceflinger/TransactionCallbackInvoker.h    |  2 +-
 .../tests/ReleaseBufferCallback_test.cpp           | 77 ++++++++++++++--------
 13 files changed, 192 insertions(+), 118 deletions(-)

(limited to 'libs/gui/SurfaceComposerClient.cpp')

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index e15e11cc20..29701168e7 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -132,8 +132,7 @@ void BLASTBufferItemConsumer::onSidebandStreamChanged() {
 
 BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp<SurfaceControl>& surface,
                                    int width, int height, int32_t format)
-      : mName(name),
-        mSurfaceControl(surface),
+      : mSurfaceControl(surface),
         mSize(width, height),
         mRequestedSize(mSize),
         mFormat(format),
@@ -150,6 +149,7 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp<SurfaceCont
                                                               GraphicBuffer::USAGE_HW_TEXTURE,
                                                       1, false);
     static int32_t id = 0;
+    mName = name + "#" + std::to_string(id);
     auto consumerName = mName + "(BLAST Consumer)" + std::to_string(id);
     mQueuedBufferTrace = "QueuedBuffer - " + mName + "BLAST#" + std::to_string(id);
     id++;
@@ -313,24 +313,24 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence
 // BBQ. This is because if the BBQ is destroyed, then the buffers will be released by the client.
 // So we pass in a weak pointer to the BBQ and if it still alive, then we release the buffer.
 // Otherwise, this is a no-op.
-static void releaseBufferCallbackThunk(wp<BLASTBufferQueue> context, uint64_t graphicBufferId,
+static void releaseBufferCallbackThunk(wp<BLASTBufferQueue> context, const ReleaseCallbackId& id,
                                        const sp<Fence>& releaseFence, uint32_t transformHint,
                                        uint32_t currentMaxAcquiredBufferCount) {
     sp<BLASTBufferQueue> blastBufferQueue = context.promote();
-    ALOGV("releaseBufferCallbackThunk graphicBufferId=%" PRIu64 " blastBufferQueue=%s",
-          graphicBufferId, blastBufferQueue ? "alive" : "dead");
     if (blastBufferQueue) {
-        blastBufferQueue->releaseBufferCallback(graphicBufferId, releaseFence, transformHint,
+        blastBufferQueue->releaseBufferCallback(id, releaseFence, transformHint,
                                                 currentMaxAcquiredBufferCount);
+    } else {
+        ALOGV("releaseBufferCallbackThunk %s blastBufferQueue is dead", id.to_string().c_str());
     }
 }
 
-void BLASTBufferQueue::releaseBufferCallback(uint64_t graphicBufferId,
+void BLASTBufferQueue::releaseBufferCallback(const ReleaseCallbackId& id,
                                              const sp<Fence>& releaseFence, uint32_t transformHint,
                                              uint32_t currentMaxAcquiredBufferCount) {
     ATRACE_CALL();
     std::unique_lock _lock{mMutex};
-    BQA_LOGV("releaseBufferCallback graphicBufferId=%" PRIu64, graphicBufferId);
+    BQA_LOGV("releaseBufferCallback %s", id.to_string().c_str());
 
     if (mSurfaceControl != nullptr) {
         mTransformHint = transformHint;
@@ -343,25 +343,26 @@ void BLASTBufferQueue::releaseBufferCallback(uint64_t graphicBufferId,
     // on a lower refresh rate than the max supported. We only do that for EGL
     // clients as others don't care about latency
     const bool isEGL = [&] {
-        const auto it = mSubmitted.find(graphicBufferId);
+        const auto it = mSubmitted.find(id);
         return it != mSubmitted.end() && it->second.mApi == NATIVE_WINDOW_API_EGL;
     }();
 
     const auto numPendingBuffersToHold =
             isEGL ? std::max(0u, mMaxAcquiredBuffers - currentMaxAcquiredBufferCount) : 0;
-    mPendingRelease.emplace_back(ReleasedBuffer{graphicBufferId, releaseFence});
+    mPendingRelease.emplace_back(ReleasedBuffer{id, releaseFence});
 
     // Release all buffers that are beyond the ones that we need to hold
     while (mPendingRelease.size() > numPendingBuffersToHold) {
         const auto releaseBuffer = mPendingRelease.front();
         mPendingRelease.pop_front();
-        auto it = mSubmitted.find(releaseBuffer.bufferId);
+        auto it = mSubmitted.find(releaseBuffer.callbackId);
         if (it == mSubmitted.end()) {
-            BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %" PRIu64,
-                     graphicBufferId);
+            BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %s",
+                     releaseBuffer.callbackId.to_string().c_str());
             return;
         }
         mNumAcquired--;
+        BQA_LOGV("released %s", id.to_string().c_str());
         mBufferItemConsumer->releaseBuffer(it->second, releaseBuffer.releaseFence);
         mSubmitted.erase(it);
         processNextBufferLocked(false /* useNextTransaction */);
@@ -428,7 +429,9 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) {
     }
 
     mNumAcquired++;
-    mSubmitted[buffer->getId()] = bufferItem;
+    mLastAcquiredFrameNumber = bufferItem.mFrameNumber;
+    ReleaseCallbackId releaseCallbackId(buffer->getId(), mLastAcquiredFrameNumber);
+    mSubmitted[releaseCallbackId] = bufferItem;
 
     bool needsDisconnect = false;
     mBufferItemConsumer->getConnectionEvents(bufferItem.mFrameNumber, &needsDisconnect);
@@ -442,7 +445,6 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) {
     incStrong((void*)transactionCallbackThunk);
 
     Rect crop = computeCrop(bufferItem);
-    mLastAcquiredFrameNumber = bufferItem.mFrameNumber;
     mLastBufferInfo.update(true /* hasBuffer */, bufferItem.mGraphicBuffer->getWidth(),
                            bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform,
                            bufferItem.mScalingMode, crop);
@@ -451,7 +453,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) {
             std::bind(releaseBufferCallbackThunk, wp<BLASTBufferQueue>(this) /* callbackContext */,
                       std::placeholders::_1, std::placeholders::_2, std::placeholders::_3,
                       std::placeholders::_4);
-    t->setBuffer(mSurfaceControl, buffer, releaseBufferCallback);
+    t->setBuffer(mSurfaceControl, buffer, releaseCallbackId, releaseBufferCallback);
     t->setDataspace(mSurfaceControl, static_cast<ui::Dataspace>(bufferItem.mDataSpace));
     t->setHdrMetadata(mSurfaceControl, bufferItem.mHdrMetadata);
     t->setSurfaceDamageRegion(mSurfaceControl, bufferItem.mSurfaceDamage);
@@ -510,11 +512,11 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) {
 
     BQA_LOGV("processNextBufferLocked size=%dx%d mFrameNumber=%" PRIu64
              " applyTransaction=%s mTimestamp=%" PRId64 "%s mPendingTransactions.size=%d"
-             " graphicBufferId=%" PRIu64,
+             " graphicBufferId=%" PRIu64 "%s",
              mSize.width, mSize.height, bufferItem.mFrameNumber, toString(applyTransaction),
              bufferItem.mTimestamp, bufferItem.mIsAutoTimestamp ? "(auto)" : "",
-             static_cast<uint32_t>(mPendingTransactions.size()),
-             bufferItem.mGraphicBuffer->getId());
+             static_cast<uint32_t>(mPendingTransactions.size()), bufferItem.mGraphicBuffer->getId(),
+             bufferItem.mAutoRefresh ? " mAutoRefresh" : "");
 }
 
 Rect BLASTBufferQueue::computeCrop(const BufferItem& item) {
diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp
index 17499ecd50..98e8b548bd 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -125,7 +125,7 @@ status_t SurfaceStats::writeToParcel(Parcel* output) const {
     for (const auto& data : jankData) {
         SAFE_PARCEL(output->writeParcelable, data);
     }
-    SAFE_PARCEL(output->writeUint64, previousBufferId);
+    SAFE_PARCEL(output->writeParcelable, previousReleaseCallbackId);
     return NO_ERROR;
 }
 
@@ -149,7 +149,7 @@ status_t SurfaceStats::readFromParcel(const Parcel* input) {
         SAFE_PARCEL(input->readParcelable, &data);
         jankData.push_back(data);
     }
-    SAFE_PARCEL(input->readUint64, &previousBufferId);
+    SAFE_PARCEL(input->readParcelable, &previousReleaseCallbackId);
     return NO_ERROR;
 }
 
@@ -253,11 +253,11 @@ public:
                                                                   stats);
     }
 
-    void onReleaseBuffer(uint64_t graphicBufferId, sp<Fence> releaseFence, uint32_t transformHint,
-                         uint32_t currentMaxAcquiredBufferCount) override {
+    void onReleaseBuffer(ReleaseCallbackId callbackId, sp<Fence> releaseFence,
+                         uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) override {
         callRemoteAsync<decltype(
                 &ITransactionCompletedListener::onReleaseBuffer)>(Tag::ON_RELEASE_BUFFER,
-                                                                  graphicBufferId, releaseFence,
+                                                                  callbackId, releaseFence,
                                                                   transformHint,
                                                                   currentMaxAcquiredBufferCount);
     }
@@ -308,4 +308,18 @@ status_t CallbackId::readFromParcel(const Parcel* input) {
     return NO_ERROR;
 }
 
+status_t ReleaseCallbackId::writeToParcel(Parcel* output) const {
+    SAFE_PARCEL(output->writeUint64, bufferId);
+    SAFE_PARCEL(output->writeUint64, framenumber);
+    return NO_ERROR;
+}
+
+status_t ReleaseCallbackId::readFromParcel(const Parcel* input) {
+    SAFE_PARCEL(input->readUint64, &bufferId);
+    SAFE_PARCEL(input->readUint64, &framenumber);
+    return NO_ERROR;
+}
+
+const ReleaseCallbackId ReleaseCallbackId::INVALID_ID = ReleaseCallbackId(0, 0);
+
 }; // namespace android
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index fd78309462..6ea070ea2f 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -197,15 +197,16 @@ void TransactionCompletedListener::removeJankListener(const sp<JankDataListener>
     }
 }
 
-void TransactionCompletedListener::setReleaseBufferCallback(uint64_t graphicBufferId,
+void TransactionCompletedListener::setReleaseBufferCallback(const ReleaseCallbackId& callbackId,
                                                             ReleaseBufferCallback listener) {
     std::scoped_lock<std::mutex> lock(mMutex);
-    mReleaseBufferCallbacks[graphicBufferId] = listener;
+    mReleaseBufferCallbacks[callbackId] = listener;
 }
 
-void TransactionCompletedListener::removeReleaseBufferCallback(uint64_t graphicBufferId) {
+void TransactionCompletedListener::removeReleaseBufferCallback(
+        const ReleaseCallbackId& callbackId) {
     std::scoped_lock<std::mutex> lock(mMutex);
-    mReleaseBufferCallbacks.erase(graphicBufferId);
+    mReleaseBufferCallbacks.erase(callbackId);
 }
 
 void TransactionCompletedListener::addSurfaceStatsListener(void* context, void* cookie,
@@ -319,14 +320,15 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
                 // and call them. This is a performance optimization when we have a transaction
                 // callback and a release buffer callback happening at the same time to avoid an
                 // additional ipc call from the server.
-                if (surfaceStats.previousBufferId) {
+                if (surfaceStats.previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) {
                     ReleaseBufferCallback callback;
                     {
                         std::scoped_lock<std::mutex> lock(mMutex);
-                        callback = popReleaseBufferCallbackLocked(surfaceStats.previousBufferId);
+                        callback = popReleaseBufferCallbackLocked(
+                                surfaceStats.previousReleaseCallbackId);
                     }
                     if (callback) {
-                        callback(surfaceStats.previousBufferId,
+                        callback(surfaceStats.previousReleaseCallbackId,
                                  surfaceStats.previousReleaseFence
                                          ? surfaceStats.previousReleaseFence
                                          : Fence::NO_FENCE,
@@ -362,25 +364,26 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
     }
 }
 
-void TransactionCompletedListener::onReleaseBuffer(uint64_t graphicBufferId, sp<Fence> releaseFence,
-                                                   uint32_t transformHint,
+void TransactionCompletedListener::onReleaseBuffer(ReleaseCallbackId callbackId,
+                                                   sp<Fence> releaseFence, uint32_t transformHint,
                                                    uint32_t currentMaxAcquiredBufferCount) {
     ReleaseBufferCallback callback;
     {
         std::scoped_lock<std::mutex> lock(mMutex);
-        callback = popReleaseBufferCallbackLocked(graphicBufferId);
+        callback = popReleaseBufferCallbackLocked(callbackId);
     }
     if (!callback) {
-        ALOGE("Could not call release buffer callback, buffer not found %" PRIu64, graphicBufferId);
+        ALOGE("Could not call release buffer callback, buffer not found %s",
+              callbackId.to_string().c_str());
         return;
     }
-    callback(graphicBufferId, releaseFence, transformHint, currentMaxAcquiredBufferCount);
+    callback(callbackId, releaseFence, transformHint, currentMaxAcquiredBufferCount);
 }
 
 ReleaseBufferCallback TransactionCompletedListener::popReleaseBufferCallbackLocked(
-        uint64_t graphicBufferId) {
+        const ReleaseCallbackId& callbackId) {
     ReleaseBufferCallback callback;
-    auto itr = mReleaseBufferCallbacks.find(graphicBufferId);
+    auto itr = mReleaseBufferCallbacks.find(callbackId);
     if (itr == mReleaseBufferCallbacks.end()) {
         return nullptr;
     }
@@ -1258,7 +1261,7 @@ SurfaceComposerClient::Transaction::setTransformToDisplayInverse(const sp<Surfac
 }
 
 SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffer(
-        const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer,
+        const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer, const ReleaseCallbackId& id,
         ReleaseBufferCallback callback) {
     layer_state_t* s = getLayerState(sc);
     if (!s) {
@@ -1271,7 +1274,7 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe
     if (mIsAutoTimestamp) {
         mDesiredPresentTime = systemTime();
     }
-    setReleaseBufferCallback(s, callback);
+    setReleaseBufferCallback(s, id, callback);
 
     registerSurfaceControlForCallback(sc);
 
@@ -1286,10 +1289,13 @@ void SurfaceComposerClient::Transaction::removeReleaseBufferCallback(layer_state
 
     s->what &= ~static_cast<uint64_t>(layer_state_t::eReleaseBufferListenerChanged);
     s->releaseBufferListener = nullptr;
-    TransactionCompletedListener::getInstance()->removeReleaseBufferCallback(s->buffer->getId());
+    auto listener = TransactionCompletedListener::getInstance();
+    listener->removeReleaseBufferCallback(s->releaseCallbackId);
+    s->releaseCallbackId = ReleaseCallbackId::INVALID_ID;
 }
 
 void SurfaceComposerClient::Transaction::setReleaseBufferCallback(layer_state_t* s,
+                                                                  const ReleaseCallbackId& id,
                                                                   ReleaseBufferCallback callback) {
     if (!callback) {
         return;
@@ -1303,8 +1309,9 @@ void SurfaceComposerClient::Transaction::setReleaseBufferCallback(layer_state_t*
 
     s->what |= layer_state_t::eReleaseBufferListenerChanged;
     s->releaseBufferListener = TransactionCompletedListener::getIInstance();
+    s->releaseCallbackId = id;
     auto listener = TransactionCompletedListener::getInstance();
-    listener->setReleaseBufferCallback(s->buffer->getId(), callback);
+    listener->setReleaseBufferCallback(id, callback);
 }
 
 SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setAcquireFence(
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 26c7285ab0..0d6a673b02 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -89,7 +89,7 @@ public:
 
     void transactionCallback(nsecs_t latchTime, const sp<Fence>& presentFence,
             const std::vector<SurfaceControlStats>& stats);
-    void releaseBufferCallback(uint64_t graphicBufferId, const sp<Fence>& releaseFence,
+    void releaseBufferCallback(const ReleaseCallbackId& id, const sp<Fence>& releaseFence,
                                uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount);
     void setNextTransaction(SurfaceComposerClient::Transaction *t);
     void mergeWithNextTransaction(SurfaceComposerClient::Transaction* t, uint64_t frameNumber);
@@ -144,13 +144,14 @@ private:
 
     // Keep a reference to the submitted buffers so we can release when surfaceflinger drops the
     // buffer or the buffer has been presented and a new buffer is ready to be presented.
-    std::unordered_map<uint64_t /* bufferId */, BufferItem> mSubmitted GUARDED_BY(mMutex);
+    std::unordered_map<ReleaseCallbackId, BufferItem, ReleaseBufferCallbackIdHash> mSubmitted
+            GUARDED_BY(mMutex);
 
     // Keep a queue of the released buffers instead of immediately releasing
     // the buffers back to the buffer queue. This would be controlled by SF
     // setting the max acquired buffer count.
     struct ReleasedBuffer {
-        uint64_t bufferId;
+        ReleaseCallbackId callbackId;
         sp<Fence> releaseFence;
     };
     std::deque<ReleasedBuffer> mPendingRelease GUARDED_BY(mMutex);
diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h
index d286c34ec8..937095c543 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -53,6 +53,36 @@ struct CallbackIdHash {
     std::size_t operator()(const CallbackId& key) const { return std::hash<int64_t>()(key.id); }
 };
 
+class ReleaseCallbackId : public Parcelable {
+public:
+    static const ReleaseCallbackId INVALID_ID;
+
+    uint64_t bufferId;
+    uint64_t framenumber;
+    ReleaseCallbackId() {}
+    ReleaseCallbackId(uint64_t bufferId, uint64_t framenumber)
+          : bufferId(bufferId), framenumber(framenumber) {}
+    status_t writeToParcel(Parcel* output) const override;
+    status_t readFromParcel(const Parcel* input) override;
+
+    bool operator==(const ReleaseCallbackId& rhs) const {
+        return bufferId == rhs.bufferId && framenumber == rhs.framenumber;
+    }
+    bool operator!=(const ReleaseCallbackId& rhs) const { return !operator==(rhs); }
+    std::string to_string() const {
+        if (*this == INVALID_ID) return "INVALID_ID";
+
+        return "bufferId:" + std::to_string(bufferId) +
+                " framenumber:" + std::to_string(framenumber);
+    }
+};
+
+struct ReleaseBufferCallbackIdHash {
+    std::size_t operator()(const ReleaseCallbackId& key) const {
+        return std::hash<uint64_t>()(key.bufferId);
+    }
+};
+
 class FrameEventHistoryStats : public Parcelable {
 public:
     status_t writeToParcel(Parcel* output) const override;
@@ -103,7 +133,7 @@ public:
     SurfaceStats(const sp<IBinder>& sc, nsecs_t time, const sp<Fence>& prevReleaseFence,
                  uint32_t hint, uint32_t currentMaxAcquiredBuffersCount,
                  FrameEventHistoryStats frameEventStats, std::vector<JankData> jankData,
-                 uint64_t previousBufferId)
+                 ReleaseCallbackId previousReleaseCallbackId)
           : surfaceControl(sc),
             acquireTime(time),
             previousReleaseFence(prevReleaseFence),
@@ -111,7 +141,7 @@ public:
             currentMaxAcquiredBufferCount(currentMaxAcquiredBuffersCount),
             eventStats(frameEventStats),
             jankData(std::move(jankData)),
-            previousBufferId(previousBufferId) {}
+            previousReleaseCallbackId(previousReleaseCallbackId) {}
 
     sp<IBinder> surfaceControl;
     nsecs_t acquireTime = -1;
@@ -120,7 +150,7 @@ public:
     uint32_t currentMaxAcquiredBufferCount = 0;
     FrameEventHistoryStats eventStats;
     std::vector<JankData> jankData;
-    uint64_t previousBufferId;
+    ReleaseCallbackId previousReleaseCallbackId;
 };
 
 class TransactionStats : public Parcelable {
@@ -161,7 +191,7 @@ public:
 
     virtual void onTransactionCompleted(ListenerStats stats) = 0;
 
-    virtual void onReleaseBuffer(uint64_t graphicBufferId, sp<Fence> releaseFence,
+    virtual void onReleaseBuffer(ReleaseCallbackId callbackId, sp<Fence> releaseFence,
                                  uint32_t transformHint,
                                  uint32_t currentMaxAcquiredBufferCount) = 0;
 };
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 16430b324d..8ac1e5d9e9 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -234,6 +234,12 @@ struct layer_state_t {
     // layers only. The callback includes a release fence as well as the graphic
     // buffer id to identify the buffer.
     sp<ITransactionCompletedListener> releaseBufferListener;
+
+    // Keeps track of the release callback id associated with the listener. This
+    // is not sent to the server since the id can be reconstructed there. This
+    // is used to remove the old callback from the client process map if it is
+    // overwritten by another setBuffer call.
+    ReleaseCallbackId releaseCallbackId;
 };
 
 struct ComposerState {
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 13994fd9eb..c2963b58df 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -82,7 +82,7 @@ using TransactionCompletedCallback =
         std::function<void(nsecs_t /*latchTime*/, const sp<Fence>& /*presentFence*/,
                            const std::vector<SurfaceControlStats>& /*stats*/)>;
 using ReleaseBufferCallback =
-        std::function<void(uint64_t /* graphicsBufferId */, const sp<Fence>& /*releaseFence*/,
+        std::function<void(const ReleaseCallbackId&, const sp<Fence>& /*releaseFence*/,
                            uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount)>;
 
 using SurfaceStatsCallback =
@@ -397,8 +397,9 @@ public:
 
         void cacheBuffers();
         void registerSurfaceControlForCallback(const sp<SurfaceControl>& sc);
-        void setReleaseBufferCallback(layer_state_t* state, ReleaseBufferCallback callback);
-        void removeReleaseBufferCallback(layer_state_t* state);
+        void setReleaseBufferCallback(layer_state_t*, const ReleaseCallbackId&,
+                                      ReleaseBufferCallback);
+        void removeReleaseBufferCallback(layer_state_t*);
 
     public:
         Transaction();
@@ -470,6 +471,7 @@ public:
         Transaction& setTransformToDisplayInverse(const sp<SurfaceControl>& sc,
                                                   bool transformToDisplayInverse);
         Transaction& setBuffer(const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer,
+                               const ReleaseCallbackId& id = ReleaseCallbackId::INVALID_ID,
                                ReleaseBufferCallback callback = nullptr);
         Transaction& setCachedBuffer(const sp<SurfaceControl>& sc, int32_t bufferId);
         Transaction& setAcquireFence(const sp<SurfaceControl>& sc, const sp<Fence>& fence);
@@ -678,7 +680,7 @@ class TransactionCompletedListener : public BnTransactionCompletedListener {
     std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> mCallbacks
             GUARDED_BY(mMutex);
     std::multimap<sp<IBinder>, sp<JankDataListener>> mJankListeners GUARDED_BY(mMutex);
-    std::unordered_map<uint64_t /* graphicsBufferId */, ReleaseBufferCallback>
+    std::unordered_map<ReleaseCallbackId, ReleaseBufferCallback, ReleaseBufferCallbackIdHash>
             mReleaseBufferCallbacks GUARDED_BY(mMutex);
 
     // This is protected by mSurfaceStatsListenerMutex, but GUARDED_BY isn't supported for
@@ -717,16 +719,16 @@ public:
                 SurfaceStatsCallback listener);
     void removeSurfaceStatsListener(void* context, void* cookie);
 
-    void setReleaseBufferCallback(uint64_t /* graphicsBufferId */, ReleaseBufferCallback);
-    void removeReleaseBufferCallback(uint64_t /* graphicsBufferId */);
+    void setReleaseBufferCallback(const ReleaseCallbackId&, ReleaseBufferCallback);
+    void removeReleaseBufferCallback(const ReleaseCallbackId&);
 
     // BnTransactionCompletedListener overrides
     void onTransactionCompleted(ListenerStats stats) override;
-    void onReleaseBuffer(uint64_t /* graphicsBufferId */, sp<Fence> releaseFence,
-                         uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) override;
+    void onReleaseBuffer(ReleaseCallbackId, sp<Fence> releaseFence, uint32_t transformHint,
+                         uint32_t currentMaxAcquiredBufferCount) override;
 
 private:
-    ReleaseBufferCallback popReleaseBufferCallbackLocked(uint64_t /* graphicsBufferId */);
+    ReleaseBufferCallback popReleaseBufferCallbackLocked(const ReleaseCallbackId&);
 };
 
 } // namespace android
diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h
index cd3d80e809..760c8b9f3c 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -133,6 +133,7 @@ protected:
         bool mTransformToDisplayInverse{false};
 
         std::shared_ptr<renderengine::ExternalTexture> mBuffer;
+        uint64_t mFrameNumber;
         int mBufferSlot{BufferQueue::INVALID_BUFFER_SLOT};
 
         bool mFrameLatencyNeeded{false};
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 6b5cf04536..8cf0d66a08 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -44,25 +44,18 @@ namespace android {
 using PresentState = frametimeline::SurfaceFrame::PresentState;
 namespace {
 void callReleaseBufferCallback(const sp<ITransactionCompletedListener>& listener,
-                               const sp<GraphicBuffer>& buffer, const sp<Fence>& releaseFence,
-                               uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) {
+                               const sp<GraphicBuffer>& buffer, uint64_t framenumber,
+                               const sp<Fence>& releaseFence, uint32_t transformHint,
+                               uint32_t currentMaxAcquiredBufferCount) {
     if (!listener) {
         return;
     }
-    listener->onReleaseBuffer(buffer->getId(), releaseFence ? releaseFence : Fence::NO_FENCE,
-                              transformHint, currentMaxAcquiredBufferCount);
+    listener->onReleaseBuffer({buffer->getId(), framenumber},
+                              releaseFence ? releaseFence : Fence::NO_FENCE, transformHint,
+                              currentMaxAcquiredBufferCount);
 }
 } // namespace
 
-// clang-format off
-const std::array<float, 16> BufferStateLayer::IDENTITY_MATRIX{
-        1, 0, 0, 0,
-        0, 1, 0, 0,
-        0, 0, 1, 0,
-        0, 0, 0, 1
-};
-// clang-format on
-
 BufferStateLayer::BufferStateLayer(const LayerCreationArgs& args)
       : BufferLayer(args), mHwcSlotGenerator(new HwcSlotGenerator()) {
     mDrawingState.dataspace = ui::Dataspace::V0_SRGB;
@@ -75,8 +68,8 @@ BufferStateLayer::~BufferStateLayer() {
     // issue with the clone layer trying to use the texture.
     if (mBufferInfo.mBuffer != nullptr && !isClone()) {
         callReleaseBufferCallback(mDrawingState.releaseBufferListener,
-                                  mBufferInfo.mBuffer->getBuffer(), mBufferInfo.mFence,
-                                  mTransformHint,
+                                  mBufferInfo.mBuffer->getBuffer(), mBufferInfo.mFrameNumber,
+                                  mBufferInfo.mFence, mTransformHint,
                                   mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(
                                           mOwnerUid));
     }
@@ -87,7 +80,7 @@ status_t BufferStateLayer::addReleaseFence(const sp<CallbackHandle>& ch,
     if (ch == nullptr) {
         return OK;
     }
-    ch->previousBufferId = mPreviousBufferId;
+    ch->previousReleaseCallbackId = mPreviousReleaseCallbackId;
     if (!ch->previousReleaseFence.get()) {
         ch->previousReleaseFence = fence;
         return OK;
@@ -214,7 +207,7 @@ void BufferStateLayer::releasePendingBuffer(nsecs_t dequeueReadyTime) {
     // see BufferStateLayer::onLayerDisplayed.
     for (auto& handle : mDrawingState.callbackHandles) {
         if (handle->releasePreviousBuffer) {
-            handle->previousBufferId = mPreviousBufferId;
+            handle->previousReleaseCallbackId = mPreviousReleaseCallbackId;
             break;
         }
     }
@@ -438,8 +431,8 @@ bool BufferStateLayer::setBuffer(const std::shared_ptr<renderengine::ExternalTex
             // dropped and we should decrement the pending buffer count and
             // call any release buffer callbacks if set.
             callReleaseBufferCallback(mDrawingState.releaseBufferListener,
-                                      mDrawingState.buffer->getBuffer(), mDrawingState.acquireFence,
-                                      mTransformHint,
+                                      mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber,
+                                      mDrawingState.acquireFence, mTransformHint,
                                       mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(
                                               mOwnerUid));
             decrementPendingBufferCount();
@@ -684,7 +677,7 @@ uint64_t BufferStateLayer::getFrameNumber(nsecs_t /*expectedPresentTime*/) const
  *     DeferTransactionUntil -> frameNumber = 2
  *     Random other stuff
  *  }
- * Now imagine getHeadFrameNumber returned mDrawingState.mFrameNumber (or mCurrentFrameNumber).
+ * Now imagine mFrameNumber returned mDrawingState.frameNumber (or mCurrentFrameNumber).
  * Prior to doTransaction SurfaceFlinger will call notifyAvailableFrames, but because we
  * haven't swapped mDrawingState to mDrawingState yet we will think the sync point
  * is not ready. So we will return false from applyPendingState and not swap
@@ -792,9 +785,10 @@ status_t BufferStateLayer::updateActiveBuffer() {
         decrementPendingBufferCount();
     }
 
-    mPreviousBufferId = getCurrentBufferId();
+    mPreviousReleaseCallbackId = {getCurrentBufferId(), mBufferInfo.mFrameNumber};
     mBufferInfo.mBuffer = s.buffer;
     mBufferInfo.mFence = s.acquireFence;
+    mBufferInfo.mFrameNumber = s.frameNumber;
 
     return NO_ERROR;
 }
@@ -969,8 +963,8 @@ void BufferStateLayer::bufferMayChange(const sp<GraphicBuffer>& newBuffer) {
         // then we will drop a buffer and should decrement the pending buffer count and
         // call any release buffer callbacks if set.
         callReleaseBufferCallback(mDrawingState.releaseBufferListener,
-                                  mDrawingState.buffer->getBuffer(), mDrawingState.acquireFence,
-                                  mTransformHint,
+                                  mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber,
+                                  mDrawingState.acquireFence, mTransformHint,
                                   mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(
                                           mOwnerUid));
         decrementPendingBufferCount();
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index 27470182bf..e5674785af 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -143,15 +143,8 @@ private:
 
     bool bufferNeedsFiltering() const override;
 
-    static const std::array<float, 16> IDENTITY_MATRIX;
-
-    std::unique_ptr<renderengine::Image> mTextureImage;
-
-    mutable uint64_t mFrameNumber{0};
-    uint64_t mFrameCounter{0};
-
     sp<Fence> mPreviousReleaseFence;
-    uint64_t mPreviousBufferId = 0;
+    ReleaseCallbackId mPreviousReleaseCallbackId = ReleaseCallbackId::INVALID_ID;
     uint64_t mPreviousReleasedFrameNumber = 0;
 
     bool mReleasePreviousBuffer = false;
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp
index fdf16a797f..6af69f0ef2 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -237,7 +237,8 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>&
                                                     handle->previousReleaseFence,
                                                     handle->transformHint,
                                                     handle->currentMaxAcquiredBufferCount,
-                                                    eventStats, jankData, handle->previousBufferId);
+                                                    eventStats, jankData,
+                                                    handle->previousReleaseCallbackId);
     }
     return NO_ERROR;
 }
diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h
index 444bec646e..6f4d812ec5 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.h
+++ b/services/surfaceflinger/TransactionCallbackInvoker.h
@@ -51,7 +51,7 @@ public:
     nsecs_t refreshStartTime = 0;
     nsecs_t dequeueReadyTime = 0;
     uint64_t frameNumber = 0;
-    uint64_t previousBufferId = 0;
+    ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID;
 };
 
 class TransactionCallbackInvoker {
diff --git a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
index 5aa809dc8b..579a26ebf4 100644
--- a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
+++ b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
@@ -29,7 +29,7 @@ using android::hardware::graphics::common::V1_1::BufferUsage;
 // b/181132765 - disabled until cuttlefish failures are investigated
 class ReleaseBufferCallbackHelper {
 public:
-    static void function(void* callbackContext, uint64_t graphicsBufferId,
+    static void function(void* callbackContext, ReleaseCallbackId callbackId,
                          const sp<Fence>& releaseFence,
                          uint32_t /*currentMaxAcquiredBufferCount*/) {
         if (!callbackContext) {
@@ -38,11 +38,11 @@ public:
         ReleaseBufferCallbackHelper* helper =
                 static_cast<ReleaseBufferCallbackHelper*>(callbackContext);
         std::lock_guard lock(helper->mMutex);
-        helper->mCallbackDataQueue.emplace(graphicsBufferId, releaseFence);
+        helper->mCallbackDataQueue.emplace(callbackId, releaseFence);
         helper->mConditionVariable.notify_all();
     }
 
-    void getCallbackData(uint64_t* bufferId) {
+    void getCallbackData(ReleaseCallbackId* callbackId) {
         std::unique_lock lock(mMutex);
         if (mCallbackDataQueue.empty()) {
             if (!mConditionVariable.wait_for(lock, std::chrono::seconds(3),
@@ -53,7 +53,7 @@ public:
 
         auto callbackData = mCallbackDataQueue.front();
         mCallbackDataQueue.pop();
-        *bufferId = callbackData.first;
+        *callbackId = callbackData.first;
     }
 
     void verifyNoCallbacks() {
@@ -72,7 +72,7 @@ public:
 
     std::mutex mMutex;
     std::condition_variable mConditionVariable;
-    std::queue<std::pair<uint64_t, sp<Fence>>> mCallbackDataQueue;
+    std::queue<std::pair<ReleaseCallbackId, sp<Fence>>> mCallbackDataQueue;
 };
 
 class ReleaseBufferCallbackTest : public LayerTransactionTest {
@@ -82,10 +82,11 @@ public:
     }
 
     static void submitBuffer(const sp<SurfaceControl>& layer, sp<GraphicBuffer> buffer,
-                             sp<Fence> fence, CallbackHelper& callback,
+                             sp<Fence> fence, CallbackHelper& callback, const ReleaseCallbackId& id,
                              ReleaseBufferCallbackHelper& releaseCallback) {
         Transaction t;
-        t.setBuffer(layer, buffer, releaseCallback.getCallback());
+        t.setFrameNumber(layer, id.framenumber);
+        t.setBuffer(layer, buffer, id, releaseCallback.getCallback());
         t.setAcquireFence(layer, fence);
         t.addTransactionCompletedCallback(callback.function, callback.getContext());
         t.apply();
@@ -98,10 +99,10 @@ public:
     }
 
     static void waitForReleaseBufferCallback(ReleaseBufferCallbackHelper& releaseCallback,
-                                             uint64_t expectedReleaseBufferId) {
-        uint64_t actualReleaseBufferId;
+                                             const ReleaseCallbackId& expectedCallbackId) {
+        ReleaseCallbackId actualReleaseBufferId;
         releaseCallback.getCallbackData(&actualReleaseBufferId);
-        EXPECT_EQ(expectedReleaseBufferId, actualReleaseBufferId);
+        EXPECT_EQ(expectedCallbackId, actualReleaseBufferId);
         releaseCallback.verifyNoCallbacks();
     }
     static ReleaseBufferCallbackHelper* getReleaseBufferCallbackHelper() {
@@ -116,6 +117,10 @@ public:
                                          BufferUsage::COMPOSER_OVERLAY,
                                  "test");
     }
+    static uint64_t generateFrameNumber() {
+        static uint64_t sFrameNumber = 0;
+        return ++sFrameNumber;
+    }
 };
 
 TEST_F(ReleaseBufferCallbackTest, DISABLED_PresentBuffer) {
@@ -125,7 +130,9 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_PresentBuffer) {
 
     // If a buffer is being presented, we should not emit a release callback.
     sp<GraphicBuffer> firstBuffer = getBuffer();
-    submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback);
+    ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
+    submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, firstBufferCallbackId,
+                 *releaseCallback);
     ExpectedResult expected;
     expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
                         ExpectedResult::Buffer::NOT_ACQUIRED);
@@ -143,13 +150,15 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_PresentBuffer) {
     // If a presented buffer is replaced, we should emit a release callback for the
     // previously presented buffer.
     sp<GraphicBuffer> secondBuffer = getBuffer();
-    submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback);
+    ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+    submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, secondBufferCallbackId,
+                 *releaseCallback);
     expected = ExpectedResult();
     expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
                         ExpectedResult::Buffer::NOT_ACQUIRED,
                         ExpectedResult::PreviousBuffer::RELEASED);
     ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected));
-    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId()));
+    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
 }
 
 TEST_F(ReleaseBufferCallbackTest, DISABLED_OffScreenLayer) {
@@ -160,7 +169,9 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_OffScreenLayer) {
 
     // If a buffer is being presented, we should not emit a release callback.
     sp<GraphicBuffer> firstBuffer = getBuffer();
-    submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback);
+    ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
+    submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, firstBufferCallbackId,
+                 *releaseCallback);
     ExpectedResult expected;
     expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
                         ExpectedResult::Buffer::NOT_ACQUIRED);
@@ -184,23 +195,27 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_OffScreenLayer) {
     // If a presented buffer is replaced, we should emit a release callback for the
     // previously presented buffer.
     sp<GraphicBuffer> secondBuffer = getBuffer();
-    submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback);
+    ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+    submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, secondBufferCallbackId,
+                 *releaseCallback);
     expected = ExpectedResult();
     expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
                         ExpectedResult::Buffer::NOT_ACQUIRED,
                         ExpectedResult::PreviousBuffer::NOT_RELEASED);
     ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected));
-    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId()));
+    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
 
     // If continue to submit buffer we continue to get release callbacks
     sp<GraphicBuffer> thirdBuffer = getBuffer();
-    submitBuffer(layer, thirdBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback);
+    ReleaseCallbackId thirdBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+    submitBuffer(layer, thirdBuffer, Fence::NO_FENCE, transactionCallback, thirdBufferCallbackId,
+                 *releaseCallback);
     expected = ExpectedResult();
     expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
                         ExpectedResult::Buffer::NOT_ACQUIRED,
                         ExpectedResult::PreviousBuffer::NOT_RELEASED);
     ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected));
-    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBuffer->getId()));
+    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBufferCallbackId));
 }
 
 TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_layerdestroy) {
@@ -210,7 +225,9 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_layerdestroy) {
 
     // If a buffer is being presented, we should not emit a release callback.
     sp<GraphicBuffer> firstBuffer = getBuffer();
-    submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, *releaseCallback);
+    ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
+    submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, firstBufferCallbackId,
+                 *releaseCallback);
     {
         ExpectedResult expected;
         expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
@@ -225,7 +242,7 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_layerdestroy) {
     t.apply();
     layer = nullptr;
 
-    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId()));
+    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
 }
 
 // Destroying a never presented layer emits a callback.
@@ -242,7 +259,9 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_OffScreenLayerDestroy)
 
     // Submitting a buffer does not emit a callback.
     sp<GraphicBuffer> firstBuffer = getBuffer();
-    submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, *releaseCallback);
+    ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
+    submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, firstBufferCallbackId,
+                 *releaseCallback);
     {
         ExpectedResult expected;
         expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
@@ -253,19 +272,21 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_OffScreenLayerDestroy)
 
     // Submitting a second buffer will replace the drawing state buffer and emit a callback.
     sp<GraphicBuffer> secondBuffer = getBuffer();
-    submitBuffer(layer, secondBuffer, Fence::NO_FENCE, *transactionCallback, *releaseCallback);
+    ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+    submitBuffer(layer, secondBuffer, Fence::NO_FENCE, *transactionCallback, secondBufferCallbackId,
+                 *releaseCallback);
     {
         ExpectedResult expected;
         expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer,
                             ExpectedResult::Buffer::NOT_ACQUIRED);
         ASSERT_NO_FATAL_FAILURE(waitForCallback(*transactionCallback, expected));
         ASSERT_NO_FATAL_FAILURE(
-                waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId()));
+                waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
     }
 
     // Destroying the offscreen layer emits a callback.
     layer = nullptr;
-    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBuffer->getId()));
+    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBufferCallbackId));
 }
 
 TEST_F(ReleaseBufferCallbackTest, DISABLED_FrameDropping) {
@@ -275,12 +296,13 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_FrameDropping) {
 
     // If a buffer is being presented, we should not emit a release callback.
     sp<GraphicBuffer> firstBuffer = getBuffer();
+    ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber());
 
     // Try to present 100ms in the future
     nsecs_t time = systemTime() + std::chrono::nanoseconds(100ms).count();
 
     Transaction t;
-    t.setBuffer(layer, firstBuffer, releaseCallback->getCallback());
+    t.setBuffer(layer, firstBuffer, firstBufferCallbackId, releaseCallback->getCallback());
     t.setAcquireFence(layer, Fence::NO_FENCE);
     t.addTransactionCompletedCallback(transactionCallback.function,
                                       transactionCallback.getContext());
@@ -295,7 +317,8 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_FrameDropping) {
 
     // Dropping frames in transaction queue emits a callback
     sp<GraphicBuffer> secondBuffer = getBuffer();
-    t.setBuffer(layer, secondBuffer, releaseCallback->getCallback());
+    ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber());
+    t.setBuffer(layer, secondBuffer, secondBufferCallbackId, releaseCallback->getCallback());
     t.setAcquireFence(layer, Fence::NO_FENCE);
     t.addTransactionCompletedCallback(transactionCallback.function,
                                       transactionCallback.getContext());
@@ -307,7 +330,7 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_FrameDropping) {
                         ExpectedResult::Buffer::NOT_ACQUIRED,
                         ExpectedResult::PreviousBuffer::RELEASED);
     ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected));
-    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId()));
+    ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId));
 }
 
 } // namespace android
-- 
cgit v1.2.3-59-g8ed1b


From a30f7c99749f48d3c13c3c1ac96fc54a80ba1295 Mon Sep 17 00:00:00 2001
From: Winson Chung <winsonc@google.com>
Date: Tue, 29 Jun 2021 15:42:56 -0700
Subject: Add mechanism for a task's windows to be trusted overlays (SF)

- Add a layer state to indicate that this layer and its children
  in the hierarchy are trusted. This can only be set by callers
  holding ACCESS_SURFACE_FLINGER, and will be used for the PIP
  task layer to indicate that activities in PIP are trusted (as
  they are controlled only by the user and SystemUI)

Bug: 191529039
Test: TBD

Change-Id: Id92ccb087bd0d8dbaeeef3ba50b67fe015e53db8
---
 cmds/surfacereplayer/proto/src/trace.proto         |  5 +++++
 libs/gui/LayerState.cpp                            |  7 ++++++
 libs/gui/SurfaceComposerClient.cpp                 | 13 +++++++++++
 libs/gui/include/gui/LayerState.h                  |  5 +++++
 libs/gui/include/gui/SurfaceComposerClient.h       |  3 +++
 services/surfaceflinger/Layer.cpp                  | 23 +++++++++++++++++++
 services/surfaceflinger/Layer.h                    |  5 +++++
 services/surfaceflinger/SurfaceFlinger.cpp         |  9 ++++++++
 services/surfaceflinger/SurfaceInterceptor.cpp     | 11 +++++++++
 services/surfaceflinger/SurfaceInterceptor.h       |  1 +
 .../surfaceflinger/layerproto/LayerProtoParser.cpp |  2 ++
 .../include/layerproto/LayerProtoParser.h          |  1 +
 services/surfaceflinger/layerproto/layers.proto    |  2 ++
 .../tests/SurfaceInterceptor_test.cpp              | 26 ++++++++++++++++++++++
 14 files changed, 113 insertions(+)

(limited to 'libs/gui/SurfaceComposerClient.cpp')

diff --git a/cmds/surfacereplayer/proto/src/trace.proto b/cmds/surfacereplayer/proto/src/trace.proto
index 3798ba73a4..03a2709075 100644
--- a/cmds/surfacereplayer/proto/src/trace.proto
+++ b/cmds/surfacereplayer/proto/src/trace.proto
@@ -52,6 +52,7 @@ message SurfaceChange {
         BackgroundBlurRadiusChange  background_blur_radius  = 20;
         ShadowRadiusChange          shadow_radius           = 21;
         BlurRegionsChange           blur_regions            = 22;
+        TrustedOverlayChange        trusted_overlay         = 23;
     }
 }
 
@@ -192,6 +193,10 @@ message ShadowRadiusChange {
     required float radius = 1;
 }
 
+message TrustedOverlayChange {
+    required float is_trusted_overlay = 1;
+}
+
 message BlurRegionsChange {
     repeated BlurRegionChange blur_regions = 1;
 }
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 2d99fc1903..076c90dd23 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -64,6 +64,7 @@ layer_state_t::layer_state_t()
         fixedTransformHint(ui::Transform::ROT_INVALID),
         frameNumber(0),
         autoRefresh(false),
+        isTrustedOverlay(false),
         bufferCrop(Rect::INVALID_RECT),
         destinationFrame(Rect::INVALID_RECT),
         releaseBufferListener(nullptr) {
@@ -170,6 +171,7 @@ status_t layer_state_t::write(Parcel& output) const
     SAFE_PARCEL(output.write, stretchEffect);
     SAFE_PARCEL(output.write, bufferCrop);
     SAFE_PARCEL(output.write, destinationFrame);
+    SAFE_PARCEL(output.writeBool, isTrustedOverlay);
 
     return NO_ERROR;
 }
@@ -300,6 +302,7 @@ status_t layer_state_t::read(const Parcel& input)
     SAFE_PARCEL(input.read, stretchEffect);
     SAFE_PARCEL(input.read, bufferCrop);
     SAFE_PARCEL(input.read, destinationFrame);
+    SAFE_PARCEL(input.readBool, &isTrustedOverlay);
 
     return NO_ERROR;
 }
@@ -532,6 +535,10 @@ void layer_state_t::merge(const layer_state_t& other) {
         what |= eAutoRefreshChanged;
         autoRefresh = other.autoRefresh;
     }
+    if (other.what & eTrustedOverlayChanged) {
+        what |= eTrustedOverlayChanged;
+        isTrustedOverlay = other.isTrustedOverlay;
+    }
     if (other.what & eReleaseBufferListenerChanged) {
         if (releaseBufferListener) {
             ALOGW("Overriding releaseBufferListener");
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 6ea070ea2f..96da8efd19 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1656,6 +1656,19 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setAutoR
     return *this;
 }
 
+SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setTrustedOverlay(
+        const sp<SurfaceControl>& sc, bool isTrustedOverlay) {
+    layer_state_t* s = getLayerState(sc);
+    if (!s) {
+        mStatus = BAD_INDEX;
+        return *this;
+    }
+
+    s->what |= layer_state_t::eTrustedOverlayChanged;
+    s->isTrustedOverlay = isTrustedOverlay;
+    return *this;
+}
+
 SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setApplyToken(
         const sp<IBinder>& applyToken) {
     mApplyToken = applyToken;
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 465e34ca33..3e57ff611e 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -118,6 +118,7 @@ struct layer_state_t {
         eBlurRegionsChanged = 0x800'00000000,
         eAutoRefreshChanged = 0x1000'00000000,
         eStretchChanged = 0x2000'00000000,
+        eTrustedOverlayChanged = 0x4000'00000000,
     };
 
     layer_state_t();
@@ -225,6 +226,10 @@ struct layer_state_t {
     // in shared buffer mode.
     bool autoRefresh;
 
+    // An inherited state that indicates that this surface control and its children
+    // should be trusted for input occlusion detection purposes
+    bool isTrustedOverlay;
+
     // Stretch effect to be applied to this layer
     StretchEffect stretchEffect;
 
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index c2963b58df..baa0567617 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -537,6 +537,9 @@ public:
         // in shared buffer mode.
         Transaction& setAutoRefresh(const sp<SurfaceControl>& sc, bool autoRefresh);
 
+        // Sets that this surface control and its children are trusted overlays for input
+        Transaction& setTrustedOverlay(const sp<SurfaceControl>& sc, bool isTrustedOverlay);
+
         // Queues up transactions using this token in SurfaceFlinger.  By default, all transactions
         // from a client are placed on the same queue. This can be used to prevent multiple
         // transactions from blocking each other.
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index e2b6a36794..dbd2793276 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -851,6 +851,23 @@ bool Layer::setRelativeLayer(const sp<IBinder>& relativeToHandle, int32_t relati
     return true;
 }
 
+bool Layer::setTrustedOverlay(bool isTrustedOverlay) {
+    if (mDrawingState.isTrustedOverlay == isTrustedOverlay) return false;
+    mDrawingState.isTrustedOverlay = isTrustedOverlay;
+    mDrawingState.modified = true;
+    mFlinger->mInputInfoChanged = true;
+    setTransactionFlags(eTransactionNeeded);
+    return true;
+}
+
+bool Layer::isTrustedOverlay() const {
+    if (getDrawingState().isTrustedOverlay) {
+        return true;
+    }
+    const auto& p = mDrawingParent.promote();
+    return (p != nullptr) && p->isTrustedOverlay();
+}
+
 bool Layer::setSize(uint32_t w, uint32_t h) {
     if (mDrawingState.requested_legacy.w == w && mDrawingState.requested_legacy.h == h)
         return false;
@@ -2041,6 +2058,7 @@ void Layer::writeToProtoDrawingState(LayerProto* layerInfo, uint32_t traceFlags,
 
         layerInfo->set_corner_radius(getRoundedCornerState().radius);
         layerInfo->set_background_blur_radius(getBackgroundBlurRadius());
+        layerInfo->set_is_trusted_overlay(isTrustedOverlay());
         LayerProtoHelper::writeToProto(transform, layerInfo->mutable_transform());
         LayerProtoHelper::writePositionToProto(transform.tx(), transform.ty(),
                                                [&]() { return layerInfo->mutable_position(); });
@@ -2327,6 +2345,11 @@ InputWindowInfo Layer::fillInputInfo(const sp<DisplayDevice>& display) {
                 toPhysicalDisplay.transform(Rect{cropLayer->mScreenBounds}));
     }
 
+    // Inherit the trusted state from the parent hierarchy, but don't clobber the trusted state
+    // if it was set by WM for a known system overlay
+    info.trustedOverlay = info.trustedOverlay || isTrustedOverlay();
+
+
     // If the layer is a clone, we need to crop the input region to cloned root to prevent
     // touches from going outside the cloned area.
     if (isClone()) {
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 6b56b2d924..c5cb17ffc7 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -275,6 +275,9 @@ public:
         // Stretch effect to apply to this layer
         StretchEffect stretchEffect;
 
+        // Whether or not this layer is a trusted overlay for input
+        bool isTrustedOverlay;
+
         Rect bufferCrop;
         Rect destinationFrame;
     };
@@ -393,6 +396,7 @@ public:
     virtual bool setBackgroundBlurRadius(int backgroundBlurRadius);
     virtual bool setBlurRegions(const std::vector<BlurRegion>& effectRegions);
     virtual bool setTransparentRegionHint(const Region& transparent);
+    virtual bool setTrustedOverlay(bool);
     virtual bool setFlags(uint32_t flags, uint32_t mask);
     virtual bool setLayerStack(uint32_t layerStack);
     virtual uint32_t getLayerStack() const;
@@ -1050,6 +1054,7 @@ private:
 
     void updateTreeHasFrameRateVote();
     void setZOrderRelativeOf(const wp<Layer>& relativeOf);
+    bool isTrustedOverlay() const;
 
     // Find the root of the cloned hierarchy, this means the first non cloned parent.
     // This will return null if first non cloned parent is not found.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2cc810986f..f44ae71896 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4156,6 +4156,15 @@ uint32_t SurfaceFlinger::setClientStateLocked(
     if (what & layer_state_t::eAutoRefreshChanged) {
         layer->setAutoRefresh(s.autoRefresh);
     }
+    if (what & layer_state_t::eTrustedOverlayChanged) {
+        if (privileged) {
+            if (layer->setTrustedOverlay(s.isTrustedOverlay)) {
+                flags |= eTraversalNeeded;
+            }
+        } else {
+            ALOGE("Attempt to set trusted overlay without permission ACCESS_SURFACE_FLINGER");
+        }
+    }
     if (what & layer_state_t::eStretchChanged) {
         if (layer->setStretchEffect(s.stretchEffect)) {
             flags |= eTraversalNeeded;
diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp
index 8ca241e53d..23ab7c8bab 100644
--- a/services/surfaceflinger/SurfaceInterceptor.cpp
+++ b/services/surfaceflinger/SurfaceInterceptor.cpp
@@ -149,6 +149,7 @@ void SurfaceInterceptor::addInitialSurfaceStateLocked(Increment* increment,
                             getLayerIdFromWeakRef(layer->mDrawingState.zOrderRelativeOf),
                             layer->mDrawingState.z);
     addShadowRadiusLocked(transaction, layerId, layer->mDrawingState.shadowRadius);
+    addTrustedOverlayLocked(transaction, layerId, layer->mDrawingState.isTrustedOverlay);
 }
 
 void SurfaceInterceptor::addInitialDisplayStateLocked(Increment* increment,
@@ -397,6 +398,13 @@ void SurfaceInterceptor::addShadowRadiusLocked(Transaction* transaction, int32_t
     overrideChange->set_radius(shadowRadius);
 }
 
+void SurfaceInterceptor::addTrustedOverlayLocked(Transaction* transaction, int32_t layerId,
+                                                 bool isTrustedOverlay) {
+    SurfaceChange* change(createSurfaceChangeLocked(transaction, layerId));
+    TrustedOverlayChange* overrideChange(change->mutable_trusted_overlay());
+    overrideChange->set_is_trusted_overlay(isTrustedOverlay);
+}
+
 void SurfaceInterceptor::addSurfaceChangesLocked(Transaction* transaction,
         const layer_state_t& state)
 {
@@ -460,6 +468,9 @@ void SurfaceInterceptor::addSurfaceChangesLocked(Transaction* transaction,
     if (state.what & layer_state_t::eShadowRadiusChanged) {
         addShadowRadiusLocked(transaction, layerId, state.shadowRadius);
     }
+    if (state.what & layer_state_t::eTrustedOverlayChanged) {
+        addTrustedOverlayLocked(transaction, layerId, state.isTrustedOverlay);
+    }
     if (state.what & layer_state_t::eStretchChanged) {
         ALOGW("SurfaceInterceptor not implemented for eStretchChanged");
     }
diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h
index 30aca8340e..673f9e789d 100644
--- a/services/surfaceflinger/SurfaceInterceptor.h
+++ b/services/surfaceflinger/SurfaceInterceptor.h
@@ -177,6 +177,7 @@ private:
     void addRelativeParentLocked(Transaction* transaction, int32_t layerId, int32_t parentId,
                                  int z);
     void addShadowRadiusLocked(Transaction* transaction, int32_t layerId, float shadowRadius);
+    void addTrustedOverlayLocked(Transaction* transaction, int32_t layerId, bool isTrustedOverlay);
 
     // Add display transactions to the trace
     DisplayChange* createDisplayChangeLocked(Transaction* transaction, int32_t sequenceId);
diff --git a/services/surfaceflinger/layerproto/LayerProtoParser.cpp b/services/surfaceflinger/layerproto/LayerProtoParser.cpp
index aef670da33..2841f7c2fd 100644
--- a/services/surfaceflinger/layerproto/LayerProtoParser.cpp
+++ b/services/surfaceflinger/layerproto/LayerProtoParser.cpp
@@ -105,6 +105,7 @@ LayerProtoParser::Layer LayerProtoParser::generateLayer(const LayerProto& layerP
     layer.queuedFrames = layerProto.queued_frames();
     layer.refreshPending = layerProto.refresh_pending();
     layer.isProtected = layerProto.is_protected();
+    layer.isTrustedOverlay = layerProto.is_trusted_overlay();
     layer.cornerRadius = layerProto.corner_radius();
     layer.backgroundBlurRadius = layerProto.background_blur_radius();
     for (const auto& entry : layerProto.metadata()) {
@@ -289,6 +290,7 @@ std::string LayerProtoParser::Layer::to_string() const {
     StringAppendF(&result, "crop=%s, ", crop.to_string().c_str());
     StringAppendF(&result, "cornerRadius=%f, ", cornerRadius);
     StringAppendF(&result, "isProtected=%1d, ", isProtected);
+    StringAppendF(&result, "isTrustedOverlay=%1d, ", isTrustedOverlay);
     StringAppendF(&result, "isOpaque=%1d, invalidate=%1d, ", isOpaque, invalidate);
     StringAppendF(&result, "dataspace=%s, ", dataspace.c_str());
     StringAppendF(&result, "defaultPixelFormat=%s, ", pixelFormat.c_str());
diff --git a/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h b/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
index c48354fe95..52503bad3a 100644
--- a/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
+++ b/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
@@ -109,6 +109,7 @@ public:
         int32_t queuedFrames;
         bool refreshPending;
         bool isProtected;
+        bool isTrustedOverlay;
         float cornerRadius;
         int backgroundBlurRadius;
         LayerMetadata metadata;
diff --git a/services/surfaceflinger/layerproto/layers.proto b/services/surfaceflinger/layerproto/layers.proto
index 9f25674f1b..dddc677715 100644
--- a/services/surfaceflinger/layerproto/layers.proto
+++ b/services/surfaceflinger/layerproto/layers.proto
@@ -128,6 +128,8 @@ message LayerProto {
 
   // Regions of a layer, where blur should be applied.
   repeated BlurRegion blur_regions = 54;
+
+  bool is_trusted_overlay = 55;
 }
 
 message PositionProto {
diff --git a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
index ee4e863474..d5890ffa79 100644
--- a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
+++ b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
@@ -193,6 +193,7 @@ public:
     bool reparentUpdateFound(const SurfaceChange& change, bool found);
     bool relativeParentUpdateFound(const SurfaceChange& change, bool found);
     bool shadowRadiusUpdateFound(const SurfaceChange& change, bool found);
+    bool trustedOverlayUpdateFound(const SurfaceChange& change, bool found);
     bool surfaceUpdateFound(const Trace& trace, SurfaceChange::SurfaceChangeCase changeCase);
 
     // Find all of the updates in the single trace
@@ -228,6 +229,7 @@ public:
     void reparentUpdate(Transaction&);
     void relativeParentUpdate(Transaction&);
     void shadowRadiusUpdate(Transaction&);
+    void trustedOverlayUpdate(Transaction&);
     void surfaceCreation(Transaction&);
     void displayCreation(Transaction&);
     void displayDeletion(Transaction&);
@@ -405,6 +407,10 @@ void SurfaceInterceptorTest::shadowRadiusUpdate(Transaction& t) {
     t.setShadowRadius(mBGSurfaceControl, SHADOW_RADIUS_UPDATE);
 }
 
+void SurfaceInterceptorTest::trustedOverlayUpdate(Transaction& t) {
+    t.setTrustedOverlay(mBGSurfaceControl, true);
+}
+
 void SurfaceInterceptorTest::displayCreation(Transaction&) {
     sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, false);
     SurfaceComposerClient::destroyDisplay(testDisplay);
@@ -433,6 +439,7 @@ void SurfaceInterceptorTest::runAllUpdates() {
     runInTransaction(&SurfaceInterceptorTest::reparentUpdate);
     runInTransaction(&SurfaceInterceptorTest::relativeParentUpdate);
     runInTransaction(&SurfaceInterceptorTest::shadowRadiusUpdate);
+    runInTransaction(&SurfaceInterceptorTest::trustedOverlayUpdate);
 }
 
 void SurfaceInterceptorTest::surfaceCreation(Transaction&) {
@@ -644,6 +651,17 @@ bool SurfaceInterceptorTest::shadowRadiusUpdateFound(const SurfaceChange& change
     return foundShadowRadius;
 }
 
+bool SurfaceInterceptorTest::trustedOverlayUpdateFound(const SurfaceChange& change,
+                                                       bool foundTrustedOverlay) {
+    bool hasTrustedOverlay(change.trusted_overlay().is_trusted_overlay());
+    if (hasTrustedOverlay && !foundTrustedOverlay) {
+        foundTrustedOverlay = true;
+    } else if (hasTrustedOverlay && foundTrustedOverlay) {
+        []() { FAIL(); }();
+    }
+    return foundTrustedOverlay;
+}
+
 bool SurfaceInterceptorTest::surfaceUpdateFound(const Trace& trace,
         SurfaceChange::SurfaceChangeCase changeCase) {
     bool foundUpdate = false;
@@ -704,6 +722,9 @@ bool SurfaceInterceptorTest::surfaceUpdateFound(const Trace& trace,
                         case SurfaceChange::SurfaceChangeCase::kShadowRadius:
                             foundUpdate = shadowRadiusUpdateFound(change, foundUpdate);
                             break;
+                        case SurfaceChange::SurfaceChangeCase::kTrustedOverlay:
+                            foundUpdate = trustedOverlayUpdateFound(change, foundUpdate);
+                            break;
                         case SurfaceChange::SurfaceChangeCase::SURFACECHANGE_NOT_SET:
                             break;
                     }
@@ -897,6 +918,11 @@ TEST_F(SurfaceInterceptorTest, InterceptShadowRadiusUpdateWorks) {
                 SurfaceChange::SurfaceChangeCase::kShadowRadius);
 }
 
+TEST_F(SurfaceInterceptorTest, InterceptTrustedOverlayUpdateWorks) {
+    captureTest(&SurfaceInterceptorTest::trustedOverlayUpdate,
+                SurfaceChange::SurfaceChangeCase::kTrustedOverlay);
+}
+
 TEST_F(SurfaceInterceptorTest, InterceptAllUpdatesWorks) {
     captureTest(&SurfaceInterceptorTest::runAllUpdates,
                 &SurfaceInterceptorTest::assertAllUpdatesFound);
-- 
cgit v1.2.3-59-g8ed1b