Switch various ValueTypeToString()s to base::Value::GetTypeName().

Switch various ValueTypeToString()s to base::Value::GetTypeName().

Simplify code along the way.

Committed: https://crrev.com/e7615d6c7a2e2cf4cbb561c13857292f99c2a1a7
Cr-Commit-Position: refs/heads/master@{#406351}

[Lei Zhang, 2016-07-15 09:25:53]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 09:26:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 10:09:27]

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

[commit-bot: I haz the power, 2016-07-15 10:09:28]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Lei Zhang, 2016-07-16 01:57:10]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-16 01:57:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Lei Zhang, 2016-07-16 02:02:22]

thestig@chromium.org changed reviewers:
+ bauerb@chromium.org, cschuet@chromium.org, rdevlin.cronin@chromium.org

[Lei Zhang, 2016-07-16 02:02:23]

devlin:
c/b/extensions
tools/json_schema_compiler

bauerb:
c/b/profiles
c/b/sessions
components/prefs

cschuet:
c/b/policy/
components/policy

https://codereview.chromium.org/2149253003/diff/20001/tools/json_schema_compi...
File tools/json_schema_compiler/util.cc (left):

https://codereview.chromium.org/2149253003/diff/20001/tools/json_schema_compi...
tools/json_schema_compiler/util.cc:180: return "number";
https://codereview.chromium.org/16462004/diff/30001/tools/json_schema_compile...
suggested "number" instead of "double" but does it really matter?

[commit-bot: I haz the power, 2016-07-16 03:04:36]

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

[commit-bot: I haz the power, 2016-07-16 03:04:37]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Bernhard Bauer, 2016-07-18 11:16:40]

LGTM, thanks!

[cschuet (SLOW), 2016-07-18 11:51:17]

On 2016/07/18 11:16:40, Bernhard Bauer wrote:
> LGTM, thanks!

LGTM, thanks as well!

[Devlin, 2016-07-18 13:29:46]

lgtm with drive by

https://codereview.chromium.org/2149253003/diff/20001/chrome/browser/profiles...
File chrome/browser/profiles/incognito_mode_policy_handler.cc (right):

https://codereview.chromium.org/2149253003/diff/20001/chrome/browser/profiles...
chrome/browser/profiles/incognito_mode_policy_handler.cc:35:
IncognitoModePrefs::Availability availability_enum_value;
nit: initialize this value.

https://codereview.chromium.org/2149253003/diff/20001/tools/json_schema_compi...
File tools/json_schema_compiler/util.cc (left):

https://codereview.chromium.org/2149253003/diff/20001/tools/json_schema_compi...
tools/json_schema_compiler/util.cc:180: return "number";
On 2016/07/16 02:02:23, Lei Zhang wrote:
>
https://codereview.chromium.org/16462004/diff/30001/tools/json_schema_compile...
> suggested "number" instead of "double" but does it really matter?

I think the motivation was since these values largely come from JS (or
JS-adjacent) code, where "number" is more canonical.  But I don't think we
really surface these errors anywhere where it will be string-sensitive, so this
is probably fine.

[Lei Zhang, 2016-07-19 01:46:17]

https://codereview.chromium.org/2149253003/diff/20001/chrome/browser/profiles...
File chrome/browser/profiles/incognito_mode_policy_handler.cc (right):

https://codereview.chromium.org/2149253003/diff/20001/chrome/browser/profiles...
chrome/browser/profiles/incognito_mode_policy_handler.cc:35:
IncognitoModePrefs::Availability availability_enum_value;
On 2016/07/18 13:29:46, Devlin wrote:
> nit: initialize this value.

Why? The value is used as an out parameter, and then it's value isn't even used.
It's only there because IncognitoModePrefs::IntToAvailability() requires the out
parameter.

[Lei Zhang, 2016-07-19 01:46:27]

The CQ bit was checked by thestig@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-19 01:47:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-19 04:03:27]

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

[commit-bot: I haz the power, 2016-07-19 04:03:28]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Devlin, 2016-07-19 15:00:07]

https://codereview.chromium.org/2149253003/diff/20001/chrome/browser/profiles...
File chrome/browser/profiles/incognito_mode_policy_handler.cc (right):

https://codereview.chromium.org/2149253003/diff/20001/chrome/browser/profiles...
chrome/browser/profiles/incognito_mode_policy_handler.cc:35:
IncognitoModePrefs::Availability availability_enum_value;
On 2016/07/19 01:46:17, Lei Zhang wrote:
> On 2016/07/18 13:29:46, Devlin wrote:
> > nit: initialize this value.
> 
> Why? The value is used as an out parameter, and then it's value isn't even
used.
> It's only there because IncognitoModePrefs::IntToAvailability() requires the
out
> parameter.

Style guide requires variable initialization with declaration.  Agreed that
right now, this doesn't matter, but IMO it's a good practice and protects
against someone later using the variable.  It'd probably never be an issue in
this case, but I still think it's worthwhile rather than trying to dissect all
the possible cases where something could go wrong in the future. :) And, of
course, is there a reason *not* to?

[Lei Zhang, 2016-07-19 19:02:08]

https://codereview.chromium.org/2149253003/diff/20001/chrome/browser/profiles...
File chrome/browser/profiles/incognito_mode_policy_handler.cc (right):

https://codereview.chromium.org/2149253003/diff/20001/chrome/browser/profiles...
chrome/browser/profiles/incognito_mode_policy_handler.cc:35:
IncognitoModePrefs::Availability availability_enum_value;
On 2016/07/19 15:00:07, Devlin wrote:
> On 2016/07/19 01:46:17, Lei Zhang wrote:
> > On 2016/07/18 13:29:46, Devlin wrote:
> > > nit: initialize this value.
> > 
> > Why? The value is used as an out parameter, and then it's value isn't even
> used.
> > It's only there because IncognitoModePrefs::IntToAvailability() requires the
> out
> > parameter.
> 
> Style guide requires variable initialization with declaration.  Agreed that
> right now, this doesn't matter, but IMO it's a good practice and protects
> against someone later using the variable.  It'd probably never be an issue in
> this case, but I still think it's worthwhile rather than trying to dissect all
> the possible cases where something could go wrong in the future. :) And, of
> course, is there a reason *not* to?

https://google.github.io/styleguide/cppguide.html#Local_Variables says
"initialize variables in the declaration." but I believe the context is for the
example below in the doc. i.e. don't write:

int i; i = f(); but write int i = foo(); instead.

The style guide says nothing about out parameters. Usually for functions that
have an out parameter and return bool, I look at the contract laid out by the
functions. For IntToAvailability(), it says it always sets its out value, so
doing that here is redundent.

[Devlin, 2016-07-19 19:12:38]

https://codereview.chromium.org/2149253003/diff/20001/chrome/browser/profiles...
File chrome/browser/profiles/incognito_mode_policy_handler.cc (right):

https://codereview.chromium.org/2149253003/diff/20001/chrome/browser/profiles...
chrome/browser/profiles/incognito_mode_policy_handler.cc:35:
IncognitoModePrefs::Availability availability_enum_value;
On 2016/07/19 19:02:08, Lei Zhang wrote:
> On 2016/07/19 15:00:07, Devlin wrote:
> > On 2016/07/19 01:46:17, Lei Zhang wrote:
> > > On 2016/07/18 13:29:46, Devlin wrote:
> > > > nit: initialize this value.
> > > 
> > > Why? The value is used as an out parameter, and then it's value isn't even
> > used.
> > > It's only there because IncognitoModePrefs::IntToAvailability() requires
the
> > out
> > > parameter.
> > 
> > Style guide requires variable initialization with declaration.  Agreed that
> > right now, this doesn't matter, but IMO it's a good practice and protects
> > against someone later using the variable.  It'd probably never be an issue
in
> > this case, but I still think it's worthwhile rather than trying to dissect
all
> > the possible cases where something could go wrong in the future. :) And, of
> > course, is there a reason *not* to?
> 
> https://google.github.io/styleguide/cppguide.html#Local_Variables says
> "initialize variables in the declaration." but I believe the context is for
the
> example below in the doc. i.e. don't write:
> 
> int i; i = f(); but write int i = foo(); instead.
> 
> The style guide says nothing about out parameters. Usually for functions that
> have an out parameter and return bool, I look at the contract laid out by the
> functions. For IntToAvailability(), it says it always sets its out value, so
> doing that here is redundent.

Given I'm not an owner here (this was just a drive-by comment), I'm fine with
whatever you decide, and don't want to block this issue on this.

[Lei Zhang, 2016-07-19 19:13:46]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2016-07-19 19:13:47]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
rdevlin.cronin@chromium.org, cschuet@chromium.org
Link to the patchset: https://codereview.chromium.org/2149253003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-07-19 19:14:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-19 19:44:00]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-19 19:47:26]

Description was changed from

==========
Switch various ValueTypeToString()s to base::Value::GetTypeName().

Simplify code along the way.
==========

to

==========
Switch various ValueTypeToString()s to base::Value::GetTypeName().

Simplify code along the way.

Committed: https://crrev.com/e7615d6c7a2e2cf4cbb561c13857292f99c2a1a7
Cr-Commit-Position: refs/heads/master@{#406351}
==========

[commit-bot: I haz the power, 2016-07-19 19:47:27]

Patchset 3 (id:??) landed as
https://crrev.com/e7615d6c7a2e2cf4cbb561c13857292f99c2a1a7
Cr-Commit-Position: refs/heads/master@{#406351}