Add new BrowserForceSigninEnabeld policy option with future:true flag.

Add new BrowserForceSigninEnabeld policy option with future:true flag.

BUG=642059
Committed: https://crrev.com/0bf3c7a9ce88ccd56d571a1383530ffa6777eafc
Cr-Commit-Position: refs/heads/master@{#419034}

[zmin, 2016-09-14 18:40:19]

Description was changed from

==========
Add new BrowserForceSigninEnabeld policy option with future:true flag.

BUG=642059
==========

to

==========
Add new BrowserForceSigninEnabeld policy option with future:true flag.

BUG=642059
==========

[zmin, 2016-09-14 18:40:20]

zmin@chromium.org changed reviewers:
+ anthonyvd@chromium.org, jwd@chromium.org, pastarmovj@chromium.org

[zmin, 2016-09-14 18:45:56]

Hi,

Here is the new policy for force sign in. Please review it.

@pastarmovj:  Can you take a look at the whole CL please?

@anthonyvd: 
chrome/browser/profiles/profiles_state.cc

@jwd: tools/metrics/histograms/histograms.xml

Thanks,
Owen

[pastarmovj, 2016-09-14 19:56:29]

A nit and a suggestion about the naming which I think is a little too verbose
right now.

https://codereview.chromium.org/2340743003/diff/1/chrome/browser/policy/confi...
File chrome/browser/policy/configuration_policy_handler_list_factory.cc (right):

https://codereview.chromium.org/2340743003/diff/1/chrome/browser/policy/confi...
chrome/browser/policy/configuration_policy_handler_list_factory.cc:367:
base::Value::TYPE_BOOLEAN },
Please revert the line removal.

https://codereview.chromium.org/2340743003/diff/1/chrome/browser/policy/confi...
chrome/browser/policy/configuration_policy_handler_list_factory.cc:575: {
key::kBrowserForceSigninEnabled,
I don't think the 'Browser' prefix is useful or rather it might make sense to
make it ForceBrowserSignin. I think you don't need the enabled suffix either.
The name already has a 'force' verb. In general I see ForceBrowserSignin as a
better name for the policy too.

PS. I think the reason the other policies here have this prefix is to
differentiate them from the same policies for ChromeOS.

[zmin, 2016-09-14 20:39:44]

https://codereview.chromium.org/2340743003/diff/1/chrome/browser/policy/confi...
File chrome/browser/policy/configuration_policy_handler_list_factory.cc (right):

https://codereview.chromium.org/2340743003/diff/1/chrome/browser/policy/confi...
chrome/browser/policy/configuration_policy_handler_list_factory.cc:367:
base::Value::TYPE_BOOLEAN },
On 2016/09/14 19:56:29, pastarmovj wrote:
> Please revert the line removal.

Done.

https://codereview.chromium.org/2340743003/diff/1/chrome/browser/policy/confi...
chrome/browser/policy/configuration_policy_handler_list_factory.cc:575: {
key::kBrowserForceSigninEnabled,
On 2016/09/14 19:56:29, pastarmovj wrote:
> I don't think the 'Browser' prefix is useful or rather it might make sense to
> make it ForceBrowserSignin. I think you don't need the enabled suffix either.
> The name already has a 'force' verb. In general I see ForceBrowserSignin as a
> better name for the policy too.
> 
> PS. I think the reason the other policies here have this prefix is to
> differentiate them from the same policies for ChromeOS. 

Yes, I use 'Browser' here just because of the two polices above as they are all
related to 'User Profile' but I don't know that is caused by ChromeOS. And yes,
I think ForceBrowserSignin is a good name since we have other 'Force' series as
well.

[pastarmovj, 2016-09-14 23:37:22]

lgtm

[anthonyvd, 2016-09-15 20:22:32]

c/b/profiles/* lgtm

[jwd, 2016-09-15 20:55:15]

histograms lgtm

[zmin, 2016-09-15 20:55:41]

The CQ bit was checked by zmin@chromium.org

[commit-bot: I haz the power, 2016-09-15 20:56:34]

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-09-15 21:50:36]

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

[commit-bot: I haz the power, 2016-09-15 21:50:37]

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_...)

[zmin, 2016-09-15 22:11:08]

The CQ bit was checked by zmin@chromium.org

[zmin, 2016-09-15 22:11:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
pastarmovj@chromium.org, anthonyvd@chromium.org, jwd@chromium.org
Link to the patchset: https://codereview.chromium.org/2340743003/#ps40001
(title: "fix trybot failure")

[commit-bot: I haz the power, 2016-09-15 22:11:32]

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-09-15 23:16:39]

Description was changed from

==========
Add new BrowserForceSigninEnabeld policy option with future:true flag.

BUG=642059
==========

to

==========
Add new BrowserForceSigninEnabeld policy option with future:true flag.

BUG=642059
==========

[commit-bot: I haz the power, 2016-09-15 23:16:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-15 23:18:56]

Description was changed from

==========
Add new BrowserForceSigninEnabeld policy option with future:true flag.

BUG=642059
==========

to

==========
Add new BrowserForceSigninEnabeld policy option with future:true flag.

BUG=642059

Committed: https://crrev.com/0bf3c7a9ce88ccd56d571a1383530ffa6777eafc
Cr-Commit-Position: refs/heads/master@{#419034}
==========

[commit-bot: I haz the power, 2016-09-15 23:18:56]

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