commit e72b4393dff93d1b77c6a5dd29764754d7cbdfac
author Peng Huang <penghuang@chromium.org> Tue Jan 23 11:53:50 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Jan 23 11:53:50 2024 +0000
tree 28498fcc78fce3c612639fd23907a6065f77855d
parent faaf33a681a793042673e514326dc445a191d2bb

Address the comments in CL 169943

Bug: dawn:2184
Change-Id: Ifa984b70bc980239e9634d83b13b664f710868f4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/171060
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Peng Huang <penghuang@chromium.org>

src/dawn/native/ShaderModule.cpp

diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp
index a50ea41..193c992 100644
--- a/src/dawn/native/ShaderModule.cpp
+++ b/src/dawn/native/ShaderModule.cpp
@@ -1292,14 +1292,19 @@
 }
 
 ShaderModuleBase::ScopedUseTintProgram ShaderModuleBase::UseTintProgram() {
-    mTintData.Use([&](auto tintData) {
-        // When the ShaderModuleBase is not referenced externally, and not used for initializing any
-        // pipeline, the mTintProgram will be released. However the ShaderModuleBase itself may
-        // still alive due to being referenced by some pipelines. In this case, when
-        // DeviceBase::APICreateShaderModule() with the same shader source code, Dawn will look up
-        // from the cache and return the same ShaderModuleBase. In this case, we have to recreate
-        // mTintProgram, when the mTintProgram is required for initializing new pipelines.
-        if (tintData->tintProgram == nullptr) {
+    return mTintData.Use(
+        [&](auto tintData) {
+            if (tintData->tintProgram) {
+                return ScopedUseTintProgram(this);
+            }
+
+            // When the ShaderModuleBase is not referenced externally, and not used for initializing
+            // any pipeline, the mTintProgram will be released. However the ShaderModuleBase itself
+            // may still alive due to being referenced by some pipelines. In this case, when
+            // DeviceBase::APICreateShaderModule() with the same shader source code, Dawn will look
+            // up from the cache and return the same ShaderModuleBase. In this case, we have to
+            // recreate mTintProgram, when the mTintProgram is required for initializing new
+            // pipelines.
             ShaderModuleDescriptor descriptor;
             ShaderModuleWGSLDescriptor wgslDescriptor;
             ShaderModuleSPIRVDescriptor sprivDescriptor;
@@ -1326,10 +1331,9 @@
 
             tintData->tintProgram = std::move(parseResult.tintProgram);
             tintData->tintProgramRecreateCount++;
-        }
-    });
 
-    return ScopedUseTintProgram(this);
+            return ScopedUseTintProgram(this);
+        });
 }
 
 Ref<TintProgram> ShaderModuleBase::GetTintProgram() const {

src/dawn/tests/unittests/RefCountedTests.cpp

diff --git a/src/dawn/tests/unittests/RefCountedTests.cpp b/src/dawn/tests/unittests/RefCountedTests.cpp
index 6ae82e8..4222243 100644
--- a/src/dawn/tests/unittests/RefCountedTests.cpp
+++ b/src/dawn/tests/unittests/RefCountedTests.cpp
@@ -61,11 +61,21 @@
 };
 
 TEST(RefCount, Increment) {
-    RefCount refCount(/*initCount=*/0, /*payload=*/0);
+    RefCount refCount0(/*initCount=*/0, /*payload=*/0);
     // Previous count is 0
-    EXPECT_TRUE(refCount.Increment());
+    EXPECT_TRUE(refCount0.Increment());
     // Previous count is 1
-    EXPECT_FALSE(refCount.Increment());
+    EXPECT_FALSE(refCount0.Increment());
+
+    EXPECT_FALSE(refCount0.Decrement());
+    EXPECT_TRUE(refCount0.Decrement());
+
+    RefCount refCount1(/*initCount=*/1, /*payload=*/0);
+    // Previous count is 1
+    EXPECT_FALSE(refCount1.Increment());
+
+    EXPECT_FALSE(refCount1.Decrement());
+    EXPECT_TRUE(refCount1.Decrement());
 }
 
 // Test that RCs start with one ref, and removing it destroys the object.