Only info collection gpu_integration_tests need to collect DX12/Vulkan

Right now we collect them for all gpu_integration tests, which caused
Win10 Intel bot to timeout.

Instead, we only need to do that for info collection test.

This also fixes a potential racing in GpuDataManager.

BUG=1004472,977034
TEST=gpu bots
R=kbr@chromium.org

Change-Id: I4010fda150ff3872b1dcefd2ac8aa7e4ad689e73
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1808012
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697045}

content/browser/devtools/protocol/system_info_handler.cc

@@ -47,9 +47,11 @@ std::unique_ptr<SystemInfo::Size> GfxSizeToSystemInfoSize(
 }
 // Give the GPU process a few seconds to provide GPU info.
 // Linux Debug builds need more time -- see Issue 796437.
-// Windows builds need more time -- see Issue 873112.
-#if (defined(OS_LINUX) && !defined(NDEBUG)) || defined(OS_WIN)
+// Windows builds need more time -- see Issue 873112 and 1004472.
+#if (defined(OS_LINUX) && !defined(NDEBUG))
 const int kGPUInfoWatchdogTimeoutMs = 20000;
+#elif defined(OS_WIN)
+const int kGPUInfoWatchdogTimeoutMs = 30000;
 #else
 const int kGPUInfoWatchdogTimeoutMs = 5000;
 #endif
@@ -288,10 +290,16 @@ class SystemInfoHandlerGpuObserver : public content::GpuDataManagerObserver {
   }
 
   void OnGpuInfoUpdate() override {
-    if (GpuDataManagerImpl::GetInstance()->IsGpuFeatureInfoAvailable() &&
-        GpuDataManagerImpl::GetInstance()->IsDx12VulkanVersionAvailable()) {
-      UnregisterAndSendResponse();
-    }
+    if (!GpuDataManagerImpl::GetInstance()->IsGpuFeatureInfoAvailable())
+      return;
+    base::CommandLine* command = base::CommandLine::ForCurrentProcess();
+    // Only wait for DX12/Vulkan info if requested at Chrome start up.
+    if (!command->HasSwitch(
+            switches::kDisableGpuProcessForDX12VulkanInfoCollection) &&
+        command->HasSwitch(switches::kNoDelayForDX12VulkanInfoCollection) &&
+        !GpuDataManagerImpl::GetInstance()->IsDx12VulkanVersionAvailable())
+      return;
+    UnregisterAndSendResponse();
   }
 
   void OnGpuProcessCrashed(base::TerminationStatus exit_code) override {

content/browser/gpu/gpu_data_manager_impl.cc

@@ -130,6 +130,11 @@ void GpuDataManagerImpl::UpdateDx12VulkanRequestStatus(bool request_continues) {
   base::AutoLock auto_lock(lock_);
   private_->UpdateDx12VulkanRequestStatus(request_continues);
 }
+
+bool GpuDataManagerImpl::Dx12VulkanRequested() const {
+  base::AutoLock auto_lock(lock_);
+  return private_->Dx12VulkanRequested();
+}
 #endif
 
 void GpuDataManagerImpl::UpdateGpuFeatureInfo(

content/browser/gpu/gpu_data_manager_impl.h

@@ -81,6 +81,7 @@ class CONTENT_EXPORT GpuDataManagerImpl : public GpuDataManager {
       const gpu::Dx12VulkanVersionInfo& dx12_vulkan_version_info);
   void UpdateDxDiagNodeRequestStatus(bool request_continues);
   void UpdateDx12VulkanRequestStatus(bool request_continues);
+  bool Dx12VulkanRequested() const;
 #endif
   // Update the GPU feature info. This updates the blacklist and enabled status
   // of GPU rasterization. In the future this will be used for more features.

content/browser/gpu/gpu_data_manager_impl_private.cc

@@ -377,7 +377,6 @@ void GpuDataManagerImplPrivate::RequestDxDiagNodeData() {
 #if defined(OS_WIN)
   if (gpu_info_dx_diag_requested_)
     return;
-
   gpu_info_dx_diag_requested_ = true;
   GpuProcessHost::CallOnIO(
       GPU_PROCESS_KIND_UNSANDBOXED_NO_GL, true /* force_create */,
@@ -400,10 +399,9 @@ void GpuDataManagerImplPrivate::RequestDxDiagNodeData() {
 void GpuDataManagerImplPrivate::RequestGpuSupportedRuntimeVersion(
     bool delayed) {
 #if defined(OS_WIN)
-  if (gpu_info_dx12_vulkan_requested_)
-    return;
-
   base::OnceClosure task = base::BindOnce([]() {
+    if (GpuDataManagerImpl::GetInstance()->Dx12VulkanRequested())
+      return;
     GpuProcessHost* host = GpuProcessHost::Get(
         GPU_PROCESS_KIND_UNSANDBOXED_NO_GL, true /* force_create */);
     if (!host) {
@@ -421,8 +419,6 @@ void GpuDataManagerImplPrivate::RequestGpuSupportedRuntimeVersion(
     base::PostDelayedTask(FROM_HERE, {BrowserThread::IO}, std::move(task),
                           base::TimeDelta::FromSeconds(120));
   } else {
-    gpu_info_dx12_vulkan_requested_ = true;
-    gpu_info_dx12_vulkan_request_failed_ = false;
     base::PostTask(FROM_HERE, {BrowserThread::IO}, std::move(task));
   }
 #endif
@@ -435,10 +431,11 @@ bool GpuDataManagerImplPrivate::IsEssentialGpuInfoAvailable() const {
 
 bool GpuDataManagerImplPrivate::IsDx12VulkanVersionAvailable() const {
 #if defined(OS_WIN)
-  // gpu_integration_test needs dx12/Vulkan info. If this info is needed,
-  // --no-delay-for-dx12-vulkan-info-collection should be added to the browser
-  // command line. This function returns the status of availability to the tests
-  // based on whether gpu info has been requested or not.
+  // Certain gpu_integration_test needs dx12/Vulkan info. If this info is
+  // needed, --no-delay-for-dx12-vulkan-info-collection should be added to the
+  // browser command line, so that the collection of this info isn't delayed.
+  // This function returns the status of availability to the tests based on
+  // whether gpu info has been requested or not.
 
   return gpu_info_dx12_vulkan_valid_ || !gpu_info_dx12_vulkan_requested_ ||
          gpu_info_dx12_vulkan_request_failed_;
@@ -567,6 +564,10 @@ void GpuDataManagerImplPrivate::UpdateDx12VulkanRequestStatus(
   if (gpu_info_dx12_vulkan_request_failed_)
     NotifyGpuInfoUpdate();
 }
+
+bool GpuDataManagerImplPrivate::Dx12VulkanRequested() const {
+  return gpu_info_dx12_vulkan_requested_;
+}
 #endif
 
 void GpuDataManagerImplPrivate::UpdateGpuFeatureInfo(

content/browser/gpu/gpu_data_manager_impl_private.h

@@ -66,6 +66,7 @@ class CONTENT_EXPORT GpuDataManagerImplPrivate {
       const gpu::Dx12VulkanVersionInfo& dx12_vulkan_version_info);
   void UpdateDx12VulkanRequestStatus(bool request_continues);
   void UpdateDxDiagNodeRequestStatus(bool request_continues);
+  bool Dx12VulkanRequested() const;
 #endif
   void UpdateGpuFeatureInfo(const gpu::GpuFeatureInfo& gpu_feature_info,
                             const base::Optional<gpu::GpuFeatureInfo>&

content/browser/gpu/gpu_internals_ui.cc

@@ -41,6 +41,7 @@
 #include "gpu/config/gpu_feature_type.h"
 #include "gpu/config/gpu_info.h"
 #include "gpu/config/gpu_lists_version.h"
+#include "gpu/config/gpu_util.h"
 #include "gpu/ipc/common/gpu_memory_buffer_support.h"
 #include "gpu/ipc/host/gpu_memory_buffer_support.h"
 #include "skia/ext/skia_commit_hash.h"
@@ -114,27 +115,6 @@ std::unique_ptr<base::ListValue> DxDiagNodeToList(const gpu::DxDiagNode& node) {
   }
   return list;
 }
-
-std::string D3dFeaturelevelToString(uint32_t d3d_feature_level) {
-  if (d3d_feature_level == 0) {
-    return "Not supported";
-  } else {
-    return base::StringPrintf("D3D %d.%d", (d3d_feature_level >> 12) & 0xF,
-                              (d3d_feature_level >> 8) & 0xF);
-  }
-}
-
-std::string VulkanVersionToString(uint32_t vulkan_version) {
-  if (vulkan_version == 0) {
-    return "Not supported";
-  } else {
-    // Vulkan version number VK_MAKE_VERSION(major, minor, patch)
-    // (((major) << 22) | ((minor) << 12) | (patch))
-    return base::StringPrintf(
-        "Vulkan API %d.%d.%d", (vulkan_version >> 22) & 0x3FF,
-        (vulkan_version >> 12) & 0x3FF, vulkan_version & 0xFFF);
-  }
-}
 #endif
 
 std::string GPUDeviceToString(const gpu::GPUInfo::GPUDevice& gpu) {
@@ -212,12 +192,13 @@ std::unique_ptr<base::ListValue> BasicGpuInfoAsListValue(
 
   basic_info->Append(NewDescriptionValuePair(
       "Driver D3D12 feature level",
-      D3dFeaturelevelToString(
+      gpu::D3DFeatureLevelToString(
           gpu_info.dx12_vulkan_version_info.d3d12_feature_level)));
 
   basic_info->Append(NewDescriptionValuePair(
       "Driver Vulkan API version",
-      VulkanVersionToString(gpu_info.dx12_vulkan_version_info.vulkan_version)));
+      gpu::VulkanVersionToString(
+          gpu_info.dx12_vulkan_version_info.vulkan_version)));
 #endif
 
   basic_info->Append(

content/test/gpu/gpu_tests/gpu_integration_test.py

@@ -80,12 +80,6 @@ class GpuIntegrationTest(
     if cls._disable_log_uploads:
       browser_options.logs_cloud_bucket = None
 
-    # A non-sandboxed, 120-seconds-delayed gpu process is currently running in
-    # the browser to collect gpu info. A command line switch is added here so
-    # the dx12/vulkan info can be collected immediately for the tests.
-    browser_args.append(
-      '--no-delay-for-dx12-vulkan-info-collection')
-
     # Append the new arguments.
     browser_options.AppendExtraBrowserArgs(browser_args)
     cls._last_launched_browser_args = set(browser_args)

content/test/gpu/gpu_tests/info_collection_test.py

@@ -27,6 +27,8 @@ class InfoCollectionTest(gpu_integration_test.GpuIntegrationTest):
             options.expected_device_id))
     yield ('InfoCollection_direct_composition', '_',
            ('_RunDirectCompositionTest', '_', '_'))
+    yield ('InfoCollection_dx12_vulkan', '_',
+           ('_RunDX12VulkanTest', '_', '_'))
 
   @classmethod
   def SetUpProcess(cls):
@@ -87,6 +89,17 @@ class InfoCollectionTest(gpu_integration_test.GpuIntegrationTest):
           self.fail('%s mismatch, expected %s but got %s.' %
               (field, self._ValueToStr(expected), self._ValueToStr(detected)))
 
+  def _RunDX12VulkanTest(self, gpu, unused_arg_0, unused_arg_1):
+    os_name = self.browser.platform.GetOSName()
+    if os_name and os_name.lower() == 'win':
+      self.RestartBrowserIfNecessaryWithArgs([
+        '--no-delay-for-dx12-vulkan-info-collection'])
+      # Need to re-request system info for DX12/Vulkan bits.
+      system_info = self.browser.GetSystemInfo()
+      if not system_info:
+        self.fail("Browser doesn't support GetSystemInfo")
+      # TODO(zmo): Verify Win GPU bots DX12/Vulkan supports are as expected.
+
   @staticmethod
   def _ValueToStr(value):
     if type(value) is str:

gpu/config/gpu_info.cc

@@ -5,6 +5,7 @@
 #include <stdint.h>
 
 #include "gpu/config/gpu_info.h"
+#include "gpu/config/gpu_util.h"
 
 namespace {
 
@@ -102,9 +103,10 @@ void EnumerateDx12VulkanVersionInfo(const gpu::Dx12VulkanVersionInfo& info,
   enumerator->BeginDx12VulkanVersionInfo();
   enumerator->AddBool("supportsDx12", info.supports_dx12);
   enumerator->AddBool("supportsVulkan", info.supports_vulkan);
-  enumerator->AddInt("dx12FeatureLevel",
-                     static_cast<int>(info.d3d12_feature_level));
-  enumerator->AddInt("vulkanVersion", static_cast<int>(info.vulkan_version));
+  enumerator->AddString("dx12FeatureLevel",
+                        gpu::D3DFeatureLevelToString(info.d3d12_feature_level));
+  enumerator->AddString("vulkanVersion",
+                        gpu::VulkanVersionToString(info.vulkan_version));
   enumerator->EndDx12VulkanVersionInfo();
 }
 #endif

gpu/config/gpu_util.cc

@@ -755,4 +755,27 @@ std::string GetIntelGpuGeneration(uint32_t vendor_id, uint32_t device_id) {
   return "";
 }
 
+#if defined(OS_WIN)
+std::string D3DFeatureLevelToString(uint32_t d3d_feature_level) {
+  if (d3d_feature_level == 0) {
+    return "Not supported";
+  } else {
+    return base::StringPrintf("D3D %d.%d", (d3d_feature_level >> 12) & 0xF,
+                              (d3d_feature_level >> 8) & 0xF);
+  }
+}
+
+std::string VulkanVersionToString(uint32_t vulkan_version) {
+  if (vulkan_version == 0) {
+    return "Not supported";
+  } else {
+    // Vulkan version number VK_MAKE_VERSION(major, minor, patch)
+    // (((major) << 22) | ((minor) << 12) | (patch))
+    return base::StringPrintf(
+        "Vulkan API %d.%d.%d", (vulkan_version >> 22) & 0x3FF,
+        (vulkan_version >> 12) & 0x3FF, vulkan_version & 0xFFF);
+  }
+}
+#endif  // OS_WIN
+
 }  // namespace gpu

gpu/config/gpu_util.h

@@ -80,6 +80,11 @@ GPU_EXPORT GpuSeriesType GetGpuSeriesType(uint32_t vendor_id,
 GPU_EXPORT std::string GetIntelGpuGeneration(uint32_t vendor_id,
                                              uint32_t device_id);
 
+#if defined(OS_WIN)
+GPU_EXPORT std::string D3DFeatureLevelToString(uint32_t d3d_feature_level);
+GPU_EXPORT std::string VulkanVersionToString(uint32_t vulkan_version);
+#endif  // OS_WIN
+
 }  // namespace gpu
 
 #endif  // GPU_CONFIG_GPU_UTIL_H_