Skip to content

GitLab

T
tangled
Commit 2251b8da authored by David's avatar David Committed by Commit Bot
Browse files
Options

Add app_locale to js crash reports for crashReportPrivate. 

Doc: http://go/backlight-better-crash-report#heading=h.148xm9y8bm2q

Note: Choosing to make this an optional field in JSErrorReport
in case future non crashReportClients don't have access to the
browser process.

Bug: b/169635499, b/167109810

Change-Id: I3d351fe2cc797fecb45b03e3a359db1be66fda20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2483945Reviewed-by: default avatarGiovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: default avatarPatti <patricialor@chromium.org>
Reviewed-by: default avatarIan Barkley-Yeung <iby@chromium.org>
Commit-Queue: David Lei <dlei@google.com>
Cr-Commit-Position: refs/heads/master@{#819255}
parent 5ce0fc03
Hide whitespace changes
Inline Side-by-side
Showing with 22 additions and 13 deletions
chrome/browser/extensions/api/crash_report_private/crash_report_private_api.cc
View file @ 2251b8da
......@@ -5,6 +5,7 @@
#include "chrome/browser/extensions/api/crash_report_private/crash_report_private_api.h"
#include "base/time/default_clock.h"
#include "chrome/browser/browser_process.h"
#include "components/crash/content/browser/error_reporting/javascript_error_report.h"
#include "components/crash/content/browser/error_reporting/send_javascript_error_report.h"
......@@ -64,6 +65,8 @@ ExtensionFunction::ResponseAction CrashReportPrivateReportErrorFunction::Run() {
error_report.stack_trace = std::move(*params->info.stack_trace);
}
error_report.app_locale = g_browser_process->GetApplicationLocale();
SendJavaScriptErrorReport(
std::move(error_report),
base::BindOnce(&CrashReportPrivateReportErrorFunction::OnReportComplete,
......
chrome/browser/extensions/api/crash_report_private/crash_report_private_apitest.cc
View file @ 2251b8da
......@@ -101,7 +101,8 @@ IN_PROC_BROWSER_TEST_F(CrashReportPrivateApiTest, Basic) {
ASSERT_TRUE(report);
EXPECT_THAT(
report->query,
MatchesRegex("browser=Chrome&browser_process_uptime_ms=\\d+&browser_"
MatchesRegex("app_locale=en-US&browser=Chrome&browser_process_uptime_ms="
"\\d+&browser_"
"version=1.2.3.4&channel=Stable&"
"error_message=hi&full_url=http%3A%2F%2Fwww.test.com%2F&"
"os=ChromeOS&os_version=" +
......@@ -132,7 +133,8 @@ IN_PROC_BROWSER_TEST_F(CrashReportPrivateApiTest, ExtraParamsAndStackTrace) {
// "Chrome%20(Chrome%20OS)" and then the second escapes the '%' into '%25'.
EXPECT_THAT(
report->query,
MatchesRegex("browser=Chrome&browser_process_uptime_ms=\\d+&browser_"
MatchesRegex("app_locale=en-US&browser=Chrome&browser_process_uptime_ms="
"\\d+&browser_"
"version=1.2.3.4&channel=Stable&column=456&"
"error_message=hi&full_url=http%3A%2F%2Fwww.test.com%2Ffoo"
"&line=123&os=ChromeOS&os_version=" +
......@@ -162,14 +164,14 @@ IN_PROC_BROWSER_TEST_F(CrashReportPrivateApiTest, StackTraceWithErrorMessage) {
ASSERT_TRUE(report);
EXPECT_THAT(
report->query,
MatchesRegex(
"browser=Chrome&browser_process_uptime_ms=\\d+&browser_version=1.2."
"3.4&channel=Stable&column=456&"
"error_message=hi&full_url=http%3A%2F%2Fwww.test.com%2Ffoo&"
"line=123&os=ChromeOS&os_version=" +
GetOsVersion() +
"&prod=TestApp&src=http%3A%2F%2Fwww.test.com%2Ffoo&type="
"JavascriptError&url=%2Ffoo&ver=1.0.0.0"));
MatchesRegex("app_locale=en-US&browser=Chrome&browser_process_uptime_ms="
"\\d+&browser_version=1.2."
"3.4&channel=Stable&column=456&"
"error_message=hi&full_url=http%3A%2F%2Fwww.test.com%2Ffoo&"
"line=123&os=ChromeOS&os_version=" +
GetOsVersion() +
"&prod=TestApp&src=http%3A%2F%2Fwww.test.com%2Ffoo&type="
"JavascriptError&url=%2Ffoo&ver=1.0.0.0"));
EXPECT_EQ(report->content, "");
}
......@@ -194,7 +196,8 @@ IN_PROC_BROWSER_TEST_F(CrashReportPrivateApiTest, RedactMessage) {
EXPECT_THAT(
report->query,
MatchesRegex(
"browser=Chrome&browser_process_uptime_ms=\\d+&browser_version=1.2."
"app_locale=en-US&browser=Chrome&browser_process_uptime_ms=\\d+&"
"browser_version=1.2."
"3.4&channel=Stable&column=456&"
"error_message=%5BMAC%20OUI%3D06%3A00%3A00%20IFACE%3D1%5D&"
"full_url=http%3A%2F%2Fwww.test.com%2Ffoo&line=123&os=ChromeOS&"
......
components/crash/content/browser/error_reporting/javascript_error_report.h
View file @ 2251b8da
......@@ -44,6 +44,9 @@ struct JavaScriptErrorReport {
// String containing the stack trace for the error. Not sent if not present.
base::Optional<std::string> stack_trace;
// String containing the application locale. Not sent if not present.
base::Optional<std::string> app_locale;
};
#endif // COMPONENTS_CRASH_CONTENT_BROWSER_ERROR_REPORTING_JAVASCRIPT_ERROR_REPORT_H_
components/crash/content/browser/error_reporting/send_javascript_error_report.cc
View file @ 2251b8da
......@@ -283,12 +283,12 @@ void OnConsentCheckCompleted(
params["line"] = base::NumberToString(*error_report->line_number);
if (error_report->column_number)
params["column"] = base::NumberToString(*error_report->column_number);
// TODO(crbug/1121816): Chrome crashes have "Process uptime" and "Process
// type" fields, eventually consider using that for process uptime.
params["browser_process_uptime_ms"] =
base::NumberToString(browser_process_uptime.InMilliseconds());
if (error_report->app_locale)
params["app_locale"] = std::move(*error_report->app_locale);
const GURL url(base::StrCat(
{crash_endpoint_string, "?", BuildPostRequestQueryString(params)}));
std::string body;
......
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