Login screen and Guest mode should use device owner preference

Login screen and Guest mode should use device owner preference
settings.clock.use_24hour_clock value for time format.

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

[Alexander Alekseev, 2013-07-31 17:34:50]

Please review:


mnissler@ for chrome/browser/prefs.*

jam@ for chrome/browser/browser_process_impl.cc

stevenjb@ for chrome/browser/chromeos/system/ash_system_tray_delegate.cc

Thank you.

[stevenjb, 2013-07-31 17:42:53]

lgtm

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
File chrome/browser/chromeos/system/ash_system_tray_delegate.cc (right):

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:988: 
nit: Probably don't need quite so much vertical whitespace, I would remove this
blank line for instance. Normally just separate major sections, e.g. only after
each 'return' in this function.

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:1017:
->SetBoolean(prefs::kOwnerPrefUse24HourClock, use24HourClock);
nit: Prefer separating this line after ( instead of before ->

[jam, 2013-07-31 21:04:25]

lgtm

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
File chrome/browser/chromeos/system/ash_system_tray_delegate.cc (right):

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:1000: bool
use24HourClock = true;
nit: here and in line 1009, use google style guide (i.e. use_24_hour_clock)

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:1017:
->SetBoolean(prefs::kOwnerPrefUse24HourClock, use24HourClock);
nit: don't start lines with "->"

[Mattias Nissler (ping if slow), 2013-08-01 08:57:58]

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
File chrome/browser/chromeos/system/ash_system_tray_delegate.cc (right):

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:1015: if
(chromeos::UserManager::Get()->IsCurrentUserOwner())
There's one issue with this: If the device doesn't have a local owner (i.e. if
the device is enterprise-managed), there's no way to set the clock default. The
proper way that would cover the enterprise case as well is to put the default
not into local_state, but into device settings (see CrosSettings and friends).

I'm not sure what the requirements are here, but I'd prefer a device settings
based implementation. Happy to discuss in more detail.

[Alexander Alekseev, 2013-08-05 15:30:22]

I've moved Use24HoursClock preference to Device Settings.

So, please review again:


mnissler@ for:
chrome/app/policy/policy_templates.json 
chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc
chrome/browser/chromeos/settings/cros_settings_names.h
chrome/browser/chromeos/settings/cros_settings_names.cc
chrome/browser/chromeos/settings/device_settings_provider.cc
chrome/browser/policy/proto/chromeos/chrome_device_policy.proto
chrome/test/data/policy/policy_test_cases.json

jam@ for:
chrome/browser/browser_process_impl.cc

stevenjb@ for:
chrome/browser/chromeos/system/ash_system_tray_delegate.cc

Thank you.

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
File chrome/browser/chromeos/system/ash_system_tray_delegate.cc (right):

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:988: 
On 2013/07/31 17:42:53, stevenjb (chromium) wrote:
> nit: Probably don't need quite so much vertical whitespace, I would remove
this
> blank line for instance. Normally just separate major sections, e.g. only
after
> each 'return' in this function.

Done.

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:1000: bool
use24HourClock = true;
On 2013/07/31 21:04:25, jam wrote:
> nit: here and in line 1009, use google style guide (i.e. use_24_hour_clock)

Done.

https://codereview.chromium.org/21133012/diff/1/chrome/browser/chromeos/syste...
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:1017:
->SetBoolean(prefs::kOwnerPrefUse24HourClock, use24HourClock);
On 2013/07/31 17:42:53, stevenjb (chromium) wrote:
> nit: Prefer separating this line after ( instead of before ->

Done.
(Actually it was suggested by "git clang-format --style=Chromium master". Don't
know why.)

[Mattias Nissler (ping if slow), 2013-08-05 18:12:13]

LGTM, thanks for the extra effort of converting this into a device setting.

https://codereview.chromium.org/21133012/diff/8001/chrome/app/policy/policy_t...
File chrome/app/policy/policy_templates.json (right):

https://codereview.chromium.org/21133012/diff/8001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:4056: 'desc': '''Specifies the clock
format be used for the device. Users can override clock format for the current
session. However, on logout it is set back to the specified value.''',
Please include a sentence that describes what happens if the policy is not
configured. I know it's trivial, but admins ask these questions :)

[commit-bot: I haz the power, 2013-08-07 14:06:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/21133012/8001

[commit-bot: I haz the power, 2013-08-07 14:06:23]

Failed to apply patch for chrome/app/policy/policy_templates.json:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/app/policy/policy_templates.json
  Hunk #1 FAILED at 112.
  Hunk #2 succeeded at 4065 (offset 23 lines).
  1 out of 2 hunks FAILED -- saving rejects to file
chrome/app/policy/policy_templates.json.rej

Patch:       chrome/app/policy/policy_templates.json
Index: chrome/app/policy/policy_templates.json
diff --git a/chrome/app/policy/policy_templates.json
b/chrome/app/policy/policy_templates.json
index
aeb4d5c9eef409dccada3f8dfa14571174aeb9ed..63f16b8ab01e42bb89504f67d2255169a2f7e24b
100644
--- a/chrome/app/policy/policy_templates.json
+++ b/chrome/app/policy/policy_templates.json
@@ -112,7 +112,7 @@
 #   persistent IDs for all fields (but not for groups!) are needed. These are
 #   specified by the 'id' keys of each policy. NEVER CHANGE EXISTING IDs,
 #   because doing so would break the deployed wire format!
-#   For your editing convenience: highest ID currently used: 234
+#   For your editing convenience: highest ID currently used: 235
 #
 # Placeholders:
 #   The following placeholder strings are automatically substituted:
@@ -4042,6 +4042,20 @@
       The format of the value follows the names of timezones in the "IANA Time
Zone Database" (see "http://en.wikipedia.org/wiki/List_of_tz_database_time"). In
particular, most timezones can be referred to by "continent/large_city" or
"ocean/large_city".''',
     },
     {
+      'name': 'SystemUse24HourClock',
+      'type': 'main',
+      'schema': { 'type': 'boolean' },
+      'supported_on': ['chrome_os:30-'],
+      'device_only': True,
+      'features': {
+        'dynamic_refresh': True,
+      },
+      'example_value': True,
+      'id': 235,
+      'caption': '''Use 24 hour clock by default''',
+      'desc': '''Specifies the clock format be used for the device. Users can
override clock format for the current session. However, on logout it is set back
to the specified value.''',
+    },
+    {
       'name': 'ShowLogoutButtonInTray',
       'type': 'main',
       'schema': { 'type': 'boolean' },

[Alexander Alekseev, 2013-08-07 14:13:35]

https://codereview.chromium.org/21133012/diff/8001/chrome/app/policy/policy_t...
File chrome/app/policy/policy_templates.json (right):

https://codereview.chromium.org/21133012/diff/8001/chrome/app/policy/policy_t...
chrome/app/policy/policy_templates.json:4056: 'desc': '''Specifies the clock
format be used for the device. Users can override clock format for the current
session. However, on logout it is set back to the specified value.''',
On 2013/08/05 18:12:13, Mattias Nissler wrote:
> Please include a sentence that describes what happens if the policy is not
> configured. I know it's trivial, but admins ask these questions :)

Done.

[commit-bot: I haz the power, 2013-08-07 14:19:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/21133012/27001

[commit-bot: I haz the power, 2013-08-07 18:10:38]

Change committed as 216232