diff options
21 files changed, 116 insertions, 22 deletions
diff --git a/chrome/app/chrome_crash_reporter_client.cc b/chrome/app/chrome_crash_reporter_client.cc index 2f6f7a8..1c9570e 100644 --- a/chrome/app/chrome_crash_reporter_client.cc +++ b/chrome/app/chrome_crash_reporter_client.cc @@ -80,10 +80,12 @@ ChromeCrashReporterClient::ChromeCrashReporterClient() {} ChromeCrashReporterClient::~ChromeCrashReporterClient() {} +#if !defined(OS_MACOSX) void ChromeCrashReporterClient::SetCrashReporterClientIdFromGUID( const std::string& client_guid) { - crash_keys::SetCrashClientIdFromGUID(client_guid); + crash_keys::SetMetricsClientIdFromGUID(client_guid); } +#endif #if defined(OS_WIN) bool ChromeCrashReporterClient::GetAlternativeCrashDumpLocation( diff --git a/chrome/app/chrome_crash_reporter_client.h b/chrome/app/chrome_crash_reporter_client.h index a48f79e..80b069f 100644 --- a/chrome/app/chrome_crash_reporter_client.h +++ b/chrome/app/chrome_crash_reporter_client.h @@ -17,8 +17,10 @@ class ChromeCrashReporterClient : public crash_reporter::CrashReporterClient { ~ChromeCrashReporterClient() override; // crash_reporter::CrashReporterClient implementation. +#if !defined(OS_MACOSX) void SetCrashReporterClientIdFromGUID( const std::string& client_guid) override; +#endif #if defined(OS_WIN) virtual bool GetAlternativeCrashDumpLocation(base::FilePath* crash_dir) override; diff --git a/chrome/app/chrome_main_delegate.cc b/chrome/app/chrome_main_delegate.cc index 3b56ecf..e25ff4f 100644 --- a/chrome/app/chrome_main_delegate.cc +++ b/chrome/app/chrome_main_delegate.cc @@ -569,6 +569,13 @@ void ChromeMainDelegate::InitMacCrashReporter( // dylib is even loaded, to catch potential early crashes. crash_reporter::InitializeCrashpad(process_type); + const bool browser_process = process_type.empty(); + if (!browser_process) { + std::string metrics_client_id = + command_line.GetSwitchValueASCII(switches::kMetricsClientID); + crash_keys::SetMetricsClientIdFromGUID(metrics_client_id); + } + // Mac Chrome is packaged with a main app bundle and a helper app bundle. // The main app bundle should only be used for the browser process, so it // should never see a --type switch (switches::kProcessType). Likewise, diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index f3d767f..836c59e 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -1238,7 +1238,14 @@ bool IsAutoReloadVisibleOnlyEnabled() { void ChromeContentBrowserClient::AppendExtraCommandLineSwitches( base::CommandLine* command_line, int child_process_id) { -#if defined(OS_POSIX) && !defined(OS_MACOSX) +#if defined(OS_MACOSX) + scoped_ptr<metrics::ClientInfo> client_info = + GoogleUpdateSettings::LoadMetricsClientInfo(); + if (client_info) { + command_line->AppendSwitchASCII(switches::kMetricsClientID, + client_info->client_id); + } +#elif defined(OS_POSIX) if (breakpad::IsCrashReporterEnabled()) { scoped_ptr<metrics::ClientInfo> client_info = GoogleUpdateSettings::LoadMetricsClientInfo(); @@ -1246,7 +1253,7 @@ void ChromeContentBrowserClient::AppendExtraCommandLineSwitches( client_info ? client_info->client_id : std::string()); } -#endif // defined(OS_POSIX) && !defined(OS_MACOSX) +#endif if (logging::DialogsAreSuppressed()) command_line->AppendSwitch(switches::kNoErrorDialogs); diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc index 2082442..f7bdca9 100644 --- a/chrome/browser/metrics/chrome_metrics_service_client.cc +++ b/chrome/browser/metrics/chrome_metrics_service_client.cc @@ -181,7 +181,11 @@ void ChromeMetricsServiceClient::RegisterPrefs(PrefRegistrySimple* registry) { void ChromeMetricsServiceClient::SetMetricsClientId( const std::string& client_id) { - crash_keys::SetCrashClientIdFromGUID(client_id); + crash_keys::SetMetricsClientIdFromGUID(client_id); +} + +void ChromeMetricsServiceClient::OnRecordingDisabled() { + crash_keys::ClearMetricsClientId(); } bool ChromeMetricsServiceClient::IsOffTheRecordSessionActive() { diff --git a/chrome/browser/metrics/chrome_metrics_service_client.h b/chrome/browser/metrics/chrome_metrics_service_client.h index a29d5c6..c240192 100644 --- a/chrome/browser/metrics/chrome_metrics_service_client.h +++ b/chrome/browser/metrics/chrome_metrics_service_client.h @@ -58,6 +58,7 @@ class ChromeMetricsServiceClient // metrics::MetricsServiceClient: void SetMetricsClientId(const std::string& client_id) override; + void OnRecordingDisabled() override; bool IsOffTheRecordSessionActive() override; int32 GetProduct() override; std::string GetApplicationLocale() override; diff --git a/chrome/common/child_process_logging_win.cc b/chrome/common/child_process_logging_win.cc index 37a8d3e..9571701 100644 --- a/chrome/common/child_process_logging_win.cc +++ b/chrome/common/child_process_logging_win.cc @@ -74,7 +74,7 @@ void Init() { scoped_ptr<metrics::ClientInfo> client_info = GoogleUpdateSettings::LoadMetricsClientInfo(); if (client_info) - crash_keys::SetCrashClientIdFromGUID(client_info->client_id); + crash_keys::SetMetricsClientIdFromGUID(client_info->client_id); } } // namespace child_process_logging diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc index 9d0a7c6..193dc25 100644 --- a/chrome/common/chrome_switches.cc +++ b/chrome/common/chrome_switches.cc @@ -1270,6 +1270,11 @@ const char kDisableSystemFullscreenForTesting[] = // chrome/browser/mac/relauncher.h. const char kRelauncherProcess[] = "relauncher"; +// This is how the metrics client ID is passed from the browser process to its +// children. With Crashpad, the metrics client ID is distinct from the crash +// client ID. +const char kMetricsClientID[] = "metrics-client-id"; + #endif // Use bubbles for content permissions requests instead of infobars. diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h index bf948bb..feb28fe6 100644 --- a/chrome/common/chrome_switches.h +++ b/chrome/common/chrome_switches.h @@ -368,6 +368,7 @@ extern const char kHostedAppQuitNotification[]; extern const char kDisableHostedAppShimCreation[]; extern const char kDisableSystemFullscreenForTesting[]; extern const char kRelauncherProcess[]; +extern const char kMetricsClientID[]; #endif #if defined(OS_WIN) diff --git a/chrome/common/crash_keys.cc b/chrome/common/crash_keys.cc index 1983f0d..dfe0f42 100644 --- a/chrome/common/crash_keys.cc +++ b/chrome/common/crash_keys.cc @@ -55,7 +55,13 @@ static_assert(kMediumSize <= kSingleChunkLength, "mac has medium size crash key chunks"); #endif +#if defined(OS_MACOSX) +// Crashpad owns the "guid" key. Chrome's metrics client ID is a separate ID +// carried in a distinct "metrics_client_id" field. +const char kMetricsClientId[] = "metrics_client_id"; +#else const char kClientId[] = "guid"; +#endif const char kChannel[] = "channel"; @@ -123,7 +129,11 @@ size_t RegisterChromeCrashKeys() { // The following keys may be chunked by the underlying crash logging system, // but ultimately constitute a single key-value pair. base::debug::CrashKey fixed_keys[] = { +#if defined(OS_MACOSX) + { kMetricsClientId, kSmallSize }, +#else { kClientId, kSmallSize }, +#endif { kChannel, kSmallSize }, { kActiveURL, kLargeSize }, { kNumSwitches, kSmallSize }, @@ -234,14 +244,40 @@ size_t RegisterChromeCrashKeys() { kSingleChunkLength); } -void SetCrashClientIdFromGUID(const std::string& client_guid) { - std::string stripped_guid(client_guid); +void SetMetricsClientIdFromGUID(const std::string& metrics_client_guid) { + std::string stripped_guid(metrics_client_guid); // Remove all instance of '-' char from the GUID. So BCD-WXY becomes BCDWXY. ReplaceSubstringsAfterOffset(&stripped_guid, 0, "-", ""); if (stripped_guid.empty()) return; +#if defined(OS_MACOSX) + // The crash client ID is maintained by Crashpad and is distinct from the + // metrics client ID, which is carried in its own key. + base::debug::SetCrashKeyValue(kMetricsClientId, stripped_guid); +#else + // The crash client ID is set by the application when Breakpad is in use. + // The same ID as the metrics client ID is used. base::debug::SetCrashKeyValue(kClientId, stripped_guid); +#endif +} + +void ClearMetricsClientId() { +#if defined(OS_MACOSX) + // Crashpad always monitors for crashes, but doesn't upload them when + // crash reporting is disabled. The preference to upload crash reports is + // linked to the preference for metrics reporting. When metrics reporting is + // disabled, don't put the metrics client ID into crash dumps. This way, crash + // reports that are saved but not uploaded will not have a metrics client ID + // from the time that metrics reporting was disabled even if they are uploaded + // by user action at a later date. + // + // Breakpad cannot be enabled or disabled without an application restart, and + // it needs to use the metrics client ID as its stable crash client ID, so + // leave its client ID intact even when metrics reporting is disabled while + // the application is running. + base::debug::ClearCrashKey(kMetricsClientId); +#endif } static bool IsBoringSwitch(const std::string& flag) { @@ -271,6 +307,11 @@ static bool IsBoringSwitch(const std::string& flag) { // (If you need to know can always look at the PEB). flag == "--flag-switches-begin" || flag == "--flag-switches-end"; +#elif defined(OS_MACOSX) + // These are carried in their own fields. + return StartsWithASCII(flag, "--channel=", true) || + StartsWithASCII(flag, "--type=", true) || + StartsWithASCII(flag, "--metrics-client-id=", true); #elif defined(OS_CHROMEOS) static const char* const kIgnoreSwitches[] = { ::switches::kEnableLogging, diff --git a/chrome/common/crash_keys.h b/chrome/common/crash_keys.h index d9dd52e..3f40ff5 100644 --- a/chrome/common/crash_keys.h +++ b/chrome/common/crash_keys.h @@ -21,11 +21,11 @@ namespace crash_keys { // reporting server. Returns the size of the union of all keys. size_t RegisterChromeCrashKeys(); -// Sets the ID (based on |client_guid| which may either be a full GUID or a -// GUID that was already stripped from its dashes -- in either cases this method -// will strip remaining dashes before setting the crash key) by which this crash -// reporting client can be identified. -void SetCrashClientIdFromGUID(const std::string& client_guid); +// Sets the ID (which may either be a full GUID or a GUID that was already +// stripped from its dashes -- in either case this method will strip remaining +// dashes before setting the crash key). +void SetMetricsClientIdFromGUID(const std::string& metrics_client_guid); +void ClearMetricsClientId(); // Sets the kSwitch and kNumSwitches keys based on the given |command_line|. void SetSwitchesFromCommandLine(const base::CommandLine* command_line); @@ -55,7 +55,16 @@ class ScopedPrinterInfo { // Crash Key Name Constants //////////////////////////////////////////////////// // The GUID used to identify this client to the crash system. +#if defined(OS_MACOSX) +// On Mac OS X, the crash reporting client ID is the responsibility of Crashpad. +// It is not set directly by Chrome. To make the metrics client ID available on +// the server, it's stored in a distinct key. +extern const char kMetricsClientID[]; +#else +// When using Breakpad instead of Crashpad, the crash reporting client ID is the +// same as the metrics client ID. extern const char kClientID[]; +#endif // The product release/distribution channel. extern const char kChannel[]; diff --git a/chromecast/browser/metrics/cast_metrics_service_client.cc b/chromecast/browser/metrics/cast_metrics_service_client.cc index 48bf45b..a367322 100644 --- a/chromecast/browser/metrics/cast_metrics_service_client.cc +++ b/chromecast/browser/metrics/cast_metrics_service_client.cc @@ -52,6 +52,9 @@ void CastMetricsServiceClient::SetMetricsClientId( PlatformSetClientID(cast_service_, client_id); } +void CastMetricsServiceClient::OnRecordingDisabled() { +} + void CastMetricsServiceClient::StoreClientInfo( const ::metrics::ClientInfo& client_info) { const std::string& client_id = client_info.client_id; diff --git a/chromecast/browser/metrics/cast_metrics_service_client.h b/chromecast/browser/metrics/cast_metrics_service_client.h index ffa17a3..abc2b57 100644 --- a/chromecast/browser/metrics/cast_metrics_service_client.h +++ b/chromecast/browser/metrics/cast_metrics_service_client.h @@ -51,6 +51,7 @@ class CastMetricsServiceClient : public ::metrics::MetricsServiceClient { // metrics::MetricsServiceClient implementation: void SetMetricsClientId(const std::string& client_id) override; + void OnRecordingDisabled() override; bool IsOffTheRecordSessionActive() override; int32_t GetProduct() override; std::string GetApplicationLocale() override; diff --git a/components/crash/app/breakpad_mac.mm b/components/crash/app/breakpad_mac.mm index 8a869d6..f435483 100644 --- a/components/crash/app/breakpad_mac.mm +++ b/components/crash/app/breakpad_mac.mm @@ -252,13 +252,6 @@ void InitCrashReporter(const std::string& process_type) { SetCrashKeyValue(@"prod", [info_dictionary objectForKey:@BREAKPAD_PRODUCT]); SetCrashKeyValue(@"plat", @"OS X"); - if (!is_browser) { - // Get the guid from the command line switch. - std::string client_guid = - command_line->GetSwitchValueASCII(switches::kEnableCrashReporter); - GetCrashReporterClient()->SetCrashReporterClientIdFromGUID(client_guid); - } - logging::SetLogMessageHandler(&FatalMessageHandler); base::debug::SetDumpWithoutCrashingFunction(&DumpHelper::DumpWithoutCrashing); diff --git a/components/crash/app/crash_reporter_client.cc b/components/crash/app/crash_reporter_client.cc index 3440a18..57edf59 100644 --- a/components/crash/app/crash_reporter_client.cc +++ b/components/crash/app/crash_reporter_client.cc @@ -27,9 +27,11 @@ CrashReporterClient* GetCrashReporterClient() { CrashReporterClient::CrashReporterClient() {} CrashReporterClient::~CrashReporterClient() {} +#if !defined(OS_MACOSX) void CrashReporterClient::SetCrashReporterClientIdFromGUID( const std::string& client_guid) { } +#endif #if defined(OS_WIN) bool CrashReporterClient::GetAlternativeCrashDumpLocation( diff --git a/components/crash/app/crash_reporter_client.h b/components/crash/app/crash_reporter_client.h index af816ef..cff8b7e 100644 --- a/components/crash/app/crash_reporter_client.h +++ b/components/crash/app/crash_reporter_client.h @@ -45,11 +45,16 @@ class CrashReporterClient { CrashReporterClient(); virtual ~CrashReporterClient(); +#if !defined(OS_MACOSX) // Sets the crash reporting client ID, a unique identifier for the client // that is sending crash reports. After it is set, it should not be changed. // |client_guid| may either be a full GUID or a GUID that was already stripped // from its dashes. + // + // On Mac OS X, this is the responsibility of Crashpad, and can not be set + // directly by the client. virtual void SetCrashReporterClientIdFromGUID(const std::string& client_guid); +#endif #if defined(OS_WIN) // Returns true if an alternative location to store the minidump files was diff --git a/components/crash/app/crashpad_mac.mm b/components/crash/app/crashpad_mac.mm index 5a3eed8..962cce6 100644 --- a/components/crash/app/crashpad_mac.mm +++ b/components/crash/app/crashpad_mac.mm @@ -158,6 +158,10 @@ void InitializeCrashpad(const std::string& process_type) { g_simple_string_dictionary = new crashpad::SimpleStringDictionary(); crashpad_info->set_simple_annotations(g_simple_string_dictionary); + + base::debug::SetCrashKeyReportingFunctions(SetCrashKeyValue, ClearCrashKey); + crash_reporter_client->RegisterCrashKeys(); + SetCrashKeyValue("ptype", browser_process ? base::StringPiece("browser") : base::StringPiece(process_type)); SetCrashKeyValue("pid", base::StringPrintf("%d", getpid())); @@ -171,9 +175,6 @@ void InitializeCrashpad(const std::string& process_type) { // the same file and line. base::debug::SetDumpWithoutCrashingFunction(DumpWithoutCrashing); - base::debug::SetCrashKeyReportingFunctions(SetCrashKeyValue, ClearCrashKey); - crash_reporter_client->RegisterCrashKeys(); - if (browser_process) { g_database = crashpad::CrashReportDatabase::Initialize(database_path).release(); diff --git a/components/metrics/metrics_service.cc b/components/metrics/metrics_service.cc index f86f33a..358d423 100644 --- a/components/metrics/metrics_service.cc +++ b/components/metrics/metrics_service.cc @@ -388,6 +388,8 @@ void MetricsService::DisableRecording() { return; recording_active_ = false; + client_->OnRecordingDisabled(); + base::RemoveActionCallback(action_callback_); for (size_t i = 0; i < metrics_providers_.size(); ++i) diff --git a/components/metrics/metrics_service_client.h b/components/metrics/metrics_service_client.h index 1b481f9..489080a 100644 --- a/components/metrics/metrics_service_client.h +++ b/components/metrics/metrics_service_client.h @@ -29,6 +29,10 @@ class MetricsServiceClient { // when metrics recording gets enabled. virtual void SetMetricsClientId(const std::string& client_id) = 0; + // Notifies the client that recording is disabled, so that other services + // (such as crash reporting) can clear any association with metrics. + virtual void OnRecordingDisabled() = 0; + // Whether there's an "off the record" (aka "Incognito") session active. virtual bool IsOffTheRecordSessionActive() = 0; diff --git a/components/metrics/test_metrics_service_client.cc b/components/metrics/test_metrics_service_client.cc index 9dcb370..b016672 100644 --- a/components/metrics/test_metrics_service_client.cc +++ b/components/metrics/test_metrics_service_client.cc @@ -26,6 +26,9 @@ void TestMetricsServiceClient::SetMetricsClientId( client_id_ = client_id; } +void TestMetricsServiceClient::OnRecordingDisabled() { +} + bool TestMetricsServiceClient::IsOffTheRecordSessionActive() { return false; } diff --git a/components/metrics/test_metrics_service_client.h b/components/metrics/test_metrics_service_client.h index 973d6e3..52f1889 100644 --- a/components/metrics/test_metrics_service_client.h +++ b/components/metrics/test_metrics_service_client.h @@ -22,6 +22,7 @@ class TestMetricsServiceClient : public MetricsServiceClient { // MetricsServiceClient: void SetMetricsClientId(const std::string& client_id) override; + void OnRecordingDisabled() override; bool IsOffTheRecordSessionActive() override; int32_t GetProduct() override; std::string GetApplicationLocale() override; |