Add policy for limiting the session length

chrome/browser/chromeos/login/user_manager_impl.cc

Index: chrome/browser/chromeos/login/user_manager_impl.cc
diff --git a/chrome/browser/chromeos/login/user_manager_impl.cc b/chrome/browser/chromeos/login/user_manager_impl.cc
index f34484a599b16c285a83f2f161efdce1b5058f72..c9896c07699cd6fef8236941b8b9c76492e849ba 100644
--- a/chrome/browser/chromeos/login/user_manager_impl.cc
+++ b/chrome/browser/chromeos/login/user_manager_impl.cc
@@ -27,6 +27,7 @@
 #include "chrome/browser/chromeos/login/remove_user_delegate.h"
 #include "chrome/browser/chromeos/login/user_image_manager_impl.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
+#include "chrome/browser/chromeos/power/session_length_limiter.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
@@ -157,6 +158,7 @@ void UserManager::RegisterPrefs(PrefService* local_state) {
                                       PrefService::UNSYNCABLE_PREF);
   local_state->RegisterDictionaryPref(kUserDisplayEmail,
                                       PrefService::UNSYNCABLE_PREF);
+  SessionLengthLimiter::RegisterPrefs(local_state);
 }
 
 UserManagerImpl::UserManagerImpl()
@@ -194,6 +196,8 @@ void UserManagerImpl::Shutdown() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   cros_settings_->RemoveSettingsObserver(kAccountsPrefDeviceLocalAccounts,
                                          this);
+  // Stop the session length limiter.
+  session_length_limiter_.reset();
 }
 
 UserImageManager* UserManagerImpl::GetUserImageManager() {
@@ -229,6 +233,10 @@ void UserManagerImpl::UserLoggedIn(const std::string& email,
     } else {
       RegularUserLoggedIn(email, browser_restart);
     }
+
+    // Start the session length limiter.
+    session_length_limiter_.reset(new SessionLengthLimiter(NULL,

[Nikita (slow), 2012/12/14 11:56:10]

Why do you need to create session length limiter for every non-Guest/non-Retail
user?

Will that be a noop for consumer user when policy is not defined? Maybe makes
sense to not create this and only create if policy that limits session lenght is
either defined during login. If policy will update during user session, you
could also create session limiter instance if needed.

[bartfab (slow), 2012/12/14 11:59:28]

Sure, I can create the limiter on demand. But that would mean moving the code
that listens for policy changes into the UserManager so that it can construct
and destruct the limiter on the fly. It seems much cleaner and maintainable to
me to encapsulate this in one place. If the policy is not enabled, the limiter
simply does nothing after all. It just waits for the policy to be enabled, just
like the UserManager would if I moved the code there.

And yes, I enabled this code for regular users as well. This is because the
policy is useful for regular users as well.

[Nikita (slow), 2012/12/14 12:14:08]

Agree, discussed.

+                                                           browser_restart));
   }
 
   NotifyOnLogin();
@@ -582,6 +590,12 @@ bool UserManagerImpl::IsSessionStarted() const {
   return session_started_;
 }
 
+bool UserManagerImpl::HasBrowserRestarted() const {
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  return command_line->HasSwitch(switches::kLoginManager) &&

[Nikita (slow), 2012/12/14 12:03:06]

When browser will be restarted after crash, --login-manager will *not* be
passed.
--login-user will be passed.

You don't have to check for --login-password here. That's a debug switch only.

Previous check also had base::chromeos::IsRunningOnChromeOS() which was not
transfered here.

[bartfab (slow), 2012/12/14 12:24:42]

The wrong pref name was a typo. Sorry. What I meant to do is to replicate the
logic from chrome_browser_main_chromeos.cc. Citing from there (line 452 onward):

// There are two use cases for kLoginUser:
//   1) if passed in tandem with kLoginPassword, to drive a "StubLogin"
//   2) if passed alone, to signal that the indicated user has already
//      logged in and we should behave accordingly.

The code here is meant to find out whether we are in case 2. Hence, it checks
whether a password was given or not.

+         !command_line->HasSwitch(switches::kLoginPassword);
+}
+
 bool UserManagerImpl::IsUserNonCryptohomeDataEphemeral(
     const std::string& email) const {
   // Data belonging to the guest, retail mode and stub users is always
@@ -614,10 +628,7 @@ bool UserManagerImpl::IsUserNonCryptohomeDataEphemeral(
   //    enabled.
   //    - or -
   // b) The browser is restarting after a crash.
-  return AreEphemeralUsersEnabled() ||
-         (base::chromeos::IsRunningOnChromeOS() &&
-          !CommandLine::ForCurrentProcess()->
-              HasSwitch(switches::kLoginManager));
+  return AreEphemeralUsersEnabled() || HasBrowserRestarted();
 }
 
 void UserManagerImpl::AddObserver(UserManager::Observer* obs) {