Login screen and Guest mode should use device owner preference

chrome/browser/chromeos/system/ash_system_tray_delegate.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/system/ash_system_tray_delegate.h"
 
 #include <algorithm>
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 962 matching lines @@
       integration_service->job_list()->AddObserver(this);
   }
 
   void UnobserveDriveUpdates() {
     DriveIntegrationService* integration_service =
         FindDriveIntegrationService();
     if (integration_service)
       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() {
     GetSystemTrayNotifier()->NotifyShowLoginButtonChanged(
         user_pref_registrar_->prefs()->GetBoolean(
             prefs::kShowLogoutButtonInTray));
   }
 
   void UpdateSessionStartTime() {
     const PrefService* local_state = local_state_registrar_->prefs();
@@ skipping 361 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(SystemTrayDelegate);
 };
 
 }  // namespace
 
 ash::SystemTrayDelegate* CreateSystemTrayDelegate() {
   return new chromeos::SystemTrayDelegate();
 }
 
 }  // namespace chromeos