Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(205)

Side by Side Diff: chrome/app/chrome_crash_reporter_client_win.cc

Issue 2031833002: Remove FilePath usage from the CrashReporterClient interface on Windows. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix build error Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright 2013 The Chromium Authors. All rights reserved. 1 // Copyright 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 // TODO(ananta/scottmg) 5 // TODO(ananta/scottmg)
6 // Add test coverage for Crashpad. 6 // Add test coverage for Crashpad.
7 #include "chrome/app/chrome_crash_reporter_client_win.h" 7 #include "chrome/app/chrome_crash_reporter_client_win.h"
8 8
9 #include <assert.h>
9 #include <windows.h> 10 #include <windows.h>
10 11
11 #include <memory> 12 #include <memory>
12 #include <string> 13 #include <string>
13 14
14 #include "base/command_line.h"
15 #include "base/files/file_path.h"
16 #include "base/logging.h"
17 #include "base/path_service.h"
18 #include "base/strings/string_split.h"
19 #include "base/strings/utf_string_conversions.h"
20 #include "base/version.h"
21 #include "build/build_config.h" 15 #include "build/build_config.h"
22 #include "chrome/common/chrome_result_codes.h" 16 #include "chrome/common/chrome_result_codes.h"
23 #include "chrome/common/crash_keys.h" 17 #include "chrome/common/crash_keys.h"
24 #include "chrome/common/env_vars.h"
25 #include "chrome/install_static/install_util.h" 18 #include "chrome/install_static/install_util.h"
26 #include "content/public/common/content_switches.h"
27 19
28 namespace { 20 namespace {
29 21
30 // This is the minimum version of google update that is required for deferred 22 // This is the minimum version of google update that is required for deferred
31 // crash uploads to work. 23 // crash uploads to work.
32 const char kMinUpdateVersion[] = "1.3.21.115"; 24 const char kMinUpdateVersion[] = "1.3.21.115";
33 25
34 } // namespace 26 } // namespace
35 27
36 ChromeCrashReporterClient::ChromeCrashReporterClient() {} 28 ChromeCrashReporterClient::ChromeCrashReporterClient() {}
37 29
38 ChromeCrashReporterClient::~ChromeCrashReporterClient() {} 30 ChromeCrashReporterClient::~ChromeCrashReporterClient() {}
39 31
40 bool ChromeCrashReporterClient::GetAlternativeCrashDumpLocation( 32 bool ChromeCrashReporterClient::GetAlternativeCrashDumpLocation(
41 base::FilePath* crash_dir) { 33 base::string16* crash_dir) {
42 // By setting the BREAKPAD_DUMP_LOCATION environment variable, an alternate 34 // By setting the BREAKPAD_DUMP_LOCATION environment variable, an alternate
43 // location to write breakpad crash dumps can be set. 35 // location to write breakpad crash dumps can be set.
44 std::string alternate_crash_dump_location = 36 *crash_dir =
45 install_static::GetEnvironmentString("BREAKPAD_DUMP_LOCATION"); 37 install_static::GetEnvironmentString16(L"BREAKPAD_DUMP_LOCATION");
46 if (!alternate_crash_dump_location.empty()) { 38 return !!crash_dir->empty();
grt (UTC plus 2) 2016/06/02 22:57:44 I think you have one too many !s here, no?
ananta 2016/06/02 23:10:30 Yes. Thanks for catching. Was not thinking when I
47 *crash_dir = base::FilePath::FromUTF8Unsafe(alternate_crash_dump_location);
48 return true;
49 }
50 return false;
51 } 39 }
52 40
53 void ChromeCrashReporterClient::GetProductNameAndVersion( 41 void ChromeCrashReporterClient::GetProductNameAndVersion(
54 const base::FilePath& exe_path, 42 const base::string16& exe_path,
55 base::string16* product_name, 43 base::string16* product_name,
56 base::string16* version, 44 base::string16* version,
57 base::string16* special_build, 45 base::string16* special_build,
58 base::string16* channel_name) { 46 base::string16* channel_name) {
59 DCHECK(product_name); 47 assert(product_name);
60 DCHECK(version); 48 assert(version);
61 DCHECK(special_build); 49 assert(special_build);
62 DCHECK(channel_name); 50 assert(channel_name);
63 51
64 install_static::GetExecutableVersionDetails( 52 install_static::GetExecutableVersionDetails(
65 exe_path.value(), product_name, version, special_build, channel_name); 53 exe_path, product_name, version, special_build, channel_name);
66 } 54 }
67 55
68 bool ChromeCrashReporterClient::ShouldShowRestartDialog(base::string16* title, 56 bool ChromeCrashReporterClient::ShouldShowRestartDialog(base::string16* title,
69 base::string16* message, 57 base::string16* message,
70 bool* is_rtl_locale) { 58 bool* is_rtl_locale) {
71 if (!install_static::HasEnvironmentVariable(env_vars::kShowRestart) || 59 if (!install_static::HasEnvironmentVariable16(
72 !install_static::HasEnvironmentVariable(env_vars::kRestartInfo) || 60 install_static::kShowRestart) ||
73 install_static::HasEnvironmentVariable(env_vars::kMetroConnected)) { 61 !install_static::HasEnvironmentVariable16(
62 install_static::kRestartInfo)) {
74 return false; 63 return false;
75 } 64 }
76 65
77 std::string restart_info = 66 base::string16 restart_info =
78 install_static::GetEnvironmentString(env_vars::kRestartInfo); 67 install_static::GetEnvironmentString16(install_static::kRestartInfo);
79 68
80 // The CHROME_RESTART var contains the dialog strings separated by '|'. 69 // The CHROME_RESTART var contains the dialog strings separated by '|'.
81 // See ChromeBrowserMainPartsWin::PrepareRestartOnCrashEnviroment() 70 // See ChromeBrowserMainPartsWin::PrepareRestartOnCrashEnviroment()
82 // for details. 71 // for details.
83 std::vector<std::string> dlg_strings = base::SplitString( 72 std::vector<base::string16> dlg_strings = install_static::TokenizeString16(
84 restart_info, "|", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL); 73 restart_info, L'|', true); // true = Trim shitespace.
scottmg 2016/06/02 22:01:49 "whitespace" (two spaces before // too)
grt (UTC plus 2) 2016/06/02 22:57:44 I totally prefer the original comment.
ananta 2016/06/02 23:10:30 Point taken in jest
ananta 2016/06/02 23:10:30 Done.
85 74
86 if (dlg_strings.size() < 3) 75 if (dlg_strings.size() < 3)
87 return false; 76 return false;
88 77
89 *title = base::UTF8ToUTF16(dlg_strings[0]); 78 *title = dlg_strings[0];
90 *message = base::UTF8ToUTF16(dlg_strings[1]); 79 *message = dlg_strings[1];
91 *is_rtl_locale = dlg_strings[2] == env_vars::kRtlLocale; 80 *is_rtl_locale = dlg_strings[2] == install_static::kRtlLocale;
92 return true; 81 return true;
93 } 82 }
94 83
95 bool ChromeCrashReporterClient::AboutToRestart() { 84 bool ChromeCrashReporterClient::AboutToRestart() {
96 if (!install_static::HasEnvironmentVariable(env_vars::kRestartInfo)) 85 if (!install_static::HasEnvironmentVariable16(install_static::kRestartInfo))
97 return false; 86 return false;
98 87
99 install_static::SetEnvironmentString(env_vars::kShowRestart, "1"); 88 install_static::SetEnvironmentString16(install_static::kShowRestart, L"1");
100 return true; 89 return true;
101 } 90 }
102 91
103 bool ChromeCrashReporterClient::GetDeferredUploadsSupported( 92 bool ChromeCrashReporterClient::GetDeferredUploadsSupported(
104 bool is_per_user_install) { 93 bool is_per_user_install) {
105 Version update_version(install_static::GetGoogleUpdateVersion()); 94 return false;
106 return update_version.IsValid() &&
107 update_version >= base::Version(kMinUpdateVersion);
108 } 95 }
109 96
110 bool ChromeCrashReporterClient::GetIsPerUserInstall( 97 bool ChromeCrashReporterClient::GetIsPerUserInstall(
111 const base::FilePath& exe_path) { 98 const base::string16& exe_path) {
112 return !install_static::IsSystemInstall(exe_path.value().c_str()); 99 return !install_static::IsSystemInstall(exe_path.c_str());
113 } 100 }
114 101
115 bool ChromeCrashReporterClient::GetShouldDumpLargerDumps( 102 bool ChromeCrashReporterClient::GetShouldDumpLargerDumps(
116 bool is_per_user_install) { 103 bool is_per_user_install) {
117 base::string16 channel_name; 104 base::string16 channel_name;
118 install_static::GetChromeChannelName(is_per_user_install, 105 install_static::GetChromeChannelName(is_per_user_install,
119 false, // !add_modifier 106 false, // !add_modifier
120 &channel_name); 107 &channel_name);
121 // Capture more detail in crash dumps for Beta, Dev, Canary channels and 108 // Capture more detail in crash dumps for Beta, Dev, Canary channels and
122 // if channel is unknown (e.g. Chromium or developer builds). 109 // if channel is unknown (e.g. Chromium or developer builds).
(...skipping 13 matching lines...) Expand all
136 // reporter. 123 // reporter.
137 // Since the configuration management infrastructure is not initialized at 124 // Since the configuration management infrastructure is not initialized at
138 // this point, we read the corresponding registry key directly. The return 125 // this point, we read the corresponding registry key directly. The return
139 // status indicates whether policy data was successfully read. If it is true, 126 // status indicates whether policy data was successfully read. If it is true,
140 // |breakpad_enabled| contains the value set by policy. 127 // |breakpad_enabled| contains the value set by policy.
141 return install_static::ReportingIsEnforcedByPolicy(crashpad_enabled); 128 return install_static::ReportingIsEnforcedByPolicy(crashpad_enabled);
142 } 129 }
143 130
144 131
145 bool ChromeCrashReporterClient::GetCrashDumpLocation( 132 bool ChromeCrashReporterClient::GetCrashDumpLocation(
146 base::FilePath* crash_dir) { 133 base::string16* crash_dir) {
147 // By setting the BREAKPAD_DUMP_LOCATION environment variable, an alternate 134 // By setting the BREAKPAD_DUMP_LOCATION environment variable, an alternate
148 // location to write breakpad crash dumps can be set. 135 // location to write breakpad crash dumps can be set.
149 // If this environment variable exists, then for the time being, 136 // If this environment variable exists, then for the time being,
150 // short-circuit how it's handled on Windows. Honoring this 137 // short-circuit how it's handled on Windows. Honoring this
151 // variable is required in order to symbolize stack traces in 138 // variable is required in order to symbolize stack traces in
152 // Telemetry based tests: http://crbug.com/561763. 139 // Telemetry based tests: http://crbug.com/561763.
153 if (GetAlternativeCrashDumpLocation(crash_dir)) 140 if (GetAlternativeCrashDumpLocation(crash_dir))
154 return true; 141 return true;
155 142
156 // TODO(scottmg): Consider supporting --user-data-dir. See 143 // TODO(scottmg): Consider supporting --user-data-dir. See
157 // https://crbug.com/565446. 144 // https://crbug.com/565446.
158 base::string16 crash_dump_location; 145 return install_static::GetDefaultCrashDumpLocation(crash_dir);
159 if (!install_static::GetDefaultCrashDumpLocation(&crash_dump_location))
160 return false;
161 *crash_dir = base::FilePath(crash_dump_location);
162 return true;
163 } 146 }
164 147
165 size_t ChromeCrashReporterClient::RegisterCrashKeys() { 148 size_t ChromeCrashReporterClient::RegisterCrashKeys() {
166 return crash_keys::RegisterChromeCrashKeys(); 149 return crash_keys::RegisterChromeCrashKeys();
167 } 150 }
168 151
169 bool ChromeCrashReporterClient::IsRunningUnattended() { 152 bool ChromeCrashReporterClient::IsRunningUnattended() {
170 return install_static::HasEnvironmentVariable(env_vars::kHeadless); 153 return install_static::HasEnvironmentVariable16(install_static::kHeadless);
171 } 154 }
172 155
173 bool ChromeCrashReporterClient::GetCollectStatsConsent() { 156 bool ChromeCrashReporterClient::GetCollectStatsConsent() {
174 return install_static::GetCollectStatsConsent(); 157 return install_static::GetCollectStatsConsent();
175 } 158 }
176 159
177 bool ChromeCrashReporterClient::EnableBreakpadForProcess( 160 bool ChromeCrashReporterClient::EnableBreakpadForProcess(
178 const std::string& process_type) { 161 const std::string& process_type) {
179 return process_type == switches::kRendererProcess || 162 return process_type == install_static::kRendererProcess ||
180 process_type == switches::kPpapiPluginProcess || 163 process_type == install_static::kPpapiPluginProcess ||
181 process_type == switches::kZygoteProcess || 164 process_type == install_static::kGpuProcess;
182 process_type == switches::kGpuProcess;
183 } 165 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698