diff options
author | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-01 16:34:45 +0000 |
---|---|---|
committer | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-01 16:34:45 +0000 |
commit | 069cd995e1f1f5f53b409dc116b3f7381e41af1b (patch) | |
tree | dcfac5ac62be5a334e30855bca82bd52e33c7b82 | |
parent | 72caffa4490b52d33aef07365a5e87cd96cdcbf3 (diff) | |
download | chromium_src-069cd995e1f1f5f53b409dc116b3f7381e41af1b.zip chromium_src-069cd995e1f1f5f53b409dc116b3f7381e41af1b.tar.gz chromium_src-069cd995e1f1f5f53b409dc116b3f7381e41af1b.tar.bz2 |
Revert 103628 (I have a simpler fix) - Fix the metrics service not working because it was initialized before the browser threads were started.
BUG=98756
Review URL: http://codereview.chromium.org/8102017
TBR=jam@chromium.org
Review URL: http://codereview.chromium.org/8101012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@103636 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/chrome_browser_main.cc | 78 | ||||
-rw-r--r-- | chrome/browser/chrome_browser_main.h | 18 | ||||
-rw-r--r-- | chrome/browser/metrics/metrics_service.cc | 1 |
3 files changed, 52 insertions, 45 deletions
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index d073763a..7c3f5d4 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -692,30 +692,19 @@ ChromeBrowserMainParts::ChromeBrowserMainParts( ChromeBrowserMainParts::~ChromeBrowserMainParts() { } -void ChromeBrowserMainParts::SetupMetricsAndFieldTrials( - MetricsService* metrics, +// This will be called after the command-line has been mutated by about:flags +MetricsService* ChromeBrowserMainParts::SetupMetricsAndFieldTrials( const CommandLine& parsed_command_line, PrefService* local_state) { - if (parsed_command_line.HasSwitch(switches::kMetricsRecordingOnly) || - parsed_command_line.HasSwitch(switches::kEnableBenchmarking)) { - // If we're testing then we don't care what the user preference is, we turn - // on recording, but not reporting, otherwise tests fail. - metrics->StartRecordingOnly(); - } else { - // If the user permits metrics reporting with the checkbox in the - // prefs, we turn on recording. We disable metrics completely for - // non-official builds. -#if defined(GOOGLE_CHROME_BUILD) -#if defined(OS_CHROMEOS) - bool enabled = - chromeos::UserCrosSettingsProvider::cached_reporting_enabled(); -#else - bool enabled = local_state->GetBoolean(prefs::kMetricsReportingEnabled); -#endif // #if defined(OS_CHROMEOS) - if (enabled) - metrics->Start(); -#endif // defined(GOOGLE_CHROME_BUILD) - } + // Must initialize metrics after labs have been converted into switches, + // but before field trials are set up (so that client ID is available for + // one-time randomized field trials). + MetricsService* metrics = InitializeMetrics(parsed_command_line, local_state); + + // Initialize FieldTrialList to support FieldTrials that use one-time + // randomization. The client ID will be empty if the user has not opted + // to send metrics. + field_trial_list_.reset(new base::FieldTrialList(metrics->GetClientId())); SetupFieldTrials(metrics->recording_active(), local_state->IsManagedPreference( @@ -726,6 +715,8 @@ void ChromeBrowserMainParts::SetupMetricsAndFieldTrials( // scope. Even though NewRunnableMethod does AddRef and Release, the object // will not be deleted after the Task is executed. field_trial_synchronizer_ = new FieldTrialSynchronizer(); + + return metrics; } // This is an A/B test for the maximum number of persistent connections per @@ -1097,9 +1088,11 @@ void ChromeBrowserMainParts::DefaultAppsFieldTrial() { } } +// ChromeBrowserMainParts: |SetupMetricsAndFieldTrials()| related -------------- + // Initializes the metrics service with the configuration for this process, // returning the created service (guaranteed non-NULL). -MetricsService* ChromeBrowserMainParts::CreateMetrics( +MetricsService* ChromeBrowserMainParts::InitializeMetrics( const CommandLine& parsed_command_line, const PrefService* local_state) { #if defined(OS_WIN) @@ -1109,15 +1102,29 @@ MetricsService* ChromeBrowserMainParts::CreateMetrics( MetricsLog::set_version_extension("-64"); #endif // defined(OS_WIN) - // Must initialize metrics after labs have been converted into switches, - // but before field trials are set up (so that client ID is available for - // one-time randomized field trials). MetricsService* metrics = g_browser_process->metrics_service(); - // Initialize FieldTrialList to support FieldTrials that use one-time - // randomization. The client ID will be empty if the user has not opted - // to send metrics. - field_trial_list_.reset(new base::FieldTrialList(metrics->GetClientId())); + if (parsed_command_line.HasSwitch(switches::kMetricsRecordingOnly) || + parsed_command_line.HasSwitch(switches::kEnableBenchmarking)) { + // If we're testing then we don't care what the user preference is, we turn + // on recording, but not reporting, otherwise tests fail. + metrics->StartRecordingOnly(); + return metrics; + } + + // If the user permits metrics reporting with the checkbox in the + // prefs, we turn on recording. We disable metrics completely for + // non-official builds. +#if defined(GOOGLE_CHROME_BUILD) +#if defined(OS_CHROMEOS) + bool enabled = chromeos::UserCrosSettingsProvider::cached_reporting_enabled(); +#else + bool enabled = local_state->GetBoolean(prefs::kMetricsReportingEnabled); +#endif // #if defined(OS_CHROMEOS) + if (enabled) { + metrics->Start(); + } +#endif // defined(GOOGLE_CHROME_BUILD) return metrics; } @@ -1394,6 +1401,11 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunInternal() { // will not be deleted after the Task is executed. histogram_synchronizer_ = new HistogramSynchronizer(); + // Now the command line has been mutated based on about:flags, we can + // set up metrics and initialize field trials. + MetricsService* metrics = + SetupMetricsAndFieldTrials(parsed_command_line(), local_state); + // Now that all preferences have been registered, set the install date // for the uninstall metrics if this is our first run. This only actually // gets used if the user has metrics reporting enabled at uninstall time. @@ -1414,14 +1426,8 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunInternal() { SecKeychainAddCallback(&KeychainCallback, 0, NULL); #endif - // Now the command line has been mutated based on about:flags, we can - // set up metrics and initialize field trials. This needs the threads started. - MetricsService* metrics = CreateMetrics(parsed_command_line(), local_state); - CreateChildThreads(browser_process_.get()); - SetupMetricsAndFieldTrials(metrics, parsed_command_line(), local_state); - #if defined(OS_CHROMEOS) // Now that the file thread exists we can record our stats. chromeos::BootTimesLoader::Get()->RecordChromeMainStats(); diff --git a/chrome/browser/chrome_browser_main.h b/chrome/browser/chrome_browser_main.h index ddfcd3a..d3e8fda 100644 --- a/chrome/browser/chrome_browser_main.h +++ b/chrome/browser/chrome_browser_main.h @@ -33,6 +33,13 @@ class ChromeBrowserMainParts : public content::BrowserMainParts { public: virtual ~ChromeBrowserMainParts(); + // Constructs metrics service and does related initialization, including + // creation of field trials. Call only after labs have been converted to + // switches. + MetricsService* SetupMetricsAndFieldTrials( + const CommandLine& parsed_command_line, + PrefService* local_state); + protected: explicit ChromeBrowserMainParts(const MainFunctionParams& parameters); @@ -73,17 +80,12 @@ class ChromeBrowserMainParts : public content::BrowserMainParts { // has on retention and general apps/webstore usage. void DefaultAppsFieldTrial(); - // Constructs metrics service and field trials. Call only after labs have been - // converted to switches. - MetricsService* CreateMetrics( + // Methods for |SetupMetricsAndFieldTrials()| -------------------------------- + + static MetricsService* InitializeMetrics( const CommandLine& parsed_command_line, const PrefService* local_state); - // Initializes metrics service and sets up the field trial experiments. - void SetupMetricsAndFieldTrials(MetricsService* metrics, - const CommandLine& parsed_command_line, - PrefService* local_state); - // Add an invocation of your field trial init function to this method. void SetupFieldTrials(bool metrics_recording_enabled, bool proxy_policy_is_set); diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc index 86b23a3..2072626 100644 --- a/chrome/browser/metrics/metrics_service.cc +++ b/chrome/browser/metrics/metrics_service.cc @@ -799,7 +799,6 @@ void MetricsService::StartRecording() { // initialization steps (such as plugin list generation) necessary // for sending the initial log. This avoids blocking the main UI // thread. - DCHECK(BrowserThread::IsMessageLoopValid(BrowserThread::FILE)); BrowserThread::PostDelayedTask(BrowserThread::FILE, FROM_HERE, base::Bind(&MetricsService::InitTaskGetHardwareClass, base::Unretained(this), |