Merge 163942 - On active, dismiss the retail mode screensaver immidiately.

When a user goes active, we try to do the login underneath the screensaver; this has the unfortunate side-effect of it looking like nothing has happened if the login takes too long. Instead, dimiss the screensaver immidiately on the user going active and show a login spinner instead. This indicates clearly to the user that the machine is logging in, instead of not being responsive.

This also removes any need for an observer for SESSION_STARTED since now we aren't waiting for the login to finish to dismiss the screensaver.

TBR'ed jhawkins for gypi change.

R=xiyuan@chromium.org
BUG=157282
TBR=jhawkins@chromium.org

Review URL: https://codereview.chromium.org/11264017

TBR=rkc@chromium.org

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

8 files changed, 49 insertions, 134 deletions

diff --git a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc
index 1daa428..15cc259 100644
--- a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc
+++ b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.cc
@@ -14,8 +14,10 @@
 #include "chrome/browser/chromeos/kiosk_mode/kiosk_mode_settings.h"
 #include "chrome/browser/chromeos/login/existing_user_controller.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
+#include "chrome/browser/chromeos/login/webui_login_display_host.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/sandboxed_unpacker.h"
+#include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_file_util.h"
@@ -128,9 +130,9 @@ KioskModeScreensaver::~KioskModeScreensaver() {
   // In case we're shutting down without ever triggering the active
   // notification and/or logging in.
   if (ash::Shell::GetInstance() &&
-      ash::Shell::GetInstance()->user_activity_detector())
+      ash::Shell::GetInstance()->user_activity_detector() &&
+      ash::Shell::GetInstance()->user_activity_detector()->HasObserver(this))
     ash::Shell::GetInstance()->user_activity_detector()->RemoveObserver(this);
-  registrar_.RemoveAll();
 }
 
 void KioskModeScreensaver::GetScreensaverCrxPath() {
@@ -172,9 +174,6 @@ void KioskModeScreensaver::SetupScreensaver(
   if (chromeos::UserManager::Get()->IsUserLoggedIn())
     return;
 
-  // Add our observers for login and user active.
-  registrar_.Add(this, chrome::NOTIFICATION_SESSION_STARTED,
-                 content::NotificationService::AllSources());
   ash::Shell::GetInstance()->user_activity_detector()->AddObserver(this);
 
   Profile* default_profile = ProfileManager::GetDefaultProfile();
@@ -188,37 +187,32 @@ void KioskModeScreensaver::SetupScreensaver(
   }
 }
 
-// NotificationObserver overrides:
-void KioskModeScreensaver::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  DCHECK_EQ(type, chrome::NOTIFICATION_SESSION_STARTED);
-
-  registrar_.RemoveAll();
-
-  ash::CloseScreensaver();
-  ShutdownKioskModeScreensaver();
-}
-
 void KioskModeScreensaver::OnUserActivity() {
   // We don't want to handle further user notifications; we'll either login
   // the user and close out or or at least close the screensaver.
   ash::Shell::GetInstance()->user_activity_detector()->RemoveObserver(this);
 
-  // User is active, log us in.
+  // Find the retail mode login page.
+  CHECK(WebUILoginDisplayHost::default_host());
+  WebUILoginDisplayHost* webui_host =
+      static_cast<WebUILoginDisplayHost*>(
+          WebUILoginDisplayHost::default_host());
+  OobeUI* oobe_ui = webui_host->GetOobeUI();
+
+  // Show the login spinner.
+  CHECK(oobe_ui);
+  oobe_ui->ShowRetailModeLoginSpinner();
+
+  // Close the screensaver, our login spinner is already showing.
+  ash::CloseScreensaver();
+
+  // Log us in.
   ExistingUserController* controller =
       ExistingUserController::current_controller();
+  CHECK(controller);
+  controller->LoginAsDemoUser();
 
-  if (controller) {
-    // Logging in will shut us down and remove the screen saver.
-    controller->LoginAsDemoUser();
-  } else {
-    // Remove the screensaver so the user can at least use the underlying
-    // login screen to be able to log in.
-    ash::CloseScreensaver();
-    ShutdownKioskModeScreensaver();
-  }
+  ShutdownKioskModeScreensaver();
 }
 
 static KioskModeScreensaver* g_kiosk_mode_screensaver = NULL;
diff --git a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.h b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.h
index 004c5fc..5b4ca4f 100644
--- a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.h
+++ b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver.h
@@ -9,8 +9,6 @@
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/memory/weak_ptr.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 
 namespace extensions {
 class Extension;
@@ -18,8 +16,7 @@ class Extension;
 
 namespace chromeos {
 
-class KioskModeScreensaver : public ash::UserActivityObserver,
-                             public content::NotificationObserver {
+class KioskModeScreensaver : public ash::UserActivityObserver {
  public:
   KioskModeScreensaver();
   virtual ~KioskModeScreensaver();
@@ -27,11 +24,6 @@ class KioskModeScreensaver : public ash::UserActivityObserver,
  private:
   friend class KioskModeScreensaverTest;
 
-  // NotificationObserver overrides:
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE;
-
   // UserActivityObserver::Observer overrides:
   virtual void OnUserActivity() OVERRIDE;
 
@@ -48,7 +40,6 @@ class KioskModeScreensaver : public ash::UserActivityObserver,
   void SetupScreensaver(scoped_refptr<extensions::Extension> extension,
                         const FilePath& extension_base_path);
 
-  content::NotificationRegistrar registrar_;
   base::WeakPtrFactory<KioskModeScreensaver> weak_ptr_factory_;
 
   FilePath extension_base_path_;
diff --git a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver_unittest.cc b/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver_unittest.cc
deleted file mode 100644
index a7462bb..0000000
--- a/chrome/browser/chromeos/kiosk_mode/kiosk_mode_screensaver_unittest.cc
+++ /dev/null
@@ -1,93 +0,0 @@
-// 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/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"
-#include "base/synchronization/waitable_event.h"
-#include "chrome/browser/chromeos/login/mock_user_manager.h"
-#include "chrome/common/chrome_notification_types.h"
-#include "chrome/common/extensions/extension.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"
-#include "content/public/test/test_browser_thread.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-using content::BrowserThread;
-using ::testing::_;
-using ::testing::AnyNumber;
-using ::testing::Return;
-using ::testing::SaveArg;
-
-namespace chromeos {
-
-class KioskModeScreensaverTest : public ash::test::AshTestBase {
- public:
-  KioskModeScreensaverTest()
-      : 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));
-
-    screensaver_ = new KioskModeScreensaver();
-    screensaver_->SetupScreensaver(NULL, FilePath());
-  }
-
-  virtual void TearDown() OVERRIDE {
-    delete screensaver_;
-    AshTestBase::TearDown();
-  }
-
-  bool LoginUserObserverRegistered() {
-    return screensaver_->registrar_.IsRegistered(
-        screensaver_,
-        chrome::NOTIFICATION_SESSION_STARTED,
-        content::NotificationService::AllSources());
-  }
-
-  ScopedMockUserManagerEnabler mock_user_manager_;
-
-  content::TestBrowserThread ui_thread_;
-  content::TestBrowserThread file_thread_;
-
-  KioskModeScreensaver* screensaver_;
-  content::NotificationRegistrar registrar_;
-
-  scoped_ptr<TestingProfileManager> profile_manager_;
-};
-
-TEST_F(KioskModeScreensaverTest, CheckObservers) {
-  EXPECT_TRUE(LoginUserObserverRegistered());
-}
-
-TEST_F(KioskModeScreensaverTest, CheckObserversAfterUserLogin) {
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_SESSION_STARTED,
-      content::Source<UserManager>(UserManager::Get()),
-      content::NotificationService::NoDetails());
-
-  RunAllPendingInMessageLoop();
-  EXPECT_FALSE(LoginUserObserverRegistered());
-}
-
-}  // namespace chromeos
diff --git a/chrome/browser/resources/chromeos/login/demo_user_login.html b/chrome/browser/resources/chromeos/login/demo_user_login.html
index f168360..19747e9 100644
--- a/chrome/browser/resources/chromeos/login/demo_user_login.html
+++ b/chrome/browser/resources/chromeos/login/demo_user_login.html
@@ -1,6 +1,8 @@
 <!DOCTYPE HTML>
 <html i18n-values="dir:textdirection">
 <head>
+<link rel="stylesheet" href="chrome://resources/css/spinner.css">
+<link rel="stylesheet" href="chrome://resources/css/chrome_shared.css">
 <link rel="stylesheet" href="demo_user_login.css">
 <script src="chrome://resources/js/util.js"></script>
 <script src="demo_user_login.js"></script>
@@ -10,9 +12,10 @@
     <div id="logo-container">
       <div>
         <img id="logo" src="images/chrome_center_logo.jpg">
+        <img id="logo-login" src="images/chrome_retail_login.png" hidden>
       </div>
-      <div id="demo-login-text" i18n-content="demoLoginMessage">
-      </div>
+      <div id="demo-login-text" i18n-content="demoLoginMessage"></div>
+      <div id="login-spinner" class="inline-spinner" hidden></div>
     </div>
   </div>
 </body>
diff --git a/chrome/browser/resources/chromeos/login/demo_user_login.js b/chrome/browser/resources/chromeos/login/demo_user_login.js
index 7828af6..269bd7e 100644
--- a/chrome/browser/resources/chromeos/login/demo_user_login.js
+++ b/chrome/browser/resources/chromeos/login/demo_user_login.js
@@ -14,6 +14,7 @@
 onClick = function(e) {
   document.removeEventListener('click', onClick);
   e.stopPropagation();
+  showLoginSpinner();
   chrome.send('launchDemoUser');
 };
 
@@ -30,5 +31,21 @@ initialize = function() {
   });
 };
 
+/**
+ * Show the login spinner.
+ */
+showLoginSpinner = function() {
+  // We're already logging in - don't login on click.
+  document.removeEventListener('click', onClick);
+
+  // Hide the "Click to start" assets.
+  $('logo').hidden = true;
+  $('demo-login-text').hidden = true;
+
+  // Show the "Logging in" assets.
+  $('logo-login').hidden = false;
+  $('login-spinner').hidden = false;
+};
+
 disableTextSelectAndDrag();
 document.addEventListener('DOMContentLoaded', initialize);
diff --git a/chrome/browser/resources/chromeos/login/images/chrome_retail_login.png b/chrome/browser/resources/chromeos/login/images/chrome_retail_login.png
new file mode 100644
index 0000000..8ea91396
--- /dev/null
+++ b/chrome/browser/resources/chromeos/login/images/chrome_retail_login.png
diff --git a/chrome/browser/ui/webui/chromeos/login/oobe_ui.cc b/chrome/browser/ui/webui/chromeos/login/oobe_ui.cc
index 9c2d8dc..eb8e307 100644
--- a/chrome/browser/ui/webui/chromeos/login/oobe_ui.cc
+++ b/chrome/browser/ui/webui/chromeos/login/oobe_ui.cc
@@ -283,6 +283,10 @@ void OobeUI::ShowOobeUI(bool show) {
   core_handler_->ShowOobeUI(show);
 }
 
+void OobeUI::ShowRetailModeLoginSpinner() {
+  signin_screen_handler_->ShowRetailModeLoginSpinner();
+}
+
 void OobeUI::ShowSigninScreen(SigninScreenHandlerDelegate* delegate) {
   signin_screen_handler_->SetDelegate(delegate);
   signin_screen_handler_->Show(core_handler_->show_oobe_ui());
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index d279eb7..12bebaf 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1162,7 +1162,6 @@
         'browser/chromeos/input_method/input_method_whitelist_unittest.cc',
         'browser/chromeos/input_method/xkeyboard_unittest.cc',
         'browser/chromeos/kiosk_mode/kiosk_mode_idle_logout_unittest.cc',
-        'browser/chromeos/kiosk_mode/kiosk_mode_screensaver_unittest.cc',
         'browser/chromeos/kiosk_mode/kiosk_mode_settings_unittest.cc',
         'browser/chromeos/language_preferences_unittest.cc',
         'browser/chromeos/login/mock_auth_attempt_state_resolver.cc',