Move kInstallDate from chrome/common/pref_names.h to components/metrics/metrics_pref_names.h

chrome/browser/chrome_browser_main.cc

Index: chrome/browser/chrome_browser_main.cc
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index f8f8b998e2176b5015e844f716d89357aa5a54bd..7fa0378a9881449b9dd305f52fb1a6e33beb6aa3 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -105,6 +105,7 @@
 #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"
@@ -939,9 +940,11 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() {
   // 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(prefs::kInstallDate);
-  if (install_date == 0)
-    local_state_->SetInt64(prefs::kInstallDate, base::Time::Now().ToTimeT());
+  int64 install_date = local_state_->GetInt64(metrics::prefs::kInstallDate);
+  if (install_date == 0) {
+    local_state_->SetInt64(metrics::prefs::kInstallDate,
+                           base::Time::Now().ToTimeT());

[Alexei Svitkine (slow), 2014/07/14 17:00:53]

I don't like the fact that we're recording install date directly here and then
accessing it from MetricsService.

I think it would be better to move the two together. Can you create a follow-up
CL that adds a static MetricsService::InitInstallDate() method with this code
and call it from here? This way, the setting and querying install date can be
done entirely in the metrics component.

[gab, 2014/07/14 19:38:13]

SGTM, in fact how about we just do this automatically when MetricsService
initializes? (i.e. no need to tell it from outside that it's time to init the
install date... it should just do it as soon as it sees it's missing on startup)

[Alexei Svitkine (slow), 2014/07/14 19:40:38]

That sounds even better assuming it's safe to do (i.e. nothing else uses that
value between this point and when MetricsService is initialized.)

[gab, 2014/07/14 19:52:37]

Well that's the same (slightly earlier in fact) as calling
MetricsService::InitInstallDate() as you originally suggested (unless you meant
to make that a static method that can be called without actually having a
service which sounds weird to me...).

The only potential early getter of this pref is
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...
is g_browser_process->metrics_service() up then? Can we use GetInstallDate()
there?

[Alexei Svitkine (slow), 2014/07/15 15:49:33]

I was originally suggesting it to be a static method called from here. While
weird, it would still be a strict improvement over the current code.

But looking at the code you linked, I think it should be fine to put it into the
ctor of MetricsService and to expose a getter from it to retrieve. The linked
code runs after MetricsService has been created, so it should be fine to pass as
a param to the SetupFieldTrials method from:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

[gab, 2014/07/15 22:13:15]

Follow-up CL posted @ https://codereview.chromium.org/396753005/

+  }
 
 #if defined(OS_MACOSX)
   // Get the Keychain API to register for distributed notifications on the main