Remove info collection states from GPUInfo.

They are really a messy design. By returning a bool at various collector
function seems enough.

BUG=808119
TEST=bots
R=piman@chromium.org,kbr@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I42dd69587d70d04930c81fdf9cfb727527e59ed6
Reviewed-on: https://chromium-review.googlesource.com/924699
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538636}

components/viz/service/gl/gpu_service_impl.cc

@@ -427,21 +427,10 @@ void GpuServiceImpl::UpdateGpuInfoPlatform(
   if (in_host_process())
     return;
 
-  DCHECK_EQ(gpu::kCollectInfoNone, gpu_info_.context_info_state);
-  gpu::CollectInfoResult result = gpu::CollectContextGraphicsInfo(&gpu_info_);
-  switch (result) {
-    case gpu::kCollectInfoFatalFailure:
-      LOG(ERROR) << "gpu::CollectGraphicsInfo failed (fatal).";
-      // TODO(piman): can we signal overall failure?
-      break;
-    case gpu::kCollectInfoNonFatalFailure:
-      DVLOG(1) << "gpu::CollectGraphicsInfo failed (non-fatal).";
-      break;
-    case gpu::kCollectInfoNone:
-      NOTREACHED();
-      break;
-    case gpu::kCollectInfoSuccess:
-      break;
+  bool success = gpu::CollectContextGraphicsInfo(&gpu_info_);
+  if (!success) {
+    LOG(ERROR) << "gpu::CollectGraphicsInfo failed.";
+    // TODO(piman): can we signal overall failure?
   }
   gpu::SetKeysForCrashLogging(gpu_info_);
   std::move(on_gpu_info_updated).Run();
@@ -471,8 +460,6 @@ void GpuServiceImpl::UpdateGpuInfoPlatform(
           [](GpuServiceImpl* gpu_service, base::OnceClosure on_gpu_info_updated,
              const gpu::DxDiagNode& dx_diag_node) {
             gpu_service->gpu_info_.dx_diagnostics = dx_diag_node;
-            gpu_service->gpu_info_.dx_diagnostics_info_state =
-                gpu::kCollectInfoSuccess;
             std::move(on_gpu_info_updated).Run();
           },
           this, std::move(on_gpu_info_updated)));

content/browser/gpu/gpu_data_manager_impl_private.cc

@@ -256,7 +256,7 @@ void DisplayReconfigCallback(CGDirectDisplayID display,
   bool gpu_changed = false;
   if (flags & kCGDisplayAddFlag) {
     gpu::GPUInfo gpu_info;
-    if (gpu::CollectBasicGraphicsInfo(&gpu_info) == gpu::kCollectInfoSuccess) {
+    if (gpu::CollectBasicGraphicsInfo(&gpu_info)) {
       gpu_changed = manager->UpdateActiveGpu(gpu_info.active_gpu().vendor_id,
                                              gpu_info.active_gpu().device_id);
     }
@@ -351,13 +351,6 @@ bool GpuDataManagerImplPrivate::GpuAccessAllowed(
   if (swiftshader_available)
     return true;
 
-  if (!gpu_process_accessible_) {
-    if (reason) {
-      *reason = "GPU process launch failed.";
-    }
-    return false;
-  }
-
   if (in_process_gpu_)
     return true;
 
@@ -400,9 +393,8 @@ void GpuDataManagerImplPrivate::RequestCompleteGpuInfoIfNeeded() {
 }
 
 bool GpuDataManagerImplPrivate::IsEssentialGpuInfoAvailable() const {
-  // Now we collect GPU info on the GPU side, so whatever returns is valid.
-  return (gpu_info_.basic_info_state != gpu::kCollectInfoNone ||
-          gpu_info_.context_info_state != gpu::kCollectInfoNone);
+  // We always update GPUInfo and GpuFeatureInfo from GPU process together.
+  return IsGpuFeatureInfoAvailable();
 }
 
 bool GpuDataManagerImplPrivate::IsGpuFeatureInfoAvailable() const {
@@ -578,11 +570,6 @@ void GpuDataManagerImplPrivate::OnGpuProcessBlocked() {
       gpu::ComputeGpuFeatureInfoWithNoGpuProcess();
   UpdateGpuFeatureInfo(gpu_feature_info);
 
-  // This is needed for IsEssentialGpuInfoAvailable() to return
-  // true. Otherwise some observers may keep waiting for the next
-  // GPUInfo update.
-  // TODO(zmo): Clean up this hack.
-  gpu_info_.context_info_state = gpu::kCollectInfoFatalFailure;
   // Some observers might be waiting.
   NotifyGpuInfoUpdate();
 }
@@ -752,7 +739,6 @@ GpuDataManagerImplPrivate::GpuDataManagerImplPrivate(GpuDataManagerImpl* owner)
       update_histograms_(true),
       domain_blocking_enabled_(true),
       owner_(owner),
-      gpu_process_accessible_(true),
       in_process_gpu_(false) {
   DCHECK(owner_);
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
@@ -897,14 +883,19 @@ void GpuDataManagerImplPrivate::Notify3DAPIBlocked(const GURL& top_origin_url,
 }
 
 void GpuDataManagerImplPrivate::OnGpuProcessInitFailure() {
-  gpu_process_accessible_ = false;
-  gpu_info_.context_info_state = gpu::kCollectInfoFatalFailure;
-#if defined(OS_WIN)
-  gpu_info_.dx_diagnostics_info_state = gpu::kCollectInfoFatalFailure;
-#endif
-  complete_gpu_info_already_requested_ = true;
-  // Some observers might be waiting.
-  NotifyGpuInfoUpdate();
+  if (!card_disabled_) {
+    DisableHardwareAcceleration();
+    return;
+  }
+  if (!swiftshader_blocked_) {
+    BlockSwiftShader();
+    return;
+  }
+  // If GPU process fails to launch with hardware GPU, and then fails
+  // to launch with SwiftShader if available, then GPU process should
+  // not launch again.
+  // TODO(zmo): In viz mode, we will have a GPU process no matter what.
+  NOTREACHED();
 }
 
 }  // namespace content

content/browser/gpu/gpu_data_manager_impl_private.h

@@ -194,8 +194,6 @@ class CONTENT_EXPORT GpuDataManagerImplPrivate {
 
   GpuDataManagerImpl* owner_;
 
-  bool gpu_process_accessible_;
-
   // True if --single-process or --in-process-gpu is passed in.
   bool in_process_gpu_;
 

content/renderer/renderer_blink_platform_impl.cc

@@ -1052,25 +1052,14 @@ static void Collect3DContextInformation(
   DCHECK(gl_info);
   gl_info->vendor_id = gpu_info.gpu.vendor_id;
   gl_info->device_id = gpu_info.gpu.device_id;
-  switch (gpu_info.context_info_state) {
-    case gpu::kCollectInfoSuccess:
-    case gpu::kCollectInfoNonFatalFailure:
-      gl_info->renderer_info = WebString::FromUTF8(gpu_info.gl_renderer);
-      gl_info->vendor_info = WebString::FromUTF8(gpu_info.gl_vendor);
-      gl_info->driver_version = WebString::FromUTF8(gpu_info.driver_version);
-      gl_info->reset_notification_strategy =
-          gpu_info.gl_reset_notification_strategy;
-      gl_info->sandboxed = gpu_info.sandboxed;
-      gl_info->amd_switchable = gpu_info.amd_switchable;
-      gl_info->optimus = gpu_info.optimus;
-      break;
-    case gpu::kCollectInfoFatalFailure:
-    case gpu::kCollectInfoNone:
-      gl_info->error_message = WebString::FromUTF8(
-          "Failed to collect gpu information, GLSurface or GLContext "
-          "creation failed");
-      break;
-  }
+  gl_info->renderer_info = WebString::FromUTF8(gpu_info.gl_renderer);
+  gl_info->vendor_info = WebString::FromUTF8(gpu_info.gl_vendor);
+  gl_info->driver_version = WebString::FromUTF8(gpu_info.driver_version);
+  gl_info->reset_notification_strategy =
+      gpu_info.gl_reset_notification_strategy;
+  gl_info->sandboxed = gpu_info.sandboxed;
+  gl_info->amd_switchable = gpu_info.amd_switchable;
+  gl_info->optimus = gpu_info.optimus;
 }
 
 std::unique_ptr<blink::WebGraphicsContext3DProvider>

gpu/config/gpu_info.cc

@@ -75,11 +75,6 @@ GPUInfo::GPUInfo()
       sandboxed(false),
       in_process_gpu(true),
       passthrough_cmd_decoder(false),
-      basic_info_state(kCollectInfoNone),
-      context_info_state(kCollectInfoNone),
-#if defined(OS_WIN)
-      dx_diagnostics_info_state(kCollectInfoNone),
-#endif
       jpeg_decode_accelerator_supported(false)
 #if defined(USE_X11)
       ,
@@ -135,10 +130,7 @@ void GPUInfo::EnumerateFields(Enumerator* enumerator) const {
     bool direct_composition;
     bool supports_overlays;
     bool can_support_threaded_texture_mailbox;
-    CollectInfoResult basic_info_state;
-    CollectInfoResult context_info_state;
 #if defined(OS_WIN)
-    CollectInfoResult dx_diagnostics_info_state;
     DxDiagNode dx_diagnostics;
 #endif
     VideoDecodeAcceleratorCapabilities video_decode_accelerator_capabilities;
@@ -196,11 +188,6 @@ void GPUInfo::EnumerateFields(Enumerator* enumerator) const {
   enumerator->AddBool("supportsOverlays", supports_overlays);
   enumerator->AddBool("canSupportThreadedTextureMailbox",
                       can_support_threaded_texture_mailbox);
-  enumerator->AddInt("basicInfoState", basic_info_state);
-  enumerator->AddInt("contextInfoState", context_info_state);
-#if defined(OS_WIN)
-  enumerator->AddInt("DxDiagnosticsInfoState", dx_diagnostics_info_state);
-#endif
   // TODO(kbr): add dx_diagnostics on Windows.
   enumerator->AddInt("videoDecodeAcceleratorFlags",
                      video_decode_accelerator_capabilities.flags);

gpu/config/gpu_info.h

@@ -26,19 +26,6 @@ typedef unsigned long VisualID;
 
 namespace gpu {
 
-// Result for the various Collect*Info* functions below.
-// Fatal failures are for cases where we can't create a context at all or
-// something, making the use of the GPU impossible.
-// Non-fatal failures are for cases where we could gather most info, but maybe
-// some is missing (e.g. unable to parse a version string or to detect the exact
-// model).
-enum CollectInfoResult {
-  kCollectInfoNone = 0,
-  kCollectInfoSuccess = 1,
-  kCollectInfoNonFatalFailure = 2,
-  kCollectInfoFatalFailure = 3
-};
-
 // Video profile.  This *must* match media::VideoCodecProfile.
 enum VideoCodecProfile {
   VIDEO_CODEC_PROFILE_UNKNOWN = -1,
@@ -230,13 +217,7 @@ struct GPU_EXPORT GPUInfo {
   // is only implemented on Android.
   bool can_support_threaded_texture_mailbox = false;
 
-  // The state of whether the basic/context/DxDiagnostics info is collected and
-  // if the collection fails or not.
-  CollectInfoResult basic_info_state;
-  CollectInfoResult context_info_state;
 #if defined(OS_WIN)
-  CollectInfoResult dx_diagnostics_info_state;
-
   // The information returned by the DirectX Diagnostics Tool.
   DxDiagNode dx_diagnostics;
 #endif

gpu/config/gpu_info_collector.cc

@@ -112,9 +112,8 @@ int StringContainsName(
 
 namespace gpu {
 
-CollectInfoResult CollectBasicGraphicsInfo(
-    const base::CommandLine* command_line,
-    GPUInfo* gpu_info) {
+bool CollectBasicGraphicsInfo(const base::CommandLine* command_line,
+                              GPUInfo* gpu_info) {
   const char* software_gl_impl_name =
       gl::GetGLImplementationName(gl::GetSoftwareGLImplementation());
   if ((command_line->GetSwitchValueASCII(switches::kUseGL) ==
@@ -133,27 +132,26 @@ CollectInfoResult CollectBasicGraphicsInfo(
     // blacklist rules.
     gpu_info->driver_vendor = software_gl_impl_name;
 
-    gpu_info->basic_info_state = gpu::kCollectInfoSuccess;
-    return kCollectInfoSuccess;
+    return true;
   }
 
   return CollectBasicGraphicsInfo(gpu_info);
 }
 
-CollectInfoResult CollectGraphicsInfoGL(GPUInfo* gpu_info) {
+bool CollectGraphicsInfoGL(GPUInfo* gpu_info) {
   TRACE_EVENT0("startup", "gpu_info_collector::CollectGraphicsInfoGL");
   DCHECK_NE(gl::GetGLImplementation(), gl::kGLImplementationNone);
 
   scoped_refptr<gl::GLSurface> surface(InitializeGLSurface());
   if (!surface.get()) {
     LOG(ERROR) << "Could not create surface for info collection.";
-    return kCollectInfoFatalFailure;
+    return false;
   }
 
   scoped_refptr<gl::GLContext> context(InitializeGLContext(surface.get()));
   if (!context.get()) {
     LOG(ERROR) << "Could not create context for info collection.";
-    return kCollectInfoFatalFailure;
+    return false;
   }
 
   gpu_info->gl_renderer = GetGLString(GL_RENDERER);
@@ -222,7 +220,8 @@ CollectInfoResult CollectGraphicsInfoGL(GPUInfo* gpu_info) {
   gpu_info->vertex_shader_version = glsl_version;
 
   IdentifyActiveGPU(gpu_info);
-  return CollectDriverInfoGL(gpu_info);
+  CollectDriverInfoGL(gpu_info);
+  return true;
 }
 
 void IdentifyActiveGPU(GPUInfo* gpu_info) {

gpu/config/gpu_info_collector.h

@@ -25,28 +25,27 @@ namespace gpu {
 // the danger of crashing), including vendor_id and device_id.
 // This is called at browser process startup time.
 // The subset each platform collects may be different.
-GPU_EXPORT CollectInfoResult CollectBasicGraphicsInfo(GPUInfo* gpu_info);
+GPU_EXPORT bool CollectBasicGraphicsInfo(GPUInfo* gpu_info);
 
 // Similar to above, except it handles the case where the software renderer of
 // the platform is used.
-GPU_EXPORT CollectInfoResult
-CollectBasicGraphicsInfo(const base::CommandLine* command_line,
-                         GPUInfo* gpu_info);
+GPU_EXPORT bool CollectBasicGraphicsInfo(const base::CommandLine* command_line,
+                                         GPUInfo* gpu_info);
 
 // Create a GL/DirectX context and collect related info.
 // This is called at GPU process startup time.
-GPU_EXPORT CollectInfoResult CollectContextGraphicsInfo(GPUInfo* gpu_info);
+GPU_EXPORT bool CollectContextGraphicsInfo(GPUInfo* gpu_info);
 
 #if defined(OS_WIN)
 // Collect the DirectX Disagnostics information about the attached displays.
 GPU_EXPORT bool GetDxDiagnostics(DxDiagNode* output);
 #endif  // OS_WIN
 
 // Create a GL context and collect GL strings and versions.
-GPU_EXPORT CollectInfoResult CollectGraphicsInfoGL(GPUInfo* gpu_info);
+GPU_EXPORT bool CollectGraphicsInfoGL(GPUInfo* gpu_info);
 
 // Each platform stores the driver version on the GL_VERSION string differently
-GPU_EXPORT CollectInfoResult CollectDriverInfoGL(GPUInfo* gpu_info);
+GPU_EXPORT void CollectDriverInfoGL(GPUInfo* gpu_info);
 
 // If more than one GPUs are identified, and GL strings are available,
 // identify the active GPU based on GL strings.

gpu/config/gpu_info_collector_android.cc

@@ -58,7 +58,7 @@ std::string GetDriverVersionFromString(const std::string& version_string) {
 
 namespace gpu {
 
-CollectInfoResult CollectContextGraphicsInfo(GPUInfo* gpu_info) {
+bool CollectContextGraphicsInfo(GPUInfo* gpu_info) {
   // When command buffer is compiled as a standalone library, the process might
   // not have a Java environment.
   if (base::android::IsVMInitialized()) {
@@ -67,23 +67,19 @@ CollectInfoResult CollectContextGraphicsInfo(GPUInfo* gpu_info) {
   }
 
   // At this point GL bindings have been initialized already.
-  CollectInfoResult result = CollectGraphicsInfoGL(gpu_info);
-  gpu_info->basic_info_state = result;
-  gpu_info->context_info_state = result;
-  return result;
+  return CollectGraphicsInfoGL(gpu_info);
 }
 
-CollectInfoResult CollectBasicGraphicsInfo(GPUInfo* gpu_info) {
+bool CollectBasicGraphicsInfo(GPUInfo* gpu_info) {
   NOTREACHED();
-  return kCollectInfoNone;
+  return false;
 }
 
-CollectInfoResult CollectDriverInfoGL(GPUInfo* gpu_info) {
+void CollectDriverInfoGL(GPUInfo* gpu_info) {
   gpu_info->driver_version = GetDriverVersionFromString(
       gpu_info->gl_version);
   gpu_info->gpu.vendor_string = gpu_info->gl_vendor;
   gpu_info->gpu.device_string = gpu_info->gl_renderer;
-  return kCollectInfoSuccess;
 }
 
 }  // namespace gpu

gpu/config/gpu_info_collector_fuchsia.cc

@@ -6,22 +6,21 @@
 
 namespace gpu {
 
-CollectInfoResult CollectContextGraphicsInfo(GPUInfo* gpu_info) {
+bool CollectContextGraphicsInfo(GPUInfo* gpu_info) {
   // TODO(crbug.com/707031): Implement this.
   NOTIMPLEMENTED();
-  return kCollectInfoFatalFailure;
+  return false;
 }
 
-CollectInfoResult CollectBasicGraphicsInfo(GPUInfo* gpu_info) {
+bool CollectBasicGraphicsInfo(GPUInfo* gpu_info) {
   // TODO(crbug.com/707031): Implement this.
   NOTIMPLEMENTED();
-  return kCollectInfoFatalFailure;
+  return false;
 }
 
-CollectInfoResult CollectDriverInfoGL(GPUInfo* gpu_info) {
+void CollectDriverInfoGL(GPUInfo* gpu_info) {
   // TODO(crbug.com/707031): Implement this.
   NOTIMPLEMENTED();
-  return kCollectInfoFatalFailure;
 }
 
 }  // namespace gpu