Deflake functor tests

sMockFunctorCounts was written to by the RenderThread, but read from the testing thread, so guard sMockFunctorCounts with a lock.

Bug: 353258633
Flag: EXEMPT test only
Test: hwui_unit_tests 100 times
Test: hwui_unit_tests --renderer skiavk 100 times
Change-Id: Icc95662f58aa675267522cf94c27eb2d9016800c

5 files changed, 32 insertions, 7 deletions

diff --git a/libs/hwui/tests/common/TestUtils.cpp b/libs/hwui/tests/common/TestUtils.cpp
index ad963dd913cf..93118aeafaaf 100644
--- a/libs/hwui/tests/common/TestUtils.cpp
+++ b/libs/hwui/tests/common/TestUtils.cpp
@@ -40,6 +40,7 @@
 namespace android {
 namespace uirenderer {
 
+std::mutex TestUtils::sMutex;
 std::unordered_map<int, TestUtils::CallCounts> TestUtils::sMockFunctorCounts{};
 
 SkColor TestUtils::interpolateColor(float fraction, SkColor start, SkColor end) {
diff --git a/libs/hwui/tests/common/TestUtils.h b/libs/hwui/tests/common/TestUtils.h
index 0ede902b1b95..8ab2b16601c3 100644
--- a/libs/hwui/tests/common/TestUtils.h
+++ b/libs/hwui/tests/common/TestUtils.h
@@ -305,22 +305,26 @@ public:
                 .onSync =
                         [](int functor, void* client_data, const WebViewSyncData& data) {
                             expectOnRenderThread("onSync");
+                            std::scoped_lock lock(sMutex);
                             sMockFunctorCounts[functor].sync++;
                         },
                 .onContextDestroyed =
                         [](int functor, void* client_data) {
                             expectOnRenderThread("onContextDestroyed");
+                            std::scoped_lock lock(sMutex);
                             sMockFunctorCounts[functor].contextDestroyed++;
                         },
                 .onDestroyed =
                         [](int functor, void* client_data) {
                             expectOnRenderThread("onDestroyed");
+                            std::scoped_lock lock(sMutex);
                             sMockFunctorCounts[functor].destroyed++;
                         },
                 .removeOverlays =
                         [](int functor, void* data,
                            void (*mergeTransaction)(ASurfaceTransaction*)) {
                             expectOnRenderThread("removeOverlays");
+                            std::scoped_lock lock(sMutex);
                             sMockFunctorCounts[functor].removeOverlays++;
                         },
         };
@@ -329,6 +333,7 @@ public:
                 callbacks.gles.draw = [](int functor, void* client_data, const DrawGlInfo& params,
                                          const WebViewOverlayData& overlay_params) {
                     expectOnRenderThread("draw");
+                    std::scoped_lock lock(sMutex);
                     sMockFunctorCounts[functor].glesDraw++;
                 };
                 break;
@@ -336,15 +341,18 @@ public:
                 callbacks.vk.initialize = [](int functor, void* data,
                                              const VkFunctorInitParams& params) {
                     expectOnRenderThread("initialize");
+                    std::scoped_lock lock(sMutex);
                     sMockFunctorCounts[functor].vkInitialize++;
                 };
                 callbacks.vk.draw = [](int functor, void* data, const VkFunctorDrawParams& params,
                                        const WebViewOverlayData& overlayParams) {
                     expectOnRenderThread("draw");
+                    std::scoped_lock lock(sMutex);
                     sMockFunctorCounts[functor].vkDraw++;
                 };
                 callbacks.vk.postDraw = [](int functor, void* data) {
                     expectOnRenderThread("postDraw");
+                    std::scoped_lock lock(sMutex);
                     sMockFunctorCounts[functor].vkPostDraw++;
                 };
                 break;
@@ -352,11 +360,16 @@ public:
         return callbacks;
     }
 
-    static CallCounts& countsForFunctor(int functor) { return sMockFunctorCounts[functor]; }
+    static CallCounts copyCountsForFunctor(int functor) {
+        std::scoped_lock lock(sMutex);
+        return sMockFunctorCounts[functor];
+    }
 
     static SkFont defaultFont();
 
 private:
+    // guards sMockFunctorCounts
+    static std::mutex sMutex;
     static std::unordered_map<int, CallCounts> sMockFunctorCounts;
 
     static void syncHierarchyPropertiesAndDisplayListImpl(RenderNode* node) {
diff --git a/libs/hwui/tests/unit/RenderNodeTests.cpp b/libs/hwui/tests/unit/RenderNodeTests.cpp
index e727ea899098..690a60a470bc 100644
--- a/libs/hwui/tests/unit/RenderNodeTests.cpp
+++ b/libs/hwui/tests/unit/RenderNodeTests.cpp
@@ -239,19 +239,21 @@ TEST(RenderNode, releasedCallback) {
     TestUtils::runOnRenderThreadUnmanaged([&] (RenderThread&) {
         TestUtils::syncHierarchyPropertiesAndDisplayList(node);
     });
-    auto& counts = TestUtils::countsForFunctor(functor);
+    auto counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(1, counts.sync);
     EXPECT_EQ(0, counts.destroyed);
 
     TestUtils::recordNode(*node, [&](Canvas& canvas) {
         canvas.drawWebViewFunctor(functor);
     });
+    counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(1, counts.sync);
     EXPECT_EQ(0, counts.destroyed);
 
     TestUtils::runOnRenderThreadUnmanaged([&] (RenderThread&) {
         TestUtils::syncHierarchyPropertiesAndDisplayList(node);
     });
+    counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(2, counts.sync);
     EXPECT_EQ(0, counts.destroyed);
 
@@ -265,6 +267,7 @@ TEST(RenderNode, releasedCallback) {
     });
     // Fence on any remaining post'd work
     TestUtils::runOnRenderThreadUnmanaged([] (RenderThread&) {});
+    counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(2, counts.sync);
     EXPECT_EQ(1, counts.destroyed);
 }
diff --git a/libs/hwui/tests/unit/SkiaDisplayListTests.cpp b/libs/hwui/tests/unit/SkiaDisplayListTests.cpp
index 064d42ec8941..26b47290d149 100644
--- a/libs/hwui/tests/unit/SkiaDisplayListTests.cpp
+++ b/libs/hwui/tests/unit/SkiaDisplayListTests.cpp
@@ -101,7 +101,7 @@ TEST(SkiaDisplayList, syncContexts) {
     SkCanvas dummyCanvas;
 
     int functor1 = TestUtils::createMockFunctor();
-    auto& counts = TestUtils::countsForFunctor(functor1);
+    auto counts = TestUtils::copyCountsForFunctor(functor1);
     skiaDL.mChildFunctors.push_back(
             skiaDL.allocateDrawable<GLFunctorDrawable>(functor1, &dummyCanvas));
     WebViewFunctor_release(functor1);
@@ -118,6 +118,7 @@ TEST(SkiaDisplayList, syncContexts) {
         });
     });
 
+    counts = TestUtils::copyCountsForFunctor(functor1);
     EXPECT_EQ(counts.sync, 1);
     EXPECT_EQ(counts.destroyed, 0);
     EXPECT_EQ(vectorDrawable.mutateProperties()->getBounds(), bounds);
@@ -126,6 +127,7 @@ TEST(SkiaDisplayList, syncContexts) {
     TestUtils::runOnRenderThread([](auto&) {
         // Fence
     });
+    counts = TestUtils::copyCountsForFunctor(functor1);
     EXPECT_EQ(counts.destroyed, 1);
 }
 
diff --git a/libs/hwui/tests/unit/WebViewFunctorManagerTests.cpp b/libs/hwui/tests/unit/WebViewFunctorManagerTests.cpp
index 5e8f13d261c7..09ce98a2e53d 100644
--- a/libs/hwui/tests/unit/WebViewFunctorManagerTests.cpp
+++ b/libs/hwui/tests/unit/WebViewFunctorManagerTests.cpp
@@ -40,7 +40,7 @@ TEST(WebViewFunctor, createDestroyGLES) {
     TestUtils::runOnRenderThreadUnmanaged([](renderthread::RenderThread&) {
         // Empty, don't care
     });
-    auto& counts = TestUtils::countsForFunctor(functor);
+    auto counts = TestUtils::copyCountsForFunctor(functor);
     // We never initialized, so contextDestroyed == 0
     EXPECT_EQ(0, counts.contextDestroyed);
     EXPECT_EQ(1, counts.destroyed);
@@ -59,7 +59,7 @@ TEST(WebViewFunctor, createSyncHandleGLES) {
     TestUtils::runOnRenderThreadUnmanaged([](renderthread::RenderThread&) {
         // fence
     });
-    auto& counts = TestUtils::countsForFunctor(functor);
+    auto counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(0, counts.sync);
     EXPECT_EQ(0, counts.contextDestroyed);
     EXPECT_EQ(0, counts.destroyed);
@@ -69,6 +69,7 @@ TEST(WebViewFunctor, createSyncHandleGLES) {
         handle->sync(syncData);
     });
 
+    counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(1, counts.sync);
 
     TestUtils::runOnRenderThreadUnmanaged([&](auto&) {
@@ -76,6 +77,7 @@ TEST(WebViewFunctor, createSyncHandleGLES) {
         handle->sync(syncData);
     });
 
+    counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(2, counts.sync);
 
     handle.clear();
@@ -84,6 +86,7 @@ TEST(WebViewFunctor, createSyncHandleGLES) {
         // fence
     });
 
+    counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(2, counts.sync);
     EXPECT_EQ(0, counts.contextDestroyed);
     EXPECT_EQ(1, counts.destroyed);
@@ -98,7 +101,6 @@ TEST(WebViewFunctor, createSyncDrawGLES) {
     auto handle = WebViewFunctorManager::instance().handleFor(functor);
     ASSERT_TRUE(handle);
     WebViewFunctor_release(functor);
-    auto& counts = TestUtils::countsForFunctor(functor);
     for (int i = 0; i < 5; i++) {
         TestUtils::runOnRenderThreadUnmanaged([&](auto&) {
             WebViewSyncData syncData;
@@ -112,6 +114,7 @@ TEST(WebViewFunctor, createSyncDrawGLES) {
     TestUtils::runOnRenderThreadUnmanaged([](renderthread::RenderThread&) {
         // fence
     });
+    auto counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(5, counts.sync);
     EXPECT_EQ(10, counts.glesDraw);
     EXPECT_EQ(1, counts.contextDestroyed);
@@ -127,13 +130,13 @@ TEST(WebViewFunctor, contextDestroyedGLES) {
     auto handle = WebViewFunctorManager::instance().handleFor(functor);
     ASSERT_TRUE(handle);
     WebViewFunctor_release(functor);
-    auto& counts = TestUtils::countsForFunctor(functor);
     TestUtils::runOnRenderThreadUnmanaged([&](auto&) {
         WebViewSyncData syncData;
         handle->sync(syncData);
         DrawGlInfo drawInfo;
         handle->drawGl(drawInfo);
     });
+    auto counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(1, counts.sync);
     EXPECT_EQ(1, counts.glesDraw);
     EXPECT_EQ(0, counts.contextDestroyed);
@@ -141,6 +144,7 @@ TEST(WebViewFunctor, contextDestroyedGLES) {
     TestUtils::runOnRenderThreadUnmanaged([](auto& rt) {
         rt.destroyRenderingContext();
     });
+    counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(1, counts.sync);
     EXPECT_EQ(1, counts.glesDraw);
     EXPECT_EQ(1, counts.contextDestroyed);
@@ -151,6 +155,7 @@ TEST(WebViewFunctor, contextDestroyedGLES) {
         DrawGlInfo drawInfo;
         handle->drawGl(drawInfo);
     });
+    counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(2, counts.sync);
     EXPECT_EQ(2, counts.glesDraw);
     EXPECT_EQ(1, counts.contextDestroyed);
@@ -159,6 +164,7 @@ TEST(WebViewFunctor, contextDestroyedGLES) {
     TestUtils::runOnRenderThreadUnmanaged([](renderthread::RenderThread&) {
         // fence
     });
+    counts = TestUtils::copyCountsForFunctor(functor);
     EXPECT_EQ(2, counts.sync);
     EXPECT_EQ(2, counts.glesDraw);
     EXPECT_EQ(2, counts.contextDestroyed);