Add RESTORE_MODE_DISABLED to RestoreMode

Add RESTORE_MODE_DISABLED to RestoreMode

This CL changes the way that device disabling is expressed in the
DeviceStateRetrievalResponse proto. Instead of being implied when
the |device_state| field is present, it is now explictly set as a
|restore_mode|.

BUG=425574
TEST=Updated unit and browser tests

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

[bartfab (slow), 2014-10-29 14:12:16]

bartfab@chromium.org changed reviewers:
+ antrim@chromium.org, mnissler@chromium.org

[bartfab (slow), 2014-10-29 14:12:16]

Hi Denis,

Could you please review:
chrome/browser/chromeos/login/*

Hi Mattias,

Could you please review:
chrome/browser/chromeos/policy/*
components/policy/proto/*


This CL is the result of a discussion with the security, server and consumer
management teams. We agreed to change the way that device disabling is expressed
in the protos sent by the server.

[Mattias Nissler (ping if slow), 2014-10-29 17:41:35]

Code LGTM, if you decide to make changes I'm happy to re-review.

https://codereview.chromium.org/690643002/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/screens/device_disabled_screen.cc (right):

https://codereview.chromium.org/690643002/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/screens/device_disabled_screen.cc:56: if
(restore_mode != policy::kDeviceStateRestoreModeDisabled ||
Hm, now that this is getting checked in more and more places, maybe it's a good
idea to make this easier by introducing a GetRestoreMode function in
server_backed_device_state.h which returns the mode as an enum value?

https://codereview.chromium.org/690643002/diff/1/chrome/browser/chromeos/poli...
File chrome/browser/chromeos/policy/server_backed_device_state.h (right):

https://codereview.chromium.org/690643002/diff/1/chrome/browser/chromeos/poli...
chrome/browser/chromeos/policy/server_backed_device_state.h:18: extern const
char kDeviceStateRestoreModeDisabled[];
Are we positive behavior will be identical for both lost and stolen cases? If
not, we might introduce two different identifiers here.

On the other hand, I guess we can always deprecated disabled and add
finer-grained identifiers when needed later.

[bartfab (slow), 2014-10-30 14:10:32]

https://codereview.chromium.org/690643002/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/screens/device_disabled_screen.cc (right):

https://codereview.chromium.org/690643002/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/screens/device_disabled_screen.cc:56: if
(restore_mode != policy::kDeviceStateRestoreModeDisabled ||
On 2014/10/29 17:41:35, Mattias Nissler wrote:
> Hm, now that this is getting checked in more and more places, maybe it's a
good
> idea to make this easier by introducing a GetRestoreMode function in
> server_backed_device_state.h which returns the mode as an enum value?

Done.

https://codereview.chromium.org/690643002/diff/1/chrome/browser/chromeos/poli...
File chrome/browser/chromeos/policy/server_backed_device_state.h (right):

https://codereview.chromium.org/690643002/diff/1/chrome/browser/chromeos/poli...
chrome/browser/chromeos/policy/server_backed_device_state.h:18: extern const
char kDeviceStateRestoreModeDisabled[];
On 2014/10/29 17:41:35, Mattias Nissler wrote:
> Are we positive behavior will be identical for both lost and stolen cases? If
> not, we might introduce two different identifiers here.
> 
> On the other hand, I guess we can always deprecated disabled and add
> finer-grained identifiers when needed later.

Yes, we are positive on that. The UI designers are worried that "lock session"
and "lock device" are confusing already. If we were to add "lock stolen device"
vs. "lock lost device", peoples' heads would explode.

[Denis Kuznetsov (DE-MUC), 2014-10-30 22:18:07]

lgtm 
if answer for my question is No

https://chromiumcodereview.appspot.com/690643002/diff/10001/chrome/browser/ch...
File chrome/browser/chromeos/login/screens/device_disabled_screen.cc (right):

https://chromiumcodereview.appspot.com/690643002/diff/10001/chrome/browser/ch...
chrome/browser/chromeos/login/screens/device_disabled_screen.cc:51: if
(policy::GetRestoreMode() != policy::RESTORE_MODE_DISABLED ||
Do we need to check for trusted mode here?

[bartfab (slow), 2014-11-03 13:32:50]

https://chromiumcodereview.appspot.com/690643002/diff/10001/chrome/browser/ch...
File chrome/browser/chromeos/login/screens/device_disabled_screen.cc (right):

https://chromiumcodereview.appspot.com/690643002/diff/10001/chrome/browser/ch...
chrome/browser/chromeos/login/screens/device_disabled_screen.cc:51: if
(policy::GetRestoreMode() != policy::RESTORE_MODE_DISABLED ||
On 2014/10/30 22:18:06, Denis Kuznetsov wrote:
> Do we need to check for trusted mode here?

Device disabling can be triggered by two different protos:
1) DeviceStateRetrievalResponse
2) PolicyData

We receive 1) during OOBE and persist its contents in local state. We trust the
persisted information if the device is not owned yet (see lines 60-75 of this
method).

We receive 2) as part of device policy and persist the whole proto on disk. We
trust the persisted information if its signature is valid, which is exactly what
cros settings trusted means.

Since we are dealing with proto 1) here, we do not need to worry about the cros
settings trusted state.

[bartfab (slow), 2014-11-03 13:33:26]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-11-03 13:33:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/690643002/10001

[commit-bot: I haz the power, 2014-11-03 14:13:54]

Committed patchset #2 (id:10001)

[commit-bot: I haz the power, 2014-11-03 14:14:33]

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