From af06b797c44ab79536bc47395ba55b943d83bf5f Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Thu, 16 Feb 2023 17:46:22 -0600 Subject: Fix WindowInfosListenerTest Fixes a race condition where the WindowInfo vector checked may not contain the updated window on the first pass. Bug: 269522974 Test: atest WindowInfosListenerTest Change-Id: I013955dd1c89858190b519c2980909e2e7e45a24 (cherry picked from commit 4c766ae50ce641672158c19eda4613a5e64baf47) --- services/surfaceflinger/SurfaceFlinger.cpp | 7 +- services/surfaceflinger/SurfaceFlinger.h | 5 +- .../surfaceflinger/WindowInfosListenerInvoker.cpp | 1 - .../surfaceflinger/WindowInfosListenerInvoker.h | 4 +- .../tests/WindowInfosListener_test.cpp | 87 ++++++++++++---------- 5 files changed, 60 insertions(+), 44 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c91ad49c9a..28b7dd652b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3576,6 +3576,10 @@ void SurfaceFlinger::commitTransactionsLocked(uint32_t transactionFlags) { }); } + if (transactionFlags & eInputInfoUpdateNeeded) { + mUpdateInputInfo = true; + } + doCommitTransactions(); } @@ -7567,8 +7571,9 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD } status_t SurfaceFlinger::addWindowInfosListener( - const sp& windowInfosListener) const { + const sp& windowInfosListener) { mWindowInfosListenerInvoker->addWindowInfosListener(windowInfosListener); + setTransactionFlags(eInputInfoUpdateNeeded); return NO_ERROR; } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index b41f414f62..03c31bb015 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -162,7 +162,8 @@ enum { eDisplayTransactionNeeded = 0x04, eTransformHintUpdateNeeded = 0x08, eTransactionFlushNeeded = 0x10, - eTransactionMask = 0x1f, + eInputInfoUpdateNeeded = 0x20, + eTransactionMask = 0x3f, }; // Latch Unsignaled buffer behaviours @@ -618,7 +619,7 @@ private: status_t getMaxAcquiredBufferCount(int* buffers) const; - status_t addWindowInfosListener(const sp& windowInfosListener) const; + status_t addWindowInfosListener(const sp& windowInfosListener); status_t removeWindowInfosListener( const sp& windowInfosListener) const; diff --git a/services/surfaceflinger/WindowInfosListenerInvoker.cpp b/services/surfaceflinger/WindowInfosListenerInvoker.cpp index a1313e3a03..292083b9bc 100644 --- a/services/surfaceflinger/WindowInfosListenerInvoker.cpp +++ b/services/surfaceflinger/WindowInfosListenerInvoker.cpp @@ -17,7 +17,6 @@ #include #include -#include "SurfaceFlinger.h" #include "WindowInfosListenerInvoker.h" namespace android { diff --git a/services/surfaceflinger/WindowInfosListenerInvoker.h b/services/surfaceflinger/WindowInfosListenerInvoker.h index a1d66a186e..d60a9c4157 100644 --- a/services/surfaceflinger/WindowInfosListenerInvoker.h +++ b/services/surfaceflinger/WindowInfosListenerInvoker.h @@ -16,6 +16,8 @@ #pragma once +#include + #include #include #include @@ -49,8 +51,6 @@ private: static constexpr size_t kStaticCapacity = 3; ftl::SmallMap, const sp, kStaticCapacity> mWindowInfosListeners GUARDED_BY(mListenersMutex); - - sp mWindowInfosReportedListener; }; } // namespace android diff --git a/services/surfaceflinger/tests/WindowInfosListener_test.cpp b/services/surfaceflinger/tests/WindowInfosListener_test.cpp index 53c3c3998f..d71486fca7 100644 --- a/services/surfaceflinger/tests/WindowInfosListener_test.cpp +++ b/services/surfaceflinger/tests/WindowInfosListener_test.cpp @@ -18,61 +18,61 @@ #include #include #include -#include "utils/TransactionUtils.h" namespace android { using Transaction = SurfaceComposerClient::Transaction; using gui::DisplayInfo; using gui::WindowInfo; +using WindowInfosPredicate = std::function&)>; + class WindowInfosListenerTest : public ::testing::Test { protected: void SetUp() override { seteuid(AID_SYSTEM); mClient = sp::make(); - mWindowInfosListener = sp::make(); - mClient->addWindowInfosListener(mWindowInfosListener); } - void TearDown() override { - mClient->removeWindowInfosListener(mWindowInfosListener); - seteuid(AID_ROOT); - } + void TearDown() override { seteuid(AID_ROOT); } - struct SyncWindowInfosListener : public gui::WindowInfosListener { + struct WindowInfosListener : public gui::WindowInfosListener { public: + WindowInfosListener(WindowInfosPredicate predicate, std::promise& promise) + : mPredicate(std::move(predicate)), mPromise(promise) {} + void onWindowInfosChanged(const std::vector& windowInfos, const std::vector&) override { - windowInfosPromise.set_value(windowInfos); - } - - std::vector waitForWindowInfos() { - std::future> windowInfosFuture = - windowInfosPromise.get_future(); - std::vector windowInfos = windowInfosFuture.get(); - windowInfosPromise = std::promise>(); - return windowInfos; + if (mPredicate(windowInfos)) { + mPromise.set_value(); + } } private: - std::promise> windowInfosPromise; + WindowInfosPredicate mPredicate; + std::promise& mPromise; }; sp mClient; - sp mWindowInfosListener; + + bool waitForWindowInfosPredicate(WindowInfosPredicate predicate) { + std::promise promise; + auto listener = sp::make(std::move(predicate), promise); + mClient->addWindowInfosListener(listener); + auto future = promise.get_future(); + bool satisfied = future.wait_for(std::chrono::seconds{1}) == std::future_status::ready; + mClient->removeWindowInfosListener(listener); + return satisfied; + } }; std::optional findMatchingWindowInfo(WindowInfo targetWindowInfo, std::vector windowInfos) { - std::optional foundWindowInfo = std::nullopt; for (WindowInfo windowInfo : windowInfos) { if (windowInfo.token == targetWindowInfo.token) { - foundWindowInfo = std::make_optional<>(windowInfo); - break; + return windowInfo; } } - - return foundWindowInfo; + return std::nullopt; } TEST_F(WindowInfosListenerTest, WindowInfoAddedAndRemoved) { @@ -92,15 +92,17 @@ TEST_F(WindowInfosListenerTest, WindowInfoAddedAndRemoved) { .setInputWindowInfo(surfaceControl, windowInfo) .apply(); - std::vector windowInfos = mWindowInfosListener->waitForWindowInfos(); - std::optional foundWindowInfo = findMatchingWindowInfo(windowInfo, windowInfos); - ASSERT_NE(std::nullopt, foundWindowInfo); + auto windowPresent = [&](const std::vector& windowInfos) { + return findMatchingWindowInfo(windowInfo, windowInfos).has_value(); + }; + ASSERT_TRUE(waitForWindowInfosPredicate(windowPresent)); Transaction().reparent(surfaceControl, nullptr).apply(); - windowInfos = mWindowInfosListener->waitForWindowInfos(); - foundWindowInfo = findMatchingWindowInfo(windowInfo, windowInfos); - ASSERT_EQ(std::nullopt, foundWindowInfo); + auto windowNotPresent = [&](const std::vector& windowInfos) { + return !findMatchingWindowInfo(windowInfo, windowInfos).has_value(); + }; + ASSERT_TRUE(waitForWindowInfosPredicate(windowNotPresent)); } TEST_F(WindowInfosListenerTest, WindowInfoChanged) { @@ -121,19 +123,28 @@ TEST_F(WindowInfosListenerTest, WindowInfoChanged) { .setInputWindowInfo(surfaceControl, windowInfo) .apply(); - std::vector windowInfos = mWindowInfosListener->waitForWindowInfos(); - std::optional foundWindowInfo = findMatchingWindowInfo(windowInfo, windowInfos); - ASSERT_NE(std::nullopt, foundWindowInfo); - ASSERT_TRUE(foundWindowInfo->touchableRegion.isEmpty()); + auto windowIsPresentAndTouchableRegionEmpty = [&](const std::vector& windowInfos) { + auto foundWindowInfo = findMatchingWindowInfo(windowInfo, windowInfos); + if (!foundWindowInfo) { + return false; + } + return foundWindowInfo->touchableRegion.isEmpty(); + }; + ASSERT_TRUE(waitForWindowInfosPredicate(windowIsPresentAndTouchableRegionEmpty)); Rect touchableRegions(0, 0, 50, 50); windowInfo.addTouchableRegion(Rect(0, 0, 50, 50)); Transaction().setInputWindowInfo(surfaceControl, windowInfo).apply(); - windowInfos = mWindowInfosListener->waitForWindowInfos(); - foundWindowInfo = findMatchingWindowInfo(windowInfo, windowInfos); - ASSERT_NE(std::nullopt, foundWindowInfo); - ASSERT_TRUE(foundWindowInfo->touchableRegion.hasSameRects(windowInfo.touchableRegion)); + auto windowIsPresentAndTouchableRegionMatches = + [&](const std::vector& windowInfos) { + auto foundWindowInfo = findMatchingWindowInfo(windowInfo, windowInfos); + if (!foundWindowInfo) { + return false; + } + return foundWindowInfo->touchableRegion.hasSameRects(windowInfo.touchableRegion); + }; + ASSERT_TRUE(waitForWindowInfosPredicate(windowIsPresentAndTouchableRegionMatches)); } } // namespace android -- cgit v1.2.3-59-g8ed1b