commit 7cbcacc712cd93a941dcaf635a2b731466a45319
author Hao Li <hao.x.li@intel.com> Wed Jun 09 08:47:37 2021 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jun 09 08:47:37 2021 +0000
tree 3140a94ad9b083f9d111e48da4414f23d40a284b
parent 11379a3b59de430a8cf403f5b542313cd3fd9af7

Disallow pipeline statistics query as UnsafeAPIs

- Pipeline statistics query is not fully implemented, disallow its
  creation as unsafe
- Add pipeline statistics creation in UnsafeAPIsTest, because it needs
  enable extension, add a separate test class for it.

BUG: chromium:1177506
Change-Id: Ic77e04c9c854b396e7240674bd9deb0caf97a513
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/53889
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>

src/dawn_native/QuerySet.cpp

diff --git a/src/dawn_native/QuerySet.cpp b/src/dawn_native/QuerySet.cpp
index 26e9c6d..e8e3c90 100644
--- a/src/dawn_native/QuerySet.cpp
+++ b/src/dawn_native/QuerySet.cpp
@@ -58,6 +58,14 @@
                 break;
 
             case wgpu::QueryType::PipelineStatistics: {
+                // TODO(crbug.com/1177506): Pipeline statistics query is not fully implemented.
+                // Disallow it as unsafe until the implementaion is completed.
+                if (device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) {
+                    return DAWN_VALIDATION_ERROR(
+                        "Pipeline statistics query is disallowed because it's not fully "
+                        "implemented");
+                }
+
                 if (!device->IsExtensionEnabled(Extension::PipelineStatisticsQuery)) {
                     return DAWN_VALIDATION_ERROR(
                         "The pipeline statistics query feature is not supported");

src/tests/unittests/validation/QueryValidationTests.cpp

diff --git a/src/tests/unittests/validation/QueryValidationTests.cpp b/src/tests/unittests/validation/QueryValidationTests.cpp
index 1ee0134..53a650f 100644
--- a/src/tests/unittests/validation/QueryValidationTests.cpp
+++ b/src/tests/unittests/validation/QueryValidationTests.cpp
@@ -226,7 +226,7 @@
   protected:
     WGPUDevice CreateTestDevice() override {
         dawn_native::DeviceDescriptor descriptor;
-        descriptor.requiredExtensions = {"timestamp_query"};
+        descriptor.requiredExtensions.push_back("timestamp_query");
         return adapter.CreateDevice(&descriptor);
     }
 };
@@ -429,7 +429,10 @@
   protected:
     WGPUDevice CreateTestDevice() override {
         dawn_native::DeviceDescriptor descriptor;
-        descriptor.requiredExtensions = {"pipeline_statistics_query"};
+        descriptor.requiredExtensions.push_back("pipeline_statistics_query");
+        // TODO(crbug.com/1177506): Pipeline statistic query is an unsafe API, disable disallowing
+        // unsafe APIs to test it.
+        descriptor.forceDisabledToggles.push_back("disallow_unsafe_apis");
         return adapter.CreateDevice(&descriptor);
     }
 };

src/tests/unittests/validation/UnsafeAPIValidationTests.cpp

diff --git a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
index 6eebeb0..02e55fd 100644
--- a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
+++ b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
@@ -201,3 +201,35 @@
         ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&desc));
     }
 }
+
+class UnsafeQueryAPIValidationTest : public ValidationTest {
+  protected:
+    WGPUDevice CreateTestDevice() override {
+        dawn_native::DeviceDescriptor descriptor;
+        descriptor.requiredExtensions.push_back("pipeline_statistics_query");
+        descriptor.forceEnabledToggles.push_back("disallow_unsafe_apis");
+        return adapter.CreateDevice(&descriptor);
+    }
+};
+
+// Check that pipeline statistics query are disallowed.
+TEST_F(UnsafeQueryAPIValidationTest, PipelineStatisticsDisallowed) {
+    wgpu::QuerySetDescriptor descriptor;
+    descriptor.count = 1;
+
+    // Control case: occlusion query creation is allowed.
+    {
+        descriptor.type = wgpu::QueryType::Occlusion;
+        device.CreateQuerySet(&descriptor);
+    }
+
+    // Error case: pipeline statistics query creation is disallowed.
+    {
+        descriptor.type = wgpu::QueryType::PipelineStatistics;
+        std::vector<wgpu::PipelineStatisticName> pipelineStatistics = {
+            wgpu::PipelineStatisticName::VertexShaderInvocations};
+        descriptor.pipelineStatistics = pipelineStatistics.data();
+        descriptor.pipelineStatisticsCount = pipelineStatistics.size();
+        ASSERT_DEVICE_ERROR(device.CreateQuerySet(&descriptor));
+    }
+}