Do not launch the GPU info collection process if the device id is 0

By the time the GPU info collection is requested, the GPU vendor id
and device id should be available. Something must be wrong if the
device id is 0. In this case, no need to collect GPU info.

This CL also renames the info collection GPU process from
GPU_PROCESS_KIND_UNSANDBOXED_NO_GL to
GPU_PROCESS_KIND_INFO_COLLECTION

Bug: 896565
Change-Id: I00e1a70448f337f148ae7ad4f016d2308c284fba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2036940
Commit-Queue: Maggie Chen <magchen@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738682}

components/viz/service/gl/gpu_service_impl.cc

@@ -601,8 +601,8 @@ void GpuServiceImpl::GetGpuSupportedRuntimeVersionAndDevicePerfInfo(
   std::move(callback).Run(gpu_info_.dx12_vulkan_version_info,
                           device_perf_info_.value());
 
-  // The unsandboxed GPU process fulfilled its duty and Dxdiag task is not
-  // running. Bye bye.
+  // The unsandboxed GPU info collection process fulfilled its duty and Dxdiag
+  // task is not running. Bye bye.
   if (!long_dx_task_different_thread_in_progress_)
     MaybeExit(false);
 }
@@ -624,7 +624,8 @@ void GpuServiceImpl::RequestCompleteGpuInfo(
           [](GpuServiceImpl* gpu_service,
              RequestCompleteGpuInfoCallback callback) {
             std::move(callback).Run(gpu_service->gpu_info_.dx_diagnostics);
-            // The unsandboxed GPU process fulfilled its duty. Bye bye.
+            // The unsandboxed GPU info collection process fulfilled its duty.
+            // Bye bye.
             gpu_service->long_dx_task_different_thread_in_progress_ = false;
             gpu_service->MaybeExit(false);
           },
@@ -992,9 +993,9 @@ void GpuServiceImpl::MaybeExit(bool for_context_loss) {
                   "from errors. GPU process will restart shortly.";
   }
   is_exiting_.Set();
-  // For the unsandboxed GPU process used for info collection, if we exit
-  // immediately, then the reply message could be lost. That's why the
-  // |exit_callback_| takes the boolean argument.
+  // For the unsandboxed GPU info collection process used for info collection,
+  // if we exit immediately, then the reply message could be lost. That's why
+  // the |exit_callback_| takes the boolean argument.
   std::move(exit_callback_).Run(/*immediately=*/for_context_loss);
 }
 

content/browser/gpu/gpu_data_manager_impl_private.cc

@@ -613,24 +613,27 @@ void GpuDataManagerImplPrivate::RequestDxDiagNodeData() {
   gpu_info_dx_diag_requested_ = true;
 
   base::OnceClosure task = base::BindOnce([]() {
-    // No info collection for software GL implementation (id == 0xffff).
-    // There are a few crash reports on exit_or_terminate_process() during
-    // process teardown.
-    const gpu::GPUInfo::GPUDevice gpu =
-        GpuDataManagerImpl::GetInstance()->GetGPUInfo().gpu;
-    if (gpu.vendor_id == 0xffff && gpu.device_id == 0xffff) {
-      GpuDataManagerImpl::GetInstance()->UpdateDxDiagNodeRequestStatus(false);
+    GpuDataManagerImpl* manager = GpuDataManagerImpl::GetInstance();
+    // No info collection for software GL implementation (id == 0xffff) or
+    // abnormal situation (id == 0). There are a few crash reports on
+    // exit_or_terminate_process() during process teardown. The GPU ID
+    // should be available by the time this task starts to run.
+    // This request comes from chrome://gpu page.
+    const gpu::GPUInfo::GPUDevice gpu = manager->GetGPUInfo().gpu;
+    if ((gpu.vendor_id == 0xffff && gpu.device_id == 0xffff) ||
+        (gpu.vendor_id == 0 && gpu.device_id == 0)) {
+      manager->UpdateDxDiagNodeRequestStatus(false);
       return;
     }
 
-    GpuProcessHost* host = GpuProcessHost::Get(
-        GPU_PROCESS_KIND_UNSANDBOXED_NO_GL, true /* force_create */);
+    GpuProcessHost* host = GpuProcessHost::Get(GPU_PROCESS_KIND_INFO_COLLECTION,
+                                               true /* force_create */);
     if (!host) {
-      GpuDataManagerImpl::GetInstance()->UpdateDxDiagNodeRequestStatus(false);
+      manager->UpdateDxDiagNodeRequestStatus(false);
       return;
     }
 
-    GpuDataManagerImpl::GetInstance()->UpdateDxDiagNodeRequestStatus(true);
+    manager->UpdateDxDiagNodeRequestStatus(true);
     host->gpu_service()->RequestCompleteGpuInfo(
         base::BindOnce([](const gpu::DxDiagNode& dx_diagnostics) {
           GpuDataManagerImpl::GetInstance()->UpdateDxDiagNode(dx_diagnostics);
@@ -644,35 +647,41 @@ void GpuDataManagerImplPrivate::RequestDxDiagNodeData() {
 void GpuDataManagerImplPrivate::RequestGpuSupportedRuntimeVersion(
     bool delayed) {
 #if defined(OS_WIN)
-  base::OnceClosure task = base::BindOnce([]() {
-    if (GpuDataManagerImpl::GetInstance()->Dx12VulkanRequested())
+  base::OnceClosure task = base::BindOnce(
+      [](bool delayed) {
+        GpuDataManagerImpl* manager = GpuDataManagerImpl::GetInstance();
+        if (manager->Dx12VulkanRequested())
           return;
 
-    // No info collection for software GL implementation (id == 0xffff).
-    // There are a few crash reports on exit_or_terminate_process() during
-    // process teardown.
-    const gpu::GPUInfo::GPUDevice gpu =
-        GpuDataManagerImpl::GetInstance()->GetGPUInfo().gpu;
-    if (gpu.vendor_id == 0xffff && gpu.device_id == 0xffff) {
-      GpuDataManagerImpl::GetInstance()->UpdateDx12VulkanRequestStatus(false);
+        // No info collection for software GL implementation (id == 0xffff) or
+        // abnormal situation (id == 0). There are a few crash reports on
+        // exit_or_terminate_process() during process teardown. The GPU ID
+        // should be available by the time this task starts to run. In the case
+        // of no delay, which is for testing only, don't check the GPU ID
+        // because the ID is not available yet.
+        const gpu::GPUInfo::GPUDevice gpu = manager->GetGPUInfo().gpu;
+        if ((gpu.vendor_id == 0xffff && gpu.device_id == 0xffff) ||
+            (delayed && gpu.vendor_id == 0 && gpu.device_id == 0)) {
+          manager->UpdateDx12VulkanRequestStatus(false);
           return;
         }
 
         GpuProcessHost* host = GpuProcessHost::Get(
-        GPU_PROCESS_KIND_UNSANDBOXED_NO_GL, true /* force_create */);
+            GPU_PROCESS_KIND_INFO_COLLECTION, true /* force_create */);
         if (!host) {
-      GpuDataManagerImpl::GetInstance()->UpdateDx12VulkanRequestStatus(false);
+          manager->UpdateDx12VulkanRequestStatus(false);
           return;
         }
 
-    GpuDataManagerImpl::GetInstance()->UpdateDx12VulkanRequestStatus(true);
+        manager->UpdateDx12VulkanRequestStatus(true);
         host->gpu_service()->GetGpuSupportedRuntimeVersionAndDevicePerfInfo(
             base::BindOnce([](const gpu::Dx12VulkanVersionInfo& info,
                               const gpu::DevicePerfInfo& device_perf_info) {
               GpuDataManagerImpl::GetInstance()->UpdateDx12VulkanInfo(info);
               // TODO(zmo): Process collected DevicePerfInfo.
             }));
-  });
+      },
+      delayed);
 
   if (delayed) {
     base::PostDelayedTask(FROM_HERE, {BrowserThread::IO}, std::move(task),
@@ -983,14 +992,14 @@ void GpuDataManagerImplPrivate::UpdateGpuPreferences(
       base::CommandLine::ForCurrentProcess();
   gpu_preferences->gpu_startup_dialog =
 #if defined(OS_WIN)
-      (kind == GPU_PROCESS_KIND_UNSANDBOXED_NO_GL &&
+      (kind == GPU_PROCESS_KIND_INFO_COLLECTION &&
        command_line->HasSwitch(switches::kGpu2StartupDialog)) ||
 #endif
       (kind == GPU_PROCESS_KIND_SANDBOXED &&
        command_line->HasSwitch(switches::kGpuStartupDialog));
 
 #if defined(OS_WIN)
-  if (kind == GPU_PROCESS_KIND_UNSANDBOXED_NO_GL) {
+  if (kind == GPU_PROCESS_KIND_INFO_COLLECTION) {
     gpu_preferences->disable_gpu_watchdog = true;
     gpu_preferences->enable_perf_data_collection = true;
   }

content/browser/gpu/gpu_process_host.cc

@@ -501,8 +501,9 @@ GpuProcessHost* GpuProcessHost::Get(GpuProcessKind kind, bool force_create) {
     return nullptr;
   }
 
-  // Do not launch the unsandboxed GPU process if GPU is disabled
-  if (kind == GPU_PROCESS_KIND_UNSANDBOXED_NO_GL) {
+  // Do not launch the unsandboxed GPU info collection process if GPU is
+  // disabled
+  if (kind == GPU_PROCESS_KIND_INFO_COLLECTION) {
     auto* command_line = base::CommandLine::ForCurrentProcess();
     if (command_line->HasSwitch(switches::kDisableGpu) ||
         command_line->HasSwitch(switches::kSingleProcess) ||
@@ -564,7 +565,7 @@ void GpuProcessHost::CallOnIO(
     bool force_create,
     base::OnceCallback<void(GpuProcessHost*)> callback) {
 #if !defined(OS_WIN)
-  DCHECK_NE(kind, GPU_PROCESS_KIND_UNSANDBOXED_NO_GL);
+  DCHECK_NE(kind, GPU_PROCESS_KIND_INFO_COLLECTION);
 #endif
   base::PostTask(FROM_HERE, {BrowserThread::IO},
                  base::BindOnce(&RunCallbackOnIO, kind, force_create,
@@ -735,7 +736,7 @@ GpuProcessHost::~GpuProcessHost() {
 
       message = "The GPU process ";
     } else {
-      message = "The unsandboxed GPU process ";
+      message = "The info collection GPU process ";
     }
 
     switch (info.status) {
@@ -970,7 +971,7 @@ void GpuProcessHost::DidInitialize(
                     "initialization time was "
                  << gpu_info.initialization_time.InMilliseconds() << " ms";
   }
-  if (kind_ != GPU_PROCESS_KIND_UNSANDBOXED_NO_GL) {
+  if (kind_ != GPU_PROCESS_KIND_INFO_COLLECTION) {
     auto* gpu_data_manager = GpuDataManagerImpl::GetInstance();
     // Update GpuFeatureInfo first, because UpdateGpuInfo() will notify all
     // listeners.
@@ -1100,7 +1101,7 @@ bool GpuProcessHost::LaunchGpuProcess() {
   cmd_line->AppendArg(switches::kPrefetchArgumentGpu);
 #endif  // defined(OS_WIN)
 
-  if (kind_ == GPU_PROCESS_KIND_UNSANDBOXED_NO_GL) {
+  if (kind_ == GPU_PROCESS_KIND_INFO_COLLECTION) {
     cmd_line->AppendSwitch(service_manager::switches::kDisableGpuSandbox);
     cmd_line->AppendSwitchASCII(switches::kUseGL,
                                 gl::kGLImplementationDisabledName);

content/public/browser/gpu_data_manager.h

@@ -25,7 +25,7 @@ struct VideoMemoryUsageStats;
 
 namespace content {
 enum GpuProcessKind {
-  GPU_PROCESS_KIND_UNSANDBOXED_NO_GL,  // Unsandboxed, no init GL bindings.
+  GPU_PROCESS_KIND_INFO_COLLECTION,  // Unsandboxed, no init GL bindings.
   GPU_PROCESS_KIND_SANDBOXED,
   GPU_PROCESS_KIND_COUNT
 };