Merge 152895 - Move retail mode's idle->active detection to Chrome.

Retail mode used power_manager to detect idle and active states; powerd now depends on Chrome letting it know of the user activity, which is rate limited to 5s. This introduces a delay in detecting any user activity for either the screensaver or the idle_logout dialog. Change the code to use Chrome's UserActivityDetector to detect user activity instead.

Additionally the user activity detector is also rate limited to 1s; reduce this to 200s since it is a cheap operation.

This CL also fixes a couple of other bugs that have made KioskMode unusable current builds.

1.) Get the profile for the user on the UI thread (this was being done on the file thread, causing a CalledOnValidThread check failure).
2.) Add a @ to the demo user since this is checked in gaia_auth_util.cc - CanonicalizeEmail. Not having the @ will cause a NOTREACHED.

R=derat@chromium.org
BUG=143706
TEST=The screensaver goes away as soon as the user presses a key or moves the mouse, even if it has been up for less than 5 seconds. Similarly, the idle logout dialog should cancel immidiately in case of any user activity.


Review URL: https://chromiumcodereview.appspot.com/10868004

TBR=rkc@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10874070

git-svn-id: svn://svn.chromium.org/chrome/branches/1229/src@153326 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 47 insertions, 93 deletions

diff --git a/ash/wm/user_activity_detector.cc b/ash/wm/user_activity_detector.cc
index 3fd845a..9b4dd64 100644
--- a/ash/wm/user_activity_detector.cc
+++ b/ash/wm/user_activity_detector.cc
@@ -12,7 +12,7 @@
 
 namespace ash {
 
-const double UserActivityDetector::kNotifyIntervalSec = 1.0;
+const double UserActivityDetector::kNotifyIntervalMs = 200.0;
 
 UserActivityDetector::UserActivityDetector() {
 }
@@ -20,6 +20,10 @@ UserActivityDetector::UserActivityDetector() {
 UserActivityDetector::~UserActivityDetector() {
 }
 
+bool UserActivityDetector::HasObserver(UserActivityObserver* observer) const {
+  return observers_.HasObserver(observer);
+}
+
 void UserActivityDetector::AddObserver(UserActivityObserver* observer) {
   observers_.AddObserver(observer);
 }
@@ -69,8 +73,8 @@ void UserActivityDetector::MaybeNotify() {
   base::TimeTicks now =
       !now_for_test_.is_null() ? now_for_test_ : base::TimeTicks::Now();
   if (last_observer_notification_time_.is_null() ||
-      (now - last_observer_notification_time_).InSecondsF() >=
-      kNotifyIntervalSec) {
+      (now - last_observer_notification_time_).InMillisecondsF() >=
+      kNotifyIntervalMs) {
     FOR_EACH_OBSERVER(UserActivityObserver, observers_, OnUserActivity());
     last_observer_notification_time_ = now;
   }
diff --git a/ash/wm/user_activity_detector.h b/ash/wm/user_activity_detector.h
index a19b57c..91d88c9 100644
--- a/ash/wm/user_activity_detector.h
+++ b/ash/wm/user_activity_detector.h
@@ -21,13 +21,14 @@ class ASH_EXPORT UserActivityDetector : public aura::EventFilter {
  public:
   // Minimum amount of time between notifications to observers that a video is
   // playing.
-  static const double kNotifyIntervalSec;
+  static const double kNotifyIntervalMs;
 
   UserActivityDetector();
   virtual ~UserActivityDetector();
 
   void set_now_for_test(base::TimeTicks now) { now_for_test_ = now; }
 
+  bool HasObserver(UserActivityObserver* observer) const;
   void AddObserver(UserActivityObserver* observer);
   void RemoveObserver(UserActivityObserver* observer);
 
diff --git a/ash/wm/user_activity_detector_unittest.cc b/ash/wm/user_activity_detector_unittest.cc
index 2d0c185..1d0ded3 100644
--- a/ash/wm/user_activity_detector_unittest.cc
+++ b/ash/wm/user_activity_detector_unittest.cc
@@ -88,7 +88,7 @@ TEST_F(UserActivityDetectorTest, Basic) {
   observer_->reset_stats();
 
   base::TimeDelta advance_delta =
-      base::TimeDelta::FromSeconds(UserActivityDetector::kNotifyIntervalSec);
+      base::TimeDelta::FromSeconds(UserActivityDetector::kNotifyIntervalMs);
   AdvanceTime(advance_delta);
   aura::MouseEvent mouse_event(
       ui::ET_MOUSE_MOVED, gfx::Point(), gfx::Point(), ui::EF_NONE);
@@ -123,22 +123,25 @@ TEST_F(UserActivityDetectorTest, RateLimitNotifications) {
   EXPECT_EQ(1, observer_->num_invocations());
   observer_->reset_stats();
 
-  // It shouldn't be notified if a second event occurs in the same second,
-  // though.
+  // It shouldn't be notified if a second event occurs
+  // in the same instant in time.
   EXPECT_FALSE(detector_->PreHandleKeyEvent(window.get(), &event));
   EXPECT_EQ(0, observer_->num_invocations());
   observer_->reset_stats();
 
   // Advance the time, but not quite enough for another notification to be sent.
   AdvanceTime(
-      base::TimeDelta::FromSeconds(
-          UserActivityDetector::kNotifyIntervalSec - 1));
+      base::TimeDelta::FromMilliseconds(
+          UserActivityDetector::kNotifyIntervalMs - 100));
   EXPECT_FALSE(detector_->PreHandleKeyEvent(window.get(), &event));
   EXPECT_EQ(0, observer_->num_invocations());
   observer_->reset_stats();
 
-  // One second later, we should send a notification again.
-  AdvanceTime(base::TimeDelta::FromSeconds(1));
+  // Advance time by the notification interval, definitely moving out of the
+  // rate limit. This should let us trigger another notification.
+  AdvanceTime(base::TimeDelta::FromMilliseconds(
+      UserActivityDetector::kNotifyIntervalMs));
+
   EXPECT_FALSE(detector_->PreHandleKeyEvent(window.get(), &event));
   EXPECT_EQ(1, observer_->num_invocations());
 }
diff --git a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.h b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.h
index d83b263..fa1ee11 100644
--- a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.h
+++ b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_idle_logout.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_CHROMEOS_KIOSK_MODE_KIOSK_MODE_IDLE_LOGOUT_H_
 #define CHROME_BROWSER_CHROMEOS_KIOSK_MODE_KIOSK_MODE_IDLE_LOGOUT_H_
 
+#include "ash/wm/user_activity_observer.h"
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "chromeos/dbus/power_manager_client.h"
@@ -13,7 +14,8 @@
 
 namespace chromeos {
 
-class KioskModeIdleLogout : public PowerManagerClient::Observer,
+class KioskModeIdleLogout : public ash::UserActivityObserver,
+                            public PowerManagerClient::Observer,
                             public content::NotificationObserver {
  public:
   static void Initialize();
@@ -33,10 +35,11 @@ class KioskModeIdleLogout : public PowerManagerClient::Observer,
 
   // Overridden from PowerManagerClient::Observer:
   virtual void IdleNotify(int64 threshold) OVERRIDE;
-  virtual void ActiveNotify() OVERRIDE;
+
+  // UserActivityObserver::Observer overrides:
+  virtual void OnUserActivity() OVERRIDE;
 
   void SetupIdleNotifications();
-  void RequestNextActiveNotification();
   void RequestNextIdleNotification();
 
   content::NotificationRegistrar registrar_;
diff --git a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver_unittest.cc b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver_unittest.cc
index 78c040a..a7462bb 100644
--- a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver_unittest.cc
+++ b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.h"
 
+#include "ash/test/ash_test_base.h"
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
@@ -11,8 +12,8 @@
 #include "chrome/browser/chromeos/login/mock_user_manager.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
-#include "chromeos/dbus/mock_dbus_thread_manager.h"
-#include "chromeos/dbus/mock_power_manager_client.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
@@ -28,51 +29,33 @@ using ::testing::SaveArg;
 
 namespace chromeos {
 
-ACTION_P(HasObserver, expected_pointer) {
-  return arg0 == *expected_pointer;
-}
-
-ACTION_P(RemoveObserver, expected_pointer) {
-  if (arg0 == *expected_pointer)
-    *expected_pointer = NULL;
-}
-
-class KioskModeScreensaverTest : public testing::Test {
+class KioskModeScreensaverTest : public ash::test::AshTestBase {
  public:
   KioskModeScreensaverTest()
-      : ui_thread_(BrowserThread::UI, &message_loop_),
-        screensaver_(NULL),
-        observer_(NULL) {
+      : ui_thread_(BrowserThread::UI, message_loop()),
+        file_thread_(BrowserThread::FILE, message_loop()),
+        screensaver_(NULL) {
   }
 
   virtual void SetUp() OVERRIDE {
+    // We need this so that the GetDefaultProfile call from within
+    // SetupScreensaver doesn't crash.
+    profile_manager_.reset(new TestingProfileManager(
+        static_cast<TestingBrowserProcess*>(g_browser_process)));
+    ASSERT_TRUE(profile_manager_->SetUp());
+
+    AshTestBase::SetUp();
     EXPECT_CALL(*mock_user_manager_.user_manager(), IsUserLoggedIn())
         .Times(AnyNumber())
         .WillRepeatedly(Return(false));
 
-    MockDBusThreadManager* mock_dbus_thread_manager =
-        new MockDBusThreadManager;
-    DBusThreadManager::InitializeForTesting(mock_dbus_thread_manager);
-
-    MockPowerManagerClient* power_manager =
-        mock_dbus_thread_manager->mock_power_manager_client();
-    EXPECT_CALL(*power_manager, HasObserver(_))
-        .Times(AnyNumber())
-        .WillRepeatedly(HasObserver(&observer_));
-    EXPECT_CALL(*power_manager, AddObserver(_))
-        .WillOnce(SaveArg<0>(&observer_));
-    EXPECT_CALL(*power_manager, RemoveObserver(_))
-        .WillOnce(RemoveObserver(&observer_));
-    EXPECT_CALL(*power_manager, RequestIdleNotification(_))
-        .Times(AnyNumber());
-
     screensaver_ = new KioskModeScreensaver();
-    screensaver_->SetupScreensaver(NULL, NULL, FilePath());
+    screensaver_->SetupScreensaver(NULL, FilePath());
   }
 
   virtual void TearDown() OVERRIDE {
     delete screensaver_;
-    DBusThreadManager::Shutdown();
+    AshTestBase::TearDown();
   }
 
   bool LoginUserObserverRegistered() {
@@ -82,26 +65,19 @@ class KioskModeScreensaverTest : public testing::Test {
         content::NotificationService::AllSources());
   }
 
-  bool PowerManagerObserverRegistered() {
-    PowerManagerClient* power_manager =
-        DBusThreadManager::Get()->GetPowerManagerClient();
-    return power_manager->HasObserver(screensaver_);
-  }
+  ScopedMockUserManagerEnabler mock_user_manager_;
 
-  MessageLoopForUI message_loop_;
   content::TestBrowserThread ui_thread_;
-
-  ScopedMockUserManagerEnabler mock_user_manager_;
+  content::TestBrowserThread file_thread_;
 
   KioskModeScreensaver* screensaver_;
   content::NotificationRegistrar registrar_;
 
-  PowerManagerClient::Observer* observer_;
+  scoped_ptr<TestingProfileManager> profile_manager_;
 };
 
 TEST_F(KioskModeScreensaverTest, CheckObservers) {
   EXPECT_TRUE(LoginUserObserverRegistered());
-  EXPECT_TRUE(PowerManagerObserverRegistered());
 }
 
 TEST_F(KioskModeScreensaverTest, CheckObserversAfterUserLogin) {
@@ -110,7 +86,8 @@ TEST_F(KioskModeScreensaverTest, CheckObserversAfterUserLogin) {
       content::Source<UserManager>(UserManager::Get()),
       content::NotificationService::NoDetails());
 
-  EXPECT_FALSE(PowerManagerObserverRegistered());
+  RunAllPendingInMessageLoop();
+  EXPECT_FALSE(LoginUserObserverRegistered());
 }
 
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/login/user.cc b/chrome/browser/chromeos/login/user.cc
index 814ed1f..339dece 100644
--- a/chrome/browser/chromeos/login/user.cc
+++ b/chrome/browser/chromeos/login/user.cc
@@ -27,7 +27,7 @@ std::string GetUserName(const std::string& email) {
 }  // namespace
 
 // The demo user is represented by a domainless username.
-const char kDemoUser[] = "demouser";
+const char kDemoUser[] = "demouser@";
 // Incognito user is represented by an empty string (since some code already
 // depends on that and it's hard to figure out what).
 const char kGuestUser[] = "";
diff --git a/chromeos/dbus/power_manager_client.cc b/chromeos/dbus/power_manager_client.cc
index 7a52368..2155713 100644
--- a/chromeos/dbus/power_manager_client.cc
+++ b/chromeos/dbus/power_manager_client.cc
@@ -91,14 +91,6 @@ class PowerManagerClientImpl : public PowerManagerClient {
 
     power_manager_proxy_->ConnectToSignal(
         power_manager::kPowerManagerInterface,
-        power_manager::kActiveNotifySignal,
-        base::Bind(&PowerManagerClientImpl::ActiveNotifySignalReceived,
-                   weak_ptr_factory_.GetWeakPtr()),
-        base::Bind(&PowerManagerClientImpl::SignalConnected,
-                   weak_ptr_factory_.GetWeakPtr()));
-
-    power_manager_proxy_->ConnectToSignal(
-        power_manager::kPowerManagerInterface,
         power_manager::kSoftwareScreenDimmingRequestedSignal,
         base::Bind(
             &PowerManagerClientImpl::SoftwareScreenDimmingRequestedReceived,
@@ -218,10 +210,6 @@ class PowerManagerClientImpl : public PowerManagerClient {
         dbus::ObjectProxy::EmptyResponseCallback());
   }
 
-  virtual void RequestActiveNotification() OVERRIDE {
-    RequestIdleNotification(0);
-  }
-
   virtual void NotifyUserActivity(
       const base::TimeTicks& last_activity_time) OVERRIDE {
     dbus::MethodCall method_call(
@@ -473,20 +461,6 @@ class PowerManagerClientImpl : public PowerManagerClient {
     FOR_EACH_OBSERVER(Observer, observers_, IdleNotify(threshold));
   }
 
-  void ActiveNotifySignalReceived(dbus::Signal* signal) {
-    dbus::MessageReader reader(signal);
-    int64 threshold = 0;
-    if (!reader.PopInt64(&threshold)) {
-      LOG(ERROR) << "Active Notify signal had incorrect parameters: "
-                 << signal->ToString();
-      return;
-    }
-    DCHECK_EQ(threshold, 0);
-
-    VLOG(1) << "Active Notify.";
-    FOR_EACH_OBSERVER(Observer, observers_, ActiveNotify());
-  }
-
   void SoftwareScreenDimmingRequestedReceived(dbus::Signal* signal) {
     dbus::MessageReader reader(signal);
     int32 signal_state = 0;
@@ -600,7 +574,6 @@ class PowerManagerClientStubImpl : public PowerManagerClient {
   }
 
   virtual void RequestIdleNotification(int64 threshold) OVERRIDE {}
-  virtual void RequestActiveNotification() OVERRIDE {}
   virtual void NotifyUserActivity(
       const base::TimeTicks& last_activity_time) OVERRIDE {}
   virtual void NotifyVideoActivity(
diff --git a/chromeos/dbus/power_manager_client.h b/chromeos/dbus/power_manager_client.h
index 63551f2..1706a0a 100644
--- a/chromeos/dbus/power_manager_client.h
+++ b/chromeos/dbus/power_manager_client.h
@@ -77,9 +77,6 @@ class CHROMEOS_EXPORT PowerManagerClient {
     // Called when we go idle for threshold time.
     virtual void IdleNotify(int64 threshold_secs) {}
 
-    // Called when we go from idle to active.
-    virtual void ActiveNotify() {}
-
     // Called when a request is received to dim or undim the screen in software
     // (as opposed to the more-common method of adjusting the backlight).
     virtual void ScreenDimmingRequested(ScreenDimmingState state) {}
@@ -154,10 +151,6 @@ class CHROMEOS_EXPORT PowerManagerClient {
   // time the machine goes idle for that much time, request again.
   virtual void RequestIdleNotification(int64 threshold_secs) = 0;
 
-  // Requests that the observers be notified in case of an Idle->Active event.
-  // NOTE: Like the previous request, this will also get triggered exactly once.
-  virtual void RequestActiveNotification() = 0;
-
   // Notifies the power manager that the user is active (i.e. generating input
   // events).
   virtual void NotifyUserActivity(