The MetricsService are not initialized unless the user have opted in...

The MetricsService are not initialized unless the user have opted in

BUG=2278

[jar (doing other things), 2009-03-02 17:48:01]

Change seems nice, safe, and correct.  Please just add one more surrounding if
block per nit below.

Thanks,

Jim

http://codereview.chromium.org/28315/diff/1/2
File chrome/browser/metrics/metrics_service.cc (right):

http://codereview.chromium.org/28315/diff/1/2#newcode426
Line 426: PrefService* pref = g_browser_process->local_state();
Nit: How about containing this block in:
if (client_id_.empty())
so that we don't re-init this each time?  Not a big deal, but since we're being
lazy, we may as well just do this once.

[Xharze, 2009-03-02 21:09:13]

Like this?

[jar (doing other things), 2009-03-02 23:29:09]

http://codereview.chromium.org/28315/diff/5/1003
File chrome/browser/metrics/metrics_service.cc (right):

http://codereview.chromium.org/28315/diff/5/1003#newcode429
Line 429: client_id_ = GenerateClientID();
Sorry to have confused the issue.  My suggestion was that you should do
something like:
if (client_id_.empty()) { // No init yet in this process.
  PrefService* pref = g_browser_process->local_state();
  client_id_ = WideToUTF8(pref->
      GetString(prefs::kMetricsClientID));
  if (client_id_.empty()) {  // Never did an init.
    client_id_ = GenerateClientID();
    ...save string...
    ...save time...
  }
}

[Xharze, 2009-03-03 00:52:50]

No problem, new patch uploaded

[jar (doing other things), 2009-03-03 00:55:00]

LGTM

[Xharze, 2009-03-04 22:45:12]

Good to know. Is it you who are going to commit it?