Call Dx12/Vulkan info request for about:gpu

After Dx12/Vulkan Info collection task is delayed for 2 minutes (the other CL),
Dx12/Vulkan info will always be requested if the data is not available for
about:gpu.

Rewrite the code to make DxDiagNode function aligned with Dx12VulkanInfo
function.

Bug:949839

Change-Id: Ifcab4885dccb13e206f707657ac9074a21cdf4a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1785827
Auto-Submit: Maggie Chen <magchen@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694960}

components/autofill/content/browser/risk/fingerprint.cc

@@ -296,7 +296,7 @@ FingerprintDataLoader::FingerprintDataLoader(
   if (gpu_data_manager_->GpuAccessAllowed(nullptr) &&
       !gpu_data_manager_->IsEssentialGpuInfoAvailable()) {
     gpu_observer_.Add(gpu_data_manager_);
-    gpu_data_manager_->RequestCompleteGpuInfoIfNeeded();
+    OnGpuInfoUpdate();
   }
 
 #if BUILDFLAG(ENABLE_PLUGINS)

components/viz/service/gl/gpu_service_impl.cc

@@ -534,8 +534,10 @@ void GpuServiceImpl::GetGpuSupportedRuntimeVersion(
       &gpu_info_.dx12_vulkan_version_info);
   std::move(callback).Run(gpu_info_.dx12_vulkan_version_info);
 
-  // The unsandboxed GPU process fulfilled its duty. Bye bye.
-  MaybeExit(false);
+  // The unsandboxed GPU process fulfilled its duty and Dxdiag task is not
+  // running. Bye bye.
+  if (!long_dx_task_different_thread_in_progress_)
+    MaybeExit(false);
 }
 
 void GpuServiceImpl::RequestCompleteGpuInfo(
@@ -556,6 +558,7 @@ void GpuServiceImpl::RequestCompleteGpuInfo(
              RequestCompleteGpuInfoCallback callback) {
             std::move(callback).Run(gpu_service->gpu_info_.dx_diagnostics);
             // The unsandboxed GPU process fulfilled its duty. Bye bye.
+            gpu_service->long_dx_task_different_thread_in_progress_ = false;
             gpu_service->MaybeExit(false);
           },
           this, std::move(callback))));
@@ -591,6 +594,7 @@ void GpuServiceImpl::UpdateGpuInfoPlatform(
 
   // We can continue on shutdown here because we're not writing any critical
   // state in this task.
+  long_dx_task_different_thread_in_progress_ = true;
   base::PostTaskAndReplyWithResult(
       base::CreateCOMSTATaskRunner(
           {base::ThreadPool(), base::TaskPriority::USER_VISIBLE,

components/viz/service/gl/gpu_service_impl.h

@@ -346,6 +346,12 @@ class VIZ_SERVICE_EXPORT GpuServiceImpl : public gpu::GpuChannelManagerDelegate,
   base::CancelableTaskTracker bind_task_tracker_;
   std::unique_ptr<mojo::BindingSet<mojom::GpuService>> bindings_;
 
+#if defined(OS_WIN)
+  // Used to track if the Dx Diag task on a different thread is still running.
+  // The status is checked before exiting the unsandboxed GPU process.
+  bool long_dx_task_different_thread_in_progress_ = false;
+#endif
+
 #if defined(OS_CHROMEOS)
   scoped_refptr<arc::ProtectedBufferManager> protected_buffer_manager_;
 #endif  // defined(OS_CHROMEOS)

content/browser/browser_main_loop.cc

@@ -1416,8 +1416,8 @@ int BrowserMainLoop::BrowserThreadsStarted() {
     // The default is to delay the secondary GPU process for 120 seconds.
     bool delayed = !parsed_command_line_.HasSwitch(
         switches::kNoDelayForDX12VulkanInfoCollection);
-    GpuDataManagerImpl::GetInstance()->RequestGpuSupportedRuntimeVersion(
-        delayed);
+    GpuDataManagerImpl::GetInstance()->RequestCompleteGpuInfoIfNeeded(
+        kGpuInfoRequestDx12Vulkan, delayed);
   }
 
 #endif

content/browser/gpu/gpu_data_manager_impl.cc

@@ -35,9 +35,10 @@ bool GpuDataManagerImpl::GpuAccessAllowed(std::string* reason) {
   return private_->GpuAccessAllowed(reason);
 }
 
-void GpuDataManagerImpl::RequestCompleteGpuInfoIfNeeded() {
+void GpuDataManagerImpl::RequestCompleteGpuInfoIfNeeded(GpuInfoRequest request,
+                                                        bool delayed) {
   base::AutoLock auto_lock(lock_);
-  private_->RequestCompleteGpuInfoIfNeeded();
+  private_->RequestCompleteGpuInfoIfNeeded(request, delayed);
 }
 
 bool GpuDataManagerImpl::IsEssentialGpuInfoAvailable() {
@@ -95,11 +96,6 @@ void GpuDataManagerImpl::AppendGpuCommandLine(base::CommandLine* command_line,
   private_->AppendGpuCommandLine(command_line, kind);
 }
 
-void GpuDataManagerImpl::RequestGpuSupportedRuntimeVersion(bool delayed) const {
-  base::AutoLock auto_lock(lock_);
-  private_->RequestGpuSupportedRuntimeVersion(delayed);
-}
-
 bool GpuDataManagerImpl::GpuProcessStartAllowed() const {
   base::AutoLock auto_lock(lock_);
   return private_->GpuProcessStartAllowed();
@@ -125,6 +121,11 @@ void GpuDataManagerImpl::UpdateDx12VulkanInfo(
   private_->UpdateDx12VulkanInfo(dx12_vulkan_version_info);
 }
 
+void GpuDataManagerImpl::UpdateDxDiagNodeRequestStatus(bool request_continues) {
+  base::AutoLock auto_lock(lock_);
+  private_->UpdateDxDiagNodeRequestStatus(request_continues);
+}
+
 void GpuDataManagerImpl::UpdateDx12VulkanRequestStatus(bool request_continues) {
   base::AutoLock auto_lock(lock_);
   private_->UpdateDx12VulkanRequestStatus(request_continues);

content/browser/gpu/gpu_data_manager_impl.h

@@ -49,7 +49,8 @@ class CONTENT_EXPORT GpuDataManagerImpl : public GpuDataManager {
   void BlacklistWebGLForTesting() override;
   gpu::GPUInfo GetGPUInfo() override;
   bool GpuAccessAllowed(std::string* reason) override;
-  void RequestCompleteGpuInfoIfNeeded() override;
+  void RequestCompleteGpuInfoIfNeeded(GpuInfoRequest request,
+                                      bool delayed) override;
   bool IsEssentialGpuInfoAvailable() override;
   void RequestVideoMemoryUsageStatsUpdate(
       VideoMemoryUsageStatsCallback callback) override;
@@ -65,7 +66,6 @@ class CONTENT_EXPORT GpuDataManagerImpl : public GpuDataManager {
   void AppendGpuCommandLine(base::CommandLine* command_line,
                             GpuProcessKind kind) override;
 
-  void RequestGpuSupportedRuntimeVersion(bool delayed) const;
   bool GpuProcessStartAllowed() const;
 
   bool IsDx12VulkanVersionAvailable() const;
@@ -76,9 +76,10 @@ class CONTENT_EXPORT GpuDataManagerImpl : public GpuDataManager {
       const gpu::GPUInfo& gpu_info,
       const base::Optional<gpu::GPUInfo>& gpu_info_for_hardware_gpu);
 #if defined(OS_WIN)
+  void UpdateDxDiagNode(const gpu::DxDiagNode& dx_diagnostics);
   void UpdateDx12VulkanInfo(
       const gpu::Dx12VulkanVersionInfo& dx12_vulkan_version_info);
-  void UpdateDxDiagNode(const gpu::DxDiagNode& dx_diagnostics);
+  void UpdateDxDiagNodeRequestStatus(bool request_continues);
   void UpdateDx12VulkanRequestStatus(bool request_continues);
 #endif
   // Update the GPU feature info. This updates the blacklist and enabled status

content/browser/gpu/gpu_data_manager_impl_private.cc

@@ -360,40 +360,48 @@ bool GpuDataManagerImplPrivate::GpuProcessStartAllowed() const {
   return false;
 }
 
-void GpuDataManagerImplPrivate::RequestCompleteGpuInfoIfNeeded() {
-  if (complete_gpu_info_already_requested_)
-    return;
-  if (!NeedsCompleteGpuInfoCollection())
-    return;
+void GpuDataManagerImplPrivate::RequestCompleteGpuInfoIfNeeded(
+    GpuInfoRequest request,
+    bool delayed) {
+  if (request & kGpuInfoRequestDxDiag) {
+    // Delay is not supported in DxDiag request
+    DCHECK(!delayed);
+    RequestDxDiagNodeData();
+  }
 
+  if (request & kGpuInfoRequestDx12Vulkan)
+    RequestGpuSupportedRuntimeVersion(delayed);
+}
+
+void GpuDataManagerImplPrivate::RequestDxDiagNodeData() {
 #if defined(OS_WIN)
-  complete_gpu_info_already_requested_ = true;
-  GpuProcessHost::CallOnIO(GPU_PROCESS_KIND_UNSANDBOXED_NO_GL,
-                           true /* force_create */,
-                           base::BindOnce([](GpuProcessHost* host) {
-                             if (!host)
-                               return;
-                             host->gpu_service()->RequestCompleteGpuInfo(
-                                 base::BindOnce(
-                                     [](const gpu::DxDiagNode& dx_diagnostics) {
-                                       GpuDataManagerImpl::GetInstance()
-                                           ->UpdateDxDiagNode(dx_diagnostics);
-                                     }));
-                           }));
-#else
-  // NeedsCompleteGpuInfoCollection() always returns false on platforms other
-  // than Windows.
-  NOTREACHED();
+  if (gpu_info_dx_diag_requested_)
+    return;
+
+  gpu_info_dx_diag_requested_ = true;
+  GpuProcessHost::CallOnIO(
+      GPU_PROCESS_KIND_UNSANDBOXED_NO_GL, true /* force_create */,
+      base::BindOnce([](GpuProcessHost* host) {
+        if (!host) {
+          GpuDataManagerImpl::GetInstance()->UpdateDxDiagNodeRequestStatus(
+              false);
+          return;
+        }
+        GpuDataManagerImpl::GetInstance()->UpdateDxDiagNodeRequestStatus(true);
+        host->gpu_service()->RequestCompleteGpuInfo(
+            base::BindOnce([](const gpu::DxDiagNode& dx_diagnostics) {
+              GpuDataManagerImpl::GetInstance()->UpdateDxDiagNode(
+                  dx_diagnostics);
+            }));
+      }));
 #endif
 }
 
 void GpuDataManagerImplPrivate::RequestGpuSupportedRuntimeVersion(
     bool delayed) {
 #if defined(OS_WIN)
-  if (gpu_info_dx12_vulkan_valid_) {
-    NotifyGpuInfoUpdate();
+  if (gpu_info_dx12_vulkan_requested_)
     return;
-  }
 
   base::OnceClosure task = base::BindOnce([]() {
     GpuProcessHost* host = GpuProcessHost::Get(
@@ -417,9 +425,6 @@ void GpuDataManagerImplPrivate::RequestGpuSupportedRuntimeVersion(
     gpu_info_dx12_vulkan_request_failed_ = false;
     base::PostTask(FROM_HERE, {BrowserThread::IO}, std::move(task));
   }
-
-#else
-  NOTREACHED();
 #endif
 }
 
@@ -533,8 +538,6 @@ void GpuDataManagerImplPrivate::UpdateGpuInfo(
 void GpuDataManagerImplPrivate::UpdateDxDiagNode(
     const gpu::DxDiagNode& dx_diagnostics) {
   gpu_info_.dx_diagnostics = dx_diagnostics;
-  if (complete_gpu_info_already_requested_)
-    complete_gpu_info_already_requested_ = false;
   // No need to call GetContentClient()->SetGpuInfo().
   NotifyGpuInfoUpdate();
 }
@@ -548,6 +551,14 @@ void GpuDataManagerImplPrivate::UpdateDx12VulkanInfo(
   NotifyGpuInfoUpdate();
 }
 
+void GpuDataManagerImplPrivate::UpdateDxDiagNodeRequestStatus(
+    bool request_continues) {
+  gpu_info_dx_diag_request_failed_ = !request_continues;
+
+  if (gpu_info_dx_diag_request_failed_)
+    NotifyGpuInfoUpdate();
+}
+
 void GpuDataManagerImplPrivate::UpdateDx12VulkanRequestStatus(
     bool request_continues) {
   gpu_info_dx12_vulkan_requested_ = true;
@@ -928,14 +939,6 @@ int64_t GpuDataManagerImplPrivate::GetBlockAllDomainsDurationInMs() const {
   return kBlockAllDomainsMs;
 }
 
-bool GpuDataManagerImplPrivate::NeedsCompleteGpuInfoCollection() const {
-#if defined(OS_WIN)
-  return gpu_info_.dx_diagnostics.IsEmpty();
-#else
-  return false;
-#endif
-}
-
 gpu::GpuMode GpuDataManagerImplPrivate::GetGpuMode() const {
   return gpu_mode_;
 }

content/browser/gpu/gpu_data_manager_impl_private.h

@@ -44,8 +44,7 @@ class CONTENT_EXPORT GpuDataManagerImplPrivate {
   gpu::GPUInfo GetGPUInfoForHardwareGpu() const;
   bool GpuAccessAllowed(std::string* reason) const;
   bool GpuProcessStartAllowed() const;
-  void RequestCompleteGpuInfoIfNeeded();
-  void RequestGpuSupportedRuntimeVersion(bool delayed);
+  void RequestCompleteGpuInfoIfNeeded(GpuInfoRequest request, bool delayed);
   bool IsEssentialGpuInfoAvailable() const;
   bool IsDx12VulkanVersionAvailable() const;
   bool IsGpuFeatureInfoAvailable() const;
@@ -66,6 +65,7 @@ class CONTENT_EXPORT GpuDataManagerImplPrivate {
   void UpdateDx12VulkanInfo(
       const gpu::Dx12VulkanVersionInfo& dx12_vulkan_version_info);
   void UpdateDx12VulkanRequestStatus(bool request_continues);
+  void UpdateDxDiagNodeRequestStatus(bool request_continues);
 #endif
   void UpdateGpuFeatureInfo(const gpu::GpuFeatureInfo& gpu_feature_info,
                             const base::Optional<gpu::GpuFeatureInfo>&
@@ -177,21 +177,19 @@ class CONTENT_EXPORT GpuDataManagerImplPrivate {
                                            base::Time at_time) const;
   int64_t GetBlockAllDomainsDurationInMs() const;
 
-  // This is platform specific. At the moment:
-  //   1) on Windows, if DxDiagnostics are missing, this returns true;
-  //   2) all other platforms, this returns false.
-  bool NeedsCompleteGpuInfoCollection() const;
-
   // Notify all observers whenever there is a GPU info update.
   void NotifyGpuInfoUpdate();
 
-  GpuDataManagerImpl* const owner_;
+  void RequestDxDiagNodeData();
+  void RequestGpuSupportedRuntimeVersion(bool delayed);
 
-  bool complete_gpu_info_already_requested_ = false;
+  GpuDataManagerImpl* const owner_;
 
   gpu::GpuFeatureInfo gpu_feature_info_;
   gpu::GPUInfo gpu_info_;
 #if defined(OS_WIN)
+  bool gpu_info_dx_diag_requested_ = false;
+  bool gpu_info_dx_diag_request_failed_ = false;
   bool gpu_info_dx12_vulkan_valid_ = false;
   bool gpu_info_dx12_vulkan_requested_ = false;
   bool gpu_info_dx12_vulkan_request_failed_ = false;

content/browser/gpu/gpu_internals_ui.cc

@@ -643,7 +643,8 @@ void GpuMessageHandler::OnBrowserBridgeInitialized(
 
   // Tell GpuDataManager it should have full GpuInfo. If the
   // Gpu process has not run yet, this will trigger its launch.
-  GpuDataManagerImpl::GetInstance()->RequestCompleteGpuInfoIfNeeded();
+  GpuDataManagerImpl::GetInstance()->RequestCompleteGpuInfoIfNeeded(
+      kGpuInfoRequestAll, /*delayed*/ false);
 
   // Run callback immediately in case the info is ready and no update in the
   // future.
@@ -726,8 +727,13 @@ void GpuMessageHandler::OnGpuInfoUpdate() {
                                          *(gpu_info_val.get()));
 }
 
+// TODO(magchen): RequestCompleteGpuInfoIfNeeded(), which collects Dxdiag and
+// Dx12/Vulkan info, only runs once. If this same information is not needed,
+// GPU switch observer should be deleted. If more information is needed, it
+// should be fixed.
 void GpuMessageHandler::OnGpuSwitched() {
-  GpuDataManagerImpl::GetInstance()->RequestCompleteGpuInfoIfNeeded();
+  GpuDataManagerImpl::GetInstance()->RequestCompleteGpuInfoIfNeeded(
+      kGpuInfoRequestDxDiag, /*delayed*/ false);
 }
 
 }  // namespace

content/public/browser/gpu_data_manager.h

@@ -28,6 +28,12 @@ enum GpuProcessKind {
   GPU_PROCESS_KIND_COUNT
 };
 
+enum GpuInfoRequest {
+  kGpuInfoRequestDxDiag = 1 << 0,
+  kGpuInfoRequestDx12Vulkan = 1 << 1,
+  kGpuInfoRequestAll = kGpuInfoRequestDxDiag | kGpuInfoRequestDx12Vulkan,
+};
+
 class GpuDataManagerObserver;
 
 // This class is fully thread-safe.
@@ -54,7 +60,8 @@ class GpuDataManager {
   virtual bool GpuAccessAllowed(std::string* reason) = 0;
 
   // Requests complete GPU info if it has not already been requested
-  virtual void RequestCompleteGpuInfoIfNeeded() = 0;
+  virtual void RequestCompleteGpuInfoIfNeeded(GpuInfoRequest request,
+                                              bool delayed) = 0;
 
   // Check if basic and context GPU info have been collected.
   virtual bool IsEssentialGpuInfoAvailable() = 0;