Skip to content

Commit

Permalink
Force Chrome to crash when GPU process crashes in testing.
Browse files Browse the repository at this point in the history
This is because Chrome could end up running with SwiftShader silently
and we fail to capture a serious bug that causes GPU process to crash.

In this CL, we only add the switch to maps integration test.

I plan to expand it to other test suites, for example, pixel tests,
webgl conformance tests, and maybe perf tests.

TEST=gpu bots
R=kbr@chromium.org

Bug: 1170819
Change-Id: I12be3516a64dcc5870cfa802248405ee6ffd5de4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2657841
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#848349}
  • Loading branch information
zhenyao authored and Chromium LUCI CQ committed Jan 29, 2021
1 parent 1d58ac9 commit f402401
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 2 deletions.
13 changes: 13 additions & 0 deletions content/browser/gpu/gpu_process_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,7 @@ GpuProcessHost::~GpuProcessHost() {
message = "The info collection GPU process ";
}

bool unexpected_exit = false;
switch (info.status) {
case base::TERMINATION_STATUS_NORMAL_TERMINATION:
// Don't block offscreen contexts (and force page reload for webgl)
Expand All @@ -789,6 +790,7 @@ GpuProcessHost::~GpuProcessHost() {
break;
case base::TERMINATION_STATUS_ABNORMAL_TERMINATION:
message += base::StringPrintf("exited with code %d.", info.exit_code);
unexpected_exit = true;
break;
case base::TERMINATION_STATUS_PROCESS_WAS_KILLED:
UMA_HISTOGRAM_ENUMERATION("GPU.GPUProcessTerminationOrigin",
Expand All @@ -798,35 +800,46 @@ GpuProcessHost::~GpuProcessHost() {
break;
case base::TERMINATION_STATUS_PROCESS_CRASHED:
message += "crashed!";
unexpected_exit = true;
break;
case base::TERMINATION_STATUS_STILL_RUNNING:
message += "hasn't exited yet.";
break;
#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
case base::TERMINATION_STATUS_PROCESS_WAS_KILLED_BY_OOM:
message += "was killed due to out of memory.";
unexpected_exit = true;
break;
#endif // BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
#if defined(OS_ANDROID)
case base::TERMINATION_STATUS_OOM_PROTECTED:
message += "was protected from out of memory kill.";
unexpected_exit = true;
break;
#endif // OS_ANDROID
case base::TERMINATION_STATUS_LAUNCH_FAILED:
message += "failed to start!";
unexpected_exit = true;
break;
case base::TERMINATION_STATUS_OOM:
message += "died due to out of memory.";
unexpected_exit = true;
break;
#if defined(OS_WIN)
case base::TERMINATION_STATUS_INTEGRITY_FAILURE:
message += "failed integrity checks.";
unexpected_exit = true;
break;
#endif
case base::TERMINATION_STATUS_MAX_ENUM:
NOTREACHED();
break;
}
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kForceBrowserCrashOnGpuCrash)) {
CHECK(!unexpected_exit)
<< "Force Chrome to crash due to unexpected GPU process crash";
}
GetUIThreadTaskRunner({})->PostTask(
FROM_HERE,
base::BindOnce(&OnGpuProcessHostDestroyedOnUI, host_id_, message));
Expand Down
3 changes: 2 additions & 1 deletion content/test/gpu/gpu_tests/common_browser_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@
ENABLE_EXPERIMENTAL_WEB_PLATFORM_FEATURES =\
'--enable-experimental-web-platform-features'
ENABLE_GPU_BENCHMARKING = '--enable-gpu-benchmarking'
ENABLE_GPU_RASTERIZATION = '--enable-gpu-rasterization'
ENABLE_LOGGING = '--enable-logging'
ENSURE_FORCED_COLOR_PROFILE = '--ensure-forced-color-profile'
FORCE_BROWSER_CRASH_ON_GPU_CRASH = '--force-browser-crash-on-gpu-crash'
FORCE_COLOR_PROFILE_SRGB = '--force-color-profile=srgb'
ENABLE_GPU_RASTERIZATION = '--enable-gpu-rasterization'
TEST_TYPE_GPU = '--test-type=gpu'
3 changes: 2 additions & 1 deletion content/test/gpu/gpu_tests/maps_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ def SetUpProcess(cls):
options.dont_restore_color_profile_after_test)
super(MapsIntegrationTest, cls).SetUpProcess()
cls.CustomizeBrowserArgs([
cba.FORCE_COLOR_PROFILE_SRGB,
cba.ENSURE_FORCED_COLOR_PROFILE,
cba.FORCE_BROWSER_CRASH_ON_GPU_CRASH,
cba.FORCE_COLOR_PROFILE_SRGB,
])
cloud_storage.GetIfChanged(
os.path.join(_MAPS_PERF_TEST_PATH, 'load_dataset'),
Expand Down
4 changes: 4 additions & 0 deletions gpu/config/gpu_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,8 @@ const char kVulkanHeapMemoryLimitMb[] = "vulkan-heap-memory-limit-mb";
// TODO(crbug/1158000): Remove this switch.
const char kVulkanSyncCpuMemoryLimitMb[] = "vulkan-sync-cpu-memory-limit-mb";

// Crash Chrome if GPU process crashes. This is to force a test to fail when
// GPU process crashes unexpectedly.
const char kForceBrowserCrashOnGpuCrash[] = "force-browser-crash-on-gpu-crash";

} // namespace switches
1 change: 1 addition & 0 deletions gpu/config/gpu_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ GPU_EXPORT extern const char kEnforceVulkanProtectedMemory[];
GPU_EXPORT extern const char kDisableVulkanFallbackToGLForTesting[];
GPU_EXPORT extern const char kVulkanHeapMemoryLimitMb[];
GPU_EXPORT extern const char kVulkanSyncCpuMemoryLimitMb[];
GPU_EXPORT extern const char kForceBrowserCrashOnGpuCrash[];

} // namespace switches

Expand Down

0 comments on commit f402401

Please sign in to comment.