The consumer management enroll and unenroll buttons should only be visible to the device owner.

The consumer management enroll and unenroll buttons should only be visible to the device owner.

Shows the enroll button when the device is not enrolled, and shows the unenroll
button when the device is enrolled.

When the enroll button is clicked, sets a variable in the local state and
reboots.

TEST=maunally test the settings page.
BUG=chromium:353050
R=bartfab@chromium.org

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

[davidyu, 2014-07-22 08:39:49]

Next step: show a owner-only, online authentication-only signin page when
consumer management enrollment is requested.

[bartfab (slow), 2014-07-23 19:27:13]

lgtm

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/options/chromeos/consumer_management_overlay.js
(right):

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/consumer_management_overlay.js:21:
$('unenroll-content').hidden = !($('enroll-content').hidden = isEnrolled);
This line is doing too much at once. Especially the second = looks like a
mistyped == until you look at it more carefully. How about splitting it into two
more palatable lines:

$('enroll-content').hidden = isEnrolled;
$('unenroll-content').hidden = !isEnrolled;

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/browser_options_handler.cc:619: const
em::PolicyData* policy_data =
Nit: Un-indent one space.

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/browser_options_handler.cc:620:
chromeos::DeviceSettingsService::Get()->policy_data();
Nit: Un-indent one space.

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/consumer_management_handler.cc
(right):

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/consumer_management_handler.cc:81: if
(!user_manager) {
Nit: Just like the other singletons, the UserManager will always be there.

[davidyu, 2014-07-24 04:28:30]

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/resources...
File chrome/browser/resources/options/chromeos/consumer_management_overlay.js
(right):

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/resources...
chrome/browser/resources/options/chromeos/consumer_management_overlay.js:21:
$('unenroll-content').hidden = !($('enroll-content').hidden = isEnrolled);
On 2014/07/23 19:27:13, bartfab wrote:
> This line is doing too much at once. Especially the second = looks like a
> mistyped == until you look at it more carefully. How about splitting it into
two
> more palatable lines:
> 
> $('enroll-content').hidden = isEnrolled;
> $('unenroll-content').hidden = !isEnrolled;

I was asked to write it this way by the previous reviewer. I also think that
having two separated lines is more readable. Done.

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/browser_options_handler.cc:619: const
em::PolicyData* policy_data =
On 2014/07/23 19:27:13, bartfab wrote:
> Nit: Un-indent one space.

Done.

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/browser_options_handler.cc:620:
chromeos::DeviceSettingsService::Get()->policy_data();
On 2014/07/23 19:27:13, bartfab wrote:
> Nit: Un-indent one space.

Done.

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/consumer_management_handler.cc
(right):

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/consumer_management_handler.cc:81: if
(!user_manager) {
On 2014/07/23 19:27:13, bartfab wrote:
> Nit: Just like the other singletons, the UserManager will always be there.

Hmm... the comment of UserManager::Get() is a little bit misleading. It says it
may return NULL on some cases, but in the implementation it actually CHECK()s
before returning the value. I removed the check statement in my change.

[davidyu, 2014-07-24 04:32:21]

Hi Steven,

Can you help review the UI changes? All these changes are guarded by
--enable-consumer-management. Thanks!

[bartfab (slow), 2014-07-24 11:54:53]

Still lgtm.

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/consumer_management_handler.cc
(right):

https://codereview.chromium.org/405383002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/consumer_management_handler.cc:81: if
(!user_manager) {
On 2014/07/24 04:28:30, davidyu wrote:
> On 2014/07/23 19:27:13, bartfab wrote:
> > Nit: Just like the other singletons, the UserManager will always be there.
> 
> Hmm... the comment of UserManager::Get() is a little bit misleading. It says
it
> may return NULL on some cases, but in the implementation it actually CHECK()s
> before returning the value. I removed the check statement in my change.

The UserManager might not exist in three cases:
* Early on in browser start-up, before enrollment can even start.
* Late in browser shut-down, when no enrollment should be happening anymore.
* In tests that do not instantiate it.

You should not be running into any of these.

https://codereview.chromium.org/405383002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/consumer_management_handler.cc
(right):

https://codereview.chromium.org/405383002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/consumer_management_handler.cc:80:
UserManager* user_manager = chromeos::UserManager::Get();
Nit: const. Or simply fold into the next line:

if (!chromeos::UserManager::Get()->IsCurrentUserOwner()) {

[stevenjb, 2014-07-24 18:26:07]

lgtm

[davidyu, 2014-07-25 03:25:33]

https://codereview.chromium.org/405383002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/consumer_management_handler.cc
(right):

https://codereview.chromium.org/405383002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/consumer_management_handler.cc:80:
UserManager* user_manager = chromeos::UserManager::Get();
On 2014/07/24 11:54:53, bartfab wrote:
> Nit: const. Or simply fold into the next line:
> 
> if (!chromeos::UserManager::Get()->IsCurrentUserOwner()) {

Done.

[davidyu, 2014-07-25 03:25:37]

The CQ bit was checked by davidyu@chromium.org

[commit-bot: I haz the power, 2014-07-25 03:26:45]

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

[davidyu, 2014-07-25 08:33:28]

The CQ bit was unchecked by davidyu@chromium.org

[davidyu, 2014-07-25 08:40:25]

The CQ bit was checked by davidyu@chromium.org

[commit-bot: I haz the power, 2014-07-25 08:41:56]

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

[commit-bot: I haz the power, 2014-07-25 12:21:33]

Change committed as 285567