Commit f95901b3 authored by Brian Sheedy's avatar Brian Sheedy Committed by Commit Bot

Fix GPU stack symbolization on Android

Adds the necessary Chromium-side changes to fix stack symbolization for
non-browser crashes in GPU Telemetry tests on Android. Specifically:

1. Makes the GPU tests use GetStackTrace() on Android instead of
   LogSymbolizedUnsymbolizedMinidumps(), as this is currently the only
   way to get stack traces through Telemetry on Android.
2. Adds a --breakpad-dump-location argument to ChromeCrashReporterClient
   that is functionally equivalent to the existing
   BREAKPAD_DUMP_LOCATION environment variable, but necessary for
   Android since we can't set environment variables before launching
   Chrome on Android.
3. Moves the dump location override to ChromeCrashReporterClient::Create
   instead of whe retrieving the crash dump location in order to avoid
   issues with Android calling blocking functions in a scope that does
   not allow blocking functions.

Bug: 949321
Change-Id: I9be2446a73a2359157cd94c889bf34951b78c86b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825719
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Auto-Submit: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700407}
parent 844ff615
...@@ -45,6 +45,27 @@ ...@@ -45,6 +45,27 @@
void ChromeCrashReporterClient::Create() { void ChromeCrashReporterClient::Create() {
static base::NoDestructor<ChromeCrashReporterClient> crash_client; static base::NoDestructor<ChromeCrashReporterClient> crash_client;
crash_reporter::SetCrashReporterClient(crash_client.get()); crash_reporter::SetCrashReporterClient(crash_client.get());
// By setting the BREAKPAD_DUMP_LOCATION environment variable, an alternate
// location to write crash dumps can be set.
std::unique_ptr<base::Environment> env(base::Environment::Create());
std::string alternate_crash_dump_location;
base::FilePath crash_dumps_dir_path;
if (env->GetVar("BREAKPAD_DUMP_LOCATION", &alternate_crash_dump_location)) {
crash_dumps_dir_path =
base::FilePath::FromUTF8Unsafe(alternate_crash_dump_location);
} else if (base::CommandLine::ForCurrentProcess()->HasSwitch(
"breakpad-dump-location")) {
// This is needed for Android tests, where we want dumps to go to a location
// where they don't get uploaded/deleted, but we can't use environment
// variables.
crash_dumps_dir_path =
base::CommandLine::ForCurrentProcess()->GetSwitchValuePath(
"breakpad-dump-location");
}
if (!crash_dumps_dir_path.empty()) {
base::PathService::Override(chrome::DIR_CRASH_DUMPS, crash_dumps_dir_path);
}
} }
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
...@@ -115,15 +136,6 @@ base::FilePath ChromeCrashReporterClient::GetReporterLogFilename() { ...@@ -115,15 +136,6 @@ base::FilePath ChromeCrashReporterClient::GetReporterLogFilename() {
bool ChromeCrashReporterClient::GetCrashDumpLocation( bool ChromeCrashReporterClient::GetCrashDumpLocation(
base::FilePath* crash_dir) { base::FilePath* crash_dir) {
// By setting the BREAKPAD_DUMP_LOCATION environment variable, an alternate
// location to write breakpad crash dumps can be set.
std::unique_ptr<base::Environment> env(base::Environment::Create());
std::string alternate_crash_dump_location;
if (env->GetVar("BREAKPAD_DUMP_LOCATION", &alternate_crash_dump_location)) {
base::FilePath crash_dumps_dir_path =
base::FilePath::FromUTF8Unsafe(alternate_crash_dump_location);
base::PathService::Override(chrome::DIR_CRASH_DUMPS, crash_dumps_dir_path);
}
return base::PathService::Get(chrome::DIR_CRASH_DUMPS, crash_dir); return base::PathService::Get(chrome::DIR_CRASH_DUMPS, crash_dir);
} }
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
import logging import logging
import re import re
from telemetry.core import android_platform
from telemetry.testing import serially_executed_browser_test_case from telemetry.testing import serially_executed_browser_test_case
from telemetry.util import screenshot from telemetry.util import screenshot
from typ import json_results from typ import json_results
...@@ -198,7 +199,14 @@ class GpuIntegrationTest( ...@@ -198,7 +199,14 @@ class GpuIntegrationTest(
# stacks could slow down the tests' running time unacceptably. # stacks could slow down the tests' running time unacceptably.
# We also don't do this if the browser failed to startup. # We also don't do this if the browser failed to startup.
if self.browser is not None: if self.browser is not None:
self.browser.LogSymbolizedUnsymbolizedMinidumps(logging.ERROR) # TODO(https://crbug.com/1008075): Remove this split once stack
# symbolization is standardized across all platforms.
if isinstance(self.browser.platform,
android_platform.AndroidPlatform):
_, output = self.browser.GetStackTrace()
logging.error(output)
else:
self.browser.LogSymbolizedUnsymbolizedMinidumps(logging.ERROR)
# This failure might have been caused by a browser or renderer # This failure might have been caused by a browser or renderer
# crash, so restart the browser to make sure any state doesn't # crash, so restart the browser to make sure any state doesn't
# propagate to the next test iteration. # propagate to the next test iteration.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment