diff options
-rw-r--r-- | libs/gui/BLASTBufferQueue.cpp | 6 | ||||
-rw-r--r-- | libs/gui/SurfaceComposerClient.cpp | 9 | ||||
-rw-r--r-- | libs/gui/include/gui/BLASTBufferQueue.h | 2 | ||||
-rw-r--r-- | libs/gui/include/gui/SurfaceComposerClient.h | 11 | ||||
-rw-r--r-- | libs/gui/include/gui/TransactionState.h | 6 | ||||
-rw-r--r-- | services/surfaceflinger/tests/DereferenceSurfaceControl_test.cpp | 6 | ||||
-rw-r--r-- | services/surfaceflinger/tests/IPC_test.cpp | 10 | ||||
-rw-r--r-- | services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp | 4 |
8 files changed, 35 insertions, 19 deletions
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 1aae13c1f4..5b0f21de91 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -1032,7 +1032,7 @@ void BLASTBufferQueue::mergeWithNextTransaction(SurfaceComposerClient::Transacti // Apply the transaction since we have already acquired the desired frame. t->setApplyToken(mApplyToken).apply(); } else { - mPendingTransactions.emplace_back(frameNumber, *t); + mPendingTransactions.emplace_back(frameNumber, std::move(*t)); // Clear the transaction so it can't be applied elsewhere. t->clear(); } @@ -1050,8 +1050,8 @@ void BLASTBufferQueue::applyPendingTransactions(uint64_t frameNumber) { void BLASTBufferQueue::mergePendingTransactions(SurfaceComposerClient::Transaction* t, uint64_t frameNumber) { auto mergeTransaction = - [&t, currentFrameNumber = frameNumber]( - std::tuple<uint64_t, SurfaceComposerClient::Transaction> pendingTransaction) { + [t, currentFrameNumber = frameNumber]( + std::pair<uint64_t, SurfaceComposerClient::Transaction>& pendingTransaction) { auto& [targetFrameNumber, transaction] = pendingTransaction; if (currentFrameNumber < targetFrameNumber) { return false; diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 65313c0fac..359a5288a6 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -828,11 +828,10 @@ SurfaceComposerClient::Transaction::Transaction() { mTransactionCompletedListener = TransactionCompletedListener::getInstance(); } -SurfaceComposerClient::Transaction::Transaction(const Transaction& other) { - mState = other.mState; - mListenerCallbacks = other.mListenerCallbacks; - mTransactionCompletedListener = TransactionCompletedListener::getInstance(); -} +SurfaceComposerClient::Transaction::Transaction(Transaction&& other) + : mTransactionCompletedListener(TransactionCompletedListener::getInstance()), + mState(std::move(other.mState)), + mListenerCallbacks(std::move(other.mListenerCallbacks)) {} void SurfaceComposerClient::Transaction::sanitize(int pid, int uid) { uint32_t permissions = LayerStatePermissions::getTransactionPermissions(pid, uid); diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index db1b9fb8eb..c69b0a79ad 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -284,7 +284,7 @@ private: std::function<void(SurfaceComposerClient::Transaction*)> mTransactionReadyCallback GUARDED_BY(mMutex); SurfaceComposerClient::Transaction* mSyncTransaction GUARDED_BY(mMutex); - std::vector<std::tuple<uint64_t /* framenumber */, SurfaceComposerClient::Transaction>> + std::vector<std::pair<uint64_t /* framenumber */, SurfaceComposerClient::Transaction>> mPendingTransactions GUARDED_BY(mMutex); std::queue<std::pair<uint64_t, FrameTimelineInfo>> mPendingFrameTimelines GUARDED_BY(mMutex); diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 15e3341ca2..668bd6fbb8 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -443,7 +443,7 @@ public: virtual ~PresentationCallbackRAII(); }; - class Transaction : public Parcelable { + class Transaction { private: static sp<IBinder> sApplyToken; static std::mutex sApplyTokenMutex; @@ -464,19 +464,20 @@ public: protected: // Accessed in tests. + explicit Transaction(Transaction const& other) = default; std::unordered_map<sp<ITransactionCompletedListener>, CallbackInfo, TCLHash> mListenerCallbacks; public: Transaction(); - virtual ~Transaction() = default; - Transaction(Transaction const& other); + Transaction(Transaction&& other); + Transaction& operator=(Transaction&& other) = default; // Factory method that creates a new Transaction instance from the parcel. static std::unique_ptr<Transaction> createFromParcel(const Parcel* parcel); - status_t writeToParcel(Parcel* parcel) const override; - status_t readFromParcel(const Parcel* parcel) override; + status_t writeToParcel(Parcel* parcel) const; + status_t readFromParcel(const Parcel* parcel); // Clears the contents of the transaction without applying it. void clear(); diff --git a/libs/gui/include/gui/TransactionState.h b/libs/gui/include/gui/TransactionState.h index 4358227dae..79124f3ed7 100644 --- a/libs/gui/include/gui/TransactionState.h +++ b/libs/gui/include/gui/TransactionState.h @@ -26,7 +26,8 @@ namespace android { class TransactionState { public: explicit TransactionState() = default; - TransactionState(TransactionState const& other) = default; + TransactionState(TransactionState&& other) = default; + TransactionState& operator=(TransactionState&& other) = default; status_t writeToParcel(Parcel* parcel) const; status_t readFromParcel(const Parcel* parcel); layer_state_t* getLayerState(const sp<SurfaceControl>& sc); @@ -86,6 +87,9 @@ public: std::vector<ListenerCallbacks> mListenerCallbacks; private: + explicit TransactionState(TransactionState const& other) = default; + friend class TransactionApplicationTest; + friend class SurfaceComposerClient; // We keep track of the last MAX_MERGE_HISTORY_LENGTH merged transaction ids. // Ordered most recently merged to least recently merged. static constexpr size_t MAX_MERGE_HISTORY_LENGTH = 10u; diff --git a/services/surfaceflinger/tests/DereferenceSurfaceControl_test.cpp b/services/surfaceflinger/tests/DereferenceSurfaceControl_test.cpp index 46b98f9193..192602d7d2 100644 --- a/services/surfaceflinger/tests/DereferenceSurfaceControl_test.cpp +++ b/services/surfaceflinger/tests/DereferenceSurfaceControl_test.cpp @@ -52,6 +52,8 @@ protected: }; TEST_F(DereferenceSurfaceControlTest, LayerNotInTransaction) { + // Last strong pointer is removed, the layer is destroyed and is removed + // from compostion. fgLayer = nullptr; { SCOPED_TRACE("after setting null"); @@ -61,7 +63,9 @@ TEST_F(DereferenceSurfaceControlTest, LayerNotInTransaction) { } TEST_F(DereferenceSurfaceControlTest, LayerInTransaction) { - auto transaction = Transaction().show(fgLayer); + Transaction transaction; + transaction.show(fgLayer); + // |transaction| retains a strong pointer, so layer is retained. fgLayer = nullptr; { SCOPED_TRACE("after setting null"); diff --git a/services/surfaceflinger/tests/IPC_test.cpp b/services/surfaceflinger/tests/IPC_test.cpp index 18bd3b92d2..94cb878311 100644 --- a/services/surfaceflinger/tests/IPC_test.cpp +++ b/services/surfaceflinger/tests/IPC_test.cpp @@ -42,14 +42,22 @@ using CallbackInfo = SurfaceComposerClient::CallbackInfo; using TCLHash = SurfaceComposerClient::TCLHash; using android::hardware::graphics::common::V1_1::BufferUsage; -class TransactionHelper : public Transaction { +class TransactionHelper : public Transaction, public Parcelable { public: + TransactionHelper() : Transaction() {} size_t getNumListeners() { return mListenerCallbacks.size(); } std::unordered_map<sp<ITransactionCompletedListener>, CallbackInfo, TCLHash> getListenerCallbacks() { return mListenerCallbacks; } + status_t writeToParcel(Parcel* parcel) const override { + return Transaction::writeToParcel(parcel); + } + + status_t readFromParcel(const Parcel* parcel) override { + return Transaction::readFromParcel(parcel); + } }; class IPCTestUtils { diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp index 6a5ac2a70e..1395fb6af3 100644 --- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp @@ -78,7 +78,7 @@ public: } }; - void checkEqual(TransactionInfo info, QueuedTransactionState state) { + void checkEqual(const TransactionInfo& info, const QueuedTransactionState& state) { EXPECT_EQ(0u, info.mComposerStates.size()); EXPECT_EQ(0u, state.states.size()); @@ -383,7 +383,7 @@ public: EXPECT_TRUE(mFlinger.getTransactionQueue().isEmpty()); EXPECT_EQ(0u, mFlinger.getPendingTransactionQueue().size()); std::unordered_set<uint32_t> createdLayers; - for (auto transaction : transactions) { + for (auto& transaction : transactions) { for (auto& state : transaction.mComposerStates) { auto layerId = static_cast<uint32_t>(state.state.layerId); if (createdLayers.find(layerId) == createdLayers.end()) { |