Move options_util.* into chrome/browser/metrics.

Move options_util.* into chrome/browser/metrics.

BUG=None
TEST=None
R=asvitkine@chromium.org, isherman@chromium.org, thestig@chromium.org
TBR=thestig@chromium.org

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

[Ilya Sherman, 2014-07-07 22:59:25]

LGTM % nits.  Thanks, Thiago.

https://codereview.chromium.org/365253004/diff/1/chrome/browser/metrics/metri...
File chrome/browser/metrics/metrics_reporting_state.cc (right):

https://codereview.chromium.org/365253004/diff/1/chrome/browser/metrics/metri...
chrome/browser/metrics/metrics_reporting_state.cc:21: DVLOG(1) << "OptionsUtil:
Unable to set crash report status to " << enabled;
nit: Please update this log stmt.

https://codereview.chromium.org/365253004/diff/1/chrome/browser/metrics/metri...
File chrome/browser/metrics/metrics_reporting_state.h (right):

https://codereview.chromium.org/365253004/diff/1/chrome/browser/metrics/metri...
chrome/browser/metrics/metrics_reporting_state.h:9: // permission match
|enabled|, returns the actual enabled setting.
This comment is pretty confusing.  I realize you're just moving the code, but
let's clean it up a bit along the way.  Here's a starting point; feel free to
improve further:

"""
Tries to set crash stats upload consent to |enabled|.  Regardless of success, if
crash stats uploading is enabled, starts the MetricsService; otherwise stops it.
 Returns whether crash stats uploading is enabled.
"""

https://codereview.chromium.org/365253004/diff/1/chrome/browser/metrics/metri...
chrome/browser/metrics/metrics_reporting_state.h:10: bool
ResolveMetricsReportingEnabled(bool enabled);
Hmm, this method is pretty confusing.  It behaves similarly to something that
I'd call "SetMetricsReportingEnabled()", except that it doesn't actually set the
appropriate pref (which, admittedly, differs by platform... which is also
confusing).  Please add a TODO and/or file a bug to clean this up and rename the
method.

[Alexei Svitkine (slow), 2014-07-15 16:46:45]

lgtm with Ilya's comments addressed

[tfarina, 2014-07-20 02:44:32]

sorry for the delay. got busy with other stuff.

https://codereview.chromium.org/365253004/diff/1/chrome/browser/metrics/metri...
File chrome/browser/metrics/metrics_reporting_state.cc (right):

https://codereview.chromium.org/365253004/diff/1/chrome/browser/metrics/metri...
chrome/browser/metrics/metrics_reporting_state.cc:21: DVLOG(1) << "OptionsUtil:
Unable to set crash report status to " << enabled;
On 2014/07/07 22:59:24, Ilya Sherman wrote:
> nit: Please update this log stmt.

Done.

https://codereview.chromium.org/365253004/diff/1/chrome/browser/metrics/metri...
File chrome/browser/metrics/metrics_reporting_state.h (right):

https://codereview.chromium.org/365253004/diff/1/chrome/browser/metrics/metri...
chrome/browser/metrics/metrics_reporting_state.h:9: // permission match
|enabled|, returns the actual enabled setting.
On 2014/07/07 22:59:24, Ilya Sherman wrote:
> This comment is pretty confusing.  I realize you're just moving the code, but
> let's clean it up a bit along the way.  Here's a starting point; feel free to
> improve further:
> 
> """
> Tries to set crash stats upload consent to |enabled|.  Regardless of success,
if
> crash stats uploading is enabled, starts the MetricsService; otherwise stops
it.
>  Returns whether crash stats uploading is enabled.
> """

Done.

https://codereview.chromium.org/365253004/diff/1/chrome/browser/metrics/metri...
chrome/browser/metrics/metrics_reporting_state.h:10: bool
ResolveMetricsReportingEnabled(bool enabled);
On 2014/07/07 22:59:24, Ilya Sherman wrote:
> Hmm, this method is pretty confusing.  It behaves similarly to something that
> I'd call "SetMetricsReportingEnabled()", except that it doesn't actually set
the
> appropriate pref (which, admittedly, differs by platform... which is also
> confusing).  Please add a TODO and/or file a bug to clean this up and rename
the
> method.

Done.

[tfarina, 2014-07-20 02:45:25]

TBRing Lei for trivial changes all over chrome/.

[Lei Zhang, 2014-07-21 18:56:22]

lgtm

[tfarina, 2014-07-24 22:47:16]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-07-24 22:49:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tfarina@chromium.org/365253004/60001

[commit-bot: I haz the power, 2014-07-25 05:20:55]

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

[commit-bot: I haz the power, 2014-07-25 05:29:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-25 05:29:27]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[tfarina, 2014-07-25 23:42:42]

Committed patchset #3 manually as r285722.