Skip to content

Commit

Permalink
Keep all references to kInstallDate inside components/metrics/
Browse files Browse the repository at this point in the history
Follow-up to https://codereview.chromium.org/370813003/ as per request.

BUG=391338
TEST=kInstallDate is still set on startup when it wasn't previously set (and no backup is present in registry)

Review URL: https://codereview.chromium.org/396753005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283467 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
gab@chromium.org committed Jul 16, 2014
1 parent 7c03214 commit 64b8652
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 28 deletions.
10 changes: 4 additions & 6 deletions chrome/browser/chrome_browser_field_trials.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,19 @@ ChromeBrowserFieldTrials::ChromeBrowserFieldTrials(
ChromeBrowserFieldTrials::~ChromeBrowserFieldTrials() {
}

void ChromeBrowserFieldTrials::SetupFieldTrials(PrefService* local_state) {
const base::Time install_time = base::Time::FromTimeT(
local_state->GetInt64(metrics::prefs::kInstallDate));
void ChromeBrowserFieldTrials::SetupFieldTrials(const base::Time& install_time,
PrefService* local_state) {
DCHECK(!install_time.is_null());

// Field trials that are shared by all platforms.
chrome_variations::SetupUniformityFieldTrials(install_time);
InstantiateDynamicTrials();

#if defined(OS_ANDROID) || defined(OS_IOS)
chrome::SetupMobileFieldTrials(
parsed_command_line_, install_time, local_state);
chrome::SetupMobileFieldTrials(parsed_command_line_);
#else
chrome::SetupDesktopFieldTrials(
parsed_command_line_, install_time, local_state);
parsed_command_line_, local_state);
#endif
}

Expand Down
9 changes: 7 additions & 2 deletions chrome/browser/chrome_browser_field_trials.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,19 @@

class PrefService;

namespace base {
class Time;
}

class ChromeBrowserFieldTrials {
public:
explicit ChromeBrowserFieldTrials(const base::CommandLine& command_line);
~ChromeBrowserFieldTrials();

// Called by the browser main sequence to set up Field Trials for this client.
// |local_state| is used to extract properties like install time.
void SetupFieldTrials(PrefService* local_state);
// |local_state| is used to set browser-wide properties.
void SetupFieldTrials(const base::Time& install_time,
PrefService* local_state);

private:
// Instantiates dynamic trials by querying their state, to ensure they get
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/chrome_browser_field_trials_desktop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ void SetupPreReadFieldTrial() {
} // namespace

void SetupDesktopFieldTrials(const CommandLine& parsed_command_line,
const base::Time& install_time,
PrefService* local_state) {
prerender::ConfigurePrerender(parsed_command_line);
AutoLaunchChromeFieldTrial();
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/chrome_browser_field_trials_desktop.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ namespace chrome {
// platforms.
// |local_state| is needed by some other methods called from within this one.
void SetupDesktopFieldTrials(const base::CommandLine& parsed_command_line,
const base::Time& install_time,
PrefService* local_state);

} // namespace chrome
Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/chrome_browser_field_trials_mobile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ void DataCompressionProxyFieldTrials() {

} // namespace

void SetupMobileFieldTrials(const CommandLine& parsed_command_line,
const base::Time& install_time,
PrefService* local_state) {
void SetupMobileFieldTrials(const CommandLine& parsed_command_line) {
DataCompressionProxyFieldTrials();
#if defined(OS_ANDROID)
prerender::ConfigurePrerender(parsed_command_line);
Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/chrome_browser_field_trials_mobile.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ namespace chrome {
// Add an invocation of your field trial init function to this method, or to
// SetupFieldTrials in chrome_browser_field_trials.cc if it is for all
// platforms.
void SetupMobileFieldTrials(const base::CommandLine& parsed_command_line,
const base::Time& install_time,
PrefService* local_state);
void SetupMobileFieldTrials(const base::CommandLine& parsed_command_line);

} // namespace chrome

Expand Down
15 changes: 3 additions & 12 deletions chrome/browser/chrome_browser_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@
#include "chrome/installer/util/google_update_settings.h"
#include "components/google/core/browser/google_util.h"
#include "components/language_usage_metrics/language_usage_metrics.h"
#include "components/metrics/metrics_pref_names.h"
#include "components/metrics/metrics_service.h"
#include "components/nacl/browser/nacl_browser.h"
#include "components/nacl/browser/nacl_process_host.h"
Expand Down Expand Up @@ -601,8 +600,9 @@ void ChromeBrowserMainParts::SetupMetricsAndFieldTrials() {
if (variations_service)
variations_service->CreateTrialsFromSeed();

// This must be called after the local state is initialized.
browser_field_trials_.SetupFieldTrials(local_state_);
// This must be called after |local_state_| is initialized.
browser_field_trials_.SetupFieldTrials(
base::Time::FromTimeT(metrics->GetInstallDate()), local_state_);

// Initialize FieldTrialSynchronizer system. This is a singleton and is used
// for posting tasks via base::Bind. Its deleted when it goes out of scope.
Expand Down Expand Up @@ -937,15 +937,6 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() {
// Initialize tracking synchronizer system.
tracking_synchronizer_ = new chrome_browser_metrics::TrackingSynchronizer();

// 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.
int64 install_date = local_state_->GetInt64(metrics::prefs::kInstallDate);
if (install_date == 0) {
local_state_->SetInt64(metrics::prefs::kInstallDate,
base::Time::Now().ToTimeT());
}

#if defined(OS_MACOSX)
// Get the Keychain API to register for distributed notifications on the main
// thread, which has a proper CFRunloop, instead of later on the I/O thread,
Expand Down
8 changes: 8 additions & 0 deletions components/metrics/metrics_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@
#include "base/threading/platform_thread.h"
#include "base/threading/thread.h"
#include "base/threading/thread_restrictions.h"
#include "base/time/time.h"
#include "base/tracked_objects.h"
#include "base/values.h"
#include "components/metrics/metrics_log.h"
Expand Down Expand Up @@ -331,6 +332,13 @@ MetricsService::MetricsService(metrics::MetricsStateManager* state_manager,
DCHECK(state_manager_);
DCHECK(client_);
DCHECK(local_state_);

// Set the install date if this is our first run.
int64 install_date = local_state_->GetInt64(metrics::prefs::kInstallDate);
if (install_date == 0) {
local_state_->SetInt64(metrics::prefs::kInstallDate,
base::Time::Now().ToTimeT());
}
}

MetricsService::~MetricsService() {
Expand Down

0 comments on commit 64b8652

Please sign in to comment.