Login screen and Guest mode should use device owner preference

chrome/browser/chromeos/system/ash_system_tray_delegate.cc

Index: chrome/browser/chromeos/system/ash_system_tray_delegate.cc
diff --git a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
index 96f6f9d5c5c8af4fdfd01116223a7a6cb86ca7e6..3c84f8ab6da53c1f1a8799e55cd0015ff654f347 100644
--- a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
+++ b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
@@ -980,13 +980,41 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
       integration_service->job_list()->RemoveObserver(this);
   }
 
+  bool ShouldUse24HourClock() const {
+    // On login screen and in guest mode owner default is used for
+    // use24HourClock preference.
+    // All other modes default to the default locale value.
+    const ash::user::LoginStatus status = GetUserLoginStatus();
+

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

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.

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

Done.

+    const PrefService* local_state = local_state_registrar_->prefs();
+
+    if (status == ash::user::LOGGED_IN_NONE)
+      return local_state->GetBoolean(prefs::kOwnerPrefUse24HourClock);
+
+    const PrefService::Preference* user_pref =
+        user_pref_registrar_->prefs()->FindPreference(prefs::kUse24HourClock);
+
+    if (status == ash::user::LOGGED_IN_GUEST && user_pref->IsDefaultValue())
+      return local_state->GetBoolean(prefs::kOwnerPrefUse24HourClock);
+
+    bool use24HourClock = true;

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

nit: here and in line 1009, use google style guide (i.e. use_24_hour_clock)

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

Done.

+    user_pref->GetValue()->GetAsBoolean(&use24HourClock);
+    return use24HourClock;
+  }
+
   void UpdateClockType() {
     if (!user_pref_registrar_)
       return;
-    clock_type_ =
-        user_pref_registrar_->prefs()->GetBoolean(prefs::kUse24HourClock) ?
-        base::k24HourClock : base::k12HourClock;
+
+    const bool use24HourClock = ShouldUse24HourClock();
+
+    clock_type_ = use24HourClock ? base::k24HourClock : base::k12HourClock;
+
     GetSystemTrayNotifier()->NotifyDateFormatChanged();
+
+    if (chromeos::UserManager::Get()->IsCurrentUserOwner())

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

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.

+      local_state_registrar_->prefs()
+          ->SetBoolean(prefs::kOwnerPrefUse24HourClock, use24HourClock);

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

nit: Prefer separating this line after ( instead of before ->

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

nit: don't start lines with "->"

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

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

   }
 
   void UpdateShowLogoutButtonInTray() {