Respect Linux user prefs with regards to crash reporting.

Respect Linux user prefs with regards to crash reporting.
This involves implementing GoogleUpdateSettings::[GS]etCollectStatsConsent, and a whole lot of refactoring.

BUG=none
TEST=delete config dir, run official Linux build, don't enable crash reporting, crash browser -> no crash reporting.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=17104

[Evan Martin, 2009-05-27 00:53:08]

http://codereview.chromium.org/115808/diff/1/2
File chrome/browser/renderer_host/browser_render_process_host.cc (right):

http://codereview.chromium.org/115808/diff/1/2#newcode308
Line 308: // switches::kDisableBreakpad to the renderer.
This makes me a bit nervous in how it's different from Windows.  How does
GoogleUpdateSettings::GetCollectStatsConsetn() normally get plumbed inot the
renderer?

[Lei Zhang, 2009-05-27 01:02:22]

On 2009/05/27 00:53:08, Evan Martin wrote:
> http://codereview.chromium.org/115808/diff/1/2
> File chrome/browser/renderer_host/browser_render_process_host.cc (right):
> 
> http://codereview.chromium.org/115808/diff/1/2#newcode308
> Line 308: // switches::kDisableBreakpad to the renderer.
> This makes me a bit nervous in how it's different from Windows.  How does
> GoogleUpdateSettings::GetCollectStatsConsetn() normally get plumbed inot the
> renderer?

It happens way earlier in wWinMain() -> InitCrashReporterWithDllPath() ->
InitCrashReporterThread()

[Lei Zhang, 2009-05-27 01:38:26]

I will refactor the Linux crash handling code to be more like the Win/Mac code.
Look for a new patch set later tonight/tomorrow morning.

[Lei Zhang, 2009-05-27 05:54:42]

Here comes a much bigger patch set 2. I still need to add a unit test for
google_update_settings_linux.cc.

[Evan Martin, 2009-05-27 05:57:09]

+agl for the crasher bits (I don't know if it's kosher to e.g. use std::string
for the renderer url)

[Evan Martin, 2009-05-27 06:00:15]

it looks like you moved some files?  you should use "svn mv" for those..

http://codereview.chromium.org/115808/diff/9/16
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/115808/diff/9/16#newcode331
Line 331: #if defined(OS_POSIX)
Unless you intended to change when this happens, the previous code was correct
-- Linux shows the first run at the same time Windows does, while Mac has this
extra hacked-in bit.

[Lei Zhang, 2009-05-27 06:05:17]

I moved the contents of chrome/renderer/render_crash_handler_linux.* into
chrome/app/breakpad_linux.*.

So it was more of an append than a mv.

http://codereview.chromium.org/115808/diff/9/16
File chrome/browser/browser_main.cc (right):

http://codereview.chromium.org/115808/diff/9/16#newcode331
Line 331: #if defined(OS_POSIX)
On 2009/05/27 06:00:15, Evan Martin wrote:
> Unless you intended to change when this happens, the previous code was correct
> -- Linux shows the first run at the same time Windows does, while Mac has this
> extra hacked-in bit.

I did it for the same reason. See the !OS_POSIX comment below. This is because I
removed setting kMetricsReportingEnabled in first_run_gtk.cc and do it below at
line 356. Should I revert this bit and put 
kMetricsReportingEnabled back in first_run_gtk.cc?

[Lei Zhang, 2009-05-28 08:45:22]

ping.

[Evan Martin, 2009-05-28 18:47:26]

http://codereview.chromium.org/115808/diff/1029/1042
File chrome/browser/google_update_settings_linux.cc (right):

http://codereview.chromium.org/115808/diff/1029/1042#newcode12
Line 12: static const char kConsentToSendStats[] = "Consent To Send Stats";
How does this work on Windows?  We should be using the prefs for this, I think.

[Lei Zhang, 2009-05-28 18:56:41]

On 2009/05/28 18:47:26, Evan Martin wrote:
> http://codereview.chromium.org/115808/diff/1029/1042
> File chrome/browser/google_update_settings_linux.cc (right):
> 
> http://codereview.chromium.org/115808/diff/1029/1042#newcode12
> Line 12: static const char kConsentToSendStats[] = "Consent To Send Stats";
> How does this work on Windows?  We should be using the prefs for this, I
think.

Windows use a registry key for this - something outside of the prefs.
See GoogleUpdateSettings::GetCollectStatsConsent in
chrome/installer/util/google_update_settings.cc. Mac uses an OS prefs facility
that provides each app with a way to get/set prefs on disk.

[Evan Martin, 2009-05-28 18:59:21]

LGTM

http://codereview.chromium.org/115808/diff/1029/1042
File chrome/browser/google_update_settings_linux.cc (right):

http://codereview.chromium.org/115808/diff/1029/1042#newcode12
Line 12: static const char kConsentToSendStats[] = "Consent To Send Stats";
Can you doc this?  e.g. "file name used in the user data dir"

http://codereview.chromium.org/115808/diff/1029/1042#newcode31
Line 31: return 0 == file_util::WriteFile(consent_file, "", 0);
rest of the code does tests the other way around *shrug*