summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorjam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-10-01 16:34:45 +0000
committerjam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-10-01 16:34:45 +0000
commit069cd995e1f1f5f53b409dc116b3f7381e41af1b (patch)
treedcfac5ac62be5a334e30855bca82bd52e33c7b82
parent72caffa4490b52d33aef07365a5e87cd96cdcbf3 (diff)
downloadchromium_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.cc78
-rw-r--r--chrome/browser/chrome_browser_main.h18
-rw-r--r--chrome/browser/metrics/metrics_service.cc1
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),