Move MetricsStateManager into the Metrics component.

Move MetricsStateManager into the Metrics component.

Moves the Metrics-related prefs and switches that MetricsStateManager uses into
the Metrics component as well.

BUG=374210
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274682

[blundell, 2014-06-02 19:53:57]

https://codereview.chromium.org/312583002/diff/1/components/metrics.gypi
File components/metrics.gypi (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics.gypi#newc...
components/metrics.gypi:16: 'variations',
Is this dependency OK from the CrOS pov do you know?

[Alexei Svitkine (slow), 2014-06-02 20:35:43]

https://codereview.chromium.org/312583002/diff/1/components/metrics.gypi
File components/metrics.gypi (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics.gypi#newc...
components/metrics.gypi:16: 'variations',
On 2014/06/02 19:53:58, blundell wrote:
> Is this dependency OK from the CrOS pov do you know?

I think it should be fine. variations is a tiny component that only depends on
base and a 1-file third party lib.

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
File components/metrics/metrics_pref_names.cc (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
components/metrics/metrics_pref_names.cc:14: const char kMetricsSessionID[] =
"user_experience_metrics.session_id";
This is only used from MetricsService, so I don't think it needs to move.

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
components/metrics/metrics_pref_names.cc:29:
"user_experience_metrics.reporting_enabled";
This pref is actually queried through a callback whose impl lives in
ChromeMetricsServiceClient, so it doesn't need to move...

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
File components/metrics/metrics_pref_names.h (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
components/metrics/metrics_pref_names.h:12: // component. Document each in the
.cc file.
Not keeping them alphabetical is fine, but let's group them in sections based on
what things use them.

e.g.

// MetricsStateManager prefs:

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_s...
File components/metrics/metrics_state_manager.cc (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_s...
components/metrics/metrics_state_manager.cc:19: #include
"components/metrics/metrics_switches.h"
Did you forget to add this file?

[blundell, 2014-06-02 20:43:40]

oops, metrics_switches added. Small question for you.

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
File components/metrics/metrics_pref_names.h (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
components/metrics/metrics_pref_names.h:12: // component. Document each in the
.cc file.
On 2014/06/02 20:35:43, Alexei Svitkine wrote:
> Not keeping them alphabetical is fine, but let's group them in sections based
on
> what things use them.
> 
> e.g.
> 
> // MetricsStateManager prefs:

I actually prefer having them be alphabetical (as I've generally seen them). I
kept these ones in the groupings they were in previously. Is it OK if I change
them to be alphabetical?

[Alexei Svitkine (slow), 2014-06-02 20:51:43]

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
File components/metrics/metrics_pref_names.h (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
components/metrics/metrics_pref_names.h:12: // component. Document each in the
.cc file.
On 2014/06/02 20:43:41, blundell wrote:
> On 2014/06/02 20:35:43, Alexei Svitkine wrote:
> > Not keeping them alphabetical is fine, but let's group them in sections
based
> on
> > what things use them.
> > 
> > e.g.
> > 
> > // MetricsStateManager prefs:
> 
> I actually prefer having them be alphabetical (as I've generally seen them). I
> kept these ones in the groupings they were in previously. Is it OK if I change
> them to be alphabetical?

Sure. I don't think the grouping is super important here.

[blundell, 2014-06-03 15:46:48]

Thanks!

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
File components/metrics/metrics_pref_names.cc (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
components/metrics/metrics_pref_names.cc:14: const char kMetricsSessionID[] =
"user_experience_metrics.session_id";
Do you mind if I keep it here now that I've moved it? If I move it back, we'll
just have to move it again when we move MetricsService.

On 2014/06/02 20:35:43, Alexei Svitkine wrote:
> This is only used from MetricsService, so I don't think it needs to move.

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
components/metrics/metrics_pref_names.cc:29:
"user_experience_metrics.reporting_enabled";
On 2014/06/02 20:35:43, Alexei Svitkine wrote:
> This pref is actually queried through a callback whose impl lives in
> ChromeMetricsServiceClient, so it doesn't need to move...

Done.

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
File components/metrics/metrics_pref_names.h (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
components/metrics/metrics_pref_names.h:12: // component. Document each in the
.cc file.
On 2014/06/02 20:51:44, Alexei Svitkine wrote:
> On 2014/06/02 20:43:41, blundell wrote:
> > On 2014/06/02 20:35:43, Alexei Svitkine wrote:
> > > Not keeping them alphabetical is fine, but let's group them in sections
> based
> > on
> > > what things use them.
> > > 
> > > e.g.
> > > 
> > > // MetricsStateManager prefs:
> > 
> > I actually prefer having them be alphabetical (as I've generally seen them).
I
> > kept these ones in the groupings they were in previously. Is it OK if I
change
> > them to be alphabetical?
> 
> Sure. I don't think the grouping is super important here.

Done.

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_s...
File components/metrics/metrics_state_manager.cc (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_s...
components/metrics/metrics_state_manager.cc:19: #include
"components/metrics/metrics_switches.h"
On 2014/06/02 20:35:43, Alexei Svitkine wrote:
> Did you forget to add this file?

Done.

[Alexei Svitkine (slow), 2014-06-03 17:37:00]

LGTM % comments, thanks!

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
File components/metrics/metrics_pref_names.cc (right):

https://codereview.chromium.org/312583002/diff/1/components/metrics/metrics_p...
components/metrics/metrics_pref_names.cc:14: const char kMetricsSessionID[] =
"user_experience_metrics.session_id";
On 2014/06/03 15:46:48, blundell wrote:
> Do you mind if I keep it here now that I've moved it? If I move it back, we'll
> just have to move it again when we move MetricsService.
> 
> On 2014/06/02 20:35:43, Alexei Svitkine wrote:
> > This is only used from MetricsService, so I don't think it needs to move.
> 

OK sure. We'll just need to remove the metrics:: prefixes from its use in
MetricsService when MetricsService moves into the component.

https://codereview.chromium.org/312583002/diff/60001/components/metrics/metri...
File components/metrics/metrics_switches.cc (right):

https://codereview.chromium.org/312583002/diff/60001/components/metrics/metri...
components/metrics/metrics_switches.cc:12: const char kResetVariationState[]    
      = "reset-variation-state";
Nit: Can have just one space before the =

https://codereview.chromium.org/312583002/diff/60001/components/metrics/metri...
File components/metrics/metrics_switches.h (right):

https://codereview.chromium.org/312583002/diff/60001/components/metrics/metri...
components/metrics/metrics_switches.h:11: // Alphabetical list of preference
names specific to the metrics
These aren't preferences.

[blundell, 2014-06-03 18:42:50]

https://codereview.chromium.org/312583002/diff/60001/components/metrics/metri...
File components/metrics/metrics_switches.cc (right):

https://codereview.chromium.org/312583002/diff/60001/components/metrics/metri...
components/metrics/metrics_switches.cc:12: const char kResetVariationState[]    
      = "reset-variation-state";
On 2014/06/03 17:37:00, Alexei Svitkine wrote:
> Nit: Can have just one space before the =

Done.

https://codereview.chromium.org/312583002/diff/60001/components/metrics/metri...
File components/metrics/metrics_switches.h (right):

https://codereview.chromium.org/312583002/diff/60001/components/metrics/metri...
components/metrics/metrics_switches.h:11: // Alphabetical list of preference
names specific to the metrics
On 2014/06/03 17:37:00, Alexei Svitkine wrote:
> These aren't preferences.

Done.

[blundell, 2014-06-03 18:42:57]

The CQ bit was checked by blundell@chromium.org

[commit-bot: I haz the power, 2014-06-03 18:44:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/blundell@chromium.org/312583002/80001

[commit-bot: I haz the power, 2014-06-03 23:51:58]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/12452)

[commit-bot: I haz the power, 2014-06-04 00:20:08]

Change committed as 274682