Integrate Kiosk Mode with enterprise policies.

chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc

Index: chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc
diff --git a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc
index 1ae86619d9270dcea434ed1fc455c4fb5b63c18e..039e5a8d95be3a47cc6e4d4dff7419857e964bde 100644
--- a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc
+++ b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.cc
@@ -4,10 +4,14 @@
 
 #include "chrome/browser/chromeos/kiosk_mode/kiosk_mode_helper.h"
 
+#include <algorithm>
+
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/cros_settings.h"
+#include "chrome/browser/chromeos/cros_settings_names.h"
 #include "chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.h"
 #include "chrome/browser/policy/cloud_policy_constants.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
@@ -15,9 +19,11 @@
 
 namespace {
 
-const int64 kScreensaverIdleTimeout = 60;
-const int64 kLoginIdleTimeout = 100;
-const int64 kLoginIdleCountdownTimeout = 20;
+const int kMaxIdleLogoutTimeout = 100000; // ms = 100s.
+const int kMinIdleLogoutTimeout = 25000; // ms = 25s.

[Mattias Nissler (ping if slow), 2012/03/16 09:56:46]

Where do this values come from? They seem overly restrictive to me. I would have
expected something in the order of max = 10minutes, min = 5seconds.

[rkc, 2012/03/19 23:59:48]

Done.

+
+const int kMaxIdleLogoutWarningTimeout = 20000; // ms = 20s.
+const int kMinIdleLogoutWarningTimeout = 5000; // ms = 5s.

[Mattias Nissler (ping if slow), 2012/03/16 09:56:46]

same here

[rkc, 2012/03/19 23:59:48]

Done.

 
 }  // namespace
 
@@ -47,6 +53,41 @@ KioskModeHelper* KioskModeHelper::Get() {
 }
 
 void KioskModeHelper::Initialize(const base::Closure& notify_initialized) {
+  chromeos::CrosSettings* cros_settings = chromeos::CrosSettings::Get();
+  if (!cros_settings->GetTrusted(
+      kIdleLogoutTimeout,
+      base::Bind(&KioskModeHelper::Initialize,
+                 base::Unretained(this),
+                 notify_initialized))) {
+    return;
+  }
+
+  // Ignored till we land the code to pull the screensaver path from the app
+  // packs with the screensaver id.
+  cros_settings->GetString(kScreenSaverExtensionId, &screensaver_id_);
+
+  int screensaver_timeout = 0;
+  int idle_logout_timeout = 0, idle_logout_warning_timeout = 0;

[Mattias Nissler (ping if slow), 2012/03/16 09:56:46]

nit: It's relatively uncommon to have declarations on the same line in the code
base other than for stuff such as x/y or width/height.

[rkc, 2012/03/19 23:59:48]

Done.

+  cros_settings->GetInteger(kScreenSaverTimeout, &screensaver_timeout);
+  cros_settings->GetInteger(kIdleLogoutTimeout, &idle_logout_timeout);
+  cros_settings->GetInteger(kIdleLogoutWarningDuration,
+                            &idle_logout_warning_timeout);
+
+  screensaver_timeout_ = base::TimeDelta::FromMilliseconds(
+      screensaver_timeout);
+
+  // Restrict idle timeouts to safe values to prevent them from being turned off
+  // or otherwise misused.
+  idle_logout_timeout_ = base::TimeDelta::FromMilliseconds(
+      std::min(idle_logout_timeout, kMaxIdleLogoutTimeout));
+  idle_logout_timeout_ = base::TimeDelta::FromMilliseconds(
+      std::max(idle_logout_timeout, kMinIdleLogoutTimeout));

[Mattias Nissler (ping if slow), 2012/03/16 09:56:46]

There's a bug here, as you clobber the value assigned in line 81. A unit test
would catch this.

[rkc, 2012/03/19 23:59:48]

Done and added unit tests to test this and other stuff.

+
+  idle_logout_warning_timeout_ = base::TimeDelta::FromMilliseconds(
+      std::min(idle_logout_warning_timeout, kMaxIdleLogoutWarningTimeout));
+  idle_logout_warning_timeout_ = base::TimeDelta::FromMilliseconds(
+      std::max(idle_logout_warning_timeout, kMinIdleLogoutWarningTimeout));

[Mattias Nissler (ping if slow), 2012/03/16 09:56:46]

ditto.

[rkc, 2012/03/19 23:59:48]

Done.

+
   is_initialized_ = true;
   notify_initialized.Run();
 }
@@ -59,24 +100,24 @@ std::string KioskModeHelper::GetScreensaverPath() const {
       GetSwitchValueASCII(switches::kKioskModeScreensaverPath);
 }
 
-int64 KioskModeHelper::GetScreensaverTimeout() const {
+base::TimeDelta KioskModeHelper::GetScreensaverTimeout() const {
   if (!is_initialized_)
-    return -1;
+    return base::TimeDelta::FromSeconds(-1);

[Mattias Nissler (ping if slow), 2012/03/16 09:56:46]

This seems like a pretty arbitrary value, since it'll translate to an internal
value of -1000000 microseconds. Can we use 0 instead?

[rkc, 2012/03/19 23:59:48]

I specifically wanted a negative value so 0 could be used as a valid value to
indicate no timeout. It shouldn't matter what the magnitude of that negative
value would be.

 
-  return kScreensaverIdleTimeout;
+  return screensaver_timeout_;
 }
 
-int64 KioskModeHelper::GetIdleLogoutTimeout() const {
+base::TimeDelta KioskModeHelper::GetIdleLogoutTimeout() const {
   if (!is_initialized_)
-    return -1;
+    return base::TimeDelta::FromSeconds(-1);
 
-  return kLoginIdleTimeout;
+  return idle_logout_timeout_;
 }

[Mattias Nissler (ping if slow), 2012/03/16 09:56:46]

insert blank line.

[rkc, 2012/03/19 23:59:48]

Done.

-int64 KioskModeHelper::GetIdleLogoutWarningTimeout() const {
+base::TimeDelta KioskModeHelper::GetIdleLogoutWarningTimeout() const {
   if (!is_initialized_)
-    return -1;
+    return base::TimeDelta::FromSeconds(-1);
 
-  return kLoginIdleCountdownTimeout;
+  return idle_logout_warning_timeout_;
 }
 
 KioskModeHelper::KioskModeHelper() : is_initialized_(false) {